From 0f43ac79f610df25d865667e41baef707aaf40da Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Wed, 10 Jan 2024 18:25:17 -0600 Subject: [PATCH] =?UTF-8?q?=E2=8F=AA=EF=B8=8F=20Revert=20encoder=20changes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts #26501 --- Marlin/src/lcd/marlinui.cpp | 114 ++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 64 deletions(-) diff --git a/Marlin/src/lcd/marlinui.cpp b/Marlin/src/lcd/marlinui.cpp index 73094761822d..dfb2a8d47c06 100644 --- a/Marlin/src/lcd/marlinui.cpp +++ b/Marlin/src/lcd/marlinui.cpp @@ -67,8 +67,6 @@ MarlinUI ui; constexpr uint8_t epps = ENCODER_PULSES_PER_STEP; -#define BLOCK_CLICK_AFTER_MOVEMENT_MS 100 - #if HAS_STATUS_MESSAGE #if ENABLED(STATUS_MESSAGE_SCROLLING) && ANY(HAS_WIRED_LCD, DWIN_LCD_PROUI) uint8_t MarlinUI::status_scroll_offset; // = 0 @@ -942,7 +940,6 @@ void MarlinUI::init() { void MarlinUI::update() { static uint16_t max_display_update_time = 0; - static millis_t next_encoder_enable_ms = 0; const millis_t ms = millis(); #if LED_POWEROFF_TIMEOUT > 0 @@ -993,12 +990,7 @@ void MarlinUI::init() { if (!touch_buttons) { // Integrated LCD click handling via button_pressed if (!external_control && button_pressed()) { - if (!wait_for_unclick) { - if (ELAPSED(ms, next_encoder_enable_ms)) - do_click(); // Handle the click - else - wait_for_unclick = true; - } + if (!wait_for_unclick) do_click(); // Handle the click } else wait_for_unclick = false; @@ -1035,73 +1027,68 @@ void MarlinUI::init() { uint8_t abs_diff = ABS(encoderDiff); #if ENCODER_PULSES_PER_STEP > 1 - #if HAS_TOUCH_SLEEP - static int8_t lastEncoderDiff; - if (lastEncoderDiff != encoderDiff) { - wakeup_screen(); - lastEncoderDiff = encoderDiff; - } - #endif + // When reversing the encoder direction, a movement step can be missed because + // encoderDiff has a non-zero residual value, making the controller unresponsive. + // The fix clears the residual value when the encoder is idle. + // Also check if past half the threshold to compensate for missed single steps. + static int8_t lastEncoderDiff; + + // Timeout? No decoder change since last check. 10 or 20 times per second. + if (encoderDiff == lastEncoderDiff && abs_diff <= epps / 2) // Same direction & size but not over a half-step? + encoderDiff = 0; // Clear residual pulses. + else if (WITHIN(abs_diff, epps / 2 + 1, epps - 1)) { // Past half of threshold? + abs_diff = epps; // Treat as a full step size + encoderDiff = (encoderDiff < 0 ? -1 : 1) * abs_diff; // ...in the spin direction. + } + TERN_(HAS_TOUCH_SLEEP, if (lastEncoderDiff != encoderDiff) wakeup_screen()); + lastEncoderDiff = encoderDiff; #endif const bool encoderPastThreshold = (abs_diff >= epps); - if (encoderPastThreshold && TERN1(IS_TFTGLCD_PANEL, !external_control)) { + if (encoderPastThreshold || lcd_clicked) { + if (encoderPastThreshold && TERN1(IS_TFTGLCD_PANEL, !external_control)) { - #if ALL(HAS_MARLINUI_MENU, ENCODER_RATE_MULTIPLIER) + #if ALL(HAS_MARLINUI_MENU, ENCODER_RATE_MULTIPLIER) - int32_t encoderMultiplier = 1; + int32_t encoderMultiplier = 1; - if (encoderRateMultiplierEnabled) { - if (lastEncoderMovementMillis) { + if (encoderRateMultiplierEnabled) { const float encoderMovementSteps = float(abs_diff) / epps; - // Note that the rate is always calculated between two passes through the - // loop and that the abs of the encoderDiff value is tracked. - const float encoderStepRate = encoderMovementSteps / float(ms - lastEncoderMovementMillis) * 1000; - - if (encoderStepRate >= ENCODER_100X_STEPS_PER_SEC) encoderMultiplier = 100; - else if (encoderStepRate >= ENCODER_10X_STEPS_PER_SEC) encoderMultiplier = 10; - - // Enable to output the encoder steps per second value - //#define ENCODER_RATE_MULTIPLIER_DEBUG - #if ENABLED(ENCODER_RATE_MULTIPLIER_DEBUG) - SERIAL_ECHO_START(); - SERIAL_ECHOPGM("Enc Step Rate: ", encoderStepRate); - SERIAL_ECHOPGM(" Multiplier: ", encoderMultiplier); - SERIAL_ECHOPGM(" ENCODER_10X_STEPS_PER_SEC: ", ENCODER_10X_STEPS_PER_SEC); - SERIAL_ECHOPGM(" ENCODER_100X_STEPS_PER_SEC: ", ENCODER_100X_STEPS_PER_SEC); - SERIAL_EOL(); - #endif - } - lastEncoderMovementMillis = ms; - } // encoderRateMultiplierEnabled + if (lastEncoderMovementMillis) { + // Note that the rate is always calculated between two passes through the + // loop and that the abs of the encoderDiff value is tracked. + const float encoderStepRate = encoderMovementSteps / float(ms - lastEncoderMovementMillis) * 1000; - #else + if (encoderStepRate >= ENCODER_100X_STEPS_PER_SEC) encoderMultiplier = 100; + else if (encoderStepRate >= ENCODER_10X_STEPS_PER_SEC) encoderMultiplier = 10; - constexpr int32_t encoderMultiplier = 1; + // Enable to output the encoder steps per second value + //#define ENCODER_RATE_MULTIPLIER_DEBUG + #if ENABLED(ENCODER_RATE_MULTIPLIER_DEBUG) + SERIAL_ECHO_START(); + SERIAL_ECHOPGM("Enc Step Rate: ", encoderStepRate); + SERIAL_ECHOPGM(" Multiplier: ", encoderMultiplier); + SERIAL_ECHOPGM(" ENCODER_10X_STEPS_PER_SEC: ", ENCODER_10X_STEPS_PER_SEC); + SERIAL_ECHOPGM(" ENCODER_100X_STEPS_PER_SEC: ", ENCODER_100X_STEPS_PER_SEC); + SERIAL_EOL(); + #endif + } - #endif // ENCODER_RATE_MULTIPLIER + lastEncoderMovementMillis = ms; + } // encoderRateMultiplierEnabled - int8_t fullSteps = encoderDiff / epps; - if (fullSteps != 0) { + #else - #if ENABLED(ENCODER_RATE_MULTIPLIER) - static bool lastFwd; - const bool fwd = fullSteps > 0; - if (encoderMultiplier != 1 && fwd != lastFwd) - fullSteps *= -1; // Fast move and direction changed? Assume glitch. - else - lastFwd = fwd; // Slow move or lastFwd==fwd already. Remember dir. - #endif + constexpr int32_t encoderMultiplier = 1; + + #endif // ENCODER_RATE_MULTIPLIER + + if (can_encode()) encoderPosition += (encoderDiff * encoderMultiplier) / epps; - next_encoder_enable_ms = ms + BLOCK_CLICK_AFTER_MOVEMENT_MS; - encoderDiff -= fullSteps * epps; - if (can_encode() && !lcd_clicked) - encoderPosition += (fullSteps * encoderMultiplier); + encoderDiff = 0; } - } - if (encoderPastThreshold || lcd_clicked) { reset_status_timeout(ms); #if HAS_BACKLIGHT_TIMEOUT @@ -1115,7 +1102,7 @@ void MarlinUI::init() { #if LED_POWEROFF_TIMEOUT > 0 if (!powerManager.psu_on) leds.reset_timeout(ms); #endif - } + } // encoder activity #endif // HAS_ENCODER_ACTION @@ -1413,10 +1400,9 @@ void MarlinUI::init() { #if HAS_ENCODER_WHEEL static uint8_t lastEncoderBits; - bool ignore = false; // Manage encoder rotation - #define ENCODER_SPIN(_E1, _E2) switch (lastEncoderBits) { case _E1: encoderDiff += encoderDirection; break; case _E2: encoderDiff -= encoderDirection; break; default: ignore = true; } + #define ENCODER_SPIN(_E1, _E2) switch (lastEncoderBits) { case _E1: encoderDiff += encoderDirection; break; case _E2: encoderDiff -= encoderDirection; } uint8_t enc = 0; if (buttons & EN_A) enc |= B01; @@ -1431,7 +1417,7 @@ void MarlinUI::init() { #if ALL(HAS_MARLINUI_MENU, AUTO_BED_LEVELING_UBL) external_encoder(); #endif - if (!ignore) lastEncoderBits = enc; + lastEncoderBits = enc; } #endif // HAS_ENCODER_WHEEL