Skip to content

Fixed point smooth linear advance (makes it available to all 32 bit boards) #27818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

dbuezas
Copy link
Contributor

@dbuezas dbuezas commented Apr 26, 2025

Port smooth linear advance to fixed point arithmetic.
Followup to #27710

There is still one float calculation inside the ISR

target_adv_steps_q13 = lookahead(t) * (Planner::extruder_advance_K[0]*(1UL<<13));// todo keep k in q format

I'll later put extruder_advance_k behind a getter setter where also a fixed point version in q18.13 format will be kept up to date for use inside the isr.

Opening the PR early because it may be already fast enough for tests in boards without an FPU.

Also changed to more conservative initial values for tau (20ms instead of 10) and smooth advance isr frequency (1kHz instead of 5kHz)

Smooth linear advance should now work in any 32 bit board and maybe (just maybe) in an 8 bit one.

@dbuezas dbuezas marked this pull request as draft April 26, 2025 11:06
@dbuezas
Copy link
Contributor Author

dbuezas commented Apr 26, 2025

Note I'm using different positions for the fixed point in different variables to maximize decimal resolution depending on the magnitude of the numbers that they store.
The position of the point is noted in the variable names with a posfix. E.g xxx_q30 means xxx<<30.

@dbuezas
Copy link
Contributor Author

dbuezas commented Apr 26, 2025

If necessary, it could be optimized a bit more by:

  • Experimentally finding the lowest isr frequency that doesn't affect quality
  • Tweaking the point positions so the int64 cast isn't necessary when multiplying (and confirm the loss of resolution doesn't affect quality)
  • Getting rid of rounding in fixed point multiplications

But that would require a lot of care to avoid overflows and I think it wont really make such a big difference, just a couple of cycles per multiplication.

@rq3
Copy link

rq3 commented Apr 26, 2025

Printing as I type on an Anycubic Predator delta with STM32F103 (no FPU).
I see no issues.

(cherry picked from commit 6e1e42c)

# Conflicts:
#	Marlin/src/gcode/feature/advance/M900.cpp
#	Marlin/src/module/planner.cpp
#	Marlin/src/module/planner.h
@dbuezas dbuezas changed the title Fixed point smooth linear advance Fixed point smooth linear advance (makes it available to all 32 bit boards) Apr 26, 2025
@dbuezas
Copy link
Contributor Author

dbuezas commented Apr 26, 2025

I'm doing some test prints to compare floating point vs fixed point branches. There is a slight (but visible) loss in quality in fixed point. This may be the lower time resolution of the less frequent ISR. May have to increase it back.

dbuezas added 5 commits April 27, 2025 14:05
(cherry picked from commit 66d41ff)
(cherry picked from commit fb324b5)
(cherry picked from commit c2e360a)
(cherry picked from commit e706ec9)
(cherry picked from commit 00403f5)
@dbuezas
Copy link
Contributor Author

dbuezas commented Apr 27, 2025

Found the problem.

15 minute benchy:

Left to right: FLOAT ---- FIXED POINT BEFORE ----- FIXED POINT NOW (fixed!)

Float and new fixed point look identical, can't distinguish them.
image

@dbuezas dbuezas marked this pull request as ready for review April 27, 2025 12:46
@dbuezas
Copy link
Contributor Author

dbuezas commented Apr 27, 2025

@rq3 Can you try again? the version you tried above had an overflow that worsened quality.

@rq3
Copy link

rq3 commented Apr 27, 2025

@rq3 Can you try again? the version you tried above had an overflow that worsened quality.

Yes indeed. Just need to mod the files, recompile, and print. Give me a few hours.

@classicrocker883
Copy link
Contributor

could have SMOOTH_LIN_ADVANCE enabled in the tests files buildroot/tests/STM32F103RE_creality when ProUI and JyersUI are enabled

@rq3
Copy link

rq3 commented Apr 27, 2025

Printed with no issues at all. The accelerometer on my print bed claims the g forces on x and y were slightlyed higher than "normal", but subjectively the extruder behavior seemed more "civilized", and it appeared that I could actually see and hear the extruder "anticipating" what was coming. That could certainly be my imagination, but the results are excellent.

Smooth_Benchy

@dbuezas
Copy link
Contributor Author

dbuezas commented Apr 27, 2025

Glad to hear!
You can now increase jerk and acceleration (particularly for inner walls and infill) and the machine will now actually do what you ask for.

@thinkyhead thinkyhead merged commit 12fdde2 into MarlinFirmware:bugfix-2.1.x May 13, 2025
66 checks passed
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request May 15, 2025
@grizzleeadam
Copy link

I've been starting to re-calibrate my printer using this new mode and the results are pretty promising. However, it seems I can't change the input shaping frequency while printing - any attempt (whether via g-code or the LCD menu) causes the printer to freeze and reboot. Wondering if it has to do with the Input_Shaping_E_Synch option (which is enabled). Any way to g-code to disable that function without reflashing the board? (I use manual mesh leveling so it's a pain to recal every time I flash)

@vovodroid
Copy link
Contributor

@grizzleeadam

I use manual mesh leveling so it's a pain to recal every time I flash

You can output existing mesh with G29, and make g-code file creating mesh back also with G29.

By a way, what is link between input shaping calibration and LA? Aren't IS parameters depend on mechanical properties only?

@grizzleeadam
Copy link

grizzleeadam commented May 17, 2025 via email

@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

I noticed that yesterday too.
The link between Smooth linear advance and input shaping is the "e-synch" feature, which tracks how input shaping affects motion and makes extrusion follow that instead of the clean planned extrusion. It's inside smooth linear advance only because the new approach makes it easy to factor in.

Interestingly the crash doesn't happen if input shaping parameters are changed via gcode. I assume the difference is that the menu doesn't synchronise with the planner and the internal shaping params (like delay and factor) are changed while in use. That must be causing ludicrous extrusion speeds which make the firmware spend 100% of the time in the stepper interrupt, starving the watchdog and eventually resulting in halting. In short: a bug in e-synch

@vovodroid
Copy link
Contributor

the link between Smooth linear advance and input shaping is the "e-synch"

But why recalibrate IS itself?

@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

Recalibration shouldn't be necessary

@grizzleeadam
Copy link

the link between Smooth linear advance and input shaping is the "e-synch"

But why recalibrate IS itself?

I rushed my calibration when I did it the first time over a year ago (haven't tinkered much since), was just doing it as a reassurance - not because this commit needed it.

I noticed that yesterday too. The link between Smooth linear advance and input shaping is the "e-synch" feature, which tracks how input shaping affects motion and makes extrusion follow that instead of the clean planned extrusion. It's inside smooth linear advance only because the new approach makes it easy to factor in.

Interestingly the crash doesn't happen if input shaping parameters are changed via gcode. I assume the difference is that the menu doesn't synchronise with the planner and the internal shaping params (like delay and factor) are changed while in use. That must be causing ludicrous extrusion speeds which make the firmware spend 100% of the time in the stepper interrupt, starving the watchdog and eventually resulting in halting. In short: a bug in e-synch

For me, the crash was happening with both gcode and with the menu. I tried adding a 100ms dwell before and after the gcode and it was still crashing at the first M593 command. I suppose I could try disabling the E sync and see if it still does it for me. I'm on a SKR E3 Mini v1.2, bit of an older setup.

@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

I suppose I could try disabling the E sync and see if it still does it for me

That would be a good test, please post the result. Maybe gcode didn't fail for me is just because I made a frequency sweep from low to high. So the delay between echos got always shorter and the algorithm never had to look at an unset value of the lookback buffer.

@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

I think I know what the problem is.
The code assumes that the minimum frequency possible is the minimum set in the config for any axis. A lower frequency would mean a bigger buffer, and if one goes below that, then there's a buffer underflow and it starts reading echoes from memory where other vars store stuff. These values may happen to be large values, the firmware tries to extrude at ridiculously high speeds and eventually resets.

Since E-SYNCH doesn't check if shaping is enabled, and disabling it sets frequency to zero, it also creates the same problem, but worse: the delay is -1 in uint, so a very large value.
Interestingly, the input shaping buffers are also set to the minimum frequency, but the menu still allows you to go lower, which would also make the input shaper go crazy

@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

@tombrazier: by default SHAPING_MIN_FREQ is the minimum of the freqs set at build time, but the menu allows going below them. Doesn't that make the input shaper break?

@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

PR to fix that: #27862

@grizzleeadam
Copy link

grizzleeadam commented May 17, 2025 via email

@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

You can also set it, the issue is that Marlin allows you to then set a lower frequency and that.
Without the PR I linked above, it will still crash when IS is disabled in the UI though

@tombrazier
Copy link
Contributor

@tombrazier: by default SHAPING_MIN_FREQ is the minimum of the freqs set at build time, but the menu allows going below them. Doesn't that make the input shaper break?

No, it just reduces the effectiveness of IS at higher speeds, as explained in the comment above the input shaping section of the config.

@grizzleeadam
Copy link

I tried the following, one at a time, using latest 2.1.x-bugfix:

  1. SMOOTH_LIN_ADV disabled: M593 to lower values works, even with manual SHAPING_MIN_FREQ not set.

  2. SMOOTH_LIN_ADV enabled, default settings - M593 freezes.

  3. Disabled INPUT_SHAPING_E_SYNCH and set SHAPING_MIN_FREQ & SHAPING_MAX_STEPRATE - M593 works. It seems all three of these must be set; any other combination causes M593 to freeze.

I'll repeat the same with your new PR linked above.

@grizzleeadam
Copy link

With your PR 27862:

SMOOTH_LIN_ADV enabled, INPUT_SHAPING_E_SYNCH enabled;
SHAPING_MIN_FREQ & SHAPING_MAX_STEPRATE commented out;

M593 worked. However, the stepper motor started making some ugly sounds as the speed increased.

thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request May 17, 2025
@dbuezas
Copy link
Contributor Author

dbuezas commented May 17, 2025

M593 worked. However, the stepper motor started making some ugly sounds as the speed increased.

Most likely you had set a frequency below SHAPING_MIN_FREQ. I'll continue the discussion in the open pr, thanks for the reports!

@CRCinAU
Copy link
Contributor

CRCinAU commented May 23, 2025

This looks to potentially have caused a new error in DWIN PROUI:

In file included from Marlin/src/lcd/e3v2/proui/dwin.cpp:36:
Marlin/src/lcd/e3v2/proui/dwin.cpp: In function 'void drawTuneMenu()':
Marlin/src/lcd/e3v2/proui/dwin.cpp:3549:104: error: lvalue required as unary '&' operand
 3549 |       EDIT_ITEM(ICON_MaxAccelerated, MSG_ADVANCE_K, onDrawPFloat3Menu, setLA_K, &planner.get_advance_k());
      |                                                                                                        ^
Marlin/src/lcd/e3v2/proui/menus.h:61:59: note: in definition of macro 'EDIT_ITEM'
   61 | #define EDIT_ITEM(I,L,V...) editItemAdd(I, GET_TEXT_F(L), V)
      |                                                           ^
Marlin/src/lcd/e3v2/proui/dwin.cpp: In function 'void drawMotionMenu()':
Marlin/src/lcd/e3v2/proui/dwin.cpp:3687:104: error: lvalue required as unary '&' operand
 3687 |       EDIT_ITEM(ICON_MaxAccelerated, MSG_ADVANCE_K, onDrawPFloat3Menu, setLA_K, &planner.get_advance_k());
      |                                                                                                        ^
Marlin/src/lcd/e3v2/proui/menus.h:61:59: note: in definition of macro 'EDIT_ITEM'
   61 | #define EDIT_ITEM(I,L,V...) editItemAdd(I, GET_TEXT_F(L), V)
      |  

thinkyhead pushed a commit that referenced this pull request May 24, 2025
@tombrazier
Copy link
Contributor

tombrazier commented Jun 9, 2025

Oh, I've been slow testing this on 8 bit! It compiles for me but uses about 6% of my RAM and I get stack overflows. Maybe leave the 32 bit sanity check in place!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants