From 700db0a39024ef2cff0f9ff8f5abbea2ae744d0d Mon Sep 17 00:00:00 2001 From: Ingo Kodba Date: Thu, 6 Aug 2026 07:58:36 +0200 Subject: [PATCH] Restore the termination guarantee in the sleep brightness step-down The GoToSleep/GoToAOD handler steps the backlight down before sleeping: while (brightnessController.Level() != Levels::Low) { brightnessController.Lower(); vTaskDelay(100); } This was safe for about three years. It originally targeted Levels::Off -- the bottom of the ladder -- and Lower() walks monotonically downwards to Off, so the loop terminated from every level. 3dca742b ("aod: PPI/RTC-based backlight brightness") changed the target from Off to Low and, in the same commit, inserted AlwaysOn into the enum between Off and Low: enum class Levels { Off, Low, Medium, High }; // before enum class Levels { Off, AlwaysOn, Low, Medium, High }; // after Lower() is a no-op at both Off and AlwaysOn. The target moved from the bottom of the ladder to the middle while two levels that cannot be lowered were placed beneath it, so the loop can no longer terminate if the handler is entered at either -- two of the five levels. AlwaysOn is also the level that AOD itself sets. Step down only from above Low, then set the level outright. If the handler is ever entered that way the failure mode is a bad one. The loop delays rather than spins, so nothing is starved: SystemTask keeps running and keeps kicking the watchdog, so there is no reset. DisplayApp never returns from Refresh(), never sends OnDisplayTaskSleeping and never redraws, so SystemTask stays in GoingToSleep indefinitely. The result is a dark, button-deaf watch that is nonetheless fully alive -- still advertising and still serving BLE -- recoverable only by holding the button to force a watchdog reset. A second unbounded loop in the same handler, `while (!lv_task_handler()) {}` (added by 2625ed39 for the go-to-clock-on-sleep path), has the same shape and the same consequence if LVGL never reports work done. Bounded here too. I have not found a reachable path that enters this handler below Low on current main, so this is not backed by a reproducer. It restores an invariant that was lost, rather than fixing an observed hang. --- src/displayapp/DisplayApp.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/displayapp/DisplayApp.cpp b/src/displayapp/DisplayApp.cpp index 84fa603622..46e85ccff3 100644 --- a/src/displayapp/DisplayApp.cpp +++ b/src/displayapp/DisplayApp.cpp @@ -309,10 +309,15 @@ void DisplayApp::Refresh() { if (state != States::Running || !systemTask->IsSleeping()) { break; } - while (brightnessController.Level() != Controllers::BrightnessController::Levels::Low) { + // Levels are ordered Off, AlwaysOn, Low, Medium, High and Lower() is a no-op at Off + // and AlwaysOn, so a plain "until the level is Low" loop cannot terminate if this + // handler is ever entered at one of those two levels. Step down only from above Low, + // then set the level outright. + while (brightnessController.Level() > Controllers::BrightnessController::Levels::Low) { brightnessController.Lower(); vTaskDelay(100); } + brightnessController.Set(Controllers::BrightnessController::Levels::Low); // Turn brightness down (or set to AlwaysOn mode) if (msg == Messages::GoToAOD) { brightnessController.Set(Controllers::BrightnessController::Levels::AlwaysOn); @@ -324,7 +329,10 @@ void DisplayApp::Refresh() { currentApp == Apps::Settings) { LoadScreen(Apps::Clock, DisplayApp::FullRefreshDirections::None); // Wait for the clock app to load before moving on. - while (!lv_task_handler()) { + // Bounded for the same reason as the brightness loop above: if lv_task_handler() + // never reports work done, spinning here wedges the display task permanently. + uint16_t attempts = 0; + while (!lv_task_handler() && ++attempts < 1000) { }; } // Clear any ongoing touch pressed events