Closed Sebastianv650 closed 5 years ago
I've reverted the PR.
I'm starting to think that Scott's idea of using fixed point rather than integer math is a better approach. The real issue is loss of precision in the deceleration_time variable due to integer math truncation.
I've reverted the PR.
👍
I'm starting to think that Scott's idea of using fixed point rather than integer math is a better approach. The real issue is loss of precision in the deceleration_time variable due to integer math truncation.
I have to say that's not the case. I rebuilt the iteration chain the acceleration / deceleration stepper loop is doing and also with all the precision Excel can offer I get nearly the exact same result that Marlin spits out on serial. It's so easy once I recognised it: We are doing an iteration, no exact calculation here! The iteration steps are the time steps. This means at each step loop "point" Marlins says: Let's assume the step rate is constant until the next step loop, and the next step loop should happen at 1/steprate. Then we calculate the new step rate based on this time as elapsed deceleration time. But in reality, this time will be always bigger than real deceleration elapsed time and on acceleration it will be always lower <- that's where the error is comming from.
As long as the step rate is high, meaning the time between pulses is low, we are iterating at a high resolution therefore the error from step to step is low. At end of deceleration when step rate becomes small and the time frames therefore bigger, the iteration resolution becomes awful low also. Therefore the speeds differ quite a lot between real deceleration curve and Marlin one within the last few steps.
That's also easy to prove if you don't know about iteration at all: In Excel, instead of iterating at 1/steprate intervals, increase the resolution for example by 100 by iterating at 1/steprate/100. Of course the amount of needed step loops is then 100 times bigger for the same end speed. But now, the delta between real end speed and Marlin calculated one is much smaler. You can increase this iteration resolution as high as you want, only limited by the row number in Excel. At 200x Marlin resolution the end result becomes quite close.
All this is done to avoid a square root in every step loop as this would be the only possible way to get the real step rate at each decel. point: step_rate = SQRT(start_step_rate^2 + 2 acceleration_rate deceleration_step_number)
As long as we use this iteration, we can't improve the accuracy. It's not a question of float or fixed point math. It's just a question of the mathematical way Marlin uses to get fast ISR loops.
Here is a screenshot of real vs Marlin deceleration, maybe it helps for understanding:
One last thing for now, here is a good (not too long and understandable) description of acceleration ramp generation which also shows the deviation described above:
@Sebastianv650 Thank you for your very thorough coverage of the issue.
While "right now" is never the best time to reform the planner, I think it would be worthwhile to re-think the way the planner works — perhaps even re-invent it from scratch — and see if we come up with something more complete and accurate than what we have now.
And/or write out the "perfect" code, using as much floating-point as it requires, and then afterward see what we can do to pre-calculate and optimize it.
@thinkyhead as a physical correct approach needs more calculation power, maybe the best would be to wait until 32bit boards are more common? When 8bit is no longer something that has to be considered, there is quite a lot I would change, like:
Calculate trapezoids only when the block gets executed and not everytime something is changed in the buffer.
Not sure I see the difference, since "changed" always means "added."
I mean the following: When we have a new block which is added to the buffer, we calculate a trapezoid assuming it's final speed will be 0. Then we add another block. Due to it's length, we can now say the final speed of our 1st block is for example 10mm/s as we could decelerate to 0 during the last segment. So we calculate the trapezoid again. Let's add another block, until we reach a length (I think the propper term would be nominal length) where we have enough length inside the buffer so we can decelerate from nominal speed to 0. This is the last time we need to recalculate the trapezoid for 1st block added, and for all next blocks until their nominal length is reached.
When I remember correctly, current versions of GRBL don't do this any more. They only do reverse and forward planning, but the trapezoid is calculated "on the fly" just before it gets executed. Saves a lot of "unnecessary" calculations, but the system has to be fast enough to calculate the trapezoid in time without creating a jerky movement.
It may be worth a try to borrow the Grbl planner in its entirety and see what it does for us.
missing label
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
This is a follow-up of #9174 where I fixed a part but not all of the issue. At the moment (and I guess since early grbl days), Marlin isn't reaching the final step rate at end of block in all cases. There are three things to mention:
Keep in mind that until this is fixed:
I will do tests where we the timer / step rate calculations differ from ideal values next.