From 213ebfc0161be9962d22395d709bef9108eb5cd2 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 6 Apr 2026 09:48:11 +0200 Subject: [PATCH] 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.