[BUG] Babystepping causes freeze on delta printer with marlin 2.0.x bugfix but not on older 1.1.x versions - caused by wrong calc with MINIMUM_STEPPER_PULSE and _PULSE_WAIT

[MSpencer0]

Description

Compiling Marlin 2 bugfix for a ANET A4 delta (Melzi board with atmega 128k) and 20x4 LCD controller.

Activated babystepping with doubleclick feature during print as well as:

• NO_WORKSPACE_OFFSETS
  Update: independent if ON or OFF -> Problem still there

• MIN_STEPS_PER_SEGMENT set to 2 -> for 20um on a A4

• S-Curve active
  -> Update: independent if ON or OFF -> Problem still there

• Adaptive step smooth active
  -> Update: independent if ON or OFF -> Problem still there

• endstops_interrupt_feature is already OFF
  -> Seen that babystepping is ISR sensitive.

• ABL and Delta Auto Calibration: With or without this active with manual/LCD leveling
  -> Problem still there

Steps to Reproduce

1. ) Printing from SD card. First layer started and entering babystepping via doubleclick.
   2.) Try to adjust with up or down button
   -> Crashes / Hangs up, after approx 2 sec all actuators are shut off. Have to do a power-on-reset to get the printer up and running again.

Expected behavior: [

Babystepping should work.

Actual behavior:

Hangs up.

Additional Information

Marlin2A4_config.zip

Build done on Arduino 1.8.8.
update: config zip attached.

Can you reproduce or is it a known issue? Which compiler switch can be related to that issue? Something already known or further investigation needed.

[MSpencer0]

Configured down with very few compiler switches and even without "doubleclick" but only babystepping accessible via TUNE-menu. That config in zip attached, but problem still exists.

Now migrating config to Marlin 1.1.x release and I will try again.
Update: Babystepping on 1.1.9 does work on the ANET A4 config!
Attached the working config for 1.1.9, too. Any idea where babystepping got lost on 1.1.9 towards 2.0.x?
Marlin1.1.9_conf_working.zip

********************************* UPDATE AND TESTS *************************************
I can provide the following summary so far:
Tested with "equivalent" configs -> provided above for 2.0.x bufgix as well as for 1.1.x release and bugfix version.
Babystepping works both on 1.1.9 release and bugfix.
But it does not work on 2.0.x bugfix as from 26.Feb. 2019 - and as of 4.March2019

• Any idea where that feature got lost during migration to 2.0.x?
• I would expect other Delta printer users with an atmega and LCD might have the same issue as well
• Due to this analysis I classified this as "bug"

[MSpencer0]

Found it! Value overflow in stepper.cpp when calculating EXTRA_CYCLES_BABYSTEP.
This finally has lead to a faulty definition of _PULSE_WAIT which is only(!) used in babystepping.

In old Marlin the default for MINIMUM_STEPPER_PULSE when not given was 2.
That changed in new Marlin to a default value of 1.

This "MINIMUM_STEPPER_PULSE" is taken in stepper.cpp to calculate EXTRA_CYCLES_BABYSTEP
the following way:

#define EXTRA_CYCLES_BABYSTEP (STEP_PULSE_CYCLES - (CYCLES_EATEN_BABYSTEP))

But it does not check if a higher value is subtracted from a smaller one (which is not the case on a 16MHz CPU with MINIMUM_STEPPER_PULSE of 2 - BUT: It takes place when MINIMUM_STEPPER_PULSE is 1 - like in the new default settings of Marlin 2 )

Solution:
Replace this line in stepper.cpp:
#define EXTRA_CYCLES_BABYSTEP (STEP_PULSE_CYCLES - (CYCLES_EATEN_BABYSTEP))

With this one:

  #if STEP_PULSE_CYCLES>CYCLES_EATEN_BABYSTEP
	#define EXTRA_CYCLES_BABYSTEP (STEP_PULSE_CYCLES - (CYCLES_EATEN_BABYSTEP))
  #else
	#define EXTRA_CYCLES_BABYSTEP 0  
  #endif

Hope this helps. I would expect this to be an issue on more machines when trying to babystep. Not only on my ANET A4 atmega128 A4988 with minstepperpulse=1 configuration.

I will leave this item open until confirmed by core-team. Maybe this bugfix is worth applying to mainline.
I do roughly remeber other babystepping related topics where people manually modified MINIMUM_STEPPER_PULSE. This seems to have worked, but is related to the same wrong calculation without overflow-checks (see above).
My personal recommendation is to add the modified #define as robustness measure.
I would not expect additional side-effects as the defines are not used somewhere else.

I will leave this topic open until inspected and evaluated if worth applying to mainline or not for core team. Just had a quick look - seems to be an issue ever since having minstepperpulse configured to 1.
With configured to 0 and 2 seems to work. All that looks strong related to the above calculation...

Thank you for your current support and efforts so far! Hope this helps a little bit! Great work! Thank You!

[tig33r]

Is this could be connected with #13309 and #13315?

[MSpencer0]

Maybe check what happens if you force
MINIMUM_STEPPER_PULSE to a value of 2 or 3 - See configuration_adv.h
Is the problem still there then?

If this solves your issue it is still not strongly related to my topic above. But maybe another side effect of changing the default of MINIMUM_STEPPER_PULSE. I haven't had that issue on my machine , yet.

[Stady234]

I just enabled
#define MINIMUM_STEPPER_PULSE 2
on line 1131 of Configuration_adv.h and now my babystep issue looks to be fix. No more lockup of the machine.

[MSpencer0]

Hi Stady234,
you are actually describing a workaround for that bug. But in general it is still a bug due to an uncaught exception/overflow.
But for a proper software even a vaild MINIMUM_STEPPER_PULSE of 1 shall not lead to a crash.
Therefore the proposed bugfix would be a clean solution for that.

[Stady234]

You are correct.. i was simply stating that changing that value back to the previous commit of 2.. solved it.. i have since made the changes in the stepper file as you showed and reset the stepper pulse back to 1 and all is well.

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