From 01a6dd865755b082b18f93c26603d2610c76d7c3 Mon Sep 17 00:00:00 2001 From: Mark Langezaal Date: Thu, 23 Jul 2020 14:44:22 +0200 Subject: [PATCH 1/2] Improved and simplified encoder handling --- Marlin/src/lcd/ultralcd.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/Marlin/src/lcd/ultralcd.cpp b/Marlin/src/lcd/ultralcd.cpp index bed72b19a399..818d2ce03cfc 100644 --- a/Marlin/src/lcd/ultralcd.cpp +++ b/Marlin/src/lcd/ultralcd.cpp @@ -901,23 +901,20 @@ void MarlinUI::update() { #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 reversed. + // 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; - int8_t prevDiff = lastEncoderDiff; - lastEncoderDiff = encoderDiff; // Store before updating encoderDiff to save actual steps - - // When not past threshold, and reversing... or past half the threshold - if (WITHIN(abs_diff, 1, (ENCODER_PULSES_PER_STEP) - 1) // Not past threshold - && (abs_diff > (ENCODER_PULSES_PER_STEP) / 2 // Passed half the threshold? Done! Call it a full step. - || (ABS(encoderDiff - prevDiff) >= (ENCODER_PULSES_PER_STEP) // A big change when abs_diff is small implies reverse - && ABS(prevDiff) < (ENCODER_PULSES_PER_STEP) // ...especially when starting from a partial or no step. - ) - ) - ) { + + // Timeout? No decoder change since last check. 10 or 20 times per second. + if (encoderDiff == lastEncoderDiff && abs_diff <= (ENCODER_PULSES_PER_STEP) / 2 ) { + encoderDiff = 0; // Clear residual value + } else + // Passed half the threshold? + if (WITHIN(abs_diff, (ENCODER_PULSES_PER_STEP) / 2 + 1, (ENCODER_PULSES_PER_STEP) - 1)) { abs_diff = ENCODER_PULSES_PER_STEP; encoderDiff = (encoderDiff < 0 ? -1 : 1) * abs_diff; // Treat as full step } + lastEncoderDiff = encoderDiff; #endif const bool encoderPastThreshold = (abs_diff >= (ENCODER_PULSES_PER_STEP)); From 59754b855fe21a46e19cdee2440e9c9495674a02 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Thu, 23 Jul 2020 19:51:36 -0500 Subject: [PATCH 2/2] Use a constexpr --- Marlin/src/lcd/ultralcd.cpp | 40 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/Marlin/src/lcd/ultralcd.cpp b/Marlin/src/lcd/ultralcd.cpp index 818d2ce03cfc..b989d6b30474 100644 --- a/Marlin/src/lcd/ultralcd.cpp +++ b/Marlin/src/lcd/ultralcd.cpp @@ -49,6 +49,8 @@ MarlinUI ui; bool MarlinUI::wait_for_move; // = false #endif +constexpr uint8_t epps = ENCODER_PULSES_PER_STEP; + #if HAS_SPI_LCD #if ENABLED(STATUS_MESSAGE_SCROLLING) uint8_t MarlinUI::status_scroll_offset; // = 0 @@ -440,13 +442,13 @@ bool MarlinUI::get_blink() { #endif { #if HAS_LCD_MENU - if (RRK(EN_KEYPAD_UP)) encoderPosition -= ENCODER_PULSES_PER_STEP; - else if (RRK(EN_KEYPAD_DOWN)) encoderPosition += ENCODER_PULSES_PER_STEP; + if (RRK(EN_KEYPAD_UP)) encoderPosition -= epps; + else if (RRK(EN_KEYPAD_DOWN)) encoderPosition += epps; else if (RRK(EN_KEYPAD_LEFT)) { MenuItem_back::action(); quick_feedback(); } else if (RRK(EN_KEYPAD_RIGHT)) encoderPosition = 0; #else - if (RRK(EN_KEYPAD_UP) || RRK(EN_KEYPAD_LEFT)) encoderPosition -= ENCODER_PULSES_PER_STEP; - else if (RRK(EN_KEYPAD_DOWN) || RRK(EN_KEYPAD_RIGHT)) encoderPosition += ENCODER_PULSES_PER_STEP; + if (RRK(EN_KEYPAD_UP) || RRK(EN_KEYPAD_LEFT)) encoderPosition -= epps; + else if (RRK(EN_KEYPAD_DOWN) || RRK(EN_KEYPAD_RIGHT)) encoderPosition += epps; #endif } #endif @@ -841,7 +843,7 @@ void MarlinUI::update() { RESET_STATUS_TIMEOUT(); if (touch_buttons & (EN_A | EN_B)) { // Menu arrows, in priority if (ELAPSED(ms, next_button_update_ms)) { - encoderDiff = (ENCODER_STEPS_PER_MENU_ITEM) * (ENCODER_PULSES_PER_STEP) * encoderDirection; + encoderDiff = (ENCODER_STEPS_PER_MENU_ITEM) * epps * encoderDirection; if (touch_buttons & EN_A) encoderDiff *= -1; TERN_(AUTO_BED_LEVELING_UBL, external_encoder()); next_button_update_ms = ms + repeat_delay; // Assume the repeat delay @@ -906,18 +908,16 @@ void MarlinUI::update() { static int8_t lastEncoderDiff; // Timeout? No decoder change since last check. 10 or 20 times per second. - if (encoderDiff == lastEncoderDiff && abs_diff <= (ENCODER_PULSES_PER_STEP) / 2 ) { - encoderDiff = 0; // Clear residual value - } else - // Passed half the threshold? - if (WITHIN(abs_diff, (ENCODER_PULSES_PER_STEP) / 2 + 1, (ENCODER_PULSES_PER_STEP) - 1)) { - abs_diff = ENCODER_PULSES_PER_STEP; - encoderDiff = (encoderDiff < 0 ? -1 : 1) * abs_diff; // Treat as full step + 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. } lastEncoderDiff = encoderDiff; #endif - const bool encoderPastThreshold = (abs_diff >= (ENCODER_PULSES_PER_STEP)); + const bool encoderPastThreshold = (abs_diff >= epps); if (encoderPastThreshold || lcd_clicked) { if (encoderPastThreshold) { @@ -926,7 +926,7 @@ void MarlinUI::update() { int32_t encoderMultiplier = 1; if (encoderRateMultiplierEnabled) { - const float encoderMovementSteps = float(abs_diff) / (ENCODER_PULSES_PER_STEP); + const float encoderMovementSteps = float(abs_diff) / epps; if (lastEncoderMovementMillis) { // Note that the rate is always calculated between two passes through the @@ -955,7 +955,7 @@ void MarlinUI::update() { #endif // ENCODER_RATE_MULTIPLIER - encoderPosition += (encoderDiff * encoderMultiplier) / (ENCODER_PULSES_PER_STEP); + encoderPosition += (encoderDiff * encoderMultiplier) / epps; encoderDiff = 0; } @@ -1188,7 +1188,7 @@ void MarlinUI::update() { // #if ANY_BUTTON(UP, DWN, LFT, RT) - const int8_t pulses = (ENCODER_PULSES_PER_STEP) * encoderDirection; + const int8_t pulses = epps * encoderDirection; if (false) { // for the else-ifs below @@ -1544,17 +1544,17 @@ void MarlinUI::update() { const int8_t xdir = col < (LCD_WIDTH ) / 2 ? -1 : 1, ydir = row < (LCD_HEIGHT) / 2 ? -1 : 1; if (on_edit_screen) - encoderDiff = (ENCODER_PULSES_PER_STEP) * ydir; + encoderDiff = epps * ydir; else if (screen_items > 0) { // Last 3 cols act as a scroll :-) if (col > (LCD_WIDTH) - 5) // 2 * LCD_HEIGHT to scroll to bottom of next page. (LCD_HEIGHT would only go 1 item down.) - encoderDiff = (ENCODER_PULSES_PER_STEP) * (encoderLine - encoderTopLine + 2 * (LCD_HEIGHT)) * ydir; + encoderDiff = epps * (encoderLine - encoderTopLine + 2 * (LCD_HEIGHT)) * ydir; else - encoderDiff = (ENCODER_PULSES_PER_STEP) * (row - encoderPosition + encoderTopLine); + encoderDiff = epps * (row - encoderPosition + encoderTopLine); } else if (!on_status_screen()) - encoderDiff = (ENCODER_PULSES_PER_STEP) * xdir; + encoderDiff = epps * xdir; } #endif