InsanityAutomation / Marlin

Optimized firmware for RepRap 3D printers based on the Arduino platform.
http://www.marlinfw.org/
GNU General Public License v3.0
449 stars 220 forks source link

CrealityDwin2.0_Bleeding Bugs using ENCODER_100X_STEPS_PER_SEC #278

Closed dewhisna closed 1 year ago

dewhisna commented 2 years ago

On the CrealityDwin2.0_Bleeding branch, in this code:

https://github.com/InsanityAutomation/Marlin/blob/8401c6d267949824b30e2c028e73591bce116b64/Marlin/src/lcd/e3v2/common/encoder.cpp#L142-L148

encoderMultiplier will always be set to 10 for all values higher than ENCODER_10X_STEPS_PER_SEC even if it's also higher than ENCODER_100X_STEPS_PER_SEC. That first if inside the #if defined(ENCODER_100X_STEPS_PER_SEC) will have no effect since (encoderStepRate >= ENCODER_10X_STEPS_PER_SEC) will always be true when (encoderStepRate >= ENCODER_100X_STEPS_PER_SEC) is true for the value it's assigned in the typical Configuration_adv.h: https://github.com/InsanityAutomation/Marlin/blob/8401c6d267949824b30e2c028e73591bce116b64/Marlin/Configuration_adv.h#L1347-L1351

I think you want to add an else statement to the end of that if inside the #if defined(ENCODER_100X_STEPS_PER_SEC) so that the ENCODER_10X_STEPS_PER_SEC if-statement won't run if the first one does (just like is done for the 5X case).

Or, better yet, reverse the order of all three of these -- put the 5X first, then the 10X, and then the 100X, so they'll be in order of ascending value, then the dangling else statements won't be needed.

If you prefer, I can write this up as a PR if you let me know which of the above two fixes is preferred.

dewhisna commented 2 years ago

Same bug also exists here:

https://github.com/InsanityAutomation/Marlin/blob/8401c6d267949824b30e2c028e73591bce116b64/Marlin/src/lcd/marlinui.cpp#L1029-L1032

InsanityAutomation commented 1 year ago

Upstream issue, purging those from here