From 06b7a0419eebd48b09b90970810dec025a76acb5 Mon Sep 17 00:00:00 2001 From: Mark Thompson <129641948+NotherNgineer@users.noreply.github.com> Date: Mon, 31 Jul 2023 23:27:15 -0500 Subject: [PATCH] Fixed Select button responsiveness when updating frequency field during heavy CPU activity (e.g. WFM Audio) (#1335) * Resolve button responsiveness * Resolve button responsiveness * Clang * Clang * Clang try again * Removed unnecessary lines * Address review comments * Add comments per reviewer suggestion * Clang test * Clang retry --- firmware/application/hw/debounce.cpp | 138 +++++++++++++++------------ firmware/application/hw/debounce.hpp | 28 ++++-- 2 files changed, 96 insertions(+), 70 deletions(-) diff --git a/firmware/application/hw/debounce.cpp b/firmware/application/hw/debounce.cpp index 77dd6b90e..1fb4751bb 100644 --- a/firmware/application/hw/debounce.cpp +++ b/firmware/application/hw/debounce.cpp @@ -1,5 +1,6 @@ /* * Copyright (C) 2014 Jared Boone, ShareBrained Technology, Inc. + * Copyright (C) 2023 Mark Thompson * * This file is part of PortaPack. * @@ -24,9 +25,8 @@ #include "utility.hpp" uint8_t Debounce::state() { - bool v = !pulse_upon_release_ && (state_ || simulated_pulse_); - if (simulated_pulse_) - simulated_pulse_ = false; + bool v = state_to_report_; + simulated_pulse_ = false; return v; } @@ -52,84 +52,100 @@ bool Debounce::long_press_occurred() { bool Debounce::feed(const uint8_t bit) { history_ = (history_ << 1) | (bit & 1); - // "Repeat" handling - simulated button release - if (repeat_ctr_) { - // Make sure the button is still being held continuously - if ((history_ == 0xFF) && !long_press_enabled_) { - // Simulate button press every REPEAT_SUBSEQUENT_DELAY ticks - if (--repeat_ctr_ == 0) { - state_ = !state_; - repeat_ctr_ = REPEAT_SUBSEQUENT_DELAY / 2; - return true; - } - } else { - // It's a real button release; stop simulating - repeat_ctr_ = 0; - } - } - - if (state_ == 0) { - // Previous button state was 0 (released); - // Has button been held for DEBOUNCE_COUNT ticks? - if ((history_ & DEBOUNCE_MASK) == DEBOUNCE_MASK) { + // Has button been held for DEBOUNCE_COUNT ticks? + if ((history_ & DEBOUNCE_MASK) == DEBOUNCE_MASK) { + // + // Button is currently pressed; + // Was previous button state 0 (released)? + // + if (state_ == 0) { + // + // Button has been pressed (after filtering glitches), transition 0->1 + // state_ = 1; - held_time_ = 0; // If long_press_enabled_, state() function masks the button press until it's released // or until LONG_PRESS_DELAY is reached if (long_press_enabled_) { pulse_upon_release_ = true; + state_to_report_ = 0; return false; } + state_to_report_ = 1; return true; } - } else { - // Previous button state was 1 (pressed); - // Has button been released for DEBOUNCE_COUNT ticks? - if ((history_ & DEBOUNCE_MASK) == 0) { - // Button has been released when long_press_enabled_ and before LONG_PRESS_DELAY was reached; + + // "Repeat" handling - simulated button release + if (repeat_ctr_ && !long_press_enabled_) { + // Simulate button press every REPEAT_SUBSEQUENT_DELAY ticks + // (by toggling reported state every 1/2 of the delay time) + if (--repeat_ctr_ == 0) { + state_to_report_ = !state_to_report_; + repeat_ctr_ = REPEAT_SUBSEQUENT_DELAY / 2; + return true; + } + } + + // Keep track of how long button has been held + held_time_++; + if (pulse_upon_release_) { + // Button is being held down and long_press support is enabled for this key: + // if LONG_PRESS_DELAY is reached then finally report that switch is pressed and set flag + // indicating it was a LONG press + // (note that repeat_support and long_press support are mutually exclusive) + if (held_time_ >= LONG_PRESS_DELAY) { + pulse_upon_release_ = false; + long_press_occurred_ = true; + state_to_report_ = 1; + return true; + } + } else if (repeat_enabled_ && !long_press_enabled_) { + // Repeat support -- 4 directional buttons only (unless long_press is enabled) + if (held_time_ >= REPEAT_INITIAL_DELAY) { + // Delay reached; trigger repeat code on NEXT tick + repeat_ctr_ = 1; + held_time_ = 0; + } + } + } else if ((history_ & DEBOUNCE_MASK) == 0) { // Has button been released for at least DEBOUNCE_COUNT ticks? + // + // Button is released; + // Was previous button state 1 (pressed)? + // + if (state_ == 1) { + // + // Button has been released (after filtering glitches), transition 1->0 + // + state_ = 0; + long_press_occurred_ = false; + + // If button released when long_press_enabled_ and before LONG_PRESS_DELAY was reached; // allow state() function to finally return a single press indication (simulated pulse). // Note: In long press mode, apps won't see button press until the button is released. if (pulse_upon_release_) { - // force state() function (called by EventDispatcher) to return simulated press for one cycle - simulated_pulse_ = true; pulse_upon_release_ = false; - } else { - state_ = 0; + simulated_pulse_ = true; + state_to_report_ = 1; + return true; } - // Reset long_press_occurred_ flag after button is released - long_press_occurred_ = false; + state_to_report_ = 0; return true; } - // Has button been held continuously? - if (history_ == 0xFF) { - held_time_++; - if (pulse_upon_release_) { - // Button is being held down and long_press support is enabled for this key: - // if LONG_PRESS_DELAY is reached then finally report that switch is pressed and set flag - // indicating it was a LONG press - // (note that repeat_support and long_press support are mutually exclusive) - if (held_time_ >= LONG_PRESS_DELAY) { - long_press_occurred_ = true; - simulated_pulse_ = true; - pulse_upon_release_ = false; - held_time_ = 0; - return true; - } - } else if (repeat_enabled_ && !long_press_enabled_) { - // Repeat support -- 4 directional buttons only (unless long_press is enabled) - if (held_time_ == REPEAT_INITIAL_DELAY) { - // Delay reached; trigger repeat code on NEXT tick - repeat_ctr_ = 1; - held_time_ = 0; - } - } - } else { - // Button not continuously pressed; reset counter - held_time_ = 0; + // Reset reported state after application/event has cleared simulated_pulse_ + if (state_to_report_ == 1 && !simulated_pulse_) { + state_to_report_ = 0; + return true; } + } else { + // + // Button is in transition between states; + // Reset counters until button inputs are stable. + // + held_time_ = 0; + repeat_ctr_ = 0; } + return false; } diff --git a/firmware/application/hw/debounce.hpp b/firmware/application/hw/debounce.hpp index 03db66f89..41ccb9da2 100644 --- a/firmware/application/hw/debounce.hpp +++ b/firmware/application/hw/debounce.hpp @@ -43,15 +43,25 @@ class Debounce { bool long_press_occurred(); private: - uint8_t history_{0}; - uint8_t state_{0}; - bool repeat_enabled_{false}; - uint16_t repeat_ctr_{0}; - uint16_t held_time_{0}; - bool pulse_upon_release_{false}; - bool simulated_pulse_{false}; - bool long_press_enabled_{false}; - bool long_press_occurred_{false}; + uint8_t history_{0}; // shift register of last 8 reads from button hardware state bit + + uint8_t state_{0}; // actual button hardware state (after debounce logic), 1=pressed + + uint8_t state_to_report_{0}; // pseudo button state reported by state() function (may be masked off or simulated presses) + + bool repeat_enabled_{false}; // TRUE if this button is enabled to auto-repeat when held down (ignored if long_press_enabled) + + uint16_t repeat_ctr_{0}; // used for timing auto-repeat simulated button presses when button is held down and repeat_enabled + + uint16_t held_time_{0}; // number of ticks that the button has been held down (compared against REPEAT and LONG_PRESS delays) + + bool pulse_upon_release_{false}; // TRUE when button is being held down when long_press_enabled and LONG_PRESS_DELAY hasn't been reached yet + + bool simulated_pulse_{false}; // TRUE if a simulated button press is active following a short button press (only when long_press_enabled) + + bool long_press_enabled_{false}; // TRUE when button is in long-press mode (takes precedence over the repeat_enabled flag) + + bool long_press_occurred_{false}; // TRUE when button is being held down and LONG_PRESS_DELAY has been reached (only when long_press_enabled) }; #endif /*__DEBOUNCE_H__*/