gin66 / FastAccelStepper

A high speed stepper library for Atmega 168/328p (nano), Atmega32u4, Atmega 2560, ESP32, ESP32S2, ESP32S3, ESP32C3 and Atmel SAM Due
MIT License
282 stars 67 forks source link

Fix attempt for https://github.com/gin66/FastAccelStepper/issues/250 #252

Closed SHWotever closed 2 months ago

SHWotever commented 2 months ago

On quick direction changes a direction toggle could be missed. My understanding is that this code branch "restarts" the motion right at a stop event. But the direction change would not be applied either doing a few steps in the wrong direction, or if a large move was initiated at this time a much more impacting movement in the wrong direction.

If it's motion restart it feels correct to toggle direction before that ultimate step, but i'm not 100% sure. Since the direction step is now occurring in the ISR multiple times I moved it as a macro like the others stepper unitary actions for readability.

gin66 commented 2 months ago

I am still wondering: If this problematic branch is triggered by a pause with direction toggle requested, then this toggle will still be lost. Perhaps the toggle should be before the test: if (e->steps > 0) ?

This would explain, why the direction delay has not solved the problem.

SHWotever commented 2 months ago

I feel like since it's a motion "restart" (it was about to stop but it was restarted right away from my understanding). It could be harmless above, if the queue processing had requested a toggle it must be applied. This morning I did further testing, and this solution appeared to be the one keeping both steppers perfectly in sync (meaning that "moves" were consistent on both steppers) the second solution of applying first the final pulse and then switching direction seemed to introduce a slight drift at the end of a long session which makes me think those steps must be applied in the newest direction. That's not done with exact unit testing unfortunately so I can't tell how really accurate is that observation.

I can try to move it up. And test, looks like my test case is really good to see any bad choices in that matter :D