diff --git a/firmware/application/apps/ui_settings.cpp b/firmware/application/apps/ui_settings.cpp index 7d77a071..7102d73b 100644 --- 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(); }; diff --git a/firmware/application/apps/ui_settings.hpp b/firmware/application/apps/ui_settings.hpp index a2f5f8e0..42937f5e 100644 --- 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}, - "+"}; - - Button button_dial_sensitivity_minus{ - {20 * 8, 4 * 16, 16, 16}, - "-"}; + NumberField field_encoder_consecutive_hits{ + {14 * 8, 10 * 16}, + 2, + {1, 10}, + 1, + ' '}; - Button button_rate_multiplier_plus{ - {20 * 8, 9 * 16, 16, 16}, - "+"}; + NumberField field_encoder_cooldown_ms{ + {14 * 8, 11 * 16}, + 3, + {0, 255}, + 5, + ' '}; - 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", }; }; diff --git a/firmware/application/hw/debounce.cpp b/firmware/application/hw/debounce.cpp index 17905edb..b6fabeed 100644 --- 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; diff --git a/firmware/application/hw/debounce.hpp b/firmware/application/hw/debounce.hpp index b4047558..a2ebda2e 100644 --- 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) diff --git a/firmware/application/hw/encoder.cpp b/firmware/application/hw/encoder.cpp index b142d054..4319a9d8 100644 --- 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; + // Decrement cooldown timer + if (direction_cooldown > 0) { + direction_cooldown--; + } - // false: normal, true: reverse - if (portapack::persistent_memory::encoder_dial_direction()) - direction = -direction; + // Require N consecutive state changes in same direction (configurable for noisy encoders) + if (direction == prev_direction && direction != 0) { + direction_stability_count++; - return direction; - } + // 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(); - // It's normal for transition map to return 0 between every +1/-1, so discarding those - if (direction != 0) - prev_direction = direction; + // 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; } diff --git a/firmware/application/hw/encoder.hpp b/firmware/application/hw/encoder.hpp index da3c269c..001fbca1 100644 --- 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__*/ diff --git a/firmware/common/portapack_persistent_memory.cpp b/firmware/common/portapack_persistent_memory.cpp index 26365d09..e5ebad0c 100644 --- 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(memory::map::backup_ram.base()); diff --git a/firmware/common/portapack_persistent_memory.hpp b/firmware/common/portapack_persistent_memory.hpp index 35810c38..5c36d99e 100644 --- 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();