[BUG] 2.1.2 Stepper motor stutters and skips at high speed due to new ISR calculation

[ansonl]

See latest post at bottom for better summary and narrowed down bug in stepper.h and stepper.cpp.

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

Yes, and the problem still exists.

Bug Description

Marlin no longer properly offsets the printhead on a toolchange between switching nozzles that have a X offset between them.

#define HOTEND_OFFSET_X { 0.0, 19.00 } // (mm) relative X-offset for each nozzle
//#define HOTEND_OFFSET_Y { 0.0, 0.00 }  // (mm) relative Y-offset for each nozzle
#define HOTEND_OFFSET_Z { 0.0, -2 }      // (mm) relative Z-offset for each nozzle

#define EVENT_GCODE_TOOLCHANGE_T0 "G90\nG0 X210 F7200\nG0 Y165\nG0 X216\nG0 X216 Y183\nG0 X210" // Extra G-code to run while executing tool-change command T0
#define EVENT_GCODE_TOOLCHANGE_T1 "G0 X229 F7200\nG0 Y199\nG0 X234\nG0 Y184\nG0 X229" // Extra G-code to run while executing tool-change command T1

When Marlin switches to a tool T1 with a positive X offset it will move the printhead quickly in the negative X direction and return to the original position before running EVENT_GCODE_TOOLCHANGE_T1 to complete the toolchange. The DXU mechanical switching nozzle mechanism has a lever on the right side of the printhead that moves to the bed limit X_BED_SIZE to touch a hook on the right side of the case which causes a nozzle lift/lower.

As of 2.1.2 Marlin no longer moves the toolhead in the negative X direction for the same distance as the "return" movement in the positive X direction before running EVENT_GCODE_TOOLCHANGE_T1. This leads to a printhead collision with the side of the gantry since the final X offset from before -> after the offset adjustment (but before EVENT_GCODE_TOOLCHANGE_T1) is no longer 0 or equal to the new HOTEND_OFFSET_X.

The last working commit I found was on 10/30/22 on PR #24553

The commit history for the PR can viewed here: https://github.com/MarlinFirmware/Marlin/pull/24553/commits
I have taken a video of the correct toolchange and the new bug toolchange that is linked below for each test.

• 10/30/22 I updated the PR Switching extruder (dondolo) without servo #24553 repo to be up to date with the main bugfix2.1.x repo. Tested to be working correctly.
• On 11/23 and 11/28 the same repo was updated to be up to date with main bugfix2.1.x repo. Tested both and they have a stepper motor failure when homing.
• On 12/19 I synced my own copy of the repo at https://github.com/ansonl/Marlin/tree/dondolotest to be updated with bugfix2.1.x to have 2.1.2 changes. Tested and stepper motor failure is gone. Toolchange offset adjustment does not work correctly.

As seen in the linked videos, the bugged toolchange initial offset adjustment does not move the full amount in negative X direction as it did in the correct toolchange. This seems to lead to the machine's real X position deviating from the software X position and it collides the side of the gantry.

The initial first toolchange from T0 -> T1 on both correct and bugged firmware does not have this problem when it is from the origin position right after homing. It is the second T0 -> T1 toolchange that this bug occurs in.

I found this bug while testing a version of PR#24553 synced to the current bugfix2.1.x and this PR has some additional definitions for Mechanical Switching nozzles and a bugfix in toolchange.cpp that prevents a bed collision. To my knowledge it doesn't modify the toolchange offset adjustment code.

Bug Timeline

New bug in 2.1.2

Expected behavior

Toolchange offset adjustment movements result in a final printhead offset equal to 0 or the HOTEND_OFFSET_X before running EVENT_GCODE_TOOLCHANGE_T1.

Actual behavior

Printhead runs into gantry when doing toolchange that is not from origin.

Steps to Reproduce

1. Setup #define HOTEND_OFFSET_X { 0.0, 19.00 } and toolchange gcode that moves that printhead to the right limit ex: if bed X size is 216, move X to 234 which is derived from 216+19 since the toolchange gcode is run from the offsets of the target hotend.
2. Home and move hotend so X position is < 19mm from right side of gantry limits.
3. Run toolchange between T0 and T1 repeatedly.
4. Printhead will collide with gantry limit due to first offset adjustment movement or offset adjustment not working correctly.

Version of Marlin Firmware

2.1.x

Printer model

Ultimaker Original+

Electronics

Ultimaker 2+ electronics

Add-ons

DXU hotend

Bed Leveling

MBL Manual Bed Leveling

Your Slicer

Cura

Host Software

OctoPrint

Don't forget to include

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

Additional information & file uploads

Configuration.zip

[Mjankor]

Sounds somewhat similar to my issue. What happens if you do it with input shaper turned off (assuming it was on)?

[ansonl]

@Mjankor I have input shaping turned off. The input shaping X and Y defines are commented out for my config. Haven't tried it with it turned on yet, not sure if I might run out of ram on atmega256 with it.

[ansonl]

I got a slow motion recording of the strange stuttering of the initial movement when switching extruders to a second extruder that is offset in the positive direction (19.00mm).
https://youtube.com/shorts/BPpDyL0Bj_o?feature=share
It looks like a 2 quick movements are called before the normal adjust offset movement. 1 quick movement in the same direction as the wipe and and 1 quick movement in the opposite direction before it starts the normal wipe movement. The sound in the video is not loud, but it's very loud in real life.

The toolchange gcode that runs to change to the second offset extruder also does not run in the offset of the second offset extruder like it used to (and should). It appears to run within the offsets of the first extruder.

Both the 2 quick movements and the normal offset movement do not happen when do_blocking_move_to_xy(destination, planner.settings.max_feedrate_mm_s[X_AXIS]); is commented out in tool_change() toolchange.cpp. I don't see any recent changes since September that could be the issue so far...

[ansonl]

@Mjankor @tombrazier
I have narrowed down the stepper motor problem to changes in module/stepper.h and module/stepper.cpp made after September 2022.

I merged in every change in the release version of Marlin 2.1.2 and this offset issue only occurs when the new stepper.h and stepper.cpp are used. I believe that it is related to changes in #24797 and #24951. I noticed that the new changes modify a lot of frequency calculations even with input shaping turned off so they must be incompatible with AVR/Atmega256/A4988 stepper motor driver/other component on the Ultimaker Ultimainboard v2.1.4. Looks related to issue #25134 with a similar workaround to reverting any stepper.cpp changes in the input shaping commit.

The stepper motor seems to be loudly skipping when moving quickly for the tool change offset adjustment to another hotend offset and then the software position gets out of sync with the real life position leading to collisions with bed limits and grinding. https://youtube.com/shorts/V2q6s6j2uzM?feature=share

[tombrazier]

It's not obvious to me how the changes in stepper.cpp / stepper.h could cause this. Are you able to selectively apply changes (leaving out all the ones that are conditionally compiled based on HAS_SHAPING or INPUT_SHAPING_X or INPUT_SHAPING_Y? See which actual change it is that makes the difference.

[ansonl]

@tombrazier I found that the stuttering and skipping occurs when the changes for #define ISR_LOOP_CYCLES(R) are applied to stepper.h.

The code changes I made were the addition of

line 129

  // Input shaping base time
  #if HAS_SHAPING
    #define ISR_SHAPING_BASE_CYCLES 290UL
  #else
    #define ISR_SHAPING_BASE_CYCLES 0UL
  #endif

line 213

// The loop takes the base time plus the time for all the bresenham logic for R pulses plus the time
// between pulses for (R-1) pulses. But the user could be enforcing a minimum time so the loop time is:
#define ISR_LOOP_CYCLES(R) ((ISR_LOOP_BASE_CYCLES + MIN_ISR_LOOP_CYCLES + MIN_STEPPER_PULSE_CYCLES) * (R - 1) + _MAX(MIN_ISR_LOOP_CYCLES, MIN_STEPPER_PULSE_CYCLES))

// Model input shaping as an extra loop call
#define ISR_SHAPING_LOOP_CYCLES(R) ((TERN0(HAS_SHAPING, ISR_LOOP_BASE_CYCLES) + TERN0(INPUT_SHAPING_X, ISR_X_STEPPER_CYCLES) + TERN0(INPUT_SHAPING_Y, ISR_Y_STEPPER_CYCLES)) * (R) + (MIN_ISR_LOOP_CYCLES) * (R - 1))

line 243

#define ISR_EXECUTION_CYCLES(R) (((ISR_BASE_CYCLES + ISR_S_CURVE_CYCLES + ISR_SHAPING_BASE_CYCLES + ISR_LOOP_CYCLES(R) + ISR_SHAPING_LOOP_CYCLES(R) + ISR_LA_BASE_CYCLES + ISR_LA_LOOP_CYCLES)) / (R))

The issue is present with both old and new values of #define ISR_BASE_CYCLES of 752UL and 1000UL. Input shaping is not enabled in my configuration.

I tested every change to stepper.h and stepper.cpp by reverting the commits 6d87daf, b4feb4f, f3108e8 and then selectively applying changes for the remaining commits of input shaping ONEand input shaping TWO.

I have uploaded my changes to both file as gists so you can compare the before and after code in stepper.h.

[stepper code with all changes applied not dependent on input shaping] (https://gist.github.com/ansonl/fb4c8b9d7733965faf8ceee983d4b571)

[stepper code with all changes applied not dependent on input shaping + ISR_LOOP_CYCLES changes in stepper.h ] https://gist.github.com/ansonl/74a9f42fbe410c4197f27c65e7748b30

[ansonl]

@tombrazier I think that ISR_LOOP_CYCLES(R) includes an extra addition of MIN_ISR_LOOP_CYCLES, MIN_STEPPER_PULSE_CYCLES.
When I change it to below, the stepper motor skipping is gone with input shaping turned off.

#define ISR_LOOP_CYCLES(R) ((ISR_LOOP_BASE_CYCLES) * (R - 1) + _MAX(MIN_ISR_LOOP_CYCLES, MIN_STEPPER_PULSE_CYCLES))

https://github.com/MarlinFirmware/Marlin/pull/24951/files/f2c06afa72850c3dfaa5a905d75a734613b1483d#r1064159829

If input shaping is enabled the motor skipping is still present after this change but reduced. Skipping occurs when the fast movement done such as moving to new tool offset with:

do_blocking_move_to_xy(destination, planner.settings.max_feedrate_mm_s[X_AXIS]);

I tested a ringing tower with input shaping + single extruder and the input shaping works great, by the way!

Videos of skipping behavior:
stepper motor no longer skipping with input shaping turned off after ISR_LOOP_CYCLES change
stepper motor still skipping with input shaping turned on but reduced skipping after ISR_LOOP_CYCLES change

stepper motor skipping with input shaping turned off before ISR_LOOP_CYCLES change

[tombrazier]

Man those A4988s are noisy!

From #24951 (review):

ISR_LOOP_CYCLES(R) revised as below fixes skipping stepper motor driver (A4988) on AVR with tested with input shaping off and on.

I suspect this change to ISR_LOOP_CYCLES is just masking the real issue. Decreasing that value should not have the effect described unless something else is going on (or I'm confused, which is also a possibility).

Is there a reason that #define ISR_BASE_CYCLES 1000UL was changed from 752UL?

Yes. I retested these values and found they have changed due to changes in the stepper code over the years. The stepper code is supposed to keep stepper CPU usage below 50% but this was not happening. In some cases (notably on the BTT SKR Mini v3) this resulted in reboots due to the watchdog timer.

I tested a ringing tower with input shaping + single extruder and the input shaping works great, by the way!

Well that at least is good to know.

My guess is this is related to the A4988s rather than the AVR because I developed IS on my AVR machine with no problem. I think it's probably about high speed moves - so only indirectly related to the tool changer which generates a high speed move to the left. Specifically, I think what is happening is that the stepper ISR is now generating double pulses where it was previously generating single pulses.

Can you confirm that the problem only occurs for high speed moves? If so, does it go away if you reduce steps/mm?

If you apply the change in https://github.com/tombrazier/Marlin/tree/25117 you can also see on the serial port whether the stepper module is single, double, triple, etc. stepping. Are you able to do that?

[ansonl]

I tried doing moves of different lengths and feedrates and can confirm that the problem only occurs for higher speed moves. I started hearing skipping at feedrate above 7000mm/min if I did a move such as G0 F8000 X100 so this problem can be observed outside of the toolchange.

When monitoring, the serial connection the multistep serial echo, I get multistep values of 1, 2, 4, 8, 16, 32. The value seems to increase with feedrate and movement length. Something like G0 F18000 X160 from X = 0 echos 1->32 and then back down to 1 from 32 in consecutive powers of 2.
The skipping gets worse when the feedrate is faster and the movement length is shorter.

[tombrazier]

Wow 32 is really high. What is the maximum you get without the changes to ISR_LOOP_CYCLES?

And I see in your config you have 80mm/s for X and Y. Is this also the M92 setting?

[ansonl]

I tried printing multistep with both versions of ISR_LOOP_CYCLES and get the same behavior where the multistep goes from 1->32 and 32->1.

80mm/s is also the M92 setting. I originally used some values from the official ultimaker config and they currently print as M92 X80.00 Y80.00 Z200.00 E369.00.

[elbarsal]

I can confirm I have the same issue with high speed movements and input shaping. Running on bugfix-2.1.x, everything is fine at commit d4d1112 (Fix missing va_end...) but then fails in commit 89334ca (Input Shaping improvements (#24951)).

Before the problem, I can G0 X70 F9000 with no problem at all, after, it maxes out at F5500 and misses steps above that.

I'm using an Azteeg X3 Pro (a ramps based ATmega2560 board) and DRV8825 drivers by Kliment. My Y axis has a fancy "CoolMuscle" closed-loop interface that acts like a stepper driver and it too is affected (stutters, but doesn't actually lose steps since it is closed loop).

Not sure how much that helps, but if I can add any more information or testing let me know.

[ansonl]

@tombrazier Were you able to find the issue with ISR_LOOP_CYCLES based on the multistep response from 1>2>4>8>16>32?

The multistep calculation in Stepper::calc_timer_interval in stepper.cpp appears to accommodate a normal max value for multistep up to 128 (1<<7) depending on the step_rate.

[cbagwell]

Here is a debug idea in case its more CPU related instead of faster step pulses related.

When you reverted value in ISR_LOOP_CYCLES, one of main things it did was allow more steps per ISR. Since the values are meant to be worst case and if I recall AVR doesn't have watchdog timers then you probably do have some CPU free to do some extra steps.

I don't pretend to fully know what's going on in the stepper code but I've noticed even on a SKR Mini E3 V3 that when these ISRs are right on an maximum edge for CPU that it can present itself as stutter.

I think a safer option than pushing the CPU to its limits in worst case (by lowering ISR_LOOP_CYCLES) to solve stutter that its better to see if you can reduce the need for more steps in the first place. In this case, the easiest option is to remove ADAPTIVE_STEP_SMOOTHING option from your config or maybe as a test you could change the logic to only use 33% of CPU instead of 50%:

-#define MIN_STEP_ISR_FREQUENCY (MAX_STEP_ISR_FREQUENCY_1X / 2)
+#define MIN_STEP_ISR_FREQUENCY (MAX_STEP_ISR_FREQUENCY_1X / 3)

But this is also where my experience is inverse of yours. You've lowered cycle counts so what it thinks should be 50% CPU is going to actually use more CPU instead which I would think introduces more stutter than solves.

[ansonl]

@cbagwell I tested it with ADAPTIVE_STEP_SMOOTHING turned off and got the same stuttering result with the new input shaping ISR_LOOP_COUNT so there seems to be no difference with or without Adaptive step smoothing.

[cbagwell]

Thanks. Its good data to have. I was hoping it was more CPU related as that more straight forward to solve. Looks like it will be a hard one to solve correctly.

[tombrazier]

if I recall AVR doesn't have watchdog timers

It does. But we still don't want to max out the CPU in the stepper ISR.

I don't pretend to fully know what's going on in the stepper code but I've noticed even on a SKR Mini E3 V3 that when these ISRs are right on an maximum edge for CPU that it can present itself as stutter.

As I understand it, the issue is not so much stuttering as lost steps.

I think a safer option than pushing the CPU to its limits in worst case (by lowering ISR_LOOP_CYCLES) to solve stutter that its better to see if you can reduce the need for more steps in the first place.

I think this is probably the most sensible approach. However it means losing functionality that did work previously.

I suspect what is going on is that in the moves where the problems are occurring the old code did something like 16 steps per ISR and the new code is doing 32 steps per ISR. The motor was just able to keep up with a sudden burst of 16 steps but it can't keep up with a burst of 32. i.e. the machine is operating right at the margins and the new code pushes it over the edge.

The objective of the code in question is to keep the stepper ISR CPU usage <= 100%. (Not to be confused with ADAPTIVE_STEP_SMOOTHING which will try to get the CPU usage to as high as possible without going over 50%. That is not happening in this case.)

The code adopts a worst case scenario approach when assessing CPU usage. So there is actually overhead in most cases and I think in the old code the CPU usage was very high but still < 100% because of this overhead. The code changes correct the CPU usage calculations and now CPU usage is well below 100% but steps per ISR are too high. One option is to intentionally allow the stepper ISR to target more than 100% CPU.

To test this please change the definition for ISR_EXECUTION_CYCLES in stepper.h to:

#define CPU_OVER_USAGE 150
#define ISR_EXECUTION_CYCLES(R) ((((ISR_BASE_CYCLES + ISR_S_CURVE_CYCLES + ISR_SHAPING_BASE_CYCLES + ISR_LOOP_CYCLES(R) + ISR_SHAPING_LOOP_CYCLES(R) + ISR_LA_BASE_CYCLES + ISR_LA_LOOP_CYCLES)) / (R)) * (CPU_OVER_USAGE) / 100)

And play around with the value of CPU_OVER_USAGE. That will at least give us info. Whether this is the "right" solution is open to question. I think if the machine is operating at its margins a better approach is to reduce max speed. Another option is to play with MAXIMUM_STEPPER_RATE.

[ansonl]

@tombrazier I tested the changes to ISR_EXECUTION_CYCLES with the input shaping off and adaptive step smoothing on. When CPU_OVER_USAGE is increased, the stuttering is more noticeable. I was not able to reduce the stuttering/rough movement completely. Results with different CPU_OVER_USAGE:

CPU_OVER_USAGE	result
150	Very loud stuttering, lost steps visibly noticeable
100	Loud stuttering, does not seem to loose steps immediately (not sure why)
90	Loud stuttering
80	Loud stuttering
75	Stuttering
70	Rough movement sound
50	Rough movement sound

Even at CPU_OVER_USAGE set to 50, the movement is audibly rougher (like a very quiet stutter) vs my modified ISR_LOOP_CYCLES when I tested a movement from G0 F18000 X150 to G0 F18000 X120.

My modified ISR_LOOP_CYCLES from above again for reference

//Modified ISR_LOOP_CYCLES
#define ISR_LOOP_CYCLES(R) ((ISR_LOOP_BASE_CYCLES) * (R - 1) + _MAX(MIN_ISR_LOOP_CYCLES, MIN_STEPPER_PULSE_CYCLES))

//Pre-input shaping ISR_LOOP_CYCLES
#define ISR_LOOP_CYCLES (ISR_LOOP_BASE_CYCLES + _MAX(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LOOP_CYCLES))

I'm not read up on stepper motors and ISR, my initial reasoning for the modified ISR_LOOP_CYCLES was that the original pre-input shaping version of the code looked like the _MAX(MIN_STEPPER_PULSE_CYCLES, MIN_ISR_LOOP_CYCLES) was only added to ISR_LOOP_BASE_CYCLES one time.
Without understanding how the stepper and ISR cycles interact, I thought the relationship between the MIN_STEPPER_PULSE_CYCLES and MIN_ISR_LOOP_CYCLES in the original code meant that the maximum of the two was added to ISR_LOOP_BASE_CYCLES either 1 or R times - but not both at the same time?

[thinkyhead]

I'm not sure what Ulendo.io will be bringing to the table next month, but it may obviate some of these issues. Once we have their updates to the Input Shaping implementation, then we will look at optimization in concert with their developers. In the meantime, I have been mulling over how to move the Input Shaping logic into the same block as the regular stepper logic so that there can be a shorter pulse delay, somewhat optimizing the performance. Perhaps that can be attacked and refined with help from the simulator, since it allows us to examine the inner workings in the most complete detail.

[cbagwell]

That's a bit of a surprise that the full range of max CPU values didn't make any differences.

For the extra data points, I would still try the recommendation on adding MAXIMUM_STEPPER_RATE to your Configuration_adv.h and lower the max rate that way as a way to prevent pushing the A4988's to their limits. The A4988 defaults to 500000 so try maybe 400000?

Also, here is something to try to confirm it is the multisteps themselves that are pushing the A4988's to their limit.

Another side effect of less accurate ISR_EXECUTION_CYCLES (when you reverted to older values) is that it thinks it has more CPU cycles free and so will less aggressively step up from 16X to 32X multistep. With the new accurate values it will do more in a for() loop to prevent running out of CPU cycles and push the A4988 harder for longer.

If you make this change to Marlin/src/module/stepper.cpp to cap out multisteps to 16X then it will introduce extra interrupt latency that might give the A4988 the break they need. If it works then I think it helps us confirm the reason lowering ISR_LOOPS_CYCLES is working for you.

    // Select the proper multistepping
    uint8_t idx = 0;
-    while (idx < 7 && step_rate > (uint32_t)pgm_read_dword(&limit[idx])) {
+    while (idx < 5 && step_rate > (uint32_t)pgm_read_dword(&limit[idx])) {
      step_rate >>= 1;
      multistep <<= 1;
      ++idx;
    };

Your CPU_OVER_USAGE change should have been indirectly changing this 32X threshold but its also changing a few other code behavior as well so it would be nice to test multistep reduction part in isolation.

[MusicJelly]

#define ISR_SHAPING_LOOP_CYCLES(R) ((TERN0(HAS_SHAPING, ISR_LOOP_BASE_CYCLES) + TERN0(INPUT_SHAPING_X, ISR_X_STEPPER_CYCLES) + TERN0(INPUT_SHAPING_Y, ISR_Y_STEPPER_CYCLES)) * (R) + (MIN_ISR_LOOP_CYCLES) * (R - 1))

maybe need modify to

#define ISR_SHAPING_LOOP_CYCLES(R) ((TERN0(HAS_SHAPING, ISR_LOOP_BASE_CYCLES) + TERN0(INPUT_SHAPING_X, ISR_X_STEPPER_CYCLES) + TERN0(INPUT_SHAPING_Y, ISR_Y_STEPPER_CYCLES)) * (R) + (TERN0(HAS_SHAPING, MIN_ISR_LOOP_CYCLES)) * (R - 1))

If we disable "input shaping", "ISR_EXECUTION_CYCLES(R)" is wrong because it plus the "ISR_SHAPING_LOOP_CYCLES(R)" which is nonzero.

[tombrazier]

@MusicJelly I think you're right. If shaping is disabled ISR_EXECUTION_CYCLES(R) should definitely be zero. Well done for unravelling all those brackets!

I'll raise a PR for this regardless. But I think I'll also try to replicate the problems and see if I can nail them down.

[tombrazier]

I have been mulling over how to move the Input Shaping logic into the same block as the regular stepper logic

@thinkyhead that would be good. I vaguely think it would be good if the end of pulse logic was generated separately all at the end of the ISR. Or, at least, the end of the last pulse if there are multiple pulses. I think this would also help straighten out the ESP32 logic.

Incidentally I think the wait between high and low happens even if SQUARE_WAVE_STEPPING is enabled. Something else I vaguely think we should check out.

[MusicJelly]

@tombrazier I find this bug while I deal with a reboot exception. And find other bug.

In this function

uint32_t Stepper::calc_timer_interval(uint32_t step_rate, uint8_t &loops)

In some condition. step_rate will greater than MAX_STEP_ISR_FREQUENCY_128X. Then you will get a small interval and 128 loop.
But the _MIN_STEP_PERIOD_NS or (_MIN_PULSE_HIGH_NS +_MIN_PULSE_LOW_NS) is greater than interval / 128.

So, the stepper isr will always into. And the watchdog timer will timeout.
Then you will get a reboot.

You can replicate the problems. LARGE _MIN_STEP_PERIOD_NS and HIGH step_rate.

EG:
The stepper driver is TB6560 which have 15000 MAXIMUM_STEPPER_RATE and 30 MINIMUM_STEPPER_PULSE.
And after subdivision the step is 300 step/mm. Then move with 100mm/s. Then wait it finished.
The MAX_STEP_ISR_FREQUENCY_1X and MAX_STEP_ISR_FREQUENCY_128X is about 12000 Hz. But We give it 30000 Hz step_rate.
Then MCU is reboot.

It usually occurs at HOME Z. With a home_z_safely, It will do_blocking_move_to_xy with 100mm/s.

But there is a way to avoid it.
Uncomment DISABLE_MULTI_STEPPING.
Because the _MIN_PULSE_LOW_NS will short out with use MAX_STEP_ISR_FREQUENCY_1X stepper_rate.
Reboot is avoided. But another problem will appear. It will wait at move end.

Here is my solution:

diff --git a/Marlin/src/HAL/AVR/math.h b/Marlin/src/HAL/AVR/math.h
index 7dd1018f..b1ce9129 100644
--- a/Marlin/src/HAL/AVR/math.h
+++ b/Marlin/src/HAL/AVR/math.h
@@ -66,7 +66,7 @@ FORCE_INLINE static uint16_t MultiU24X32toH16(uint32_t longIn1, uint32_t longIn2
     A("add %[tmp2], r1")
     A("adc %A[intRes], %[tmp1]")
     A("adc %B[intRes], %[tmp1]")
-    A("lsr %[tmp2]")
+    A("lsl %[tmp2]")
     A("adc %A[intRes], %[tmp1]")
     A("adc %B[intRes], %[tmp1]")
     A("mul %D[longIn2], %A[longIn1]")
diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp
index 46b21a20..aa06220b 100644
--- a/Marlin/src/module/stepper.cpp
+++ b/Marlin/src/module/stepper.cpp
@@ -2144,6 +2144,7 @@ uint32_t Stepper::block_phase_isr() {
 
       // Are we in acceleration phase ?
       if (step_events_completed <= accelerate_until) { // Calculate new timer value
+        uint32_t step_rate;
 
         #if ENABLED(S_CURVE_ACCELERATION)
           // Get the next speed to use (Jerk limited!)
@@ -2151,20 +2152,20 @@ uint32_t Stepper::block_phase_isr() {
                                    ? _eval_bezier_curve(acceleration_time)
                                    : current_block->cruise_rate;
         #else
-          acc_step_rate = STEP_MULTIPLY(acceleration_time, current_block->acceleration_rate) + current_block->initial_rate;
-          NOMORE(acc_step_rate, current_block->nominal_rate);
+          acc_step_rate = step_rate = STEP_MULTIPLY(acceleration_time, current_block->acceleration_rate) + current_block->initial_rate;
+          NOMORE(step_rate, current_block->nominal_rate);
         #endif
 
         // acc_step_rate is in steps/second
 
         // step_rate to timer interval and steps per stepper isr
-        interval = calc_timer_interval(acc_step_rate << oversampling_factor, steps_per_isr);
+        interval = calc_timer_interval(step_rate << oversampling_factor, steps_per_isr);
         acceleration_time += interval;
 
         #if ENABLED(LIN_ADVANCE)
           if (current_block->la_advance_rate) {
             const uint32_t la_step_rate = la_advance_steps < current_block->max_adv_steps ? current_block->la_advance_rate : 0;
-            la_interval = calc_timer_interval(acc_step_rate + la_step_rate) << current_block->la_scaling;
+            la_interval = calc_timer_interval(step_rate + la_step_rate) << current_block->la_scaling;
           }
         #endif
 
diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h
index b279f2b9..c28df327 100644
--- a/Marlin/src/module/stepper.h
+++ b/Marlin/src/module/stepper.h
@@ -50,7 +50,7 @@
 #endif
 
 // Disable multiple steps per ISR
-//#define DISABLE_MULTI_STEPPING
+#define DISABLE_MULTI_STEPPING
 
 //
 // Estimate the amount of time the Stepper ISR will take to execute
@@ -219,7 +219,7 @@
 #define ISR_LOOP_CYCLES(R) ((ISR_LOOP_BASE_CYCLES + MIN_ISR_LOOP_CYCLES + MIN_STEPPER_PULSE_CYCLES) * (R - 1) + _MAX(MIN_ISR_LOOP_CYCLES, MIN_STEPPER_PULSE_CYCLES))
 
 // Model input shaping as an extra loop call
-#define ISR_SHAPING_LOOP_CYCLES(R) ((TERN0(HAS_SHAPING, ISR_LOOP_BASE_CYCLES) + TERN0(INPUT_SHAPING_X, ISR_X_STEPPER_CYCLES) + TERN0(INPUT_SHAPING_Y, ISR_Y_STEPPER_CYCLES)) * (R) + (MIN_ISR_LOOP_CYCLES) * (R - 1))
+#define ISR_SHAPING_LOOP_CYCLES(R) ((TERN0(HAS_SHAPING, ISR_LOOP_BASE_CYCLES) + TERN0(INPUT_SHAPING_X, ISR_X_STEPPER_CYCLES) + TERN0(INPUT_SHAPING_Y, ISR_Y_STEPPER_CYCLES)) * (R) + (TERN0(INPUT_SHAPING_Y, MIN_ISR_LOOP_CYCLES)) * (R - 1))
 
 // If linear advance is enabled, then it is handled separately
 #if ENABLED(LIN_ADVANCE)

[j-be]

I will give it a try on Friday.

[tombrazier]

The only thing I encountered is with linear advance stuttering as object is further from X=0. But don't know if it's related to this issue or should I file another one as AFAIR it also happened in 2.1.1.

Can you check with 2.1.1 please to be sure?

[er1z]

@tombrazier, confirmed, stuttering occurs on 2.1.1 as well (for what I plotted on the diagram).

[dc740]

I finally got time to test it. The situation improved a lot, BUT I still have missed steps when moving around the axis (specially on X axis).

(input shaping enabled for x and y and S-Curve disabled)

[dc740]

Sorry about the double post, but... is it possible that it skips steps because I didn't go through the IS calibration process?

[tombrazier]

is it possible that it skips steps because I didn't go through the IS calibration process?

I don't think so.

The situation improved a lot, BUT I still have missed steps when moving around the axis (specially on X axis).

Is this still only a problem at higher movement speeds? And how does it compare to 2.1.1?

I am in the process of further optimising the dynamic multi-stepping. So I think it ought to get even better soon.

[dc740]

2.1.1 was/is rock solid. But again, 2.1.1 was with S-Curve enabled. I will test S-Curve (instead of IS) as soon as possible, but I only got time to test IS so far.

[j-be]

@tombrazier Tested your branch with my config, extruder still broken with LinearAdvance and K=2.3. My config can be found here.

TL;DR: Bowden setup with Voron M4 (ESteps=565), LinearAdvance and SCurve. Extruder retracts more than it pushes forward, hence no material is coming out of the nozzle.

[dc740]

I just retested your branch (I pulled your latest changes again, there was a force push on that branch and I wanted to see what was there):

IS enabled:
misses a few steps when the motor is slowing down (when there is inertia). Starts and speed ups are fine though.

S-Curve enabled (NO linear advance, just S-Curve):
works fine, very smooth and no missed steps even at the highest speeds.

config-tevo3d-mks_genL_1.3-SCurve.zip

[thinkyhead]

Please give LA another test with 2.1.2.1 and compare to the current bugfix-2.1.x (proceed with caution during the current motion refactors), and then we can also try reverting some of the changes from #25541 and #24533 to see if they have any effect. I can create a branch with partial reversions for testing, if needed, to help isolate specific changes.

[dc740]

I tested 2.1.2.1 and bugfix-2.1.x (yesterday's checkout). But I'm still tuning it

2.1.2.1:
general movement OK. I'm printing the IS calibration tower now. I have to reduce the extruder feedrate, but that can be explained by a runout sensor that I haven't tunned properly,so the extruder has to push harder. So, all in all, it seems OK.

bugfix-2.1.x:
LOTS of grinding noises in all axes (not just the extruder). BUT I was able to print for two hours, and I can't see the missed steps. I'm thinking that maybe it was the extruder too, but it was way louder, and it happened more often too.

NOTE: This is all under review. I have issues with the extruder and the testing was not conclusive>

[dc740]

I can't get anything conclusive. It's like input shaping is disabled all times. (UPDATE: config attached below. IS + Lin Adv enabled)

Tried printing normally (80mm/s outer walls), tried printing too fast for my printer (outer walls at 200mm/s). I can't see any difference on the ringing tower height, and inspecting the gcode I see M593 commands, so the slicer (prusa slicer 2.6beta2) is inserting them just fine. Of course more speed == more artifacts on the print, but nothing changes with height. It's like input shaping is not doing anything when configuring it with M593.

After printing the ringing tower two more times, my results are the same as my previous comment: 2.1.2.1 does not skip steps (although I had to reduce the extruder feedrate to 5mm/s). bugfix-2.1.x has a lot of skipped steps, so the extruder feedrate needs to be set even slower.

I also tried the volumetric limit (enabling VOLUMETRIC_EXTRUDER_LIMIT). But they were completely ignored too. Tried enabling and disabling them through gcode. No changes at all. G0 E50 always outputs at the same speed. It only changes with F...., but no upper limit is applied to the feedrate. So I disabled the option after I was done testing. As it is, I'll revert to 2.1.1 for a couple of months to get the printer back in shape.

Printer details
It's a Tevo Tornado. I changed the motherboard for an MKS Gen v1.4 + DRV8825 drivers with a fast decay mod and smoothers on all axis. The extruder is the stock one, but with a volcano hotend, and the only add-on on the carriage is the bed level thing, which is a Geeetech 3d touch pro v3.2. Finally, the hotbed does not have springs. It's fixed, so there is very little ringing on the Y axis. These mods work OK on 2.1.1
;;;;;;
Config and gcode attached
Configuration.zip

[cbagwell]

@dc740 For what it's worth, when I tested input shaping + LA with a high K value then it messed with acceleration enough that I could not produce ringing either. Similar issue from my slicer (Cura) where minimum layer time of 10 seconds was reducing my feedrate enough to prevent ringing. Neither caused stepper noise though.

During tuning phase, you should have LA off and disable any slicer settings that slow down feedrate or acceleration to be able to produce ringing. Once tuned, you can re-enable LA and those settings.

What acceleration are you using? You'll need something high like 4000 but your config shows max of 2000 so max sure to increase both acceleration and max accelerations during tuning as well.

Finally, during your tuning, I suggest updating your Configuration_adv.h and uncommenting SHAPING_MIN_FREQ and set its value to what ever is the lowest frequency you're testing on your tower. If your board doesn't have enough memory, you will need to set the minimum to something like 20 and then update your tuning tower to start testing from that 20 frequency. Otherwise, there will not be enough buffers reserved and could cause lower parts of tower to look same as upper parts of tower.

[dc740]

Thanks for the tips. I couldn't get to the point of finding the sweet spot, but I had to revert back to 2.1.1. I don't know if it's the drivers, the fact that I'm using 1/32 stepping, or both. But I keep getting missed steps issues with the extruder. It's like the motors is not strong enough after the ISR got changed. Tried master (which is even worse) and 2.1.2.1. The extruder clearly misses steps and I have to reduce the speed A LOT to avoid them, and I really mean a lot, retractions are too slow, you can clearly see it takes a couple of seconds to get past a single retraction. Reverted back to 2.1.1 (and reverted the speeds) and everything is back to normal. I even enabled UBL and LA with the extra ram from not using IS. To be fair, the results are not as good, but I can finally stop fighting with the extruder speed.

I'd hook an oscilloscope to show the difference, but I don't have one anymore and I'm not planning to get a new one in the near future.

I'll leave the testing for someone with bigger micro steppings. It probably works on those configurations and I'll keep it in mind if I ever want to upgrade Marlin in the future. I was hitting the RAM limit (8k) already anyway and I could barely move through the menus when printing, so all in all I think I reached the best I can do with this board.Thank you all.

[ansonl]

I tested the latest bugfix2.1.x on my Atmega2560ext with A4988 stepper drivers with ADAPTIVE_STEP_SMOOTHING enabled, IS enabled, and MULTISTEPPING_LIMIT set to the default 16. It works well at lower speeds. I set the max feedrate to 300mm/s and it seems to self limit the speed to 250mm/s and acceleration even if those are set higher due to the MULTISTEPPING_LIMIT. Increasing the multistepping limit to 32 and running at high acceleration and feedrate like 9000mm/s2 and 300mm/s leads to lost steps.

[tombrazier]

Increasing the multistepping limit to 32 and running at high acceleration and feedrate like 9000mm/s2 and 300mm/s leads to lost steps.

That is what I would expect, which is why MULTISTEPPING_LIMIT was introduced. The A4988s don't go beyond 16x microstepping, if I remember correctly.

[er1z]

It looks like 2.1.2.1 is working much better, no stuttering noticed, so far. Needed to lower max speed (by 0,5 mm/s) of the extruder but finally works.

[dc740]

To those who got it working. Did you see the benefit? I modded my printer to test this better, set it to 1/16 microstepping to match the most common configuration, made a direct drive mount, calibrated Linear Advance, increased the acceleration above 5k and fired the IS test tower. The results were surprisingly disappointing. It can still be a problem with my printer, of course, but I didn't see the ringing reduced more than a small amount. I calibrated it to the best result I saw, which was at very low height, around 1cm from the bottom. The ringing was way more noticeable at the top, so I assume IS was doing something. The good news is that I don't see any missed steps anymore. (turns out I had a clogged barrel on my last 2 tests, before doing the conversion to direct extruding)

[tombrazier]

IS has had an incredible effect on my 12V bedslinger. I'm presently working on a sub 20 minute benchy and the limits are extrusion speed and cooling, not layer skipping.

But I did find that the test tower (which, in fairness, we stole borrowed from Klipper) mixes up the X and Y resonant frequencies. I have written a python script which generates a single layer test pattern that is much better (and faster and cheaper on filament). It's a bit rough and ready. You have to set one of freq_pattern, zeta_pattern_y or zeta_pattern_x to True and it will generate an appropriate pattern.

The freq_pattern script generates a gcode file that will draw X and Y lines with increasing frequency at right angles to the line. Pick the spot where the oscillations are largest (or layer skips are worst!) measure from the end of the line in mm. Divide by 2 (or whatever wavelength) is set to and that's your resonant frequency.

The other two scripts draw a set of lines with increasing zeta/damping values from 0.05 to 0.5 in increments of 0.05.

Yes, before you ask, this really ought to be added to the M593 gcode command. I will hopefully get round to it some day unless someone beats me to it. (I really hope someone beats me to it!)

[dc740]

Awesome. I can finally tune it properly!
Your script was a game changer (of course I had to set the start g-code to my temps and machine setup). I can finally see the results.

Regarding the damping. The lowest damping gives me the best line on the X axis, but the Y axis shows bad results in all.
Y damping

X damping

Am I interpreting these correctly? This is with the new IS frequencies already in the firmware.

[tombrazier]

Sorry I didn't explain how to interpret the results. You want the zigzags to have about the same amplitude across the whole line. On Y that's about 6 lines from the bottom and on X it's quite close to the right.

…

On Sun, 30 Jul 2023, 14:12 Emilio Moretti, ***@***.***> wrote: Awesome. I can finally tune it properly! Your script was a game changer (of course I had to set the start g-code to my temps and machine setup). I can finally see the results. Regarding the damping. The lowest damping gives me the best line on the X axis, but the Y axis shows bad results in all. Y damping [image: image] <https://user-images.githubusercontent.com/546408/257052903-8ac1a27c-df01-4fd7-af22-409d540aaf52.png> X damping [image: image] <https://user-images.githubusercontent.com/546408/257053013-45d2dcdd-3be3-4b4e-a59f-05b5e27fb811.png> Am I interpreting these correctly? This is with the new IS frequencies already in the firmware. — Reply to this email directly, view it on GitHub <#25117 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQNZXQJ6NBXSVOJSVYTCRQ3XSZMSZANCNFSM6AAAAAATD6J3KI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[dc740]

Thank you. I'm not seeing issues on 2.1.2.1, but I haven't tried the bugfix branch and I ran out of time. I'll probably try it in some distant future again. My concerns regarding this particular ticket are over, but someone else may still have issues.

[tombrazier]

I think this issue is resolved. Anyone disagree?

[ellensp]

closing, if it turns out not to be resolved will reopen

[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.