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

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2014 Jared Boone, ShareBrained Technology, Inc.
+ * Copyright (C) 2023 Mark Thompson
  *
  * This file is part of PortaPack.
  *
@@ -24,8 +25,7 @@
 #include "utility.hpp"
 
 uint8_t Debounce::state() {
-    bool v = !pulse_upon_release_ && (state_ || simulated_pulse_);
+    bool v = state_to_report_;
-    if (simulated_pulse_)
     simulated_pulse_ = false;
     return v;
 }
@@ -52,59 +52,41 @@ 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) {
+        //
+        // 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;
             }
-            return true;
+            state_to_report_ = 1;
-        }
-    } 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;
-            // 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;
-            }
-
-            // Reset long_press_occurred_ flag after button is released
-            long_press_occurred_ = false;
             return true;
         }
 
-        // Has button been held continuously?
+        // "Repeat" handling - simulated button release
-        if (history_ == 0xFF) {
+        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:
@@ -112,24 +94,58 @@ bool Debounce::feed(const uint8_t bit) {
             // 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;
+                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) {
+            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_) {
+                pulse_upon_release_ = false;
+                simulated_pulse_ = true;
+                state_to_report_ = 1;
+                return true;
+            }
+
+            state_to_report_ = 0;
+            return true;
+        }
+
+        // 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 not continuously pressed; reset counter
+        //
+        // Button is in transition between states;
+        // Reset counters until button inputs are stable.
+        //
         held_time_ = 0;
+        repeat_ctr_ = 0;
     }
-    }
+
     return false;
 }

firmware/application/hw/debounce.hpp

@@ -43,15 +43,25 @@ class Debounce {
     bool long_press_occurred();
 
    private:
-    uint8_t history_{0};
+    uint8_t history_{0};  // shift register of last 8 reads from button hardware state bit
-    uint8_t state_{0};
+
-    bool repeat_enabled_{false};
+    uint8_t state_{0};  // actual button hardware state (after debounce logic), 1=pressed
-    uint16_t repeat_ctr_{0};
+
-    uint16_t held_time_{0};
+    uint8_t state_to_report_{0};  // pseudo button state reported by state() function (may be masked off or simulated presses)
-    bool pulse_upon_release_{false};
+
-    bool simulated_pulse_{false};
+    bool repeat_enabled_{false};  // TRUE if this button is enabled to auto-repeat when held down (ignored if long_press_enabled)
-    bool long_press_enabled_{false};
+
-    bool long_press_occurred_{false};
+    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__*/