[LinuxCNC/linuxcnc PR#380] 2.7 limit3 constraint violation fixes About GRBL HAL

Issue #380 | 状态: 已关闭 | 作者: zultron | 创建时间: 2017-12-15

This set of patches fixes #378. The patch that actually fixes the acceleration blips is 2e741d5.

The second and third patches from top go on to fix the max/min limit overshoot issue (no more fudge!) by fixing the uniform acceleration math to better predict when to start decelerating and by fixing a problem where the output attempted to jump onto the limit position too early, and had to overshoot the following cycle to avoid acceleration violations.

The top patch fixes another problem following those fixes. When we always use maximum positive or negative acceleration, sometimes the output never gets both close enough and slow enough to the limit to lock on. That patch teaches the output to use some finesse to slow down and move close enough to get a lock within a couple of periods.

One more possible improvement would be to adapt these same techniques to follow the input better. I have a branch with that all working except for the analog of the top patch here. The way it is now (both the merged version and this one), with the crude max pos/neg acceleration, there can be some oscillation as it tries to lock onto the input. In our test cases, that oscillation is less than a tenth percent, IIRC.

This needs to be proven with a bunch more testing. I hope @samcoinc is up for it! If we find a lot of problems that we don’t want to fix, we can always punt and go back to the simple fix in 2e741d5.

Thanks, @SebKuzminsky for the comprehensive test cases. I always write tests first and develop from there, and it’s sure nice to have all that groundwork already completed before I even start.

#1 – zultron 于 2017-12-15

Ugh, I wasn’t paying attention to the base. I’ll rebase this stuff tomorrow.

#2 – samcoinc 于 2017-12-15

I have pulled your 2.7-limit3-constraints-lcnc378 branch and started testing this morning. No vel or acc errors in initial testing. Yay! Great work!

#3 – SebKuzminsky 于 2017-12-15

@zultron this looks really good to me.

I edited the PR to change the destination from master to 2.7. Do you still want to rebase before merging?

#4 – SebKuzminsky 于 2017-12-15

Hmm, this branch fails the old limit3.* tests (large steps in input with different limit3 constraints).

Looks like the new limit3 component in this branch accelerates much slower than the one currently in 2.7 (714d8565). Our tests in tests/limit3/ catch constraint violations* (of which we now have none, which is awesome!), but they do not test for sluggish response so we missed this regression. I’ll try to extend the tests to catch this.

Also I see that limit3.0 and limit3.1 are functionally identical, so I’ll remove one of them.

#5 – zultron 于 2017-12-16

Sorry, I left the orig limit3 tests out, d’oh! Anyway, I found the same regression by other means, and need to dig in again to see what’s going on in those cases. Thanks for the extra eyeballs; we’ll need them again.

#6 – SebKuzminsky 于 2017-12-17

I pushed a couple of commits to my version of this branch (2.7-constraint-violations) that extend the limit3 tests’ checkresult program to identify this new acceleration/velocity under-performance problem.

In @zultron’s original refactor (what’s currently in 2.7 as of 714d8565) the limit3 component does use all available acceleration and velocity track its input (this can be seen by running the tests in commit 30ff48df). But the version that’s currently in this PR (as of 97e74eaa) sometimes does not accelerate as hard as it should, leading to increased following error.

#7 – zultron 于 2017-12-21

I’m getting pretty close with a branch that feels pretty solid. It needs a lot of cleaning to be presentable. I’ll explain the ‘sluggish start’ question, though, since it calls into question whether the new code is the issue or whether the test itself is the issue.

Take, for example, the limit3.2 [test][1] (limit3.0 and limit3.1 are similar), with maxa set at 1000. The output at sample 0 is 0.001, where sample -1 would be 0 with vel. 0. Using the ddt comp’s algorithm, this looks good: sample 0 vel. = (final position – initial position)/dt = (0.001 – 0.000)/0.001 = 1.0, and accel. = (final velocity – initial velocity)/dt = (1.0 – 0.0)/0.001 = 1000, obeying max accel.

If we’re assuming uniform acceleration, though, the sample 0 velocity reverse-calculation is incorrect. The forward calculation looks like this: At sample -1, initial velocity is 0, and at constant accel. 1000, will ramp up linearly to final velocity 1.0 after 0.001s at sample 0. The calculation should be displacement = avg. velocity dt = (final velocity + initial velocity)/2 dt = (1.0 + 0.0)/2 * 0.001 = 0.0005! That output at sample 0 is 0.001, twice that, implies the acceleration is also twice, or 2000, which breaks acceleration limits.

The reason the ddt comp output shows velocity 1.0 at sample 0 is because it also calculates average velocity, not final velocity, using (current output – previous output)/fperiod.

I’m not sure what to think about this yet. Maybe it’s what @jmkasunich was referring to when he [mentioned][2] “the discrete time nature of the code vs continuous time of normal physics.” I haven’t really determined the overall effect across multiple periods; maybe it’s insignificant and we can safely ignore this issue. Maybe there are cases where this will matter and the calculations (and tests) should be fixed. I’d appreciate hearing others’ thoughts, and in the meantime, my new branch does the calculation the old way.

[1]: https://github.com/LinuxCNC/linuxcnc/blob/2.7/tests/limit3.2/
[2]: https://github.com/LinuxCNC/linuxcnc/issues/240#issuecomment-337326649

#8 – zultron 于 2018-01-12

This one is obsoleted by #396; closing. Above-referenced issues should be addressed there.

原始Issue: https://github.com/LinuxCNC/linuxcnc/pull/380

评论 (8)