Improved Debounce for Encoders (#1837)

* Fix variable type declaration

* Fix typo

* Two-bit encoder debouncing

* Slight optimization

* Comment change

firmware/application/hw/debounce.cpp

@@ -25,7 +25,7 @@
 #include "utility.hpp"
 
 uint8_t Debounce::state() {
-    bool v = state_to_report_;
+    uint8_t v = state_to_report_;
     simulated_pulse_ = false;
     return v;
 }
@@ -149,3 +149,24 @@ bool Debounce::feed(const uint8_t bit) {
 
     return false;
 }
+
+uint8_t EncoderDebounce::state() {
+    return state_;
+}
+
+// Returns TRUE if encoder position phase bits changed (after debouncing)
+bool EncoderDebounce::feed(const uint8_t phase_bits) {
+    history_ = (history_ << 2) | phase_bits;
+
+    // Has input been constant for 4 ticks? (phase_bits * 01010101b should be 0x00, 0x55, 0xAA, or 0xFF)
+    if (history_ == (phase_bits * 0x55)) {
+        // Has the debounced input value changed?
+        if (state_ != phase_bits) {
+            state_ = phase_bits;
+            return true;
+        }
+    }
+
+    // Unstable input, or no change
+    return false;
+}

firmware/application/hw/debounce.hpp

@@ -64,4 +64,16 @@ class Debounce {
     bool long_press_occurred_{false};  // TRUE when button is being held down and LONG_PRESS_DELAY has been reached (only when long_press_enabled)
 };
 
+class EncoderDebounce {
+   public:
+    bool feed(const uint8_t phase_bits);  // returns TRUE if state changed after debouncing
+
+    uint8_t state();  // returns debounced phase bits from encoder
+
+   private:
+    uint8_t history_{0};  // shift register of previous reads from encoder
+
+    uint8_t state_{0};  // actual encoder output state (after debounce logic)
+};
+
 #endif /*__DEBOUNCE_H__*/

firmware/application/hw/encoder.cpp

@@ -94,13 +94,8 @@ static const int8_t transition_map[][16] = {
     },
 };
 
-int_fast8_t Encoder::update(
+int_fast8_t Encoder::update(const uint_fast8_t phase_bits) {
-    const uint_fast8_t phase_0,
+    state = (state << 2) | phase_bits;
-    const uint_fast8_t phase_1) {
-    state <<= 1;
-    state |= phase_0;
-    state <<= 1;
-    state |= phase_1;
 
     // dial sensitivity setting is stored in pmem
     return transition_map[portapack::persistent_memory::config_encoder_dial_sensitivity()][state & 0xf];

firmware/application/hw/encoder.hpp

@@ -26,9 +26,7 @@
 
 class Encoder {
    public:
-    int_fast8_t update(
+    int_fast8_t update(const uint_fast8_t phase_bits);
-        const uint_fast8_t phase_0,
-        const uint_fast8_t phase_1);
 
    private:
     uint_fast8_t state{0};

firmware/application/irq_controls.cpp

@@ -43,7 +43,7 @@ static Thread* thread_controls_event = NULL;
 
 // Index with the Switch enum.
 static std::array<Debounce, 6> switch_debounce;
-static std::array<Debounce, 2> encoder_debounce;
+static EncoderDebounce encoder_debounce;
 
 static_assert(std::size(switch_debounce) == toUType(Switch::Dfu) + 1);
 
@@ -162,16 +162,12 @@ static bool switches_update(const uint8_t raw) {
 }
 
 static bool encoder_update(const uint8_t raw) {
-    bool encoder_changed = false;
+    // TODO: swap +1/-1 directions in encoder.update() to avoid needless swizzing of phase bits here
-
+    return encoder_debounce.feed(((raw >> 7) & 0x01) | ((raw >> 5) & 0x02));
-    encoder_changed |= encoder_debounce[0].feed((raw >> 6) & 0x01);
-    encoder_changed |= encoder_debounce[1].feed((raw >> 7) & 0x01);
-
-    return encoder_changed;
 }
 
 static bool encoder_read() {
-    auto delta = encoder.update(encoder_debounce[0].state(), encoder_debounce[1].state());
+    auto delta = encoder.update(encoder_debounce.state());
 
     if (injected_encoder > 0) {
         if (injected_encoder == 1) delta = -1;