⚡️regular_timer: arm for the next due callback instead of ticking at 1 Hz

[Coral-coder]

⚡ Power tweak — one of a series of idle-wakeup reductions that let the SoC spend more time in deep sleep. Companion PRs: #1658 · #1659 · #1660 · #1661 · #1662 · #1663.

The service woke the system every second even when nothing was due: minute callbacks only need a wakeup at the minute boundary, and multisecond callbacks only at their next deadline. Replace the 1 Hz repeating timer with a one-shot armed for the soonest-due callback.

Elapsed whole seconds are credited against multisecond countdowns at each fire, measured on the monotonic tick clock so wall-clock changes can't stall or double-credit them. Seconds-list deadlines are armed on that same tick-clock grid, anchored at the last crediting pass, so a fire can never land short of the seconds it is meant to credit; on boards where the hardware RTC and the tick clock are independent oscillators (e.g. SF32LB52), arming against wall boundaries instead would drift and cause double-wakes. The minutes list fires on wall-minute changes, so its deadline aims just past the next wall-minute boundary; an early landing there costs one extra pass. New registrations are padded by the elapsed-but-uncredited seconds so they wait their full interval, and the anchor is re-based when the timer was idle.

On an idle watch this removes the perpetual 1 Hz wakeup, leaving the minute boundary as the only recurring deadline.

[gmarull]

Deep review. The core design is sound: I verified the anchor/pad arithmetic (correct for arbitrary anchor staleness, since pad and credit use the same anchor), and checked task_timer.c for deadlock — new_timer_start/stop only take the manager mutex, which is never held while callbacks execute, so calling them from prv_arm_timer() under the callback-list mutex is safe, including the re-arm from within timer_callback itself. The epoch-minute change also quietly fixes the old tm_min aliasing (a clock jump of exactly 60 minutes was previously missed). CI is green on all boards.

I found one real race-class bug (two windows, same root cause — see the inline comments on timer_callback and do_callbacks) plus a few smaller hardenings. Summary of the non-inline points:

• The power claim needs the companion PRs. On current main, cron_service_init (services/cron/service.c:92), the main.c watchdog, and the system-task idle watchdog all register permanent 1 s callbacks, so this PR alone doesn't remove the 1 Hz wakeup. Fine as a series (#1658–#1663), but landing order matters for anyone bisecting power numbers.
• Catch-up semantics changed (probably fine). After a stall longer than a callback's period (debugger halt, missed deadline), the old code fired back-to-back catch-up ticks; the new code credits MIN(elapsed, count) and fires once, discarding the excess for that callback. Periodic rate is preserved but fire count isn't — only relevant to callbacks that integrate per fire (e.g. power_tracking, battery sampling) and only after abnormal stalls. Worth a sentence in the commit message.
• Early minute landings on split-oscillator boards. On SF32LB52 (RC10K tick clock) a 60 s tick-domain delay can land before the wall boundary, costing an extra wake+rearm most minutes. The code comment acknowledges this; if it proves chatty, a slightly larger bias on the minute deadline trades latency for wakes.
• Tests: no tests added, and the service is only compiled into test_data_logging incidentally. Every bug I found lives in the anchor/pad/credit arithmetic — a dedicated clar test using fake_rtc plus a controllable new_timer stub (registration padding, idle rebase, mid-pass registration, clamp behavior, minute-boundary init) would be high-value.
• Nits: include ordering is now inconsistent (pbl/util/math.h after the system/ headers); add_multisecond_callback calls rtc_get_ticks() twice where one read could serve both the rebase and uncredited.

Verdict: I'd hold this for the crediting race (the mid-pass-registration window is reachable in normal operation once long-interval seconds callbacks exist — exactly the world this PR series creates). The rest are small hardenings; the overall approach is correct and the power win is real once the series lands.

[gmarull]

Race: the anchor is advanced outside the mutex. rtc_get_ticks(), elapsed, and this s_last_run_ticks += all run before do_callbacks() takes the list mutex. regular_timer_add_multisecond_callback reads s_last_run_ticks under the mutex, but that doesn't exclude this unlocked write:

• A registration landing between the anchor advance and do_callbacks' mutex_lock computes uncredited = 0, is appended to the list, and is then credited elapsed in the same pass → fires up to elapsed seconds early.
• RtcTicks is uint64_t; on 32-bit ARM this unlocked write can tear against the locked read. A torn read yields a garbage uncredited → immediate fire, or a count clamped to UINT16_MAX (callback delayed ~18 h). Astronomically narrow window, but watch uptimes are months.

Suggested fix (also closes the do_callbacks window below): make the seconds-list pass two-phase — under one mutex hold, advance the anchor and credit/mark all due nodes; then execute the marked callbacks with the existing unlock-around-cb pattern.

[gmarull]

Race: a callback registered mid-pass is credited seconds it didn't live through. do_callbacks releases the mutex while each callback runs; a callback registered in that window is appended at the tail (list_append), so this iteration reaches it and credits it the full elapsed, even though its registration pad is ~0 (the anchor was already advanced at the top of timer_callback).

Normally elapsed is small, but it equals the whole inter-fire gap: with only a 300 s callback registered, a 10 s callback registered while that callback executes fires immediately instead of in 10 s. This is reachable in normal operation once long-interval seconds callbacks exist — exactly what this PR series creates.

A two-phase pass (credit+mark under one lock hold, then execute marked nodes) fixes this: nodes appended during execution are unmarked and uncredited.

[gmarull]

First minutes callback can wait up to ~2 minutes. This -1 initialization now happens on the first timer fire ever; previously the free-running 1 Hz tick did it within a second of boot. If the first registration is a minutes callback (realistic once the companion PRs land — e.g. rtc_sync_timer in drivers/rtc/nrf5.c), the timer is armed exactly at the boundary and that first boundary is consumed by initialization, so the first fire comes at the second boundary.

Suggest initializing s_last_minute_fired = rtc_get_time() / SECONDS_PER_MINUTE in regular_timer_init() — the RTC driver initializes much earlier.

[gmarull]

Anchor rebase hole on the minutes path (edge case). Only add_multisecond_callback re-bases the stale anchor, and only when both lists are empty. Sequence: lists empty for hours → add_multiminute_callback (no rebase) → add_multisecond_callback before the first minute fire → uncredited = the entire idle gap. The pad arithmetic still mostly cancels, but when seconds + uncredited exceeds the UINT16_MAX clamp the cancellation breaks and the callback fires early by the overflow amount.

Hard to hit, but the fix is one line: do the same both-lists-empty rebase here (shared helper), which caps uncredited at ~60 s forever and makes the clamp unreachable.

[gmarull]

Behavior change worth documenting: seconds ticks lose wall-second alignment. The removed code deliberately armed the first tick at 1000 - milliseconds so 1 Hz fires landed at ~:000 ms of each wall second. The seconds grid is now anchored at registration time — arbitrary phase within the second, varying per boot. tick_timer/service.c drives PEBBLE_TICK_EVENT from this, so a seconds watchface updates up to ~1 s out of phase with the true boundary.

The commit message covers the oscillator-drift rationale for tick-grid arming, but not this user-visible tradeoff — it should. If alignment matters, the anchor could be best-effort rounded to a wall-second boundary at rebase time (drift on split-oscillator boards would remain).

[Coral-coder]

Yep, still working on coalescing the timers. It's coming along nicely tho. 😁