From cbfacb1a595d20858a818d37e9dc7f92adbe9a0c 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 --- Marlin/src/lcd/marlinui.cpp | 110 ++++++++++++++++++++---------------- buildroot/tests/mega2560 | 1 + 2 files changed, 61 insertions(+), 50 deletions(-) diff --git a/Marlin/src/lcd/marlinui.cpp b/Marlin/src/lcd/marlinui.cpp index 178f0b74ecc5e..4eb58f52aedf0 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 @@ -880,8 +882,8 @@ void MarlinUI::init() { void MarlinUI::external_encoder() { if (external_control && encoderDiff) { bedlevel.encoder_diff += encoderDiff; // Encoder for UBL G29 mesh editing - encoderDiff = 0; // Hide encoder events from the screen handler - refresh(LCDVIEW_REDRAW_NOW); // ...but keep the refresh. + encoderDiff = 0; // Hide encoder events from the screen handler + refresh(LCDVIEW_REDRAW_NOW); // ...but keep the refresh. } } @@ -932,7 +934,8 @@ void MarlinUI::init() { void MarlinUI::update() { static uint16_t max_display_update_time = 0; - millis_t ms = millis(); + static millis_t next_encoder_enable_ms = 0; + const millis_t ms = millis(); #if LED_POWEROFF_TIMEOUT > 0 leds.update_timeout(powerManager.psu_on); @@ -982,7 +985,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; @@ -1019,68 +1027,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. - } TERN_(HAS_TOUCH_SLEEP, if (lastEncoderDiff != encoderDiff) wakeup_screen()); 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 LCD_BACKLIGHT_TIMEOUT_MINS @@ -1094,7 +1103,7 @@ void MarlinUI::init() { #if LED_POWEROFF_TIMEOUT > 0 if (!powerManager.psu_on) leds.reset_timeout(ms); #endif - } // encoder activity + } #endif // HAS_ENCODER_ACTION @@ -1393,9 +1402,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; @@ -1410,7 +1420,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 diff --git a/buildroot/tests/mega2560 b/buildroot/tests/mega2560 index d647a6ddb16c5..1fbdd6b58b201 100755 --- a/buildroot/tests/mega2560 +++ b/buildroot/tests/mega2560 @@ -33,6 +33,7 @@ opt_enable AUTO_BED_LEVELING_UBL AVOID_OBSTACLES RESTORE_LEVELING_AFTER_G28 DEBU EMERGENCY_PARSER MULTI_NOZZLE_DUPLICATION CLASSIC_JERK LIN_ADVANCE ADVANCE_K_EXTRA QUICK_HOME \ SET_PROGRESS_MANUALLY SET_PROGRESS_PERCENT PRINT_PROGRESS_SHOW_DECIMALS SHOW_REMAINING_TIME \ ENCODER_NOISE_FILTER BABYSTEPPING BABYSTEP_XY NANODLP_Z_SYNC I2C_POSITION_ENCODERS M114_DETAIL +opt_disable ENCODER_RATE_MULTIPLIER exec_test $1 $2 "Azteeg X3 Pro | EXTRUDERS 5 | RRDFGSC | UBL | LIN_ADVANCE ..." "$3" #