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
This commit is contained in:
cottongin
2026-03-09 06:08:31 -04:00
parent 6cf212d12a
commit 7c80227e54
3 changed files with 66 additions and 29 deletions

View File

@@ -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

View File

@@ -416,13 +416,9 @@ void loop() {
if (lastRenderedMinute < 0) { if (lastRenderedMinute < 0) {
lastRenderedMinute = currentMinute; lastRenderedMinute = currentMinute;
if (sawInvalidTime) { if (sawInvalidTime) {
lastActivityTime = millis();
powerManager.setPowerSaving(false);
activityManager.requestUpdate(); activityManager.requestUpdate();
} }
} else if (currentMinute != lastRenderedMinute) { } else if (currentMinute != lastRenderedMinute) {
lastActivityTime = millis();
powerManager.setPowerSaving(false);
activityManager.requestUpdate(); activityManager.requestUpdate();
lastRenderedMinute = currentMinute; lastRenderedMinute = currentMinute;
} }

View File

@@ -94,36 +94,39 @@ static bool tryConnectToSavedNetwork(const TaskParams& params) {
} }
static void taskFunc(void* param) { static void taskFunc(void* param) {
HalPowerManager::Lock powerLock; {
auto* params = static_cast<TaskParams*>(param); HalPowerManager::Lock powerLock;
auto* params = static_cast<TaskParams*>(param);
bool connected = tryConnectToSavedNetwork(*params); bool connected = tryConnectToSavedNetwork(*params);
if (!connected && running) { if (!connected && running) {
LOG_DBG("BNTP", "First scan failed, retrying in 3s..."); LOG_DBG("BNTP", "First scan failed, retrying in 3s...");
vTaskDelay(3000 / portTICK_PERIOD_MS); vTaskDelay(3000 / portTICK_PERIOD_MS);
if (running) { if (running) {
connected = tryConnectToSavedNetwork(*params); 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; running = false;
taskHandle = nullptr; taskHandle = nullptr;
LOG_DBG("BNTP", "Boot NTP task complete"); LOG_DBG("BNTP", "Boot NTP task complete");