Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add configurable delay between DIR change and STEP #83

Closed
gin66 opened this issue Sep 12, 2021 · 11 comments
Closed

Add configurable delay between DIR change and STEP #83

gin66 opened this issue Sep 12, 2021 · 11 comments
Assignees

Comments

@gin66
Copy link
Owner

gin66 commented Sep 12, 2021

Great stuff. Thanks a lot. A pleasure to read your code and comments. I have added a few comments to the code. Please have a look at it.

For the DIR pin delay. As there are several places, where you have issued a 30 microsecond delay, better to move this to a higher level, while enqueueing commands. At that place, the delay can be easily implemented with a pause performing the dir change and latter step commands do not need to change the DIR-Pin. Additional benefit, the API can be extended to add a configurable delay (>MIN_CMD_TICKS) or turn it off.

Later I will change it accordingly and remove those delays in the sam architecture.

Originally posted by @gin66 in #82 (comment)

@clazarowitz
Copy link
Collaborator

Just to clarify, the delay isn't exactly because of the architecture other than the fact that the speed, I suspect the ESP would have the same issue on this junky stepper driver. (Its a "TB6600" (probably fake) from amazon where I got 4 of them from amazon for $30. Definitely not the highest quality drivers! I can't find the page online anymore, but I found someone having similar issues who recommended a 30 microsecond delay, so I went with that and my problem went away. Even the better drivers on my mower, CLA86T hybrid drivers say they need a minimum of 5 microseconds from setting the dir pin to outputting a step! I put #ifdef ARDUINO_ARCH_SAM around it just to really call out that it was my code, not because of the specific architecture. I do not remember what I saw on the scope for direction change signal vs steps, but it was quite fast, fast enough that I knew I needed a delay for the junk microcontroller, and for some reason I thought it would be necessary on the CLA86T's. Either I thought it too needed 30 microseconds, or the code was running within 5 microseconds. Not sure, but microseconds sounds crazy....but then, 30 is pretty fast too! :) Either way, I think you're right that the correct way is to make it a higher level function and make it configurable so any junky stepper driver is able to be made to work on any platform!

gin66 added a commit that referenced this issue Oct 7, 2021
@gin66
Copy link
Owner Author

gin66 commented Oct 7, 2021

Is implemented in pre-0.24.0 and will be released after test.

@gin66
Copy link
Owner Author

gin66 commented Oct 7, 2021

@clazarowitz Please check on your side with SAM due, if this delay works as expected for your needed delay

@clazarowitz
Copy link
Collaborator

I have most of my test setup torn down, so it will take a bit to build it up. I'll be working on it tonight and this weekend.

I have been running the previous version on my mower project, and I did drive it via Radio control routed through the autopilot, so, it seems like it was working well so far! The only reason I'm not calling everything done is I had hope, but not really believed, that I would "zero" the sticks by using the "disable" position on the mower. Unfortunately, they still have some slop, so I need to put some sensors on the mower to sense the position so I can trim the throttle, and get it all working. I have printed circuit boards that hopefully will still be delivered today!

@clazarowitz
Copy link
Collaborator

So, it took less to get back running than I thought :) One immediate issue was the
define MIN_DIR_DELAY_US (MIN_CMD_TICKS * (TICKS_PER_S / 1000000)) should be
define MIN_DIR_DELAY_US (MIN_CMD_TICKS / (TICKS_PER_S / 1000000))

Its now doing something quite odd though, I'm going to have to get my scope back up here (instead of by the mower :) ) The stepper jumps a bunch of steps then moves. Do you expect that I would have needed to modify the ISR?

I'll look more into it a little later :)

@gin66
Copy link
Owner Author

gin66 commented Oct 17, 2021

Thanks for the correction in regard to MIN_DIR_DELAY_US. The fix is updated in the latest master version (not yet released/tagged).

Based on your finding, I too have taken out my test set up and checked on esp32 and avr. In order to do this, I have extended the StepperDemo to allow the configuration of the direction pin. During testing I have found another phenomenon: If auto enable delay is used, the direction pin change delay is not applied. Actually this could be ok, if the enable time is larger than the direction pin change delay. But if the enable time is smaller, then the defined direction pin change delay is not guaranteed. Even though in real life enable time should be always much larger than the direction pin change delay, I have fixed this behavior.

What I could not reproduce is this "The stepper jumps a bunch of steps then moves.". Neither on esp32, nor on avr.

Neither for avr, nor for esp32 a change in the ISR is required. Rationale is, that the direction pin change delay is just an additional command (pause) issued to the stepper queue. Consequently, no change in ISR is required.

If all works, then in the SAM code the microsecond delays in the ISR and commandAddedToQueue() after direction pin changes, can be removed and in the application, the direction pin configured appropriately.

BTW: In your ISR the lines 353-357 are duplicated in 393-397.

      if (e->toggle_dir) {                                                     \
        e->toggle_dir = 0;                                                     \
        *q->_dirPinPort ^= q->_dirPinMask;                                     \
        delayMicroseconds(20);                                                 \
      }    

Without deep insight in your code, I would guess that code should be before line 376 and only there. And then e->toggle_dir = 0; is not needed anymore. Please check, if my understanding is correct and the proposal is valid/useful

@clazarowitz
Copy link
Collaborator

Sorry it took me so long to get back to this....I have far too many projects :)

I did get my scope connected and the problem is pretty obvious, in fact, it wouldn't have warranted a comment, I apologize :) I haven't found the code mistake yet, but I know 100% its my code. Something is turning on the pulse generator early. So the lurching is trying to run at a pretty good speed without acceleration. Famous last words here, but I think I should be able to fix this pretty quickly.

@clazarowitz
Copy link
Collaborator

Ok, I got it to what I think is fixed....the problem was that in startPreparedQueue, I just always assumed it would start with a pulse :) I actually didn't make that mistake elsewhere, or at least corrected it. It also exposed an issue in the PWM ISR...a missing line, which would cause an oscillating missed pulse. Depending on my time, I may run the long test on it tomorrow. If not, I think it will be this weekend. Just depends on how my "day job" goes.

For a fun update on the mower, I actually drove the mower using this code! I had Radio Control for an RC Quad Copter, which was routed through the ardupilot hardware, which generated servo pulses which the due read synchronously, then generated the stepper pulses for the given servo signal and current stepper position. Unfortunately when I went to tune the throttle PID loops, my fears were realized - I'd need limit switches on the mower stick positions so it would repeatably return to "0" every time, which would allow adjusting trims to get the wheels moving at similar speeds for similar throttle outputs. I had some PCB's fabricated....then refabricated since the PCA9615 is nowhere to be found, and not available until December of 2022! So I redesigned on the PCA9616 - Differential I2C transceiver, and a Sil7210 hall effect sensor. This did require drilling....into a stainless steel part, so no paint damage, so its sort-of acceptable. The magnets will need to be epoxied to the mower body. No big deal there. I actually found a SiL7210 library online, which I "arduinoified" (It was C, so I put a C++ wrapper around it) and was able to detect my magnets, and its very repeatable. Next step is to write the initialization code which will move the sticks to their extents, finding the magnets via the hall effect, then return to a 0 position, and begin to accept servo commands. Not much work...Mowing season is basically over though, so most testing will likely happen next spring :)

@gin66
Copy link
Owner Author

gin66 commented Nov 6, 2021

Thanks for your update. I am quite interested in your mowing project. This mower is remote controlled and the arduino translates servo commands into stepper motor control ? And the steppers drive the wheels directly or adjust the mower controls for its motor ? And this mower is how big ? Wonder if you could upload a video on youtube, so it is better to understand, how your setup is working.

@clazarowitz
Copy link
Collaborator

Sorry about the delay. Somehow in my mind, I had already pushed the code after running the long endurance test. I realized today that was not the case :)

For the mower, it will actually be autonomous using the ardupilot autopilot software and the PixHawk4 autopilot hardware. (Both open source).

You are correct, the Due is taking in the servo pulse width and translating that to a stepper position and commanding the move with this library. The remote control I mentioned happened in two phases. The first with just a regular Spektrum receiver, an arduino mega, this library, and an updated faster/more accurate PWM capture routine I wrote. The second, and most recent iteration which I ran with the Due code was the Spektrum receiver was connected to the PX4 autopilot, which sent out the PWM servo signals (with some smoothing and acceleration rate controls) to the Due, which then measured the signals using hardware timers, and again translated to stepper signals. The Due was necessary because the autopilot sent out synchronous PWM signals. For large differences in pulse width, this was fine. For equal length pulses one would get about 12 microseconds added to its pulse width, and for signals differing by less than 12 microseconds, they'd get 12-(difference) microseconds added to the pulse width. I wasn't happy with that level of imprecision so I used the due with hardware timer registers. The 84MHz clock speed and 32 bit ALU means I still get both pulses measured and accounted for in less time than the Mega could do!

The mower is a 1.4m cutting width gasoline powered "zero turn" lawnmower. It has two controls that control 2 hydraulic motors, one on each rear wheel. The front wheels can rotate in 2 axes freely, allowing them to follow the control of the back wheels. Its called "zero turn" because inputs equal in magnitude and opposite in direction will result in the mower spinning with a turn radius of zero.

The steppers drive levers on the hydraulic valves. So it is still currently gasoline powered. I do not think I could make it all electric without significant modification. My goal with this is to have 0 or well, near 0 modification to the original mower, in fact, I want it to operate manually the same way it did without the autopilot present. Once that works, I may see about designing an electric mower from the ground up. I have an old video of the original radio controls https://youtu.be/Qrap6iHbRHM but nothing since then. It is incredibly difficult to control because the mower has no suspension, and I have not modified the control that requires a person to be sitting on it, so as the mower bounces, so do your thumbs on the sticks! My control through the autopilot was a bit more clumsy than this. Looking back, I've made a lot of progress since this video :)

I'm still working on the limit switches. I didn't realize all of the SiL7210's with identical part numbers had identical I2C addresses. For a quick fix, I got a different part number. (but it doesn't use an open drain interrupt pin...) Long term, I'll be designing around an I2C switch in between the PCA9616s.

I'll be writing the initialization code tonight. In fact, that is what reminded me I hadn't uploaded the due changes! Once I get a reliable "0" position, I think I should be able to put it under autonomous control. I also have some and/or gates I'll be using to replace the built in version, and allow another input - autopilot - to override the seat sensor. I think the original mower circuitry is making clever use of relays as logic units for this purpose!

@gin66
Copy link
Owner Author

gin66 commented Nov 14, 2021

Thanks for pushing the changes to the repository. With your changes integrated just have released version 0.24.0.

Thanks for sharing the information and the link to the cool video. Now I have a much better understanding, which size of lawn and lawn mower, you are talking about. Hope can add in the future a video link in the Readme, with your lawn mower running in autopilot and with FastAccelStepper in use. That would be great. As the mowing season has passed, perhaps in spring ?

@gin66 gin66 closed this as completed Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants