From d84a78cc1de0a6f288e641c438b1354770a164c2 Mon Sep 17 00:00:00 2001 From: philmoz Date: Fri, 21 Jun 2024 12:00:44 +1000 Subject: [PATCH] fix(radio): issues with rotary encoder (#5194) - detection of initial direction of encoder after startup - change of direction issue on radios with incorrectly wired encoder - handle all wiring variations (without needing special #defines) - handle the case when the radio is started with the encoder not on a detent position --- radio/src/gui/colorlcd/LvglWrapper.cpp | 6 +- radio/src/hal/rotary_encoder.h | 4 - .../arm/stm32/rotary_encoder_driver.cpp | 150 ++++++++++-------- radio/src/targets/horus/hal.h | 4 - radio/src/targets/simu/simpgmspace.cpp | 2 - radio/src/targets/taranis/hal.h | 3 - 6 files changed, 84 insertions(+), 85 deletions(-) diff --git a/radio/src/gui/colorlcd/LvglWrapper.cpp b/radio/src/gui/colorlcd/LvglWrapper.cpp index 666c83823..8eac03776 100644 --- a/radio/src/gui/colorlcd/LvglWrapper.cpp +++ b/radio/src/gui/colorlcd/LvglWrapper.cpp @@ -272,14 +272,14 @@ static void rotaryDriverRead(lv_indev_drv_t *drv, lv_indev_data_t *data) static int8_t prevDir = 0; static uint32_t lastDt = 0; - rotenc_t newPos = rotaryEncoderGetRawValue(); - rotenc_t diff = (newPos - prevPos) / ROTARY_ENCODER_GRANULARITY; - prevPos += diff * ROTARY_ENCODER_GRANULARITY; + rotenc_t newPos = rotaryEncoderGetValue(); + rotenc_t diff = newPos - prevPos; data->enc_diff = (int16_t)diff; data->state = LV_INDEV_STATE_RELEASED; if (diff != 0) { + prevPos = newPos; reset_inactivity(); int8_t dir = 0; diff --git a/radio/src/hal/rotary_encoder.h b/radio/src/hal/rotary_encoder.h index 510c6d598..370d3cb40 100644 --- a/radio/src/hal/rotary_encoder.h +++ b/radio/src/hal/rotary_encoder.h @@ -40,9 +40,5 @@ void rotaryEncoderInit(); // return impulses / granularity rotenc_t rotaryEncoderGetValue(); -// returns raw # impulses -rotenc_t rotaryEncoderGetRawValue(); - - int8_t rotaryEncoderGetAccel(); void rotaryEncoderResetAccel(); diff --git a/radio/src/targets/common/arm/stm32/rotary_encoder_driver.cpp b/radio/src/targets/common/arm/stm32/rotary_encoder_driver.cpp index 458803266..23400a7b4 100644 --- a/radio/src/targets/common/arm/stm32/rotary_encoder_driver.cpp +++ b/radio/src/targets/common/arm/stm32/rotary_encoder_driver.cpp @@ -35,98 +35,106 @@ #include "opentx.h" #endif +#if ROTARY_ENCODER_GRANULARITY == 2 + #define ON_DETENT(p) ((p == 3) || (p == 0)) +#elif ROTARY_ENCODER_GRANULARITY == 4 + #define ON_DETENT(p) (p == 3) +#elif +#error "Unknown ROTARY_ENCODER_GRANULARITY" +#endif + volatile rotenc_t rotencValue = 0; volatile uint32_t rotencDt = 0; -#if defined(BOOT) -#define INC_ROT 1 -#define INC_ROT_2 2 -#else -#define INC_ROT \ - (g_eeGeneral.rotEncMode == ROTARY_ENCODER_MODE_INVERT_BOTH ? -1 : 1); -#define INC_ROT_2 \ - (g_eeGeneral.rotEncMode == ROTARY_ENCODER_MODE_INVERT_BOTH ? -2 : 2); -#endif +// Last encoder pins state +static uint8_t lastPins = 0; +// Record encoder position change between detents +int8_t reChgPos = 0; +// Used on start to ignore movement until encoder position on detent +bool skipUntilDetent = false; rotenc_t rotaryEncoderGetValue() -{ - return rotencValue / ROTARY_ENCODER_GRANULARITY; -} - -rotenc_t rotaryEncoderGetRawValue() { return rotencValue; } void rotaryEncoderCheck() { - static uint8_t state = 0; - static uint8_t re_count = 0; + // Value increment for each state transition of the RE pins +#if defined(ROTARY_ENCODER_INVERTED) + static int8_t reInc[4][4] = { + // Prev = 0 + { 0, -1, 1, -2 }, + // Prev = 1 + { 1, 0, 0, -1 }, + // Prev = 2 + { -1, 0, 0, 1 }, + // Prev = 3 + { 2, 1, -1, 0 }, + }; +#else + static int8_t reInc[4][4] = { + // Prev = 0 + { 0, 1, -1, 2 }, + // Prev = 1 + { -1, 0, 0, 1 }, + // Prev = 2 + { 1, 0, 0, -1 }, + // Prev = 3 + { -2, -1, 1, 0 }, + }; +#endif + uint8_t pins = ROTARY_ENCODER_POSITION(); -#if defined(ROTARY_ENCODER_SUPPORT_BUGGY_WIRING) - if (pins != (state & 0x03) && !(readKeys() & (1 << KEY_ENTER))) { - if (re_count == 0) { - // Need at least 2 values to correctly determine initial direction - re_count = 1; - } else { - if ((pins ^ (state & 0x03)) == 0x03) { - if (pins == 3) { - rotencValue += INC_ROT_2; - } else { - rotencValue -= INC_ROT_2; - } - } else { - if ((state & 0x01) ^ ((pins & 0x02) >> 1)) { - rotencValue -= INC_ROT; - } else { - rotencValue += INC_ROT; - } - } - - if (re_count == 1) - { - re_count = 2; - // Assume 1st value is same direction as 2nd value - rotencValue = rotencValue * 2; - } - } - state &= ~0x03; - state |= pins; + // No change - do nothing + if (pins == lastPins) { + return; } -#else - if (pins != state && !(readKeys() & (1 << KEY_ENTER))) { - if (re_count == 0) { - // Need at least 2 values to correctly determine initial direction - re_count = 1; - } else { -#if defined(ROTARY_ENCODER_INVERTED) - if (!(state & 0x01) ^ ((pins & 0x02) >> 1)) { -#else - if ((state & 0x01) ^ ((pins & 0x02) >> 1)) { -#endif - rotencValue -= INC_ROT; - } else { - rotencValue += INC_ROT; - } - if (re_count == 1) - { - re_count = 2; - // Assume 1st value is same direction as 2nd value - rotencValue = rotencValue * 2; - } + // Handle case where radio started with encoder not on detent position + if (skipUntilDetent) { + if (ON_DETENT(pins)) { + lastPins = pins; + skipUntilDetent = false; } - state = pins; + return; } + + // Get increment value for pin state transition + int inc = reInc[lastPins][pins]; + +#if !defined(BOOT) + if (g_eeGeneral.rotEncMode == ROTARY_ENCODER_MODE_INVERT_BOTH) + inc = -inc; #endif + // Update position change between detents + reChgPos += inc; + + // Update reported value on full detent change + if (reChgPos >= ROTARY_ENCODER_GRANULARITY) { + // If ENTER pressed - ignore scrolling + if ((readKeys() & (1 << KEY_ENTER)) == 0) { + rotencValue += 1; + } + reChgPos -= ROTARY_ENCODER_GRANULARITY; + } else if (reChgPos <= -ROTARY_ENCODER_GRANULARITY) { + // If ENTER pressed - ignore scrolling + if ((readKeys() & (1 << KEY_ENTER)) == 0) { + rotencValue -= 1; + } + reChgPos += ROTARY_ENCODER_GRANULARITY; + } + + lastPins = pins; + #if !defined(BOOT) && defined(COLORLCD) static uint32_t last_tick = 0; static rotenc_t last_value = 0; rotenc_t value = rotencValue; - rotenc_t diff = (value - last_value) / ROTARY_ENCODER_GRANULARITY; + rotenc_t diff = (value - last_value); if (diff != 0) { uint32_t now = RTOS_GET_MS(); @@ -134,7 +142,7 @@ void rotaryEncoderCheck() // pre-compute accumulated dt (dx/dt is done later in LVGL driver) rotencDt += dt; last_tick = now; - last_value += diff * ROTARY_ENCODER_GRANULARITY; + last_value = value; } #endif } @@ -181,6 +189,10 @@ void rotaryEncoderInit() NVIC_EnableIRQ(ROTARY_ENCODER_TIMER_IRQn); NVIC_SetPriority(ROTARY_ENCODER_TIMER_IRQn, 7); + + // Get initial position + lastPins = ROTARY_ENCODER_POSITION(); + skipUntilDetent = !ON_DETENT(lastPins); } extern "C" void ROTARY_ENCODER_TIMER_IRQHandler(void) diff --git a/radio/src/targets/horus/hal.h b/radio/src/targets/horus/hal.h index 2c9598624..2f58a95c0 100644 --- a/radio/src/targets/horus/hal.h +++ b/radio/src/targets/horus/hal.h @@ -111,10 +111,6 @@ #define ROTARY_ENCODER_INVERTED #endif -#if defined(RADIO_FAMILY_T16) && !defined(RADIO_T18) && !defined(RADIO_T15) - #define ROTARY_ENCODER_SUPPORT_BUGGY_WIRING -#endif - // Switches #if defined(RADIO_T15) #define STORAGE_SWITCH_A diff --git a/radio/src/targets/simu/simpgmspace.cpp b/radio/src/targets/simu/simpgmspace.cpp index 31ce381d2..9fc805385 100644 --- a/radio/src/targets/simu/simpgmspace.cpp +++ b/radio/src/targets/simu/simpgmspace.cpp @@ -63,8 +63,6 @@ rotenc_t rotaryEncoderGetValue() return rotencValue / ROTARY_ENCODER_GRANULARITY; } -rotenc_t rotaryEncoderGetRawValue() { return rotencValue; } - // TODO: remove all STM32 defs extern const etx_hal_adc_driver_t simu_adc_driver; diff --git a/radio/src/targets/taranis/hal.h b/radio/src/targets/taranis/hal.h index 6d2d89b24..edd62e00d 100644 --- a/radio/src/targets/taranis/hal.h +++ b/radio/src/targets/taranis/hal.h @@ -303,9 +303,6 @@ #define USE_EXTI15_10_IRQ #define EXTI15_10_IRQ_Priority 5 #endif - #if defined(RADIO_TX12) - #define ROTARY_ENCODER_SUPPORT_BUGGY_WIRING - #endif #if defined(RADIO_TX12MK2) || defined(RADIO_BOXER) || defined(RADIO_ZORRO) || defined(RADIO_MT12) || defined(RADIO_POCKET) || defined(RADIO_T14) #define ROTARY_ENCODER_INVERTED #endif