Merge pull request #22 from jpirnay/fix-powerdebounce

fix: Second attempt at proper power key debouncing

lib/hal/HalGPIO.cpp

@@ -223,26 +223,46 @@ bool HalGPIO::wasAnyReleased() const { return inputMgr.wasAnyReleased(); }
 
 unsigned long HalGPIO::getHeldTime() const { return inputMgr.getHeldTime(); }
 
-void HalGPIO::startDeepSleep() {
-  // Ensure that the power button has been released to avoid immediately turning back on if you're holding it
-  while (inputMgr.isPressed(BTN_POWER)) {
-    delay(50);
-    inputMgr.update();
+void HalGPIO::waitForStablePowerRelease() {
+  // Wait until the raw power-button pin reads HIGH (released) for RELEASE_STABLE_MS
+  // consecutive milliseconds.  The InputManager debounce (5 ms) is too short for
+  // mechanical switch bounce which can last 10-50 ms, so we bypass it entirely here.
+  constexpr unsigned long RELEASE_STABLE_MS = 200;
+  const unsigned long waitStart = millis();
+  unsigned long stableStart = 0;
+  while (true) {
+    if (digitalRead(InputManager::POWER_BUTTON_PIN) == HIGH) {
+      if (stableStart == 0) stableStart = millis();
+      if (millis() - stableStart >= RELEASE_STABLE_MS) break;
+    } else {
+      stableStart = 0;
+    }
+    delay(10);
   }
+  LOG_DBG("GPIO", "Power button stable-released after %lu ms", millis() - waitStart);
+}
+
+void HalGPIO::startDeepSleep() {
+  LOG_DBG("GPIO", "startDeepSleep: waiting for power button release (isPressed=%d, rawPin=%d)",
+          inputMgr.isPressed(BTN_POWER), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW);
+  waitForStablePowerRelease();
   // Arm the wakeup trigger *after* the button is released
   esp_deep_sleep_enable_gpio_wakeup(1ULL << InputManager::POWER_BUTTON_PIN, ESP_GPIO_WAKEUP_GPIO_LOW);
+  LOG_DBG("GPIO", "startDeepSleep: entering deep sleep now");
   // Enter Deep Sleep
   esp_deep_sleep_start();
 }
 
 void HalGPIO::verifyPowerButtonWakeup(uint16_t requiredDurationMs, bool shortPressAllowed) {
   if (shortPressAllowed) {
-    // Fast path - no duration check needed
+    LOG_DBG("GPIO", "verifyPowerButtonWakeup: shortPressAllowed, skipping verification");
     return;
   }
   // Calibrate: subtract boot time already elapsed, assuming button held since boot
   const uint16_t calibration = millis();
   const uint16_t calibratedDuration = (calibration < requiredDurationMs) ? (requiredDurationMs - calibration) : 1;
+  LOG_DBG("GPIO", "verifyPowerButtonWakeup: requiredMs=%u, calibration=%u, calibratedMs=%u", requiredDurationMs,
+          calibration, calibratedDuration);
 
   const auto start = millis();
   inputMgr.update();
@@ -251,25 +271,38 @@ void HalGPIO::verifyPowerButtonWakeup(uint16_t requiredDurationMs, bool shortPre
     delay(10);
     inputMgr.update();
   }
+  LOG_DBG("GPIO", "verifyPowerButtonWakeup: initial detect took %lu ms, isPressed=%d, rawPin=%d", millis() - start,
+          inputMgr.isPressed(BTN_POWER), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW);
+
   if (inputMgr.isPressed(BTN_POWER)) {
     // Use wall-clock elapsed time instead of getHeldTime() which resets on bounce.
     // Tolerate brief release gaps (bouncing switch) up to BOUNCE_TOLERANCE_MS.
     constexpr unsigned long BOUNCE_TOLERANCE_MS = 100;
     unsigned long lastSeenPressed = millis();
     const auto holdStart = millis();
+    unsigned long bounceCount = 0;
 
     while (millis() - holdStart < calibratedDuration) {
       delay(10);
       inputMgr.update();
       if (inputMgr.isPressed(BTN_POWER)) {
+        if (millis() - lastSeenPressed > 20) {
+          bounceCount++;
+        }
         lastSeenPressed = millis();
       } else if (millis() - lastSeenPressed >= BOUNCE_TOLERANCE_MS) {
         // Button released for longer than bounce tolerance — truly released
+        LOG_DBG("GPIO",
+                "verifyPowerButtonWakeup: released during hold check after %lu ms (bounces=%lu), going to sleep",
+                millis() - holdStart, bounceCount);
         startDeepSleep();
       }
     }
+    LOG_DBG("GPIO", "verifyPowerButtonWakeup: hold verified after %lu ms (bounces=%lu), proceeding with boot",
+            millis() - holdStart, bounceCount);
     // Held long enough (tolerating brief bounces) — proceed with boot
   } else {
+    LOG_DBG("GPIO", "verifyPowerButtonWakeup: button not pressed after 1s wait, going to sleep");
     startDeepSleep();
   }
 }
@@ -296,6 +329,8 @@ HalGPIO::WakeupReason HalGPIO::getWakeupReason() const {
   const auto resetReason = esp_reset_reason();
 
   const bool usbConnected = isUsbConnected();
+  LOG_DBG("GPIO", "getWakeupReason: wakeupCause=%d, resetReason=%d, usbConnected=%d", static_cast<int>(wakeupCause),
+          static_cast<int>(resetReason), usbConnected);
 
   if ((wakeupCause == ESP_SLEEP_WAKEUP_UNDEFINED && resetReason == ESP_RST_POWERON && !usbConnected) ||
       (wakeupCause == ESP_SLEEP_WAKEUP_GPIO && resetReason == ESP_RST_DEEPSLEEP && usbConnected)) {

lib/hal/HalGPIO.h

@@ -71,6 +71,11 @@ class HalGPIO {
   bool wasAnyReleased() const;
   unsigned long getHeldTime() const;
 
+  // Wait until the raw power-button GPIO reads HIGH (released) for a sustained period.
+  // Uses the raw pin directly instead of the InputManager debounced state to avoid
+  // the 5 ms debounce being fooled by mechanical switch bounce during release.
+  void waitForStablePowerRelease();
+
   // Setup wake up GPIO and enter deep sleep
   void startDeepSleep();
 

lib/hal/HalPowerManager.cpp

@@ -61,11 +61,11 @@ void HalPowerManager::setPowerSaving(bool enabled) {
 }
 
 void HalPowerManager::startDeepSleep(HalGPIO& gpio, bool keepClockAlive) const {
-  // Ensure that the power button has been released to avoid immediately turning back on if you're holding it
-  while (gpio.isPressed(HalGPIO::BTN_POWER)) {
-    delay(50);
-    gpio.update();
-  }
+  LOG_DBG("PWR", "startDeepSleep: isPressed=%d, rawPin=%d, keepClock=%d", gpio.isPressed(HalGPIO::BTN_POWER),
+          digitalRead(InputManager::POWER_BUTTON_PIN) == LOW, keepClockAlive);
+  // Perform all hardware preparation immediately (while the button may still be held)
+  // so the user gets instant visual feedback (display already off). Only block for
+  // button release at the very end, right before entering sleep.
   // GPIO13 is connected to the battery latch MOSFET.
   // When keepClockAlive is false (default): GPIO13 goes LOW, the MCU is
   // completely powered off during sleep (including the LP timer / RTC memory).
@@ -84,12 +84,18 @@ void HalPowerManager::startDeepSleep(HalGPIO& gpio, bool keepClockAlive) const {
   gpio_deep_sleep_hold_en();
   gpio_hold_en(GPIO_SPIWP);
   pinMode(InputManager::POWER_BUTTON_PIN, INPUT_PULLUP);
+
+  // Now wait for the power button to be fully released before arming the wakeup
+  // trigger and entering sleep — prevents immediate re-wake from a held button.
+  gpio.waitForStablePowerRelease();
+
   // Arm the wakeup trigger *after* the button is released
   // Note: when keepClockAlive is false, this is only useful for waking up on USB power. On battery, the MCU will be
   // completely powered off, so the power button is hard-wired to briefly provide power to the MCU, waking it up
   // regardless of the wakeup source configuration.
   // When keepClockAlive is true, this is the actual wakeup mechanism since the MCU stays powered.
   esp_deep_sleep_enable_gpio_wakeup(1ULL << InputManager::POWER_BUTTON_PIN, ESP_GPIO_WAKEUP_GPIO_LOW);
+  LOG_DBG("PWR", "startDeepSleep: entering deep sleep now");
   // Enter Deep Sleep
   esp_deep_sleep_start();
 }

src/main.cpp

@@ -124,63 +124,10 @@ EpdFont ui12RegularFont(&ubuntu_12_regular);
 EpdFont ui12BoldFont(&ubuntu_12_bold);
 EpdFontFamily ui12FontFamily(&ui12RegularFont, &ui12BoldFont);
 
-// measurement of power button press duration calibration value
-unsigned long t1 = 0;
-unsigned long t2 = 0;
-
-// Verify power button press duration on wake-up from deep sleep
-// Pre-condition: isWakeupByPowerButton() == true
-void verifyPowerButtonDuration() {
-  if (SETTINGS.shortPwrBtn == CrossPointSettings::SHORT_PWRBTN::SLEEP) {
-    // Fast path for short press
-    // Needed because inputManager.isPressed() may take up to ~500ms to return the correct state
-    return;
-  }
-
-  // Give the user up to 1000ms to start holding the power button, and must hold for SETTINGS.getPowerButtonDuration()
-  const auto start = millis();
-  bool abort = false;
-  // Subtract the current time, because inputManager only starts counting the HeldTime from the first update()
-  // This way, we remove the time we already took to reach here from the duration,
-  // assuming the button was held until now from millis()==0 (i.e. device start time).
-  const uint16_t calibration = start;
-  const uint16_t calibratedPressDuration =
-      (calibration < SETTINGS.getPowerButtonDuration()) ? SETTINGS.getPowerButtonDuration() - calibration : 1;
-
-  gpio.update();
-  // Needed because inputManager.isPressed() may take up to ~500ms to return the correct state
-  while (!gpio.isPressed(HalGPIO::BTN_POWER) && millis() - start < 1000) {
-    delay(10);  // only wait 10ms each iteration to not delay too much in case of short configured duration.
-    gpio.update();
-  }
-
-  t2 = millis();
-  if (gpio.isPressed(HalGPIO::BTN_POWER)) {
-    do {
-      delay(10);
-      gpio.update();
-    } while (gpio.isPressed(HalGPIO::BTN_POWER) && gpio.getHeldTime() < calibratedPressDuration);
-    abort = gpio.getHeldTime() < calibratedPressDuration;
-  } else {
-    abort = true;
-  }
-
-  if (abort) {
-    // Button released too early. Returning to sleep.
-    // IMPORTANT: Re-arm the wakeup trigger before sleeping again
-    powerManager.startDeepSleep(gpio);
-  }
-}
-void waitForPowerRelease() {
-  gpio.update();
-  while (gpio.isPressed(HalGPIO::BTN_POWER)) {
-    delay(50);
-    gpio.update();
-  }
-}
-
 // Enter deep sleep mode
 void enterDeepSleep() {
+  LOG_DBG("MAIN", "enterDeepSleep called at millis=%lu, powerBtn isPressed=%d, rawPin=%d", millis(),
+          gpio.isPressed(HalGPIO::BTN_POWER), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW);
   HalPowerManager::Lock powerLock;  // Ensure we are at normal CPU frequency for sleep preparation
   APP_STATE.lastSleepFromReader = activityManager.isReaderActivity();
   HalClock::saveBeforeSleep(SETTINGS.useClock);
@@ -189,7 +136,8 @@ void enterDeepSleep() {
   activityManager.goToSleep();
 
   display.deepSleep();
-  LOG_DBG("MAIN", "Entering deep sleep");
+  LOG_DBG("MAIN", "Entering deep sleep (powerBtn isPressed=%d, rawPin=%d)", gpio.isPressed(HalGPIO::BTN_POWER),
+          digitalRead(InputManager::POWER_BUTTON_PIN) == LOW);
 
   powerManager.startDeepSleep(gpio, SETTINGS.useClock);
 }
@@ -228,8 +176,6 @@ void setupDisplayAndFonts() {
 }
 
 void setup() {
-  t1 = millis();
-
   HalSystem::begin();
   gpio.begin();
   powerManager.begin();
@@ -268,11 +214,15 @@ void setup() {
   ButtonNavigator::setMappedInputManager(mappedInputManager);
 
   const auto wakeupReason = gpio.getWakeupReason();
+  LOG_DBG("MAIN", "Wakeup reason: %d, millis=%lu, rawPowerPin=%d", static_cast<int>(wakeupReason), millis(),
+          digitalRead(InputManager::POWER_BUTTON_PIN) == LOW);
   switch (wakeupReason) {
     case HalGPIO::WakeupReason::PowerButton:
-      LOG_DBG("MAIN", "Verifying power button press duration");
+      LOG_DBG("MAIN", "Verifying power button press duration (required=%u ms, shortPress=%d)",
+              SETTINGS.getPowerButtonDuration(), SETTINGS.shortPwrBtn == CrossPointSettings::SHORT_PWRBTN::SLEEP);
       gpio.verifyPowerButtonWakeup(SETTINGS.getPowerButtonDuration(),
                                    SETTINGS.shortPwrBtn == CrossPointSettings::SHORT_PWRBTN::SLEEP);
+      LOG_DBG("MAIN", "Power button verification passed, millis=%lu", millis());
       break;
     case HalGPIO::WakeupReason::AfterUSBPower:
       // If USB power caused a cold boot, go back to sleep
@@ -310,9 +260,6 @@ void setup() {
     APP_STATE.saveToFile();
     activityManager.goToReader(path);
   }
-
-  // Ensure we're not still holding the power button before leaving setup
-  waitForPowerRelease();
 }
 
 void loop() {
@@ -379,14 +326,30 @@ void loop() {
     return;
   }
 
-  if (gpio.isPressed(HalGPIO::BTN_POWER) && gpio.getHeldTime() > SETTINGS.getPowerButtonDuration()) {
-    // If the screenshot combination is potentially being pressed, don't sleep
-    if (gpio.isPressed(HalGPIO::BTN_DOWN)) {
+  // Track power button hold for sleep.  We require a fresh press edge (wasPressed)
+  // before starting to measure hold time, so that a hold carried over from boot
+  // (wake-up press) is never misinterpreted as a "go to sleep" press.
+  static unsigned long powerHoldStart = 0;
+  if (gpio.wasPressed(HalGPIO::BTN_POWER)) {
+    powerHoldStart = millis();
+    LOG_DBG("MAIN", "loop: power button press detected (fresh edge)");
+  }
+  if (gpio.isPressed(HalGPIO::BTN_POWER) && powerHoldStart > 0) {
+    const unsigned long heldTime = millis() - powerHoldStart;
+    if (heldTime > SETTINGS.getPowerButtonDuration()) {
+      // If the screenshot combination is potentially being pressed, don't sleep
+      if (gpio.isPressed(HalGPIO::BTN_DOWN)) {
+        return;
+      }
+      LOG_DBG("MAIN", "loop: power button held for %lu ms (> %u ms), entering deep sleep", heldTime,
+              SETTINGS.getPowerButtonDuration());
+      enterDeepSleep();
+      // This should never be hit as `enterDeepSleep` calls esp_deep_sleep_start
       return;
     }
-    enterDeepSleep();
-    // This should never be hit as `enterDeepSleep` calls esp_deep_sleep_start
-    return;
+  }
+  if (!gpio.isPressed(HalGPIO::BTN_POWER)) {
+    powerHoldStart = 0;
   }
 
   // Refresh the battery icon when USB is plugged or unplugged.