From e446c34fd0b116469b4a9f2031cd6cf29b0b9b67 Mon Sep 17 00:00:00 2001 From: David Buezas Date: Sat, 9 Dec 2023 07:48:57 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8=20Encoder=20improvements=20(#26501?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Conflicts: # Marlin/src/lcd/marlinui.cpp --- Marlin/src/lcd/marlinui.cpp | 102 ++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/Marlin/src/lcd/marlinui.cpp b/Marlin/src/lcd/marlinui.cpp index 313131d87abaa..05016aafca2e7 100644 --- a/Marlin/src/lcd/marlinui.cpp +++ b/Marlin/src/lcd/marlinui.cpp @@ -67,6 +67,8 @@ 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 @@ -943,6 +945,7 @@ 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,7 +996,12 @@ void MarlinUI::init() { if (!touch_buttons) { // Integrated LCD click handling via button_pressed if (!external_control && button_pressed()) { - if (!wait_for_unclick) do_click(); // Handle the click + if (!wait_for_unclick) { + if (ELAPSED(ms, next_encoder_enable_ms)) + do_click(); // Handle the click + else + wait_for_unclick = true; + } } else wait_for_unclick = false; @@ -1030,68 +1038,69 @@ void MarlinUI::init() { uint8_t abs_diff = ABS(encoderDiff); #if ENCODER_PULSES_PER_STEP > 1 - // 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. - } if (lastEncoderDiff != encoderDiff) wake_display(); lastEncoderDiff = encoderDiff; #endif const bool encoderPastThreshold = (abs_diff >= epps); - if (encoderPastThreshold || lcd_clicked) { - if (encoderPastThreshold && TERN1(IS_TFTGLCD_PANEL, !external_control)) { + 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 (encoderRateMultiplierEnabled) { + if (lastEncoderMovementMillis) { 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 + } - 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; - - 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 - lastEncoderMovementMillis = ms; - } // encoderRateMultiplierEnabled + #else - #else + constexpr int32_t encoderMultiplier = 1; - constexpr int32_t encoderMultiplier = 1; + #endif // ENCODER_RATE_MULTIPLIER - #endif // ENCODER_RATE_MULTIPLIER + int8_t fullSteps = encoderDiff / epps; + if (fullSteps != 0) { - if (can_encode()) encoderPosition += (encoderDiff * encoderMultiplier) / epps; + #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 - encoderDiff = 0; + next_encoder_enable_ms = ms + BLOCK_CLICK_AFTER_MOVEMENT_MS; + encoderDiff -= fullSteps * epps; + if (can_encode() && !lcd_clicked) + encoderPosition += (fullSteps * encoderMultiplier); } + } + if (encoderPastThreshold || lcd_clicked) { reset_status_timeout(ms); #if HAS_BACKLIGHT_TIMEOUT @@ -1403,9 +1412,10 @@ 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; } + #define ENCODER_SPIN(_E1, _E2) switch (lastEncoderBits) { case _E1: encoderDiff += encoderDirection; break; case _E2: encoderDiff -= encoderDirection; break; default: ignore = true; } uint8_t enc = 0; if (buttons & EN_A) enc |= B01; @@ -1420,7 +1430,7 @@ void MarlinUI::init() { #if ALL(HAS_MARLINUI_MENU, AUTO_BED_LEVELING_UBL) external_encoder(); #endif - lastEncoderBits = enc; + if (!ignore) lastEncoderBits = enc; } #endif // HAS_ENCODER_WHEEL