[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) #8697

Bob-the-Kuhn · 2017-12-07T05:07:06Z

This is from Issue #8573

In a band of feed rates the CORE machines will have short (10mS – 30mS) pauses.

The root cause is the current code doesn’t apply jerk until the head movement rate exceeds the jerk setting. On a CORE machine one of the steppers is always moving faster than the head. The result is a band of feed rates where the stepper needs jerk but the head rate isn’t yet calling for it.

In that band of feed rates, negative values for block->accelerate_until are fed to the stepper routine. These negative values result in the short pauses of about 10mS - 30mS spaced about 275mS apart..

This update calculates the jerk requirements on a per stepper basis. It also applies the worst case rather than just the one with the maximum configured jerk.

nominal_speed is now based on maximum stepper speed and start_speed is based on the worst case jerk.

CPU loading is not affected by the code change. Execution times for _buffer_steps() are similar.

Execution times are in milliseconds.

current  1.499  2.008  2.258  2.326  2.212  2.143
         1.533  1.977  2.258  2.326  2.212  2.125
         1.546  1.973  2.258  2.319  2.392  2.010
         1.505  2.002  2.211  2.380  2.206  2.136

update   1.616  1.900  2.271  2.321  2.254  2.126
         1.627  1.905  2.271  2.321  2.312  2.033
         1.627  1.901  2.251  2.321  2.102  2.207
         1.632  1.905  2.288  2.321  2.324  2.033

The above uses the following gcode program

  g1 x100 y100 f1000
  g1 x50  y200 f1000
  g1 x0   y0   f1000
  g1 x275 y25  f1000
  g1 x100 y100 f100

Build sizes and RAM usage are the same or a little less.

         text       data     bss 
current  51642       192    2406
update   51776       192    2406

Here's an example of the stuttering when the feed rate is in problem band.

Here are logic analyzer captures of the step pulses. They include new & old software, COREXY & Cartesian and various feedrates. The gcode program is given above. X jerk was set to 20 and Y was set to 10 so that it is possible to see what happens when axis need different accelerations.

The very bottom trace is the time spend in the _buffer_steps() routine.

You'll need the Saleae software to view them.

stepper captures.zip

removed test statements

thinkyhead · 2017-12-07T05:44:56Z

This is interesting, and kind of scary… because it looks like it throws away a bunch of carefully-crafted jerk code, and kind of seems like a regression.

Has some other firmware taken this approach, and are we sure it's safe compared to all the other possible approaches that could be taken? After all, we've been flirting with possibly using an even fancier jerk method that takes angular change into account… and this PR goes the opposite way, towards what looks like utter simplicity.

I want to believe… and merge… but it seems too easy.

thinkyhead · 2017-12-07T06:09:32Z

Well, let's loose this thing into the wild and see how it fares… And I'll merge for 2.0.x too.

Bob-the-Kuhn · 2017-12-07T06:30:15Z

WOW!! That was merged a lot faster than I expected.

I'm also nervous about going away from the old method.

In a simple Cartesian system the amount of energy supplied by the X stepper to make a change in the X direction of the head does not depend on what is happening in the Y or Z directions.

I'll need to do some thought experiments before I'm confident in how a CORE system works.

Delta math is still a mystery to me.

thinkyhead · 2017-12-08T07:10:36Z

If the new Jerk code ends up in 1.1.7 (coming soon!) then we'll need to give it a fast sounding nickname in the release notes. The "Rocky Rakuhn" Jerk Algorithm…

thinkyhead · 2017-12-08T07:33:39Z

In a simple Cartesian system the amount of energy supplied by the X stepper to make a change in the X direction of the head does not depend on what is happening in the Y or Z directions.

You're down the rabbit hole now! As you can see the core underlying system of Marlin presumes Cartesian mechanics to a fault. Delta was bolted on by @jcrocholl for the Kossel, and so there is no formal system in place to deal with the linear acceleration of the effector in a Delta or with the rotational degrees-per-second feedrates of a SCARA. So, we continue to apply patches and look for patterns in the chaos. For the most part we keep getting closer, though each problem we tackle ends up having scattered pieces — what with so many different specialized motion functions, and all.

The fact that Delta acceleration works is down to the segmentation of moves. Each segment of the move is supposed to take a certain amount of time. So wherever possible, we should apply feed-rates that accomplish the target duration.

It might even make sense to start storing and working with feed-rates more explicitly in terms of move duration. Instead of saying "move this fast" tell each move "take this amount of time."

All that said, the rates of motion in a Core system —in my view— should be considered in terms of the carriage, with the feedrates reckoned and limited at the X_HEAD/Y_HEAD/Z_HEAD level, and there should be no limits applied to the ABC level. Their speeds should be whatever the result is when limits are applied in XYZ terms.

So, maybe that means there's a part of the stepper ISR that could be skipped or replaced, if its job is to apply unnecessary limits to ABC that it should be applying to XYZ. Or, maybe that's what it already tries to do…?

…menr [bugfix-1.1.x] minor planner.cpp speed improvement (follow up to COREXY stutter changes PR #8697)

…ware#8697)

…rlinFirmware#8697)" This reverts commit 86b65e5.

…ware#8697)

…rlinFirmware#8697)" This reverts commit 86b65e5.

initial load

499321f

removed test statements

Bob-the-Kuhn added Bug: Confirmed ! PR: Bug Fix Release: RCBugfix and removed Release: RCBugfix labels Dec 7, 2017

thinkyhead removed the Release: 1.1.x label Dec 7, 2017

thinkyhead added 2 commits December 7, 2017 00:00

Update planner.cpp

e777a80

Update planner.cpp

ff5d257

thinkyhead mentioned this pull request Dec 7, 2017

[2.0.x] Improved Core-compatible jerk code #8698

Merged

thinkyhead merged commit 86b65e5 into MarlinFirmware:bugfix-1.1.x Dec 7, 2017

Bob-the-Kuhn added a commit that referenced this pull request Dec 9, 2017

Merge pull request #8718 from Bob-the-Kuhn/1.1x-planner-speed-improve…

360c553

…menr [bugfix-1.1.x] minor planner.cpp speed improvement (follow up to COREXY stutter changes PR #8697)

Bob-the-Kuhn deleted the 1.1.x-CORE-accel/decel branch December 9, 2017 00:45

AnHardt mentioned this pull request Dec 9, 2017

[1.1.x] better reverse pass #8720

Closed

mtm88001 pushed a commit to mtm88001/Marlin that referenced this pull request Dec 9, 2017

[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) (MarlinFirm…

88fc9bb

…ware#8697)

thinkyhead added a commit to thinkyhead/Marlin that referenced this pull request Dec 20, 2017

Revert "[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) (Ma…

477e36a

…rlinFirmware#8697)" This reverts commit 86b65e5.

thinkyhead mentioned this pull request Dec 21, 2017

[1.1.x] Restore position_float to LIN_ADVANCE #8847

Merged

damicreabox pushed a commit to damicreabox/Marlin that referenced this pull request Sep 14, 2018

[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) (MarlinFirm…

8f86bdc

…ware#8697)

damicreabox pushed a commit to damicreabox/Marlin that referenced this pull request Sep 14, 2018

Revert "[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) (Ma…

a0ba8df

…rlinFirmware#8697)" This reverts commit 86b65e5.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) #8697

[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) #8697

Bob-the-Kuhn commented Dec 7, 2017

thinkyhead commented Dec 7, 2017

thinkyhead commented Dec 7, 2017 •

edited

Bob-the-Kuhn commented Dec 7, 2017

thinkyhead commented Dec 8, 2017

thinkyhead commented Dec 8, 2017 •

edited

[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) #8697

[bugfix-1.1.x] COREXY stutter moves (planner.cpp changes) #8697

Conversation

Bob-the-Kuhn commented Dec 7, 2017

thinkyhead commented Dec 7, 2017

thinkyhead commented Dec 7, 2017 • edited

Bob-the-Kuhn commented Dec 7, 2017

thinkyhead commented Dec 8, 2017

thinkyhead commented Dec 8, 2017 • edited

thinkyhead commented Dec 7, 2017 •

edited

thinkyhead commented Dec 8, 2017 •

edited