deadsy / grbl_stm32f4

The grbl g-code interpreter ported to the STM32F4
61 stars 33 forks source link

double use of USB #1

Open J-Dunn opened 9 years ago

J-Dunn commented 9 years ago

Nice effort, this will be most useful.

I quickly built and flashed this GRBL port on Fedora ( I also did the modprobe quirks config. )

Hopefully it will not be too difficult to port it for a more recent GRBL.

deadsy commented 9 years ago

I quickly built and flashed this GRBL port on Fedora but I'm only seeing one usb device: /dev/stlink1 ( I also did the modprobe quirks config. )

There are 2 physical ports on the disco board. One is connected to the micro-controller running the st-link debugger. The other is connected through to the USB-OTG port of the main controller. You should see a USB CDC device show up on the USB of the main controller.

Having said that - I think the USB driver code that is part of the SDK is buggy - it work, but it's not reliable. It would freeze up in the midst of a long stream of gcode. One day I may dig into it and work out what's going on - or not.

The UART code works much better- I haven't had any trouble with that. It's less convenient in so far as you need to wire up a level converter- but once you've done that it gets the job done.

J-Dunn commented 9 years ago

Thanks for the reply. I realised after posting that I had not changed the #define to activate the USB-OTG and was a bit mislead by the re-enumerations thing producing two devices for the STLINK port. I thought that was the second USB port.

I cropped my comment down once I realised but you seem to have taken a copy to reply to.

This really wants to be updated to a recent >=0.9g GRBL : apparently there are major performance improvements. But it looks like you've broken the back of the task, so that's a great help.

I did have a report from someone else using this board about the CDC not giving the speed it should when streaming data ( not by a long way ) this may be the same issue. It's a little disappointing that someone with the resources of ST can't write and test this sort of thing for their own hardware and get it right.

Anyway, these boards are a lot of bang for your buck and open to door to doing some more interesting stuff with GRBL that would not be possible on AVR.

deadsy commented 9 years ago

This really wants to be updated to a recent >=0.9g GRBL : apparently there are major performance >improvements. But it looks like you've broken the back of the task, so that's a great help.

You'll notice that the changes to the grbl source that I've made are either patches or file overlays- so the set of things that need to be changed from the original source are clearly specified.

The biggest changes is the use of STM32 hardware for stepper pulse generation. Other than that the grbl code is reasonably platform independent... after a bit of hacking.

J-Dunn commented 9 years ago

Yes, I'd had a look at the way you'd done it, very helpful for updates. There has been a significant rewrite from 0.8 to 0.9 but most of it will not concern what you've done in porting it.

Hopefully allowing some fuzzy matching for the patches will get most of it through.

I've made quite a lot of changes to GRBL myself to accommodate the SPI driven L6474 stepper drivers. Looks like time for me to roll up my sleeves and get more familiar with the STM32f4 family h/w. I had not intended to get deeply into it this soon but thanks to what you've provided here I may get straight into it.

J-Dunn commented 9 years ago

What platform did you test the OTG on ? The other person that I'm aware of having problems with this was using Windows, so part of the problem could be with the host OS driver, rather than the STM code.

USART is more direct but I'd like to avoid cluttering things with a level shifter and an extra plug if possible. Is this likely to be a major effort to debug?

I have not done any testing on this myself yet, can you suggest a test case that will provoke the bug?

Thanks.

deadsy commented 9 years ago

What platform did you test the OTG on ?

Linux.

USB Issues... Is this likely to be a major effort to debug?

You'd need to understand the USB hardware quite well. I don't think the SDK code is trustworthy- so you would need to either debug that driver or write your own.

can you suggest a test case that will provoke the bug

If you do a serial loopback in the code and drive a lot of characters through it, it will probably lockup.

SPI driven L6474 stepper drivers

How's that working out? Grbl assumes step/direction pins going to the stepper driver....

btw - I started a port to 0.9j on the "development" branch

J-Dunn commented 9 years ago

Thanks, I noticed the recent changes last night. No sense in my working on that and duplicating effort.

L6474 is basically working subject to further testing trying to break it. It is still step / dir driven but does config and status reporting via SPI. It produces a h/w error signal from a variety of conditions from a non critical temp warming to over-current or over-temp conditions that will have already taken the chip output into HiZ state, requiring emergeny stop on all axes.

The flag interrupt requires getting the status word from the ( three ?) drivers , I got that to under 19us on a Uno for a stack of three. There is then a little overhead to compare to a mask of bits that represent the critical error conditions and do the equivalent of a limit switch input in GRBL or perhaps a soft stop / feedhold for a temp warning that does not lose motor positions.

This could add something like 20us jitter to the core interrupts, it may require reducing GRBL max of 30kHz to 20 or 25. This is where some stress testing is needed. I used INT0 which is not ideal but I did not want to completely reorganise core interrupts since Uno was just a stepping stone to putting all this STM32 anyway.

This OTG thing sounds like something that ST ought to fix. Selling discovery boards for their products which demonstrate that they don't work and that ST can't write a USB driver for thier own h/w is something I think they would want to attend to.

J-Dunn commented 9 years ago

Is there anything testable in development yet, or is just stubs?

deadsy commented 9 years ago

The dev branch builds clean and the host facing serial port stuff works properly. The stepper motor driver is a WIP. I wouldn't expect that pulses get generated correctly at this point.

J-Dunn commented 9 years ago

Many thanks for the clarification. There was a considerable reworking of the core code ( which is mainly in steppers.c ) between 0.8 and 0.9 , so your patches probably won't apply cleanly. However, I would not have thought many of the changes would be relevant to porting to STM32, other than what you have already done.

deadsy commented 9 years ago

They have actually made it a bit easier in 0.9j by moving the avr includes to a single file and splitting out the step and direction variables. Still - stepper.c is the tricky one. It's intertwined with avr timer code and has to be understood before it can be changed.

J-Dunn commented 9 years ago

It fairly simple but obviously does need understanding before much can be done. It's also very simple in it's use of timers, which should be readily portable.

It uses one main timer to raise the step pin, its ISR triggers another timer whose ISR drops it. A third timer is used to provide an optional delay before the step signal rises. ( Don't recall why , that was a recent extra to cover some external requirement.). No PWM

Some of the variable names are a bit confusing. I've made some recommendations for changes. There was one that was called a mask but which was actually a collection of pin numbers which needed shifting and ORing to create a mask. I'll look out the details later if it may be of use. No time now.

J-Dunn commented 9 years ago

config.h

// Creates a delay between the direction pin setting and corresponding step pulse by creating
// another interrupt (Timer2 compare) to manage it. The main Grbl interrupt (Timer1 compare) 
// sets the direction pins, and does not immediately set the stepper pins, as it would in 
// normal operation. The Timer2 compare fires next to set the stepper pins after the step 
// pulse delay time, and Timer2 overflow will complete the step pulse, except now delayed 
// by the step pulse time plus the step pulse delay. (Thanks langwadt for the idea!)

The STEP pin is controlled by one free-running timer that fires another single shot timer. Pretty trivial.

deadsy commented 9 years ago

A stepper controller (Eg the G540) generally specifies a minimum delay between any changes to the direction pin and the following step pulse. That's why the delay is needed. On the STM32 I've setup a timer whose period matches the desired step pulse period. Then I use two CCR registers to generate an interrupt at the end of the delay period, end of the step pulse and the end/start of the step period. I assume I can use the same scheme for 0.9j even though they have changed the way they are calculating the dynamics of the stepper movement. See board/timer.c

J-Dunn commented 9 years ago

Of course, I was forgetting what that was for. On AVR, it is slow enough that the few clock cycles needed for the preceding line provides enough delay. This is much more likely to be needed on STM32.

J-Dunn commented 9 years ago

deadsy, there is someone from ST asking for specifics on the OTG issues, if you'd like to get them to fix this, it may be a good time to provide some input: https://my.st.com/public/STe2ecommunities/mcu/Lists/STM32Java/Flat.aspx?RootFolder=https%3a%2f%2fmy.st.com%2fpublic%2fSTe2ecommunities%2fmcu%2fLists%2fSTM32Java%2freliability%20of%20OTG%20USB%20on%20stm32f4%20Disco&FolderCTID=0x01200200770978C69A1141439FE559EB459D758000F9A0E3A95BA69146A17C2E80209ADC21&currentviews=11

J-Dunn commented 9 years ago

Hi, no response from ST on this yet, so not holding my breath. However, I did find this which sounds like it may be the same root cause. Original poster seems to have pinpointed the problem. What do you think?

https://my.st.com/public/STe2ecommunities/mcu/Lists/STM32Java/Flat.aspx?RootFolder=https%3a%2f%2fmy.st.com%2fpublic%2fSTe2ecommunities%2fmcu%2fLists%2fSTM32Java%2fSTM32Cube%20USB%20CDC%20%28possible%29%20Bug%20Found%20-%20Rx%20Tx%20Race%20condition&FolderCTID=0x01200200770978C69A1141439FE559EB459D758000F9A0E3A95BA69146A17C2E80209ADC21&currentviews=172 BTW are you still working on this port or is it on a back-burner for you now?

I'm still managing to shoe-horn everything in to arduino and working on the mechanics. STM32 will be next step once I have a mechanically rigid proto with tree axes.

deadsy commented 9 years ago

That might be the same issue as I have seen.

are you still working on this port or is it on a back-burner

At the moment the port to 0.9 is not getting any cycles. Perhaps in the next few weeks I'll get the motor driver done.

J-Dunn commented 8 years ago

reliable USB-OTG seems to be a key problem on STM32 using the maker's HAL. However, there is quite a bit of action on this at Chibios which has its own, independent HAL. Their HAL wrapper on RTOS is not important here but the underlying, low level USB code should be of use.

deadsy commented 8 years ago

Thanks for looking into this. I'll try to get the 0.9 motor driver working in the next week or so. I'll revisit the USB issues and see if some of this info/code can be used to help the USB CDC reliability. Failing that, perhaps some simple code to reproduce the problem might get ST to look at it.

J-Dunn commented 8 years ago

I have not had time to test it yet but they seem have some competent guys on this at chibios. It should be relatively easy to locate the relevant, platform specific code, all the RTOS stuff is abstracted out to a higher level.

I really don't have any hope of ST getting their code together.

A USB stress-test would be useful in a number of contexts. I would have thought that the first thing to do with a USB driver is see whether it can deliver USB data rates for a sustained period without falling on its arse.

It seems there is far too much stuff going out with little more than a "works for me" test.

Chibios seem to be testing on a variety of end-points like MSD , audio and video so their base code must be fairly solid.

There is a test using MSD on USB-OTG port and requiring a serial connection for output information. Since you already have that to replace the broken ST USB code, it should be plug and play. http://forum.chibios.org/phpbb/viewtopic.php?f=16&t=2968#p23036

J-Dunn commented 8 years ago

I've just come across this project. It may be an alternative to the ST stack. https://github.com/libopencm3

Anyway I've stuck a 3232 driver on this now, I've spent enough time chasing buggy CDC implementations. This is not ideal since I wanted to run the hardware with a laptop but in some ways a true serial port is better: solid screwable connectors and possibility of hardware handshaking. It makes the whole thing a bit more robust.

Was there an important reason not to retain GRBL single shot pulses and move to using PWM?

I would have thought that on STM32 it should be possible to do step by step accelerations curves. One of the limitations of GRBL is the simplistic rectangular acceleration profiles. When stress testing GRBL it nearly always drops out when going from zero accel at max pulse rate to max deceleration.

One of my major reasons for porting to something more capable than Arduino is do better acceleration curves.

J-Dunn commented 8 years ago

Just a quick note: it seems like your editor may be configured to remove trailing space at end of line. This is producing a lot rather spurious patch volume. eg 260-limits.patch is quite large but at least half seems to be just removing one or two spaces.

This makes it quite hard to follow what has really been changed in the code.

Anyway it's a good start to porting. Since you don't seem to be having much time to continue with 0.9 , I'm going to start digging myself. The patches will certainly be a great help.

deadsy commented 8 years ago

It may be an alternative to the ST stack.

I'm not a big fan of the ST cube code - It's better than what I've seen from some hw manufacturers but its still not written very well. Too many layers of junk to deal with all of the device variants. They need to learn how to inline and/or use macros. Maybe the opencm3 stuff would make a better hal.

solid screwable connectors

You can get screwed USB connectors - but yes. A traditional UART has admirable simplicity and just works for this application, which is what you want.

Was there an important reason not to retain GRBL single shot pulses and move to using PWM?

It's not PWM. It's just dir/step. It may look like pwm on the scope, but that's neither here nor there to the stepper controller since it's only looking at edges. A minimum size pulse for step would work just as well, I just found the end of period interrupt a more convenient place to pull the step line low.

better acceleration curves

The motion planning code is probably some of the trickier stuff. Fortunately it's platform independent. It might be instructive to take a look at the firmware used on some of the 3d printers (marlin, reptier, etc.) They have a similar problem and they may have some interesting approaches.

remove trailing space at end of line

Sorry. I generally try to make patches minimal.

J-Dunn commented 8 years ago

Sorry about the PWM comment. I had a closer look at your code yesterday and it seems that you are taking a similar approach as GRBL. I'm still reading the details of stm32fxx ref manual to understand what these "channels" are about. I've found there is a "one pulse mode" for some timers that seems to closely fit what is needed.

Yes Marlin and tinyG implement variable acceleration ( unlike GRBL's all or nothing approach ). There is also talk of "S-curves" in Smoothieware which is the same thing. It applies constant rate of change of accel. ie third time derivative of position or "jerk" in tinyG. Motor speed is a parabolic ramp instead of a linear one. This is quite interesting since triangular ramp, with zero constant speed can become something close to a sinusoid, which would be mechanically smoothest.

Mechanically this means a constant rate of change in accel force/torque instead of hitting the hardware with a hammer blow to get it moving. The price for less violence is a longer A to B path time. On the plus side it should reduce tendancy to miss a step and drop out.

I've noted that GRBL tends to fail when going from const speed ( ie max step rate ) into max decel when it gets hit by the "slow down hammer" blow. Even moderate rounding of the transitions should be beneficial.

That is what I want to do once I've got a 32bit platform.

ChibiOS does not use Cube other than the header files for the constant names. It has its own HAL. As such there may be something that is better done in the new USB stack that I pointed to. Their standard USB-CDC also seems dubious.

Another advantage of RS232 is hardware handshaking. Stopping the sender quickly enough when the buffer is full seems to be a problem for USB-CDC. Is this related to the bug you were seeing?

J-Dunn commented 8 years ago

BTW have you ever done anything with openCM3? It looks interesting

https://github.com/libopencm3/libopencm3-examples/tree/master/examples/stm32/f4/stm32f429i-discovery/usb_cdcacm

I have one of these boards if you would like to come up with a test case for the USB breakage, I can try it.

J-Dunn commented 8 years ago

Jason, I've just looked at the STLINK upgrade from v2 to v2.1 , the Release Notes contain this comment:

http://www.st.com/web/en/catalog/tools/PF260217

Removed risk of lockup when using the Virtual Com Port intensively in both directions
(Rx/Tx) simultaneously

Now this is rather a strange comment since v2 does not provide a virtual com port interface on the STLINK, so it would seem that it can only be referring to the OTG port.

Does this imply that STLINK was somehow interfering with timing of the OTG port. I'll hold off on doing the update if you can come up with a test case.

What STLINK firmware do you have ? This is visible in the USB prodID: It is the STLINK version , not the board : 374b is v2.1 ; 3748 is v2