fix(radio): issues with rotary encoder (#5194)

philmoz · web-flow · commit d84a78cc1de0 · 2024-06-21T12:00:44.000+10:00
- 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
diff --git 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
@@ -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
@@ -35,106 +35,114 @@
   #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;
-      }
+  // No change - do nothing
+  if (pins == lastPins) {
+    return;
+  }
+
+  // Handle case where radio started with encoder not on detent position
+  if (skipUntilDetent) {
+    if (ON_DETENT(pins)) {
+      lastPins = pins;
+      skipUntilDetent = false;
     }
-    state &= ~0x03;
-    state |= pins;
+    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)) {
+
+  // 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
-        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;
-      }
+
+  // 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;
     }
-    state = pins;
+    reChgPos += ROTARY_ENCODER_GRANULARITY;
   }
-#endif
+
+  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();
     uint32_t dt = now - last_tick;
     // 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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -63,8 +63,6 @@ rotenc_t rotaryEncoderGetValue()`
`63`	`63`	`return rotencValue / ROTARY_ENCODER_GRANULARITY;`
`64`	`64`	`}`
`65`	`65`
`66`		`-rotenc_t rotaryEncoderGetRawValue() { return rotencValue; }`
`67`		`-`
`68`	`66`	`// TODO: remove all STM32 defs`
`69`	`67`
`70`	`68`	`extern const etx_hal_adc_driver_t simu_adc_driver;`