Fix long-uptime clock display bugs (colon stalls, stray PM/seconds dot)

Bullpuph123 commented

2026-07-05 06:24:22 +00:00

(Migrated from github.com)

Summary

Fixes three ClockWidget symptoms that only appear after long uptime and clear on reboot: irregular colon blinking, the PM indicator showing up on a digit orb, and a stray seconds dot on a digit orb.
GlobalTime: the 1s scheduler reset itself with m_updateTimer = millis() after a > 1000ms check, so its period drifted a few ms per cycle relative to the NTP epoch's exact 1000ms grid (same millis() clock). Each time the drift crossed a second boundary, a second was skipped — and since the colon toggled on seconds parity, it froze for 2+s at irregular intervals. Now advances by whole periods (rollover-safe, drift-free) so seconds are never skipped.
ClockWidget: colon now blinks on a dedicated 500ms millis()-based timer (stable 1s cycle) instead of seconds parity; AM/PM is only redrawn when it changes, eliminating the per-second TTF font unload/reload cycle (~86k FreeType heap alloc cycles/day) that fragmented the heap over multi-day uptimes; middle/status orb ownership of colon/seconds tick/AM-PM is explicit with bounds guards on orb indexes and seconds values; update() uses rollover-safe unsigned elapsed-time math and avoids per-loop String allocations.
ScreenManager: selectScreen() deselects all CS lines before asserting the target and validates the index, so two panels can never be briefly selected during a transition (stray pixels latched into adjacent orbs).
Adds a host-side simulation (firmware/test/host_sim_long_uptime.py) that replicates the exact uint32 arithmetic: the old scheduler skips 53 seconds in 3 simulated hours; the new logic shows 0 skips including across the 49.7-day millis() rollover, holds a 500ms colon cadence with 2ms cumulative drift over 24h, and keeps all seconds-dot draws bounded to the middle orb.

Test plan

Host simulation: python firmware/test/host_sim_long_uptime.py (all checks pass, incl. rollover boundary and blocked-loop catch-up)
pio run -e esp32doit-devkit-v1 builds clean (no new warnings)
Flashed to ESP32 hardware; verified normal boot, WiFi reconnect, and stable operation
Long-run soak on hardware across multiple days

🤖 Generated with Claude Code

## Summary - Fixes three ClockWidget symptoms that only appear after long uptime and clear on reboot: irregular colon blinking, the PM indicator showing up on a digit orb, and a stray seconds dot on a digit orb. - **GlobalTime**: the 1s scheduler reset itself with `m_updateTimer = millis()` after a `> 1000ms` check, so its period drifted a few ms per cycle relative to the NTP epoch's exact 1000ms grid (same `millis()` clock). Each time the drift crossed a second boundary, a second was skipped — and since the colon toggled on seconds parity, it froze for 2+s at irregular intervals. Now advances by whole periods (rollover-safe, drift-free) so seconds are never skipped. - **ClockWidget**: colon now blinks on a dedicated 500ms `millis()`-based timer (stable 1s cycle) instead of seconds parity; AM/PM is only redrawn when it changes, eliminating the per-second TTF font unload/reload cycle (~86k FreeType heap alloc cycles/day) that fragmented the heap over multi-day uptimes; middle/status orb ownership of colon/seconds tick/AM-PM is explicit with bounds guards on orb indexes and seconds values; `update()` uses rollover-safe unsigned elapsed-time math and avoids per-loop String allocations. - **ScreenManager**: `selectScreen()` deselects all CS lines before asserting the target and validates the index, so two panels can never be briefly selected during a transition (stray pixels latched into adjacent orbs). - Adds a host-side simulation (`firmware/test/host_sim_long_uptime.py`) that replicates the exact uint32 arithmetic: the old scheduler skips 53 seconds in 3 simulated hours; the new logic shows 0 skips including across the 49.7-day `millis()` rollover, holds a 500ms colon cadence with 2ms cumulative drift over 24h, and keeps all seconds-dot draws bounded to the middle orb. ## Test plan - [x] Host simulation: `python firmware/test/host_sim_long_uptime.py` (all checks pass, incl. rollover boundary and blocked-loop catch-up) - [x] `pio run -e esp32doit-devkit-v1` builds clean (no new warnings) - [x] Flashed to ESP32 hardware; verified normal boot, WiFi reconnect, and stable operation - [x] Long-run soak on hardware across multiple days 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Bullpuph123 commented

2026-07-06 01:49:41 +00:00

(Migrated from github.com)

Follow-up fix pushed (b8becdf): during on-hardware soak testing, the colon blink degraded from 500ms to 1s on / 1s off after several hours of uptime — a new reproduction of the original "works after reboot, degrades over time" pattern, this time with a different root cause.

Root cause: when the NTP server stops responding (pool rotation, router NAT timeout, etc.), NTPClient::forceUpdate() blocks for ~1010ms waiting on the UDP reply and returns without advancing _lastUpdate on failure. NTPClient::update() therefore re-attempts the blocking call on every invocation once a refresh is due — and GlobalTime::updateTime() calls it once per second. The entire render loop degrades to ~1 iteration/second, so the colon (max one toggle per draw) blinks at exactly half rate. Timekeeping continues via millis(), so the digits and seconds dot stay correct — only the colon shows the fault. A reboot picks a responsive server via fresh DNS and masks the bug.

Changes:

GlobalTime: NTP attempts are now gated to once per 61s (5s until the first successful sync), with the attempt timestamp recorded regardless of outcome. A dead NTP server now costs at most one ~1s hiccup per minute instead of one per second, and failures are logged to serial.
ClockWidget: the colon catch-up toggles by elapsed-period parity, so a stalled loop can never invert the blink phase — it stays locked to the 500ms grid.
Host simulation: new check reproduces the failure with the old code (median toggle interval ~1010ms) and verifies zero phase errors over 2 simulated hours with the fix.

Verified on hardware: builds clean, flashed, normal boot with WiFi/timezone intact, stable 500ms blink.

**Follow-up fix pushed** (b8becdf): during on-hardware soak testing, the colon blink degraded from 500ms to 1s on / 1s off after several hours of uptime — a new reproduction of the original "works after reboot, degrades over time" pattern, this time with a different root cause. **Root cause**: when the NTP server stops responding (pool rotation, router NAT timeout, etc.), `NTPClient::forceUpdate()` blocks for ~1010ms waiting on the UDP reply and returns *without* advancing `_lastUpdate` on failure. `NTPClient::update()` therefore re-attempts the blocking call on **every** invocation once a refresh is due — and `GlobalTime::updateTime()` calls it once per second. The entire render loop degrades to ~1 iteration/second, so the colon (max one toggle per draw) blinks at exactly half rate. Timekeeping continues via `millis()`, so the digits and seconds dot stay correct — only the colon shows the fault. A reboot picks a responsive server via fresh DNS and masks the bug. **Changes**: - `GlobalTime`: NTP attempts are now gated to once per 61s (5s until the first successful sync), with the attempt timestamp recorded regardless of outcome. A dead NTP server now costs at most one ~1s hiccup per minute instead of one per second, and failures are logged to serial. - `ClockWidget`: the colon catch-up toggles by elapsed-period *parity*, so a stalled loop can never invert the blink phase — it stays locked to the 500ms grid. - Host simulation: new check reproduces the failure with the old code (median toggle interval ~1010ms) and verifies zero phase errors over 2 simulated hours with the fix. Verified on hardware: builds clean, flashed, normal boot with WiFi/timezone intact, stable 500ms blink.

Bullpuph123 commented

2026-07-06 05:59:31 +00:00

(Migrated from github.com)

Follow-up commit 0c4fbf1 fixes three more interaction bugs found while reviewing how the event loops interact:

1. Timezone API retry storm (same class as the NTP bug above)
GlobalTime::updateTime() called getTimeZoneOffsetFromAPI() on every 1-second tick whenever m_timeZoneOffset == -1 (boot, until the first successful fetch) or when the epoch passed m_nextTimeZoneUpdate (DST changeover). The call is a blocking HTTP request (up to ~5s on an unreachable server), and timezonedb rate-limits free keys to ~1 req/s — so hammering it once per second both stalls the render loop and makes the requests likely to fail. Attempts are now gated: retry every 30s until the first success, then every 5 minutes if a scheduled refresh fails. A successful refresh still reschedules via zoneEnd as before.

2. Uninitialized loop index in WidgetSet::updateAll()
for (int8_t i; i < m_widgetCount; i++) read an uninitialized i — undefined behavior executed on every boot via initializeAllWidgetsData(). If the stack garbage was ever non-zero, widgets would be silently skipped (or worse, m_widgets[i] indexed out of bounds). Now int8_t i = 0.

3. Button ISRs not placed in IRAM
The three ISR wrappers in main.cpp and Button::isrButtonChange() lacked IRAM_ATTR. On the ESP32, an interrupt that executes from flash-cached code while the flash cache is disabled (e.g. during an NVS/config write) crashes with "Cache disabled but cached memory region accessed" — i.e. pressing a button at the wrong moment could reboot the device. All four functions are now IRAM_ATTR (millis() and digitalRead() are IRAM-safe in the ESP32 Arduino core).

Build verified (RAM 16.2%, Flash 77.2%) and the host-side timing simulation in firmware/test/host_sim_long_uptime.py still passes all checks.

Follow-up commit 0c4fbf1 fixes three more interaction bugs found while reviewing how the event loops interact: **1. Timezone API retry storm (same class as the NTP bug above)** `GlobalTime::updateTime()` called `getTimeZoneOffsetFromAPI()` on every 1-second tick whenever `m_timeZoneOffset == -1` (boot, until the first successful fetch) or when the epoch passed `m_nextTimeZoneUpdate` (DST changeover). The call is a blocking HTTP request (up to ~5s on an unreachable server), and timezonedb rate-limits free keys to ~1 req/s — so hammering it once per second both stalls the render loop and makes the requests likely to fail. Attempts are now gated: retry every 30s until the first success, then every 5 minutes if a scheduled refresh fails. A successful refresh still reschedules via `zoneEnd` as before. **2. Uninitialized loop index in `WidgetSet::updateAll()`** `for (int8_t i; i < m_widgetCount; i++)` read an uninitialized `i` — undefined behavior executed on every boot via `initializeAllWidgetsData()`. If the stack garbage was ever non-zero, widgets would be silently skipped (or worse, `m_widgets[i]` indexed out of bounds). Now `int8_t i = 0`. **3. Button ISRs not placed in IRAM** The three ISR wrappers in `main.cpp` and `Button::isrButtonChange()` lacked `IRAM_ATTR`. On the ESP32, an interrupt that executes from flash-cached code while the flash cache is disabled (e.g. during an NVS/config write) crashes with "Cache disabled but cached memory region accessed" — i.e. pressing a button at the wrong moment could reboot the device. All four functions are now `IRAM_ATTR` (`millis()` and `digitalRead()` are IRAM-safe in the ESP32 Arduino core). Build verified (RAM 16.2%, Flash 77.2%) and the host-side timing simulation in `firmware/test/host_sim_long_uptime.py` still passes all checks.

ericthelin commented

2026-07-14 18:10:02 +00:00

(Migrated from github.com)

I updated the target branch to be dev instead of main because I think that is still where we are doing development then merging dev into main at release points. (Unless I missed something in the time I have been in active). But unfortunately that triggered merge conflicts. That said please do fix the conflicts and update because I fully support your change and want to see it merged. I chased the tick delay issues for a long time and didn't find a solution. So I really want this merged. That said it also would be a good thing to split this into a few separate PRs as it is now doing multiple things at once.

I updated the target branch to be `dev` instead of `main` because I think that is still where we are doing development then merging dev into main at release points. (Unless I missed something in the time I have been in active). But unfortunately that triggered merge conflicts. That said please do fix the conflicts and update because I fully support your change and want to see it merged. I chased the tick delay issues for a long time and didn't find a solution. So I really want this merged. That said it also would be a good thing to split this into a few separate PRs as it is now doing multiple things at once.

Bullpuph123 commented

2026-07-15 00:47:45 +00:00

(Migrated from github.com)

Thanks for the support and for retargeting to dev! As requested, I've split this into three focused PRs, each re-implemented directly on top of dev (rather than resolving conflicts here, since dev's ConfigManager/WidgetTimer architecture diverged quite a bit from main):

#334 — GlobalTime: drift-free 1s scheduler + gated NTP/timezone retries (the tick-delay root cause)
#335 — ClockWidget: millis-based colon blinker + AM/PM redraw only on change + bounds guards
#336 — ScreenManager: deselect-all-first CS handling + IRAM_ATTR on button ISRs

One note: the uninitialized loop index fix in WidgetSet::updateAll from this PR was dropped — it's already fixed on dev.

Closing this in favor of the three above.

Thanks for the support and for retargeting to dev! As requested, I've split this into three focused PRs, each re-implemented directly on top of dev (rather than resolving conflicts here, since dev's ConfigManager/WidgetTimer architecture diverged quite a bit from main): - #334 — GlobalTime: drift-free 1s scheduler + gated NTP/timezone retries (the tick-delay root cause) - #335 — ClockWidget: millis-based colon blinker + AM/PM redraw only on change + bounds guards - #336 — ScreenManager: deselect-all-first CS handling + IRAM_ATTR on button ISRs One note: the uninitialized loop index fix in WidgetSet::updateAll from this PR was dropped — it's already fixed on dev. Closing this in favor of the three above.

Pull request closed

Please reopen this pull request to perform a merge.

Rows
Columns

Fix long-uptime clock display bugs (colon stalls, stray PM/seconds dot) #330

Summary

Test plan

Pull request closed