Merge pull request #176 from jpirnay/fix-powerbutton

fix: Amend powerbutton logic (less actions on going back to sleep)

lib/hal/HalGPIO.cpp

@@ -254,19 +254,26 @@ void HalGPIO::startDeepSleep() {
 }
 
 void HalGPIO::verifyPowerButtonWakeup(uint16_t requiredDurationMs, bool shortPressAllowed) {
+  // The wakeup reason was already confirmed as a power button press before this is called,
+  // so we know a real press occurred. When short presses are allowed, nothing more to verify.
   if (shortPressAllowed) {
-    LOG_DBG("GPIO", "verifyPowerButtonWakeup: shortPressAllowed, skipping verification");
+    LOG_DBG("GPIO", "verifyPowerButtonWakeup: shortPressAllowed, skipping hold verification");
     return;
   }
-  // Calibrate: subtract boot time already elapsed, assuming button held since boot
+
+  // Calibrate: subtract boot time already elapsed, assuming button held since boot.
+  // Never collapse to less than BOUNCE_TOLERANCE_MS so the hold loop always has time to
+  // sample the button and detect a release (early release = unintentional tap).
+  constexpr unsigned long BOUNCE_TOLERANCE_MS = 100;
   const uint16_t calibration = millis();
-  const uint16_t calibratedDuration = (calibration < requiredDurationMs) ? (requiredDurationMs - calibration) : 1;
+  const uint16_t calibratedDuration =
+      (calibration < requiredDurationMs) ? (requiredDurationMs - calibration) : BOUNCE_TOLERANCE_MS;
   LOG_DBG("GPIO", "verifyPowerButtonWakeup: requiredMs=%u, calibration=%u, calibratedMs=%u", requiredDurationMs,
           calibration, calibratedDuration);
 
   const auto start = millis();
   inputMgr.update();
-  // inputMgr.isPressed() may take up to ~500ms to return correct state
+  // inputMgr.isPressed() may take up to ~500ms to return correct state after boot
   while (!inputMgr.isPressed(BTN_POWER) && millis() - start < 1000) {
     delay(10);
     inputMgr.update();
@@ -275,9 +282,8 @@ void HalGPIO::verifyPowerButtonWakeup(uint16_t requiredDurationMs, bool shortPre
           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;
+    // Monitor the hold for calibratedDuration, tolerating brief bounces up to BOUNCE_TOLERANCE_MS.
+    // Early release beyond the bounce window means an unintentional tap — go back to sleep.
     unsigned long lastSeenPressed = millis();
     const auto holdStart = millis();
     unsigned long bounceCount = 0;
@@ -291,16 +297,13 @@ void HalGPIO::verifyPowerButtonWakeup(uint16_t requiredDurationMs, bool shortPre
         }
         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",
+        LOG_DBG("GPIO", "verifyPowerButtonWakeup: released early 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();

src/CrossPointSettings.cpp

@@ -99,8 +99,29 @@ void CrossPointSettings::validateFrontButtonMapping(CrossPointSettings& settings
   }
 }
 
+#include <Preferences.h>
+
+void CrossPointSettings::loadStartupFromNvs() {
+  Preferences nvs;
+  nvs.begin("Crosspoint", true);  // read-only
+  btnShortPower = nvs.getUChar("bSPwr", BTN_DEFAULT);
+  btnDoublePower = nvs.getUChar("bDPwr", BTN_DEFAULT);
+  useClock = nvs.getUChar("useClk", 0);
+  nvs.end();
+}
+
+void CrossPointSettings::saveStartupToNvs() const {
+  Preferences nvs;
+  nvs.begin("Crosspoint", false);  // read-write
+  nvs.putUChar("bSPwr", btnShortPower);
+  nvs.putUChar("bDPwr", btnDoublePower);
+  nvs.putUChar("useClk", useClock);
+  nvs.end();
+}
+
 bool CrossPointSettings::saveToFile() const {
   Storage.mkdir("/.crosspoint");
+  saveStartupToNvs();
   return JsonSettingsIO::saveSettings(*this, SETTINGS_FILE_JSON);
 }
 
@@ -113,6 +134,7 @@ bool CrossPointSettings::loadFromFile() {
       bool result = JsonSettingsIO::loadSettings(*this, json.c_str(), &resave);
       if (result) {
         enforceFixedShortActions(*this);
+        saveStartupToNvs();  // Ensure NVS is in sync on boot
         if (resave) {
           if (saveToFile()) {
             LOG_DBG("CPS", "Resaved settings to update format");

src/CrossPointSettings.h

@@ -339,6 +339,8 @@ class CrossPointSettings {
 
   bool saveToFile() const;
   bool loadFromFile();
+  void loadStartupFromNvs();
+  void saveStartupToNvs() const;
 
   static void validateFrontButtonMapping(CrossPointSettings& settings);
 

src/main.cpp

@@ -195,6 +195,15 @@ void setup() {
   powerManager.begin();
   gpio_deep_sleep_hold_dis();  // Release deep sleep GPIO hold state from previous sleep cycle
 
+  const auto wakeupReason = gpio.getWakeupReason();
+
+  if (wakeupReason == HalGPIO::WakeupReason::AfterUSBPower) {
+    // If USB power caused a cold boot, go back to sleep immediately without initializing subsystems
+    LOG_DBG("MAIN", "Wakeup reason: After USB Power => Deep sleep");
+    powerManager.startDeepSleep(gpio);
+    return;
+  }
+
 #ifdef ENABLE_SERIAL_LOG
   if (gpio.isUsbConnected()) {
     Serial.begin(115200);
@@ -206,6 +215,25 @@ void setup() {
 #endif
 
   LOG_INF("MAIN", "Hardware detect: %s", gpio.deviceIsX3() ? "X3" : "X4");
+  LOG_DBG("MAIN", "Wakeup reason: %d, millis=%lu, rawPowerPin=%d", static_cast<int>(wakeupReason), millis(),
+          digitalRead(InputManager::POWER_BUTTON_PIN) == LOW);
+
+  // Load just the settings we need *before* initializing the SD card to speed up and reduce power on unverified wakes
+  SETTINGS.loadStartupFromNvs();
+
+  if (wakeupReason == HalGPIO::WakeupReason::PowerButton) {
+    LOG_DBG("MAIN", "Verifying power button press duration (required=%u ms)",
+            CrossPointSettings::getPowerButtonDuration());
+
+    // We only want to skip the hold verification (allowing a short press to wake) if the short
+    // press or double press actually have an action assigned, or if the clock screensaver is active.
+    // Otherwise, short presses from sleep should be ignored entirely and return to sleep.
+    bool allowShortPress = (SETTINGS.useClock != 0) || (SETTINGS.btnShortPower != CrossPointSettings::BTN_DEFAULT) ||
+                           (SETTINGS.btnDoublePower != CrossPointSettings::BTN_DEFAULT);
+
+    gpio.verifyPowerButtonWakeup(CrossPointSettings::getPowerButtonDuration(), allowShortPress);
+    LOG_DBG("MAIN", "Power button verification passed, millis=%lu", millis());
+  }
 
   // SD Card Initialization
   // We need 6 open files concurrently when parsing a new chapter
@@ -216,8 +244,9 @@ void setup() {
     return;
   }
 
-  HalSystem::checkPanic();
   SETTINGS.loadFromFile();
+
+  HalSystem::checkPanic();
   HalSystem::clearPanic();  // TODO: move this to an activity when we have one to display the panic info
   HalClock::applyTimezone(SETTINGS.timeZone);
   I18N.loadSettings();
@@ -227,31 +256,6 @@ void setup() {
   UITheme::getInstance().reload();
   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: {
-      constexpr uint16_t defaultPowerButtonDurationMs = 400;
-      LOG_DBG("MAIN", "Verifying power button press duration (required=%u ms, default only)",
-              defaultPowerButtonDurationMs);
-      gpio.verifyPowerButtonWakeup(defaultPowerButtonDurationMs, false);
-      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
-      LOG_DBG("MAIN", "Wakeup reason: After USB Power");
-      powerManager.startDeepSleep(gpio);
-      break;
-    case HalGPIO::WakeupReason::AfterFlash:
-      // After flashing, just proceed to boot
-    case HalGPIO::WakeupReason::Other:
-    default:
-      break;
-  }
-
   // First serial output only here to avoid timing inconsistencies for power button press duration verification
   LOG_DBG("MAIN", "Starting CrossPoint version " CROSSPOINT_VERSION);
 
@@ -277,6 +281,13 @@ void setup() {
     APP_STATE.saveToFile();
     activityManager.goToReader(path);
   }
+
+  // Ensure we're not still holding the power button before leaving setup
+  // waitForStablePowerRelease protects against switch bounce that might register as a false double-press.
+  gpio.waitForStablePowerRelease();
+  // Flush any pin state transitions that occurred during boot before entering the main loop
+  mappedInputManager.update();
+  buttonEventManager.drain();
 }
 
 void loop() {
@@ -500,8 +511,8 @@ void loop() {
           activityManager.goHome();
           break;
         case BA::BTN_SLEEP:
-          activityManager.goToSleep();
-          break;
+          enterDeepSleep();
+          return;  // enterDeepSleep() never returns, but return here to stop processing
         case BA::BTN_FORCE_REFRESH: {
           RenderLock lock;
           renderer.displayBuffer(HalDisplay::HALF_REFRESH);