[BUG] Input Shaping + MULTISTEPPING_LIMIT cause motor noise?

[classicrocker883]

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

Apparently since March, when MULTISTEPPING_LIMIT was added and module/stepper.cpp | stepper.h amended, the stepper motors would sort of grind when moving at higher speeds, acceleration, jerk.

having #define OLD_ADAPTIVE_MULTISTEPPING 1 did fix the issue. but there is no option to enable it, it was arbitrarily added to Configuration_adv.h

I am wondering if the stepper code can be improved? or is the old adaptive multistepping good enough ?

Bug Timeline

March

Expected behavior

Smooth motor movements

Actual behavior

Noisy at higher speeds

video of issue

Steps to Reproduce

Enable input_shaping
Start printing w/ high speeds,
go into a terminal and send commands to printer

G1 X200 F15000
G28X ;home
G1 X200 F10000
G28X ;home
G1 X200 F7000
G28X ;home
G1 X200 F6000 ;(no noticeable noise here)

Version of Marlin Firmware

bugfix-2.1.x

Printer model

Voxelab Aquila (Ender3V2)

Electronics

No response

Add-ons

No response

Bed Leveling

UBL Bilinear mesh

Your Slicer

None

Host Software

None

Don't forget to include

• A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

ref to issue classicrocker883/MRiscoCProUI#27

MarlinConfigs.zip

[cbagwell]

So your board is a Creality V4.2.2? I don't think there was a report of noisy steppers+INPUT_SHAPING on that board yet.

There was a report about some underpowered (8-bit I think) boards having noisy steppers and losing steps when INPUT_SHAPING as enabled and I think it was traced to MULTISTEPPING getting too aggressive. Input Shaping is changing the stepper ISR time just enough that multistepping can cause issues on low power boards but V4.2.2 isn't considered that really.

Have you tried reducing new MULTISTEPPING_LIMIT from 16 to 8? I have done that preemptively on my under powered BTT SKR Mini E3 to reduce chance of lost steps... but I also wasn't having noisy steppers.

[classicrocker883]

technically it's an Aquila board but uses the same creality v4 pinout. the Aquila is an ender 3v2 clone.

I think there was also an issue having linear advance, so I would assume they are related

[classicrocker883]

what is the benefits of MULTISTEPPING_LIMIT or without OLD_ADAPTIVE_MULTISTEPPING (default setting) vs lowering MULTISTEPPING_LIMIT and enable OLD_ADAPTIVE_MULTISTEPPING?

[cbagwell]

Multistepping feature attempts to combine multiple step requests into a single ISR to squeeze more performance out of CPU's (you get back overhead spent switching to ISR routine). Both old and new code computes how many steps to do in 1 ISR. Old is based on hard coded compile time knowledge of worst case timing and new code is based on runtime computed values. The new way should be more accurate since it's based on actual measurements.

In both old and new, it's pretty easy to get a bit aggressive on combining steps into single ISR; especially on underpowered hardware. Turning on optional features such as INPUT_SHAPING+ADAPTIVE_STEP_SMOOTHING+S_SCURVE_ACCEL make it that much more aggressive at combining. Side effects of combining are losing steps and strange stepper noises. MULTISTEP_LIMIT is new config item and can put an upper limit to prevent being aggressive with 8 being a good choice on underpowered hardware regardless of new vs old.

But then again, its very dependent on your config... If you disable INPUT_SHAPING then 16 might actually be best value... you want the highest but stablest value possible for your config.

Is old or new logic better to use? New way is very new but I assume the best choice... but thats why old way was left around so people could try both.

[classicrocker883]

ah thank you for the very informative reply. this was very helpful!

[cbagwell]

Would you mind testing with OLD_ADAPTIVE_MULTISTEPPING commented out and lowering the MULTISTEPPING_LIMIT and see if that also solves your issue? It would help to confirm that your steppers need the lower multi-steps vs. some unknown bug unique to the new adaptive multi stepping logic that still needs further debugging.

[tombrazier]

A slight clarification on MULTISTEPPING_LIMIT: it is mainly there to prevent multistepping causing missed steps. If multistepping exceeds the microstepping setting for your stepper driver then there is a good chance it will cause the stepper to get out of sync.

I could believe there might be audible noise with the new multistepping algorithm if it keeps switching the multistepping level up and then down again. If it is doing this, it's getting the maximum performance possible out of your MCU - albeit at the cost of extra noise. We are squeezing the most we can out of the old 8 bit and the slower 32 bit hardware.

The existence of OLD_ADAPTIVE_MULTISTEPPING by the way is a fail safe in case there turns out to be anything seriously wrong with the new way of doing multistepping. Not that we think there is anything wrong with it, but it felt like a good idea to have a way out if there was as it is a significant change.

[classicrocker883]

Would you mind testing with OLD_ADAPTIVE_MULTISTEPPING commented out and lowering the MULTISTEPPING_LIMIT and see if that also solves your issue?

lowering MULTISTEPPING_LIMIT to 8, 4 2 or even 1 seems to not fix the issue. 1 is less noise but still the issue remaining.

some investigation and setting
time_spent_out_isr = 0; gives no noise. @

I wouldnt say it's "fixed" by this arbitrary change, but

what if the code were something like if (time_spent_out_isr < 0) time_spent_out_isr = 0; if (time_spent_out_isr > 0) time_spent_out_isr = 0;

[tombrazier]

time_spent_out_isr = 0; gives no noise.

This is an excellent test in terms of the information it gives. It tends to confirm my suspicion that the noise is from the multi-stepping level flipping between two values.

However setting time_spent_out_isr (or the equivalent if statement code you suggested) negates the purpose of the multi-stepping logic. It just causes maximum multi-stepping to run all the time.

I guess the question is how much of a problem is the extra noise? It would be possible to change

if (steps_per_isr > 1 && time_spent_out_isr >= time_spent_in_isr + time_spent)```

to something like

if (steps_per_isr > 1 && time_spent_out_isr >= MULTISTEPPING_RESET_FACTOR * (time_spent_in_isr + time_spent))```

on around line 2200 so that multi-stepping is not decreased as readily, for example. But that leads to an extra config option and extra complexity generally.

[github-actions]

This issue has had no activity in the last 60 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 10 days.

[tombrazier]

Idle because we talked about it at classicrocker883/MRiscoCProUI#27

[github-actions]

Greetings from the Marlin AutoBot!
This issue has had no activity for the last 90 days.
Do you still see this issue with the latest bugfix-2.1.x code?
Please add a reply within 14 days or this issue will be automatically closed.
To keep a confirmed issue open we can also add a "Bug: Confirmed" tag.

Disclaimer: This is an open community project with lots of activity and limited
resources. The main project contributors will do a bug sweep ahead of the next
release, but any skilled member of the community may jump in at any time to fix
this issue. That can take a while depending on our busy lives so please be patient,
and take advantage of other resources such as the MarlinFirmware Discord to help
solve the issue.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.