From 7c80227e54db7fb48896c90706e3fa730db785d9 Mon Sep 17 00:00:00 2001 From: cottongin Date: Mon, 9 Mar 2026 06:08:31 -0400 Subject: [PATCH] fix: scope NTP power lock before vTaskDelete, revert clock refresh override vTaskDelete(nullptr) does not unwind the C++ stack, so the RAII HalPowerManager::Lock destructor never ran -- permanently holding the lock and preventing low-power mode. Fix by wrapping the lock in a block scope that exits before vTaskDelete(). Revert the setPowerSaving(false) calls in the clock refresh block; the RenderLock already handles CPU frequency during display updates. Made-with: Cursor --- chat-summaries/2026-03-09_06-30-summary.md | 38 ++++++++++++++++ src/main.cpp | 4 -- src/util/BootNtpSync.cpp | 53 ++++++++++++---------- 3 files changed, 66 insertions(+), 29 deletions(-) create mode 100644 chat-summaries/2026-03-09_06-30-summary.md diff --git a/chat-summaries/2026-03-09_06-30-summary.md b/chat-summaries/2026-03-09_06-30-summary.md new file mode 100644 index 00000000..fecf4bf7 --- /dev/null +++ b/chat-summaries/2026-03-09_06-30-summary.md @@ -0,0 +1,38 @@ +# Fix Idle Freeze: NTP Power Lock (Corrected) + +**Date**: 2026-03-09 + +## Task Description + +Fix device freeze during idle caused by power management issues with NTP sync and clock refresh. This is a correction to the initial fix (commit `6cf212d`) which introduced two new problems. + +## Root Cause + +The original freeze at the clock minute boundary was caused by the BootNtpSync background task running WiFi/NTP operations without a power lock, allowing the main loop to drop the CPU to 10 MHz during active WiFi/SPI operations. + +## Initial Fix Attempt (commit 6cf212d) -- Two Problems + +1. **BootNtpSync Lock**: Added `HalPowerManager::Lock` as a local variable in `taskFunc()`. However, `vTaskDelete(nullptr)` at the end of the function kills the FreeRTOS task immediately without C++ stack unwinding, so the lock destructor never ran. The lock was permanently held, preventing the device from ever entering low-power mode. + +2. **Clock Refresh Power Override**: Added `setPowerSaving(false)` and `lastActivityTime` reset in the clock refresh block of `main.cpp`. This was unnecessary -- the `RenderLock` inside the render path already handles CPU frequency restoration. The explicit calls prevented the device from entering low-power mode by resetting the idle timer every minute. + +## Corrected Fix + +### `src/util/BootNtpSync.cpp` +- Wrapped the `HalPowerManager::Lock` and all WiFi/NTP work in a block scope `{}` that ends before `vTaskDelete(nullptr)` +- The lock destructor now runs when the block scope exits, properly releasing the power lock +- `running = false`, `taskHandle = nullptr`, log message, and `vTaskDelete()` remain outside the scope since they don't need full CPU speed + +### `src/main.cpp` +- Reverted the `setPowerSaving(false)` and `lastActivityTime = millis()` additions from both clock refresh branches +- Clock refresh relies on the existing `RenderLock` mechanism to handle CPU frequency during display updates + +## Key Lesson + +FreeRTOS `vTaskDelete(nullptr)` does not unwind the C++ stack. RAII objects (like `HalPowerManager::Lock`) placed as local variables in a task function will never have their destructors called. Always use explicit block scoping or manual release before `vTaskDelete()`. + +## Follow-up Items + +- Verify on device that low-power mode is entered after idle and that NTP sync still completes +- Verify the clock still updates at minute boundaries without freezing +- Consider auditing other FreeRTOS task functions in the codebase for similar RAII + `vTaskDelete` issues diff --git a/src/main.cpp b/src/main.cpp index 110e95d7..0924681f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -416,13 +416,9 @@ void loop() { if (lastRenderedMinute < 0) { lastRenderedMinute = currentMinute; if (sawInvalidTime) { - lastActivityTime = millis(); - powerManager.setPowerSaving(false); activityManager.requestUpdate(); } } else if (currentMinute != lastRenderedMinute) { - lastActivityTime = millis(); - powerManager.setPowerSaving(false); activityManager.requestUpdate(); lastRenderedMinute = currentMinute; } diff --git a/src/util/BootNtpSync.cpp b/src/util/BootNtpSync.cpp index 61ef904c..a81b1360 100644 --- a/src/util/BootNtpSync.cpp +++ b/src/util/BootNtpSync.cpp @@ -94,36 +94,39 @@ static bool tryConnectToSavedNetwork(const TaskParams& params) { } static void taskFunc(void* param) { - HalPowerManager::Lock powerLock; - auto* params = static_cast(param); + { + HalPowerManager::Lock powerLock; + auto* params = static_cast(param); - bool connected = tryConnectToSavedNetwork(*params); + bool connected = tryConnectToSavedNetwork(*params); - if (!connected && running) { - LOG_DBG("BNTP", "First scan failed, retrying in 3s..."); - vTaskDelay(3000 / portTICK_PERIOD_MS); - if (running) { - connected = tryConnectToSavedNetwork(*params); + if (!connected && running) { + LOG_DBG("BNTP", "First scan failed, retrying in 3s..."); + vTaskDelay(3000 / portTICK_PERIOD_MS); + if (running) { + connected = tryConnectToSavedNetwork(*params); + } } + + if (connected && running) { + LOG_DBG("BNTP", "Starting NTP sync..."); + bool synced = TimeSync::waitForNtpSync(5000); + TimeSync::stopNtpSync(); + if (synced) { + LOG_DBG("BNTP", "NTP sync successful"); + } else { + LOG_DBG("BNTP", "NTP sync timed out, continuing without time"); + } + } + + WiFi.disconnect(false); + vTaskDelay(100 / portTICK_PERIOD_MS); + WiFi.mode(WIFI_OFF); + vTaskDelay(100 / portTICK_PERIOD_MS); + + delete params; } - if (connected && running) { - LOG_DBG("BNTP", "Starting NTP sync..."); - bool synced = TimeSync::waitForNtpSync(5000); - TimeSync::stopNtpSync(); - if (synced) { - LOG_DBG("BNTP", "NTP sync successful"); - } else { - LOG_DBG("BNTP", "NTP sync timed out, continuing without time"); - } - } - - WiFi.disconnect(false); - vTaskDelay(100 / portTICK_PERIOD_MS); - WiFi.mode(WIFI_OFF); - vTaskDelay(100 / portTICK_PERIOD_MS); - - delete params; running = false; taskHandle = nullptr; LOG_DBG("BNTP", "Boot NTP task complete");