Adapt Jerk / Speed code from Prusa MK2

[thinkyhead]

This PR adapts the of the speed limiting code from the Prusa i3 MK2 branch of Marlin. It purports to correct an issue where the Z axis could end up with its speed not properly limited.

Summary:

• Use a single byte with flag bits instead of two bool flags
• Add previous_safe_speed for testing whether the move is halting
• Optimize / condense max acceleration limit code (See also Adjustments to planner acceleration limit #4365)
• Perform initial jerk speed limit as per MK2 branch
• Perform junction speed limit as per MK2 branch
• Use const where it makes sense

References: #4936, #4930, #4365

[Sebastianv650]

I was excited what the changes inside buffer_line() means for execution speed so I did a quick and dirty comparison with and without this PR. The numbers are only for reference and not exact because the planner is interrupted by the ISR so it's more a sum of something.
I did 4 runs (2 with each FW) with the exact same stl and measured 400 buffer_line executions per run so it should have some meaning :)

An average run with RCBugFix state takes 4308µs.
An average run with this PR takes 4000µs.

I would call this significant, and if it also fixes some possible bugs, that's awsome!

[thinkyhead]

That does seem pretty significant. 308µs is about 4928 CPU cycles, or an average savings of ~12.3 cycles per invocation. When it comes to code in the planner, even 12.3 cycles can add up to a big difference.

This PR makes some significant changes to the speed junction code, but nothing too crazy-looking. I noticed that the Prusa MK2 branch also features a pretty different-looking trapezoid generator, so that might be the next PR.

I'm not sure if it's just older Marlin (1.0.x) code, or if it's actually a rewrite of that section. I haven't determined the MK2 branch starting-point, but once I do I can perform a side-by-side comparison and see where all the major changes are.

[thinkyhead]

@clexpert @petrzjunior Do we like the MK2?

[bubnikv]

I have re-written the trapezoidal generator in the Prusa3D branch of Marlin from floating point to fixed point. For the small bed size, the code shall produce exactly the same result with very significant CPU clock savings. I recommend for the official Marlin to use a conditional block to activate this fixed point planner as long as the intermediate results fit the 32bit integer.

[clexpert]

@thinkyhead Hi Scott, thank you for the question. I see that you have found my photo 😄 We have one Velleman K8200, one Velleman K8400 Vertex, a lot of Rebel 2 (Czech open source printer), and one Prusa i3 MK2. I am satisfied with the Prusa printer but I am frustrated from the latest heatbed 😃

[thinkyhead]

@bubnikv Thanks for pointing that out. I've been looking forward to using the fixed-point code when possible. Can I assume that the current MK2 branch is the most up to date?

[thinkyhead]

@bubnikv One hazard to note. Since G92 can be used to set any coordinate value, and the planner/stepper code incorporates the coordinate space, probably something as simple as G92 X32000 is enough to cause bad things to happen. (Of course, note that G92 behavior has also been changed recently so software endstops move along with it. So the MK2 branch won't let X move right after that command.)

[FHeilmann]

Wouldn't it be possible to limit G92 in a way that it cannot set positions outside of the actual bed positions?

The only time where you would want to set a coordinate outside of the build volume would be if you're trying to "figure out" your printers limits I imagine.

I would envision if you call a G92 with X, Y or Z values outside +AXIS_LENGTH>=value>=-AXIS_LENGTH it should simply not execute the command at all.

Of course that wouldn't solve issues where someone actually needs exorbitant values (I'd love to see a printer with a 3.2 meter long build axis :D)

[bubnikv]

G92 does not behave as intended with the software end stops indeed. The way
the G92 code works now is more a calibration aid than a coordinate system
offset. The way it shall work is to save the offset somewhere at the high
level and then to add it to the input coordinates before clamping by the
soft end stops. So if I go to zero and say G92 X100 Y100, I would have the
bed centered around zero on our Prusa I3 printers.

This is not an issue for a normal user, as the usual slicers use the G92
code for zeroing the extruder coordinate only. But it will break, if
someone tries to write a G-code by hand and use G92 for shifting the
coordinate system. This is really the only other use case for G92 on the
FFF printers.

Wouldn't it be possible to limit G92 in a way that it cannot set
positions outside of the actual bed positions?

IMHO no. The user shall have the freedom to shift the offset as she/he
wishes.

On Thu, Oct 20, 2016 at 9:49 AM, Florian Heilmann [email protected]
wrote:

Wouldn't it be possible to limit G92 in a way that it cannot set positions
outside of the actual bed positions?

The only time where you would want to set a coordinate outside of the
build volume would be if you're trying to "figure out" your printers limits
I imagine.

I would envision if you call a G92 with X, Y or Z values outside
+AXIS_LENGTH>=value>=-AXIS_LENGTH it should simply not execute the
command at all.

Of course that wouldn't solve issues where someone actually needs
exorbitant values (I'd love to see a printer with a 3.2 meter long build
axis :D)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4997 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFj5I3AqdH4gcIg3wwwBPEMTiweHh5Yeks5q1x0DgaJpZM4KTfn6
.

[bubnikv]

So the offset given by G92 is really a virtual value, which shall be added
to the input value. It has nothing to do with the physical dimensions of
the machine.

On Thu, Oct 20, 2016 at 10:04 AM, bubnikv . [email protected] wrote:

G92 does not behave as intended with the software end stops indeed. The
way the G92 code works now is more a calibration aid than a coordinate
system offset. The way it shall work is to save the offset somewhere at the
high level and then to add it to the input coordinates before clamping by
the soft end stops. So if I go to zero and say G92 X100 Y100, I would have
the bed centered around zero on our Prusa I3 printers.

This is not an issue for a normal user, as the usual slicers use the G92
code for zeroing the extruder coordinate only. But it will break, if
someone tries to write a G-code by hand and use G92 for shifting the
coordinate system. This is really the only other use case for G92 on the
FFF printers.

Wouldn't it be possible to limit G92 in a way that it cannot set
positions outside of the actual bed positions?

IMHO no. The user shall have the freedom to shift the offset as she/he
wishes.

On Thu, Oct 20, 2016 at 9:49 AM, Florian Heilmann <
[email protected]> wrote:

Wouldn't it be possible to limit G92 in a way that it cannot set
positions outside of the actual bed positions?

The only time where you would want to set a coordinate outside of the
build volume would be if you're trying to "figure out" your printers limits
I imagine.

I would envision if you call a G92 with X, Y or Z values outside
+AXIS_LENGTH>=value>=-AXIS_LENGTH it should simply not execute the
command at all.

Of course that wouldn't solve issues where someone actually needs
exorbitant values (I'd love to see a printer with a 3.2 meter long build
axis :D)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4997 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFj5I3AqdH4gcIg3wwwBPEMTiweHh5Yeks5q1x0DgaJpZM4KTfn6
.

[thinkyhead]

It has nothing to do with the physical dimensions

@bubnikv

Correct. You can think of G92 as redefining what the current physical position means, logically. It doesn't just force set the current position, which is how we previously thought of it. The only change to G92 in 1.1.x is that the software endstops are moved along with the current position.

So, for example, suppose the current position is in the middle of the bed, X100 Y100, while the software endstops are X:0,200 Y:0,200. If you do G92 X0 Y0 in Marlin 1.0.x, the current position changes to {0,0} but the software endstops remain the same, so you can no longer move left or frontward.

In Marlin 1.1.x this is no longer the case. The G92 X0 Y0 command will also change the software endstops to X:-100,100 Y-100,100 so you can still move the nozzle over the entire bed.

The downside of this is, if you want to use G92 to do nozzle lowering tricks, you can't, because the Z software endstop moves. But, we also separated the concept of minimum position from home position, so in fact, you can configure a machine to allow moving Z below the bed (all the time), but still home with Z=0 at the endstop level.

In the code we now call the current position the "logical position." That's the position that includes offsets to the coordinate space as applied by either M206 or G92 commands. The logical position is currently sent to the planner (unless kinematics are applied!) and the steppers also use the logical position, converted to steps. That needs to change so the planner/stepper represent the "physical position" (i.e., the raw position) instead.

I will be auditing the code soon to find all the places where logical coordinates are converted to "raw coordinates" (the coordinate space without the offsets), and then attempt to sweep the code so that the Planner and Stepper classes always work within the raw coordinate space, and the offsets are applied (or unapplied) when converting back and forth. It probably should have been made that way in the first place. We're just at a point of organization where it should be fairly straightforward to accomplish.

[bubnikv]

I see, so this has been fixed in Marlin 1.1. Very good, thanks.

On Fri, Oct 21, 2016 at 3:26 AM, Scott Lahteine [email protected]
wrote:

You can think of G92 as redefining what the current position means. It
doesn't just force set the current position, which is how we previously
thought of it. The only change to G92 in 1.1.x is that the software
endstops are moved too.

For example, imagine the current position is in the middle of the bed,
X100 Y100, while the software endstops are X:0,200 Y:0,200. If you do G92
X0 Y0 in Marlin 1.0.x, the current position changes to {0,0} but the
software endstops remain the same, so you can no longer move left or
forward. In Marlin 1.1.x this is no longer the case. The G92 command now
changes the software endstops to X:-100,100 Y-100,100 so you can still move
the nozzle over the entire bed.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4997 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFj5I3n35Yi9pD9HRyd6pcMS67KXoSCBks5q2BTYgaJpZM4KTfn6
.