Additional debounce control parameters for rotary encoder settings (for bad/noisy encoders) (#2841)

* Added debounce control options for rotary encoder settings * ran format-code.sh to adjust whitespace --------- Co-authored-by: Robert McKay <robert.mckay@ubermorgen.land>
2025-12-01 15:37:57 +00:00 · 2025-10-27 22:56:27 +00:00
parent 81afec9ea4
commit bf18851b6b
8 changed files with 144 additions and 64 deletions
--- a/firmware/application/apps/ui_settings.cpp
+++ b/firmware/application/apps/ui_settings.cpp
@@ -681,35 +681,27 @@ SetEncoderDialView::SetEncoderDialView(NavigationView& nav) {
    add_children({&labels,
                  &field_encoder_dial_sensitivity,
                  &field_encoder_rate_multiplier,
+                  &field_encoder_dial_direction,
+                  &field_encoder_consecutive_hits,
+                  &field_encoder_cooldown_ms,
+                  &field_encoder_debounce_ms,
                  &button_save,
-                  &button_cancel,
-                  &button_dial_sensitivity_plus,
-                  &button_dial_sensitivity_minus,
-                  &button_rate_multiplier_plus,
-                  &button_rate_multiplier_minus,
-                  &field_encoder_dial_direction});
+                  &button_cancel});

    field_encoder_dial_sensitivity.set_by_value(pmem::encoder_dial_sensitivity());
    field_encoder_rate_multiplier.set_value(pmem::encoder_rate_multiplier());
    field_encoder_dial_direction.set_by_value(pmem::encoder_dial_direction());
-
-    button_dial_sensitivity_plus.on_select = [this](Button&) {
-        field_encoder_dial_sensitivity.on_encoder(1);
-    };
-    button_dial_sensitivity_minus.on_select = [this](Button&) {
-        field_encoder_dial_sensitivity.on_encoder(-1);
-    };
-    button_rate_multiplier_plus.on_select = [this](Button&) {
-        field_encoder_rate_multiplier.on_encoder(1);
-    };
-    button_rate_multiplier_minus.on_select = [this](Button&) {
-        field_encoder_rate_multiplier.on_encoder(-1);
-    };
+    field_encoder_consecutive_hits.set_value(pmem::encoder_consecutive_hits());
+    field_encoder_cooldown_ms.set_value(pmem::encoder_cooldown_ms());
+    field_encoder_debounce_ms.set_value(pmem::encoder_debounce_ms());

    button_save.on_select = [&nav, this](Button&) {
        pmem::set_encoder_dial_sensitivity(field_encoder_dial_sensitivity.selected_index_value());
        pmem::set_encoder_rate_multiplier(field_encoder_rate_multiplier.value());
        pmem::set_encoder_dial_direction(field_encoder_dial_direction.selected_index_value());
+        pmem::set_encoder_consecutive_hits(field_encoder_consecutive_hits.value());
+        pmem::set_encoder_cooldown_ms(field_encoder_cooldown_ms.value());
+        pmem::set_encoder_debounce_ms(field_encoder_debounce_ms.value());
        nav.pop();
    };

--- a/firmware/application/apps/ui_settings.hpp
+++ b/firmware/application/apps/ui_settings.hpp
@@ -559,58 +559,63 @@ class SetEncoderDialView : public View {

   private:
    Labels labels{
-        {{UI_POS_X(0), UI_POS_Y(0)}, "Sensitivity to dial rotation", Theme::getInstance()->fg_light->foreground},
-        {{UI_POS_X(0), 1 * 16}, "position (x steps per 360):", Theme::getInstance()->fg_light->foreground},
-        {{1 * 8, 3 * 16}, "Sensitivity:", Theme::getInstance()->fg_light->foreground},
-        {{UI_POS_X(0), 7 * 16}, "Rotation rate (default 1", Theme::getInstance()->fg_light->foreground},
-        {{UI_POS_X(0), 8 * 16}, "means no rate dependency):", Theme::getInstance()->fg_light->foreground},
-        {{2 * 8, 10 * 16}, "Rate multiplier:", Theme::getInstance()->fg_light->foreground},
-        {{4 * 8, 14 * 16}, "Direction:", Theme::getInstance()->fg_light->foreground},
+        {{1 * 8, 1 * 16}, "Sensitivity:", Theme::getInstance()->fg_light->foreground},
+        {{1 * 8, 4 * 16}, "Rate mult:", Theme::getInstance()->fg_light->foreground},
+        {{1 * 8, 7 * 16}, "Direction:", Theme::getInstance()->fg_light->foreground},
+        {{UI_POS_X(0), 9 * 16}, "--- Debounce (noisy dial) ---", Theme::getInstance()->fg_light->foreground},
+        {{1 * 8, 10 * 16}, "Consec hits:", Theme::getInstance()->fg_light->foreground},
+        {{1 * 8, 11 * 16}, "Cooldown ms:", Theme::getInstance()->fg_light->foreground},
+        {{1 * 8, 12 * 16}, "Debounce ms:", Theme::getInstance()->fg_light->foreground},

    };

    OptionsField field_encoder_dial_sensitivity{
-        {20 * 8, 3 * 16},
+        {14 * 8, 1 * 16},
        6,
        {{"LOW", encoder_dial_sensitivity::DIAL_SENSITIVITY_LOW},
         {"NORMAL", encoder_dial_sensitivity::DIAL_SENSITIVITY_NORMAL},
         {"HIGH", encoder_dial_sensitivity::DIAL_SENSITIVITY_HIGH}}};

    NumberField field_encoder_rate_multiplier{
-        {20 * 8, 10 * 16},
+        {14 * 8, 4 * 16},
        2,
        {1, 15},
        1,
        ' '};

    OptionsField field_encoder_dial_direction{
-        {18 * 8, 14 * 16},
+        {14 * 8, 7 * 16},
        7,
        {{"NORMAL", false},
         {"REVERSE", true}}};

-    Button button_dial_sensitivity_plus{
-        {20 * 8, 2 * 16, 16, 16},
-        "+"};
+    NumberField field_encoder_consecutive_hits{
+        {14 * 8, 10 * 16},
+        2,
+        {1, 10},
+        1,
+        ' '};

-    Button button_dial_sensitivity_minus{
-        {20 * 8, 4 * 16, 16, 16},
-        "-"};
+    NumberField field_encoder_cooldown_ms{
+        {14 * 8, 11 * 16},
+        3,
+        {0, 255},
+        5,
+        ' '};

-    Button button_rate_multiplier_plus{
-        {20 * 8, 9 * 16, 16, 16},
-        "+"};
-
-    Button button_rate_multiplier_minus{
-        {20 * 8, 11 * 16, 16, 16},
-        "-"};
+    NumberField field_encoder_debounce_ms{
+        {14 * 8, 12 * 16},
+        2,
+        {4, 32},
+        2,
+        ' '};

    Button button_save{
-        {UI_POS_X_CENTER(12) - UI_POS_WIDTH(8), UI_POS_Y_BOTTOM(4), 12 * 8, 32},
+        {2 * 8, 14 * 16, 12 * 8, 24},
        "Save"};

    Button button_cancel{
-        {UI_POS_X_CENTER(12) + UI_POS_WIDTH(8), UI_POS_Y_BOTTOM(4), 12 * 8, 32},
+        {16 * 8, 14 * 16, 12 * 8, 24},
        "Cancel",
    };
 };
--- a/firmware/application/hw/debounce.cpp
+++ b/firmware/application/hw/debounce.cpp
@@ -163,15 +163,28 @@ uint8_t EncoderDebounce::rotation_rate() {

 // Returns TRUE if encoder position phase bits changed (after debouncing)
 bool EncoderDebounce::feed(const uint8_t phase_bits) {
+    // Shift in new 2-bit sample into 32-bit history (16 samples total)
    history_ = (history_ << 2) | phase_bits;

-    // If both inputs have been stable for the last 4 ticks, history_ should equal 0x00, 0x55, 0xAA, or 0xFF.
-    uint8_t expected_stable_history = phase_bits * 0b01010101;
+    // For very noisy encoders: require BOTH bits stable for N consecutive ticks
+    // Get configurable debounce window (4-32ms)
+    uint8_t debounce_samples = portapack::persistent_memory::encoder_debounce_ms();

-    // But, checking for equal seems to cause issues with at least 1 user's encoder, so we're treating the input
-    // as "stable" if at least ONE input bit is consistent for 4 ticks...
-    uint8_t diff = (history_ ^ expected_stable_history);
-    if (((diff & 0b01010101) == 0) || ((diff & 0b10101010) == 0)) {
+    // Build expected pattern: phase_bits repeated debounce_samples times
+    // For phase_bits=0b00: 0x00000000, 0b01: 0x55555555, 0b10: 0xAAAAAAAA, 0b11: 0xFFFFFFFF
+    uint32_t expected_stable_history = 0;
+    for (uint8_t i = 0; i < debounce_samples; i++) {
+        expected_stable_history = (expected_stable_history << 2) | phase_bits;
+    }
+
+    // Create mask for the number of samples we're checking
+    uint32_t mask = 0;
+    for (uint8_t i = 0; i < debounce_samples; i++) {
+        mask = (mask << 2) | 0x3;
+    }
+
+    // Require exact match - both bits must be stable for configured ms
+    if ((history_ & mask) == expected_stable_history) {
        // Has the debounced input value changed?
        if (state_ != phase_bits) {
            state_ = phase_bits;
--- a/firmware/application/hw/debounce.hpp
+++ b/firmware/application/hw/debounce.hpp
@@ -78,7 +78,7 @@ class EncoderDebounce {
    uint8_t rotation_rate();  // returns last rotation rate

   private:
-    uint8_t history_{0};  // shift register of previous reads from encoder
+    uint32_t history_{0};  // shift register of previous reads from encoder (16 samples @ 2 bits each)

    uint8_t state_{0};  // actual encoder output state (after debounce logic)

--- a/firmware/application/hw/encoder.cpp
+++ b/firmware/application/hw/encoder.cpp
@@ -62,21 +62,41 @@ int_fast8_t Encoder::update(const uint_fast8_t phase_bits) {

    int_fast8_t direction = transition_map[state];

-    // Require 2 state changes in same direction to register movement -- for additional level of contact switch debouncing
-    if (direction == prev_direction) {
-        if ((sensitivity_map[portapack::persistent_memory::encoder_dial_sensitivity()] & (1 << state)) == 0)
-            return 0;
-
-        // false: normal, true: reverse
-        if (portapack::persistent_memory::encoder_dial_direction())
-            direction = -direction;
-
-        return direction;
+    // Decrement cooldown timer
+    if (direction_cooldown > 0) {
+        direction_cooldown--;
    }

-    // It's normal for transition map to return 0 between every +1/-1, so discarding those
-    if (direction != 0)
-        prev_direction = direction;
+    // Require N consecutive state changes in same direction (configurable for noisy encoders)
+    if (direction == prev_direction && direction != 0) {
+        direction_stability_count++;
+
+        // Need N consecutive same-direction changes AND cooldown expired
+        uint8_t required_hits = portapack::persistent_memory::encoder_consecutive_hits();
+        if (direction_stability_count >= required_hits && direction_cooldown == 0) {
+            if ((sensitivity_map[portapack::persistent_memory::encoder_dial_sensitivity()] & (1 << state)) == 0)
+                return 0;
+
+            // Successfully registered movement - reset stability and set cooldown
+            direction_stability_count = 0;
+            direction_cooldown = portapack::persistent_memory::encoder_cooldown_ms();
+
+            // false: normal, true: reverse
+            if (portapack::persistent_memory::encoder_dial_direction())
+                direction = -direction;
+
+            return direction;
+        }
+    } else if (direction != 0 && direction != prev_direction) {
+        // Direction changed - only accept if cooldown expired (prevents bounce-induced reversals)
+        if (direction_cooldown == 0) {
+            prev_direction = direction;
+            direction_stability_count = 1;  // Start counting this new direction
+        } else {
+            // During cooldown, completely ignore opposite direction - reset stability count
+            direction_stability_count = 0;
+        }
+    }

    return 0;
 }
--- a/firmware/application/hw/encoder.hpp
+++ b/firmware/application/hw/encoder.hpp
@@ -32,6 +32,8 @@ class Encoder {
   private:
    uint_fast8_t state{0};
    int_fast8_t prev_direction{0};
+    uint8_t direction_stability_count{0};  // count consecutive same-direction changes
+    uint8_t direction_cooldown{0};         // prevent rapid direction reversals
 };

 #endif /*__ENCODER_H__*/
--- a/firmware/common/portapack_persistent_memory.cpp
+++ b/firmware/common/portapack_persistent_memory.cpp
@@ -234,6 +234,11 @@ struct data_t {

    uint16_t UNUSED : 4;

+    // Encoder debounce parameters for noisy hardware
+    uint8_t encoder_consecutive_hits;  // Number of consecutive same-direction hits required (1-10)
+    uint8_t encoder_cooldown_ms;       // Cooldown period in ms after movement (0-255ms)
+    uint8_t encoder_debounce_ms;       // Debounce stability window in ms (4-32ms)
+
    // Headphone volume in centibels.
    int16_t headphone_volume_cb;

@@ -304,6 +309,10 @@ struct data_t {
          encoder_rate_multiplier(1),
          UNUSED(0),

+          encoder_consecutive_hits(3),  // Default: 3 hits (Version 1 setting)
+          encoder_cooldown_ms(20),      // Default: 20ms (Version 1 setting)
+          encoder_debounce_ms(16),      // Default: 16ms stability window
+
          headphone_volume_cb(-600),
          misc_config(),
          ui_config2(),
@@ -1121,6 +1130,38 @@ void set_encoder_dial_direction(bool v) {
    data->encoder_dial_direction = v;
 }

+// Encoder debounce parameters for noisy hardware
+uint8_t encoder_consecutive_hits() {
+    uint8_t v = data->encoder_consecutive_hits;
+    if (v == 0) v = 3;   // default to 3 if not set
+    if (v > 10) v = 10;  // cap at 10
+    return v;
+}
+void set_encoder_consecutive_hits(uint8_t v) {
+    if (v < 1) v = 1;    // minimum 1
+    if (v > 10) v = 10;  // maximum 10
+    data->encoder_consecutive_hits = v;
+}
+
+uint8_t encoder_cooldown_ms() {
+    return data->encoder_cooldown_ms;
+}
+void set_encoder_cooldown_ms(uint8_t v) {
+    data->encoder_cooldown_ms = v;
+}
+
+uint8_t encoder_debounce_ms() {
+    uint8_t v = data->encoder_debounce_ms;
+    if (v < 4) v = 16;   // default to 16ms if not set or too low
+    if (v > 32) v = 32;  // cap at 32ms
+    return v;
+}
+void set_encoder_debounce_ms(uint8_t v) {
+    if (v < 4) v = 4;    // minimum 4ms
+    if (v > 32) v = 32;  // maximum 32ms
+    data->encoder_debounce_ms = v;
+}
+
 // Recovery mode magic value storage
 static data_t* data_direct_access = reinterpret_cast<data_t*>(memory::map::backup_ram.base());

--- a/firmware/common/portapack_persistent_memory.hpp
+++ b/firmware/common/portapack_persistent_memory.hpp
@@ -259,6 +259,13 @@ void set_encoder_rate_multiplier(uint8_t v);
 bool encoder_dial_direction();
 void set_encoder_dial_direction(bool v);

+uint8_t encoder_consecutive_hits();
+void set_encoder_consecutive_hits(uint8_t v);
+uint8_t encoder_cooldown_ms();
+void set_encoder_cooldown_ms(uint8_t v);
+uint8_t encoder_debounce_ms();
+void set_encoder_debounce_ms(uint8_t v);
+
 uint32_t config_mode_storage_direct();
 void set_config_mode_storage_direct(uint32_t v);
 bool config_disable_config_mode_direct();