langwadt / grbl_stm32

grbl for stm32 with nucleo stepper drivers
32 stars 33 forks source link

serial priority #4

Open J-Dunn opened 8 years ago

J-Dunn commented 8 years ago

One of my main reasons for wanting to get off AVR was serial interrupt priority being non modifiable and fixed at a level higher than the stepper interrupts. This means that many GRBL commands are disabled unless the machine is in idle state, since the USB comms could get stuck in a wait loop and wreck the motor control.

One example is that you can not do $$ when a motor is running. This is explicitly blocked in GRBL:


system.c : 

      switch( line[char_counter] ) {
        case '$' : // Prints Grbl settings
          if ( sys.state & (STATE_CYCLE | STATE_HOLD) ) { return(STATUS_IDLE_ERROR); } // Block during cycle. Takes too long to print.
          else { report_grbl_settings(); }
          break;

I have not tested this yet but it would seem a simple change of priority level would allow removal of this annoying limitation.

 NVIC_InitStructure.NVIC_IRQChannel = Open_USART_IRQn;
  NVIC_InitStructure.NVIC_IRQChannelPreemptionPriority = 1;  //oder 0?
  NVIC_InitStructure.NVIC_IRQChannelSubPriority = 0;         //oder 1?
  NVIC_InitStructure.NVIC_IRQChannelCmd = ENABLE;
  NVIC_Init(&NVIC_InitStructure);

The STM32 h/w should be smart enough to be able to chew gum and walk at the same time :)

langwadt commented 8 years ago

I've changed the code to put the serial interrupt priority behind the two stepper interrupts, but that still won't allow too long operations in the main loop because it has to keep running fast enough to keep the segment buffer filled

J-Dunn commented 8 years ago

Yes, I had a quick poke and set lower priority and removed the check on $$ : NVIC_InitStructure.NVIC_IRQChannelPreemptionPriority = 3;

Doing $$ during a simple axis displacement z5 ; z0 under load , caused some skipping. I have not had time to look into what was happening. I'm a little surprised that simple move like that does not get calculated in in one hit. I have not checked how many blocks are being created for the linear accel / decel sections.

Your comment suggests that time critical stuff in the main loop should probably be put in INT itself to ensure proper prioritisation of tasks. GRBL is obviously done in a very simple way suit the AVR target. This could possibly be done better now freed of that limitation.

I'm anticipating using some of the other features available on this platform so the motor control s/w needs to be protected and given an assigned priority, not just left hanging around scavenging for CPU time in a 'main loop'.

It's true that , as it stands, even a low priority serial output can interrupt the motor control software, not good. I'll have to give this some thought, later.

J-Dunn commented 8 years ago

Much of the problem here is the rediculously small RX, TX buffers that were needed to shoe-horn GRBL into the Atmel chip. These should probably be set to more useful values now there is some breathing space. 1024 bytes will take all the output from $$ , this can then be sent during motor movement, with the lower serial INT priority.

The block on $$ can then be released but possibly there should still be a check that the buffer is empty first, to prevent it jamming if the host is not processing it for some reason.

#ifndef TX_BUFFER_SIZE
#ifdef STM32
  #define TX_BUFFER_SIZE 1024  // $# = circa 280b ; $$ = circa 34x30b
#else
  #define TX_BUFFER_SIZE 64
#endif
#endif
langwadt commented 8 years ago

yes there is plenty of room for buffers and probably also room for improvement in how they are handled. Increasing the RX buffer will probably not do much unless you change the PC side too

J-Dunn commented 8 years ago

Indeed, the current char by char send is pretty clunky but effort spent making it more efficient probably would not yield a material difference to functionality. I'd rather see a situation where a dumb wait loop in serial send cannot end up impacting motion control.

J-Dunn commented 8 years ago

In the same vein, we can now increase line limit to the correct NIST length of 256:

cpu_map_stm32f411_nucleos.h #define LINE_BUFFER_SIZE 256

J-Dunn commented 8 years ago

Just to confirm, increasing TX buffer to 1024 allows doing an $$ command without disrupting motors. However, pasting two $$ lines causes a skip and motor stalling.

I have the following INT priorities set and the motor pulses should interrupt the serial output ISR STM32, so I can't see what is breaking the regular pulse train and causing the stall.


    NVIC_InitStructure.NVIC_IRQChannel = TIM2_IRQn;
    NVIC_InitStructure.NVIC_IRQChannelPreemptionPriority = 0;
    NVIC_InitStructure.NVIC_IRQChannelSubPriority = 0;
    NVIC_InitStructure.NVIC_IRQChannelCmd = ENABLE;
    NVIC_Init(&NVIC_InitStructure);

    NVIC_InitStructure.NVIC_IRQChannel = DMA1_Stream7_IRQn;
    NVIC_InitStructure.NVIC_IRQChannelPreemptionPriority = 1;
    NVIC_InitStructure.NVIC_IRQChannelSubPriority = 0;
    NVIC_InitStructure.NVIC_IRQChannelCmd = ENABLE;
    NVIC_Init(&NVIC_InitStructure);
  NVIC_InitStructure.NVIC_IRQChannel = Open_USART_IRQn;
  NVIC_InitStructure.NVIC_IRQChannelPreemptionPriority = 2;  //oder 0?
  NVIC_InitStructure.NVIC_IRQChannelSubPriority = 0;         //oder 1?
  NVIC_InitStructure.NVIC_IRQChannelCmd = ENABLE;
  NVIC_Init(&NVIC_InitStructure);

I send the $$ commands during the plateau section of a long z-axis movement, so any planned moves should be in place in the buffer.

I can't see why the pulse train is not stable and continuous.

langwadt commented 8 years ago

I had a look at it and the interrupt priorities looks correct, i.e. the stepper interrupts can interrupt the serial interrupt. It isn't an interrupt issue, as I can tell the stall is caused by the main loop not running often enough to feed the stepper interrupt. Adding a delay to the main loop will also cause the stepper to stutter

J-Dunn commented 8 years ago

Thanks for taking a look.

If I follow what you're saying, it's the serial output which has a interrupt priority and is thus starving the main thread. This comes back to my original idea that the block processing in the main loop needs to be interrupt driven too, and thus getting an appropriate and defined priority rather than just scavenging for free cycles.

That is part of my intent in messing around with trying to make $$ work. It's not such an important idea in itself but it should work. If it does not it means that the system has not been designed correctly. It's a test case.

There was not much choice on AVR but now we are working on more capable platforms it is time to get this coded in a more rigorous and engineered fashion.

langwadt commented 8 years ago

it isn't limited to the serial output or interrupts, anything that takes too long in the main loop will cause it, it kinda say so in the comment on st_prep_buffer();

Putting a limit on how long an operation you can run in the main loop is simple and efficient, changing how it works opens a whole new can of worms with things no longer being done in order and strings being cut in half

nsiatras commented 8 years ago

@J-Dunn Do you run it under a 411RE ?

wrljet commented 8 years ago

I have it running on an unmodified Nucleo 411RE. 115200 baud rate.

nsiatras commented 8 years ago

I am trying (really hard) to make it run on the Nucleo 401RE but I still get garbage data on the serial.

I think it is something that has to do with the clock. Any ideas?

nsiatras commented 8 years ago

Did you change anything on the code to make it run on the 411 or just build and upload ?

wrljet commented 8 years ago

I didn't change anything. In fact this is the first time I used the System Workbench. (I do have a lot of experience with other Eclipse based setups for the STM32, though)

I was going to try using the 401 board, because I already had that board. But felt lazy, and I wasn't in a hurry. So I bought a 411 from Mouser (in U.S.A.) and used that.

J-Dunn commented 8 years ago

yes, I'm using F411RE plus a stack of three L6474 boards, communicating with minicom on linux.115200 bd These boards expose two USB devices via the same cable, one for the debugger one for virtual comm port serial output.

nsiatras commented 8 years ago

I want to use the protoneer GRBL shield. So I guess I have to get a 411RE and run the code on it.

wrljet commented 8 years ago

Nikos, I dug out my 401RE board, fiddled with a few settings in the project, and it does run. I only tested the interaction to the terminal window. Didn't try to run any motors.

I edited the defined symbols, told it to use the other chip, and changed the RAM aspects of the linker file. -Bill

nsiatras commented 8 years ago

Hello, and thanks for the reply. Can you please send me the binary and/or the changed files. I'm trying for two days now to make it work.. :(

wrljet commented 8 years ago

Nikos, Here's what I changed. This was done quickly without considering carefully what I was doing. And I am not an expert. So keep that in mind.

In menu Project -> Properties Expand C/C++ Build in the left panel Under that click on Settings

Then in the main panel, Target tab, change MCU to STM32F401RETx and change Board to NUCLEO-F401RE

Click on Tool Settings tab, and inside there click on Symbols under MCU GCC Compiler I changed those so the CPU specific ones were for 401 instead of 411: NUCLEO_F401RE STM32F401RETx STM32F401xx

Again, I don't know if this is "correct" but it worked for me. Apply and close that properties dialog.

Then in the Project Explorer on the left, expand the project and open file LinkerScript.ld to edit it I didn't remember how much RAM the 401 had vs the 411, so I just guessed 64k. I was too lazy to look that up.

I changed: _estack = 0x20020000; /* end of RAM */ to: _estack = 0x20010000; /* end of RAM */

And I changed: RAM (xrw) : ORIGIN = 0x20000000, LENGTH = 128K to: RAM (xrw) : ORIGIN = 0x20000000, LENGTH = 64K

Save that file.

Then do Clean Project, and Build Project and you should be all set.

Bill

nsiatras commented 8 years ago

Hello Bill,

I made the changes you suggested and I managed to build it! I added the protoneer shield and I managed to turn motors but motors turns too slow.

Now I think I have to figure out what F_CPU and COUNTS_PER_MICROSECOND (inside delay.c) must be.

Thank you!

langwadt commented 8 years ago

the 401 should run at 84MHz so F_CPU should be set to 21000000 and COUNTS_PER_MICROSECOND I expect 27 or 28 it is not that critical

the 401 has 96KB ram so,

_estack = 0x20018000; /* end of RAM */

RAM (xrw) : ORIGIN = 0x20000000, LENGTH = 96K

nsiatras commented 8 years ago

Hello,

It works now ! The only problem is that the acceleration seems to be a bit different than the original GRBL.

I set the "x max rate" to 4000mm/min and the acceleration to 250mm/sec^2 and it goes slower than the setup I had with an Arduino Due and the Original GRBL.

Is this a timer issue ?

nsiatras commented 8 years ago

I just changed the line 1082 of stepper.c from TIM_TimeBaseStructure.TIM_Prescaler = 3; to TIM_TimeBaseStructure.TIM_Prescaler = 2;

and now works a bit faster.

langwadt commented 8 years ago

it could be that the CPU isn't actually been setup to run at 84MHz and istead runs a some slower clock

try timing a dwell (G4) of a few seconds, if COUNTS_PER_MICROSECOND and time i way off the clock is wrong

else try timing a movement with a slow feedrate (so acceleration doesn't matter) and see if the mm/sec and time makes sense, if not it indicates the timer setup is wrong

you shouldn't have to change the timer settings

I think I found the problem, try adding the definition of the symbol HSE_VALUE and set it to 8000000 then change line 371 in system_stm32f4xx.c from :

define PLL_M 25

to

define PLL_M 8

nsiatras commented 8 years ago

I set the speed to 1000mm/min and made the machine move for 1000mm. It stopped after 1:34" so it was 34" late...

I am doing the changes you suggest and tell you.

Thank you very much!

nsiatras commented 8 years ago

Now everything is good! I changed the PLL_M to 8 and everything is fine!

Thank you very much for the help. I really appreciate it!

langwadt commented 8 years ago

Glad to hear you got it working, the STM library code can be a bit of a mess with these kind of details

wrljet commented 8 years ago

Yes, these chips and the libraries are awfully complicated.

Nikos, good show getting it to run. Bill

nsiatras commented 8 years ago

It was my first "touch" on the STM32 but you are right, they are complicated a lot.

Thanks again for your help Nikos