From 213ebfc0161be9962d22395d709bef9108eb5cd2 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 6 Apr 2026 09:48:11 +0200 Subject: [PATCH 1/3] Second attempt at proper debouncing --- lib/hal/HalGPIO.cpp | 49 ++++++++++++++++--- lib/hal/HalGPIO.h | 5 ++ lib/hal/HalPowerManager.cpp | 13 ++++-- src/main.cpp | 93 +++++++++++++------------------------ 4 files changed, 87 insertions(+), 73 deletions(-) diff --git a/lib/hal/HalGPIO.cpp b/lib/hal/HalGPIO.cpp index 51beaf84..f3755dba 100644 --- a/lib/hal/HalGPIO.cpp +++ b/lib/hal/HalGPIO.cpp @@ -223,26 +223,48 @@ 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); + // Re-sync the InputManager so its debounced state matches reality + inputMgr.update(); +} + +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 +273,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 +331,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(wakeupCause), + static_cast(resetReason), usbConnected); if ((wakeupCause == ESP_SLEEP_WAKEUP_UNDEFINED && resetReason == ESP_RST_POWERON && !usbConnected) || (wakeupCause == ESP_SLEEP_WAKEUP_GPIO && resetReason == ESP_RST_DEEPSLEEP && usbConnected)) { diff --git a/lib/hal/HalGPIO.h b/lib/hal/HalGPIO.h index 6337cf9e..9a0bbe9a 100644 --- a/lib/hal/HalGPIO.h +++ b/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(); diff --git a/lib/hal/HalPowerManager.cpp b/lib/hal/HalPowerManager.cpp index ff976bc5..61e8a202 100644 --- a/lib/hal/HalPowerManager.cpp +++ b/lib/hal/HalPowerManager.cpp @@ -61,11 +61,9 @@ 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: waiting for power button release (isPressed=%d, rawPin=%d, keepClock=%d)", + gpio.isPressed(HalGPIO::BTN_POWER), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW, keepClockAlive); + gpio.waitForStablePowerRelease(); // 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). @@ -90,6 +88,11 @@ void HalPowerManager::startDeepSleep(HalGPIO& gpio, bool keepClockAlive) const { // 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); + // Final check: is the raw pin still LOW (button still physically pressed)? + if (digitalRead(InputManager::POWER_BUTTON_PIN) == LOW) { + LOG_DBG("PWR", "startDeepSleep: WARNING raw pin still LOW after release wait — may wake immediately!"); + } + LOG_DBG("PWR", "startDeepSleep: entering deep sleep now"); // Enter Deep Sleep esp_deep_sleep_start(); } diff --git a/src/main.cpp b/src/main.cpp index 3bf94ea2..b4ebdcd2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -124,63 +124,15 @@ 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(); - } + LOG_DBG("MAIN", "waitForPowerRelease: rawPin=%d", digitalRead(InputManager::POWER_BUTTON_PIN) == LOW); + gpio.waitForStablePowerRelease(); } // 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 +141,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 +181,6 @@ void setupDisplayAndFonts() { } void setup() { - t1 = millis(); - HalSystem::begin(); gpio.begin(); powerManager.begin(); @@ -268,11 +219,15 @@ void setup() { ButtonNavigator::setMappedInputManager(mappedInputManager); const auto wakeupReason = gpio.getWakeupReason(); + LOG_DBG("MAIN", "Wakeup reason: %d, millis=%lu, rawPowerPin=%d", static_cast(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 @@ -379,14 +334,28 @@ 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)) { + static bool powerHoldLogged = false; + if (gpio.isPressed(HalGPIO::BTN_POWER)) { + const unsigned long heldTime = gpio.getHeldTime(); + if (!powerHoldLogged) { + LOG_DBG("MAIN", "loop: power button pressed, heldTime=%lu ms, required=%u ms, rawPin=%d", heldTime, + SETTINGS.getPowerButtonDuration(), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW); + powerHoldLogged = true; + } + 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()); + powerHoldLogged = false; + 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; + } else { + powerHoldLogged = false; } // Refresh the battery icon when USB is plugged or unplugged. From 633cc04f4115b0799961f376766f4751308442f8 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 6 Apr 2026 09:56:27 +0200 Subject: [PATCH 2/3] Additional checks --- lib/hal/HalGPIO.cpp | 8 +++++++- lib/hal/HalPowerManager.cpp | 17 ++++++++++------- src/main.cpp | 24 +++++++++++++----------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/lib/hal/HalGPIO.cpp b/lib/hal/HalGPIO.cpp index f3755dba..2904c9a4 100644 --- a/lib/hal/HalGPIO.cpp +++ b/lib/hal/HalGPIO.cpp @@ -240,7 +240,13 @@ void HalGPIO::waitForStablePowerRelease() { delay(10); } LOG_DBG("GPIO", "Power button stable-released after %lu ms", millis() - waitStart); - // Re-sync the InputManager so its debounced state matches reality + // Flush the InputManager debounced state to match reality. + // A single update() is insufficient: if lastState was stale ("pressed"), the first + // call resets the debounce timer but cannot update currentState until a second call + // arrives after DEBOUNCE_DELAY (5 ms). Without this, isPressed() / getHeldTime() + // would carry stale values into the next loop() iteration. + inputMgr.update(); + delay(10); // > InputManager DEBOUNCE_DELAY (5 ms) inputMgr.update(); } diff --git a/lib/hal/HalPowerManager.cpp b/lib/hal/HalPowerManager.cpp index 61e8a202..180d3b79 100644 --- a/lib/hal/HalPowerManager.cpp +++ b/lib/hal/HalPowerManager.cpp @@ -61,9 +61,11 @@ void HalPowerManager::setPowerSaving(bool enabled) { } void HalPowerManager::startDeepSleep(HalGPIO& gpio, bool keepClockAlive) const { - LOG_DBG("PWR", "startDeepSleep: waiting for power button release (isPressed=%d, rawPin=%d, keepClock=%d)", - gpio.isPressed(HalGPIO::BTN_POWER), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW, keepClockAlive); - gpio.waitForStablePowerRelease(); + 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). @@ -82,16 +84,17 @@ 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); - // Final check: is the raw pin still LOW (button still physically pressed)? - if (digitalRead(InputManager::POWER_BUTTON_PIN) == LOW) { - LOG_DBG("PWR", "startDeepSleep: WARNING raw pin still LOW after release wait — may wake immediately!"); - } LOG_DBG("PWR", "startDeepSleep: entering deep sleep now"); // Enter Deep Sleep esp_deep_sleep_start(); diff --git a/src/main.cpp b/src/main.cpp index b4ebdcd2..4bcbc763 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -334,14 +334,16 @@ void loop() { return; } - static bool powerHoldLogged = false; - if (gpio.isPressed(HalGPIO::BTN_POWER)) { - const unsigned long heldTime = gpio.getHeldTime(); - if (!powerHoldLogged) { - LOG_DBG("MAIN", "loop: power button pressed, heldTime=%lu ms, required=%u ms, rawPin=%d", heldTime, - SETTINGS.getPowerButtonDuration(), digitalRead(InputManager::POWER_BUTTON_PIN) == LOW); - powerHoldLogged = true; - } + // 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)) { @@ -349,13 +351,13 @@ void loop() { } LOG_DBG("MAIN", "loop: power button held for %lu ms (> %u ms), entering deep sleep", heldTime, SETTINGS.getPowerButtonDuration()); - powerHoldLogged = false; enterDeepSleep(); // This should never be hit as `enterDeepSleep` calls esp_deep_sleep_start return; } - } else { - powerHoldLogged = false; + } + if (!gpio.isPressed(HalGPIO::BTN_POWER)) { + powerHoldStart = 0; } // Refresh the battery icon when USB is plugged or unplugged. From 4f00ed634831c0a1b28633d65c17e028f3fd76fa Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 6 Apr 2026 10:07:50 +0200 Subject: [PATCH 3/3] Fast wakeup (not waiting for button release) --- lib/hal/HalGPIO.cpp | 8 -------- src/main.cpp | 8 -------- 2 files changed, 16 deletions(-) diff --git a/lib/hal/HalGPIO.cpp b/lib/hal/HalGPIO.cpp index 2904c9a4..c1734b0e 100644 --- a/lib/hal/HalGPIO.cpp +++ b/lib/hal/HalGPIO.cpp @@ -240,14 +240,6 @@ void HalGPIO::waitForStablePowerRelease() { delay(10); } LOG_DBG("GPIO", "Power button stable-released after %lu ms", millis() - waitStart); - // Flush the InputManager debounced state to match reality. - // A single update() is insufficient: if lastState was stale ("pressed"), the first - // call resets the debounce timer but cannot update currentState until a second call - // arrives after DEBOUNCE_DELAY (5 ms). Without this, isPressed() / getHeldTime() - // would carry stale values into the next loop() iteration. - inputMgr.update(); - delay(10); // > InputManager DEBOUNCE_DELAY (5 ms) - inputMgr.update(); } void HalGPIO::startDeepSleep() { diff --git a/src/main.cpp b/src/main.cpp index 4bcbc763..e601a153 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -124,11 +124,6 @@ EpdFont ui12RegularFont(&ubuntu_12_regular); EpdFont ui12BoldFont(&ubuntu_12_bold); EpdFontFamily ui12FontFamily(&ui12RegularFont, &ui12BoldFont); -void waitForPowerRelease() { - LOG_DBG("MAIN", "waitForPowerRelease: rawPin=%d", digitalRead(InputManager::POWER_BUTTON_PIN) == LOW); - gpio.waitForStablePowerRelease(); -} - // Enter deep sleep mode void enterDeepSleep() { LOG_DBG("MAIN", "enterDeepSleep called at millis=%lu, powerBtn isPressed=%d, rawPin=%d", millis(), @@ -265,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() {