[FR] Get Linear Advance + S_Curve completed and reliable

[MarkusThur]

Is your feature request related to a problem? Please describe.

This feature request, or better first Request for discussion, is related with the issue described in https://github.com/MarlinFirmware/Marlin/issues/14728 [BUG] Linear advance incompatible with S-Curve acceleration

If I see it right this issue is not solved, except by the sanity check that warns / throws a error if you enable LIN_ADVANCE and S_CURVE_ACCELERATION without marking you explicitly want it by activating EXPERIMENTAL_SCURVE at the same time. Where EXPERIMENTAL_SCURVE does not activate any special s-curve feature, just only marks that you definitely want it.

I use https://github.com/knutwurst/Marlin-2-0-x-Anycubic-i3-MEGA-S, which relies on Marlin 2.0.5.4 on a Anycubic Mega S. There Linear_Advance and S_Curve are activated by default and work very well. The described problems are not seen.

Problem observed is (that obviously) the time prediction is a mess, as triangular movement is exspected and S-Curve Movements are obviously slower.

But Linear_advance with a K factor of 0.51 works well with printing speed of 50 to 70 mm/s, as well as printing acceleration of 3200 mm/s^2 is possible without interfering with print or causing strange "sound" of the extruder. Starting with a print acceleration of 3600 mm/s^2 the print gets a little "dirty" as the break is to hard and causes vibration / resonance, but does not cause issues on the extruder.

Travel acceleration even up to 4500 mm/s^2 is possible, but 4000 mm/s^2 travels more nicely and vibration / resonance on brake is absorbed without hampering a print at travel speed of 120 mm/s. Faster travels are possible, but may end in step losses of the bed and / or imprecise positioning at end-position.

Are you looking for hardware support?

NO Nevertheless, related with statement above is following hardware: Anycubic Mega S with: Trigorilla 0.0.2 board, using a ATMEGA2560 TMC2208_standalone Steppers So obviously S_Curve + Lin_Advance is possible on a AVR Controller, or was possible till 2.0.5.4

Describe the feature you want

What I want to do / the feature to be requested is as follows.

1. Look into the issue, especially what may changed between there and now in S_CURVE and / or LINEAR_ADVANCE

2. Discuss potential solutions (independent if it works together or not, there is a issue in time prediction of the S_CURVE and therewith the point where LIN_ADVANCE has to start doing it's work)

3. Implement the solution

Additional context

From a first feeling, the main issue is the longer and "hard" to predict time an S_CURVE accelerated movement will need.

The S-curve accelerated movement takes a longer time, as the maximum acceleration of the S_CURVE is the same as the one of a "triangular" movement. In worst case, the S_Curve is not even able to reach max. speed and does not reach maximum acceleration, similar to the "normal" triangular movement, which at some point is not able to reach travel speed, before braking has to start. But this point is obvious earlier for S_Curved movements.

But for any S_Curve accelerated movement, there is a "corresponding" triangular movement, that has the "same" timing, e.g. a triangular acceleration from 0 to 50 mm/s with a acceleration of 2500 ,mm/s^2 obviously needs 0.02s to reach the max speed and will travel 0.5mm during that time. (exact half of what it would have been at full speed, or an unaccelerated movement with 25mm/s). An S_Curve accelerated movement with a max_acceleration of 2500 mm/s^2 will need a little longer due to the lower accel at the beginning and ending of the acceleration phase, For ease of explaination I use a Z_Curve with following parameter,

form 0 to 12.5 mm/s use 1250 mm/s^2 (accel/2) from 12.5 to 37.5 mm/s use 2500mm/s^2 (accel) from 37.5 to 50 mm/s use 1250 mm/s^2 (accel/2)

This way we will need 0.3s to reach travel speed. And the moving distance will be:

distance after each Phase of the Z:
s(0.01) = (0.5*1250*0.01*0.01+0+0)mm = 0,0625mm
s(0.02) = (0.5*2500*0.01*0.01 + 12.5*0.01 + 0.0625)mm = 0.3125mm
s(0.03) = (0.5*1250*0.01*0.01 + 37.5*0.01 + 0.3125)mm = 0,75mm

This exactly equals an unaccelerated movement with 25mm/s for 0.03 seconds. so this also shall equal a movement with a linear acceleration of:

(50mm/s)/0.03s = 1666.66mm/s²
s(0.03) = (0.5*1666.66*0.03*0.03)mm = 0.75mm

So the idea is, to either

• calculate the equivalent acceleration of the S_Curve and use it in Linear_advance or
• to calculate a S_Curve acceleration that is equivalent to the triangular acceleration imposed by the desired acceleration.

Advantage of solution one is, the set max_accel is not exceeded, disadvantage is, time prediction for the total print still stays a mess. Advantage of solution two is time prediction of the print does not need to know if it is S_Curved or not, disadvantage is the max Accel is exceeded by the "form factor" of the S_Curve. Solution two imposes, that the max_accel of printers, that already use S_Curve, would have to reduce their max_accel value, as it now would represents the "equivalent" acceleration instead of the real acceleration.

Both solutions could be implemented in a first approach, using the EXPERIMENTAL_SCURVE flag, which would give the flag a more "useful" meaning.

I am happy to hear your opinions on it.

[thisiskeithb]

Resolved in https://github.com/MarlinFirmware/Marlin/pull/24533

[tombrazier]

Hold on @thisiskeithb, it's not quite resolved yet. But #24533 brings it a lot closer. It's easy now - by which I mean it is easy to see what needs to be done. But there is some tightly optimised assembler to modify. Will need another PR.

[ejtagle]

@tombrazier : I was the one who wrote the Bézier curve code in assembly. I have read that Linear Advance could work with it, if the _eval_bezier_curve() function could also calculate acceleration. I see at least 2 ways to accomplish it: Direct evaluation of the formulas;

V_f(t) = A*t^5 + B*t^4 + C*t^3 + F => This is the Bézier curve that is evaluated. It returns speed (in steps/second)

If we need to get acceleration, we should differenciate that formula respect from time:

A_f(t) = t^2*(t*(5*A*t+4*B)+3*C)

That is the expression to compute, assuming we want to get acceleration in steps/second^2

5*A , 4*B, 3*C all are constants, so can be computed just once: A_f(t) = t*t*(t*(5A*t+4B)+3C)

This eventually requires 4 extra multiplications, and 2 additions. The original code to compute speed required 8 multiplications, and 3 additions. Computing acceleration would add 4 extra multiplications, and 2 extra additions, slowing down the computations 50%. For 32bit boards, this is absolutely an non issue, but for AVR, i am not sure if it is important or not: The code would take approximately 150*1.5 = 225 cycles to execute.

The other question is the exact acceleration units needed for linear advance computations...

[tombrazier]

@ejtagle Your name is familiar to me from exploring your code, which is excellent.

I was planning to differentiate the polynomial as you suggest. I am running on an AVR and it has a fair bit of processor capacity still available after some optimisations so I am somewhat hopeful. Anyway, the main thing was that it was hand coded in assembler and I just needed the time to modify that. With one thing and another I haven't got round to it yet.

LA needs the same units as the rest of the stepper code, steps and seconds. In this code and the similar deceleration code, the variable current_block->la_advance_rate needs to be replaced with acceleration (in steps/s^2) * K factor (which the planner could populate into current_block.

https://github.com/MarlinFirmware/Marlin/blob/0d8a695ea96bc137059fcb88e65befc5c662adbd/Marlin/src/module/stepper.cpp#L2006-L2027

[tombrazier]

Do you fancy doing the assembler code work?

[ejtagle]

Do you fancy doing the assembler code work?

I could do it. The main issue here is getting sensible units, because the ASM is using fixed point to compute things. There is an alternate way to do it, just computing acceleration from speed differences, but that requires a division... So not sure if it will be a win to do it that way...

[tombrazier]

I didn't like the cost of the division either which is why I have not explored that option.

However it has suddenly just struck me that there doesn't need to be a division. The time interval between calls to Stepper::_eval_bezier_curve() is the inverse of speed! So acceleration = (speed2 - speed1) * speed1. This is slightly complicated by ADAPTIVE_STEP_SMOOTHING and multi-stepping but the same thing holds: the division can be replaced by a multiplication.

[thinkyhead]

ADAPTIVE_STEP_SMOOTHING

Fortunately, this just doubles (and doubles), so somewhere in there a shift operation using oversampling should be able to compensate.

[tombrazier]

Indeed. It's in Stepper::calc_timer_interval().

I'll finish my work on IS first and then hit this. Hopefully will be quick.

[ejtagle]

I didn't like the cost of the division either which is why I have not explored that option.

However it has suddenly just struck me that there doesn't need to be a division. The time interval between calls to Stepper::_eval_bezier_curve() is the inverse of speed! So acceleration = (speed2 - speed1) * speed1. This is slightly complicated by ADAPTIVE_STEP_SMOOTHING and multi-stepping but the same thing holds: the division can be replaced by a multiplication.

You are right. There is already a division done in code... Do you want to smile a bit ? ... ;) ... I also was the one that implemented the function get_period_inverse() that calculates 1/d using a fast newton-rapson iteration xD ... I just forgot about that ... If needed, that routine could also be reused, but you are right. Time is the inverse of speed, and also, as @thinkyhead said, ADAPTIVE_STEP_SMOOTHING uses integer factors (yes, I was also the one that did it, the bresenham.txt file explaining the ADAPTIVE_STEP_SMOOTHING is mine) so, the division can be replaced by a way faster multiplication 👍

[Sophist-UK]

There have been several people on Discord asking whether this work was ever finished and whether...

• S-Curve;
• Linear Advance;
• Input Shaping;
• XY Frequency Limit; and
• Adaptive Step Smoothing

are now compatible with one another.

Is anyone who was involved in this FR able to comment on this please?

(As an aside, in the LCD menus and Gcode, you can turn Linear Advance can be adjusted to/from zero and Input Shaping can be turned on and off in each dimension, but AFAIK there is no UI or Gcode to turn on/off or adjust S-Curve, XY Frequency Limit or Adaptive Step Smoothing. Are these things that really should have Advanced Configuration menu entries and Gcodes?)

[tombrazier]

For LA & S_CURVE, I never did implement the solution I mentioned in https://github.com/MarlinFirmware/Marlin/issues/22547#issuecomment-1278939604. The main reason was that with input shaping, S_CURVE is now far less relevant. Also LA & S_CURVE can coexist and it is still better to have LA than not have it when using S_CURVE even though it is less than theoretically perfect. Other considerations were that I have also added a lot of optimisations to the stepper code which needed to settle and that I just haven't had the time. But mainly, lacking the motivation because of the existence of IS, I don't think I will ever get round to it.

IS will work with any of the other items on the list (that is the ZV one I implemented - I don't really know about the FT_MOTION implementation).

ASS is also compatible with everything, I think. As far as I can remember it always has been but I might have forgotten something.

I don't know XY frequency limit well but I think it just adjusts print speed, so should be independent of everything else and work with everything else.

i.e. I think the only limitation is LA & S_CURVE but am open to correction. And rather than use S_CURVE, I'd just recommend IS now. (Which is sad because S_CURVE is really lovely code.)

[Sophist-UK]

@tombrazier Thanks for such a prompt response. If I may, can I just check that I have understood this ok. Your recommendations are:

1. If you are going to use only one of LA or S_CURVE, then you should use LA by itself, but if you are going to use S_CURVE then with LA is better than S_CURVE by itself?

2. IS/XY/ASS are all OK to use in conjunction with whatever you decide in point 1.?

[tombrazier]

1. If it is a choice between LA and S_CURVE, then I would say forget S_CURVE and use IS instead. But, yes, if using S_CURVE then with LA should be better than without LA.
2. Yes, I think so.

Note also, the combination of IS with S_CURVE will work but is probably unnecessary. Maybe, just maybe, there's an edge case where there are multiple resonant frequencies and S_CURVE performs better than ZV input shaping. But even in such a case the key limitation of S_CURVE is that it does not affect cornering discontinuities which is where ringing and layer shifting primarily originate. S_CURVE is only applied to the acceleration segments of the trapezoidal speed profiles. IS is applied to both.

[Sophist-UK]

@tombrazier Sorry - still confused by your most recent point 1.

Is the use of "IS" intentional or a typo i.e. did you mean "forget S_CURVE and use LA instead"?

Ultimately I (and probably most everyone else) just want a version of firmware that gives the best results.

[tombrazier]

IS is input shaping.

[InsanityAutomation]

I still see solid value in S-Curve, even with IS. When IS compiled in, I can enable / disable as needed depending on project accuracy requirements. When I have it off, I want S-Curve active. On a home users machine, just relying on IS is acceptable but its not a viable solution from an professional / engineering standpoint.

Adaptive Step Smoothing if my memory is correct drastically increases the stepper ISR rate, so it is called even more frequently than steps could possibly be sent there, in order to ensure planned moves are started ASAP. As far as I know there has never been an incompatibility issue with it. It just complicates the math slightly for some of the execution count code discussed above because it means the rate isnt fixed and needs a calc.

[Sophist-UK]

@InsanityAutomation So I should just enable everything?

Is there a menu or Gcode switch to turn on/off S-Curve?

(I have posted a PR for Marlin to allow you to compile firmware with Input Shaping included but turned off (by specifying frequencies of zero.)

[thisiskeithb]

(I have posted a PR for Marlin to allow you to compile firmware with Input Shaping included but turned off (by specifying frequencies of zero.)

You can already compile IS in by default by enabling & setting SHAPING_MIN_FREQ to a value like 10/below any frequency that would affect a print. This allows you set INPUT_SHAPING_X/INPUT_SHAPING_Y to 0.

[Sophist-UK]

I am compiling Marlin for several hundred combinations of variants of a particular model.

For the model I have (and those with the same physical shape and extruder (there are size variants and extruder variants) I can calibrate IS frequencies and Zetas, but for other variants where I cannot calibrate because I don't have a physical machine I want to enable IS but turn it off - and I don't want to include a SHAPING MIN FREQ if I have no idea what that should be.

[thisiskeithb]

and I don't want to include a SHAPING MIN FREQ if I have no idea what that should be.

You'd set it to a value like 10 so it's below any frequency that would affect a print.

[Sophist-UK]

I was under the impression that the lower the min. freq. the greater the RAM usage.

[thisiskeithb]

I was under the impression that the lower the min. freq. the greater the RAM usage.

Yes, that's how it works. Marlin needs to know how much ram to allocate to IS buffers at compile time and that is done through SHAPING_MIN_FREQ when you want to compile IS in, but leave it disabled.

[Sophist-UK]

Hmmm. I think you are right - in the absence of any actual frequencies, you need to guess a frequency.

Since this is for an AVR board, there is a minimum frequency supported by the board anyway, so I need to work out what that is and use it.

[oliof]

since the usual frequency for IS is between 40 and 50Hz and the algorithm centers on a set frequency, a minimum frequency of 30Hz is very likely to be more than sufficient.

[Sophist-UK]

I am having trouble determining the Y frequency on my bed-flinger. Obviously the bed has significant mass, so I would imagine that the resonant frequency would be quite low, however determining what this is using either the MarlinFW.org tests or the th3dstudio.com tests is proving difficult. The IS values as best as I can determine from both these sets of tests are:

#define SHAPING_FREQ_X  37.42f
#define SHAPING_ZETA_X   0.10f
#define SHAPING_FREQ_Y  18.48f
#define SHAPING_ZETA_Y   0.50f 

Also, the results on a calibration cube are showing almost exactly the same ringing in both directions with and without IS on (but I really need to print a few more experiments on this to check it).

[tombrazier]

Hmmm. I think you are right - in the absence of any actual frequencies, you need to guess a frequency.

Since this is for an AVR board, there is a minimum frequency supported by the board anyway, so I need to work out what that is and use it.

If the actual frequency at runtime ends up lower than SHAPING_MIN_FREQ then the effect is just that input shaping becomes less effective at higher speeds. But since input shaping is needed most on corners where the speed is lower, this is not as bad a limitation as it sounds.

I also have an AVR bedslinger and shaping works really well. When RAM is a limitation, I think the best way to pick SHAPING_MIN_FREQ is to pick a value that results in enough spare RAM for the stack. You need about 1k of RAM for this. So on an ATmega2560, you need a maximum of about 7k of RAM used as reported by the compiler.

[tombrazier]

I am having trouble determining the Y frequency on my bed-flinger. Obviously the bed has significant mass, so I would imagine that the resonant frequency would be quite low, however determining what this is using either the MarlinFW.org tests or the th3dstudio.com tests is proving difficult. The IS values as best as I can determine from both these sets of tests are:

#define SHAPING_FREQ_X  37.42f
#define SHAPING_ZETA_X   0.10f
#define SHAPING_FREQ_Y  18.48f
#define SHAPING_ZETA_Y   0.50f 

Also, the results on a calibration cube are showing almost exactly the same ringing in both directions with and without IS on (but I really need to print a few more experiments on this to check it).

These frequencies look reasonable for a bedslinger. Zeta of 0.5 for Y looks looks a bit high.

The best way to measure the frequencies in Marlin is with the single layer pattern at https://marlinfw.org/tools/input_shaping/freq-calibr.html. I suggest you do this, take a photo of it and ask for help interpreting it in #support-chat on the Marlin Discord server.

[Sophist-UK]

I don't think that my problem is interpreting them. The explanation on the MarlinFW page is clear, and the prints come out looking similar in general terms. The problem is that the Y line doesn't actually show much vibration (because of the mass of the bed I think) - I tried editing the gcode, reversing the zig-zags to keep the zig length the same but half the width and so keep the frequencies the same but more exaggerated movement, but this didn't help. And most of the Y zeta pattern except for the 0.5 line was so broken up it was difficult to determine anything.

As soon as I get time I will redo these tests and ask on Discord as you suggest.

The th3dstudio ringing tower printed well, and so I took the frequencies (as best I could) off that. I will share that on Discord too.