From b9541d8e7cb8d7a810fe861ef83361163aebac38 Mon Sep 17 00:00:00 2001 From: David Lee <247393336+davelee98@users.noreply.github.com> Date: Sat, 1 Aug 2026 21:27:39 -0400 Subject: [PATCH 1/2] feat(nrf): hardware watchdog with reset-reason and phase breadcrumbs nRF had no watchdog, so any unbounded wait was permanent: nrfx_spim.c's `while (!nrf_spim_event_check(END)){}` and the six Wire_nRF52 TWIM spins have no timeout and no yield, and checkTransferTimeouts() cannot help because it runs FROM loop(), which is what is stuck. This makes that class recoverable -- the residual PLAN_PHASE2_BOUND_WAITS D-L accepted and D-K assumed unrecoverable. Portable module, not an nRF-only one: every feed site and breadcrumb stamp lives in code that compiles for both targets, so an nRF-only API would mean #ifdef TARGET_NRF around ~20 call sites in shared files. Follows the ble_transport pattern -- one header with no vendor includes, two whole-file-gated implementations, ESP32 stubbed. Its reset-reason decode moves out of main.cpp, a net #ifdef reduction there. Timeout is 300 s, and the number alone is not what makes it safe. The longest span the firmware cannot instrument is a REFRESH_FULL on a 7-colour split-buffer panel: 4 BUSY_WAIT entries x 30 s, sent to BOTH controllers, ~240 s inside one bbepRefresh(). What keeps that from resetting a healthy device is the feed immediately before all 15 bb_epaper entry points, so the dog faces one call rather than that call plus everything preceding it. Margin is ~1.25x -- re-check it when adding a panel. Three details that are easy to get wrong: - RESETREAS must come from readResetReason(). The core reads AND clears the register in init() before setup(), so reading the peripheral (or sd_power_reset_reason_get) returns zero forever and reports every watchdog reset as a power-on. - GPREGRET2 needs two access paths. sd_power_gpregret_* are numbered from SOC_SVC_BASE_NOT_AVAILABLE and cannot be used before ble.begin(); direct register access is correct while the SoftDevice is disabled, SVCs once it is enabled. - A running WDT cannot be stopped or reconfigured, and which resets clear it is NOT established by anything in-tree. So inherit-detection via RUNSTATUS runs on every build INCLUDING the disabled one, and feeds every enabled RREN channel of whatever it finds. A disabled build that inherited a live dog it never fed would be a brick. Boot-loop containment: armed before the boot panel path so boot wedges are covered, with a 3-strike counter in GPREGRET2 bits 5:4 entering a safe mode that refuses panel work at both epdSessionAcquire and pwrmgm. Strikes clear after 10 min of uptime rather than on a successful refresh -- refresh-based clearing would never accumulate (every boot refreshes) and would make safe mode permanent (safe mode never refreshes). Not verified on hardware. The 240 s figure is read from bb_ep.inl, not measured; T2 in the plan is the gate before this reaches devices. Safe mode rejects transfers late and generically -- a clean NACK needs a "device in safe mode" code, which must originate in opendisplay-protocol. Plan, decisions and four rounds of review findings: docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md --- docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md | 396 ++++++++++++++++++ platformio.ini | 35 +- src/boot_screen.cpp | 5 + src/display_service.cpp | 63 +++ src/main.cpp | 63 ++- src/watchdog.h | 85 ++++ src/watchdog_esp32.cpp | 81 ++++ src/watchdog_nrf.cpp | 386 +++++++++++++++++ 8 files changed, 1084 insertions(+), 30 deletions(-) create mode 100644 docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md create mode 100644 src/watchdog.h create mode 100644 src/watchdog_esp32.cpp create mode 100644 src/watchdog_nrf.cpp diff --git a/docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md b/docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md new file mode 100644 index 0000000..526c08c --- /dev/null +++ b/docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md @@ -0,0 +1,396 @@ +# nRF Hardware Watchdog — Implementation Plan + +**Date:** 2026-08-01 +**Revision:** 2 — rewritten after an adversarial review found four blocking errors in rev 1. +Corrections are marked **[R1]** throughout; §10 lists them. +**Target:** the watchdog itself is `TARGET_NRF` only (nRF52840, Bluefruit / S140 v7.3.0). +The **module is portable** (W-0): a target-neutral `watchdog.h` with two self-gated +implementations, ESP32 stubbed (W-6). ESP32 behaviour is unchanged except that its existing +reset-reason decode relocates out of `main.cpp`. +**Goal:** recover the device when an *unbounded* wait — in `loop()`, or below it in a +vendored driver we cannot instrument — wedges the panel permanently. + +## 1. What this closes + +Two accepted residuals and one new finding converge on the same gap. + +**Accepted residual 1 — nRF's unbounded I2C spins.** +[PLAN_PHASE2_BOUND_WAITS_2026-07-26.md](PLAN_PHASE2_BOUND_WAITS_2026-07-26.md) decision +**D-L** found that `Wire_nRF52.cpp:166-181` / `:230-247` spin on TWIM events with no deadline +and no yield, and chose option **(a) accept and document**. + +**Accepted residual 2 — the scheduler-starving fault class.** +Decision **D-K** accepted that `millis()` bounds all fail together if the scheduler stalls, on +the grounds that it is "a fault class we cannot recover from anyway." A hardware watchdog is +what makes that class recoverable, so D-K's premise no longer holds once this lands. + +**New finding (2026-08-01) — the same shape on SPI.** +`nrfx_spim.c:598` blocks in `while (!nrf_spim_event_check(p_spim, NRF_SPIM_EVENT_END)){}` with +no timeout and no yield, because `SPIClass` initialises nrfx with a NULL handler +(`SPI.cpp:101`). On nRF the Arduino API drives the panel **one byte at a time** +(`arduino_io.inl:214-221`), so the firmware enters that spin ~96,000 times per full refresh. + +Underlying all three: no firmware-armed watchdog exists on nRF. See **V1** for the corrected +statement of what exists on ESP32 — rev 1 got that wrong. + +## 2. Verification done before writing this plan + +Checked against source in this workspace. Rows corrected after review are marked **[R1]**. + +| # | Fact | Evidence | +|---|---|---| +| V1 **[R1]** | nRF has **no** watchdog. ESP32's IDF task watchdog **is enabled and initialised** (`CONFIG_ESP_TASK_WDT_EN=y`, `INIT=y`, `TIMEOUT_S=5`) — but **watches nothing that would catch a wedged `loop()`**: S3/classic set only `CHECK_IDLE_TASK_CPU0=y` while `CONFIG_ARDUINO_RUNNING_CORE=1` puts `loopTask` on CPU1; C3/C6 subscribe no idle task at all | `framework-arduinoespressif32-libs/{esp32s3,esp32,esp32c3,esp32c6}/sdkconfig`. Rev 1 claimed "no watchdog on either target" — the *conclusion* for `loop()` holds, the *reason* was wrong | +| V2 **[R1]** | Neither the core nor Bluefruit **operates** `NRF_WDT` | grep over `cores/`+`libraries/` finds only MDK register definitions. Rev 1 said "only MDK defs exist"; `drivers/include/nrfx_wdt.h` is also installed — header only | +| V3 **[R1]** | The nrfx WDT **implementation** is not compiled | no `drivers/src/nrfx_wdt.c`; `NRFX_WDT_ENABLED` absent from `nrfx_config.h`. The *header* `nrfx_wdt.h` does exist. Use `hal/nrf_wdt.h` | +| V4 **[R1]** | ⚠️ **`RESETREAS` is read and cleared by the core before `setup()`**, and exposed via `readResetReason()` | `cores/nRF5/wiring.c:37-40` — `_reset_reason = NRF_POWER->RESETREAS;` then `RESETREAS \|= RESETREAS` (write-1-to-clear); getter at `:72`. **Rev 1's `sd_power_reset_reason_get()` design would read zero forever**, and its "bits accumulate across boots" claim was false | +| V5 | `GPREGRET` (id 0) is already taken by the DFU handshake | [device_control.cpp:868-869](../src/device_control.cpp#L868-L869) | +| V6 **[R1]** | `GPREGRET2` (id 1) is reachable under the SD and unused by app + framework source | `nrf_soc.h:641-666`. **Not fully verified against the installed bootloader image**, which is not in these sources — see risk in W-3 | +| V7 **[R1]** | No `.noinit` section exists in the packaged linker scripts | `grep -l noinit cores/nRF5/linker/*.ld` → none. Rev 1 concluded "package edit or nothing" — **wrong**: PlatformIO supports `board_build.ldscript` (`platforms/nordicnrf52/builder/frameworks/arduino/adafruit.py:190-196`). GPREGRET2 is still preferred, but as a choice, not a necessity | +| V8 **[R1]** | WDT is clocked from LFCLK — but **LFCLK is already started by the core before `setup()`** | `wiring.c:45-57` (`TASKS_LFCLKSTART = 1`). **Rev 1's conclusion that this forces arming after `ble.begin()` was wrong**; arm ordering is now a policy choice (W-3), not a clock constraint | +| V9 | Reload uses a fixed magic and per-register enables | `nrf_wdt.h:56` `NRF_WDT_RR_VALUE 0x6E524635`; `WDT_RREN_RR0_*`, `nrf52840_bitfields.h:17376` | +| V10 | Behaviour in sleep / debug-halt is configurable | `WDT_CONFIG_SLEEP_Pos = 0`, `WDT_CONFIG_HALT_Pos = 3`, `nrf52840_bitfields.h:17385,17391` | +| V11 **[R2]** | **The WDT cannot be stopped once started.** The register block has no `TASKS_STOP` and no `ENABLE`, and `CRV`/`RREN`/`CONFIG` latch at `START`. **Which resets clear it is NOT established** — a power-on reset certainly does; whether a soft/`DOG`/pin reset does is unverified, so the code feeds any watchdog it finds running regardless (W-1) | nRF52840 architectural property; the HAL exposes START and reload, no stop. Constrains W-3 and §5's DFU residual | +| V12 **[R1]** | **Worst-case uninstrumentable span = ~240 s.** The 8.1" Spectra is `BBEP_SPLIT_BUFFER \| BBEP_7COLOR`; its init list holds **4** `BUSY_WAIT` entries, and `REFRESH_FULL` sends the whole sequence to CS1 **and again to CS2** | `bb_ep.inl:3704-3726` (4 × `BUSY_WAIT`), `:4373-4380` (CS1 then CS2), `:3967-3969` (30 s cap for 3/4/7-colour). 8 × 30 s = 240 s inside **one** `bbepRefresh()` call | +| V13 **[R1]** | `enterDFUMode()` **jumps to the bootloader without a system reset** | [device_control.cpp:868-884](../src/device_control.cpp#L868-L884) — `sd_softdevice_disable()`, vector-table move, `bootloader_util_app_start()`. No `NVIC_SystemReset()`. With **V11**, an armed WDT keeps counting into the bootloader | + +## 3. The central design problem + +A watchdog is easy to get catastrophically wrong here, because this firmware **legitimately +blocks for minutes** on a healthy device. + +### 3.1 Blocking-span inventory + +| Span | Worst case | Feed possible inside? | +|---|---|---| +| **`bbepRefresh(REFRESH_FULL)` on a 7-colour split-buffer panel** | **~240 s** (**V12**) | **No** — one libdep call | +| `waitforrefresh(60)` | ~126 s — 6000 iterations × ~21 ms (`delay(10)` + `bbepIsBusy`'s own `delay(10)`+`delay(1)`, `bb_ep.inl:3984-3986`). The argument is **not** seconds | **Yes** — [display_service.cpp:857-870](../src/display_service.cpp#L857-L870) | +| `bbepSendCMDSequence` (init only) | N × 5 s (B/W) or N × 30 s (multicolour) | No — libdep | +| `pwrmgm(true)` | 900 ms | Yes | +| `nrfx_spim` / `Wire` spins | **unbounded** (the fault) | No — and deliberately not (§W-2) | + +### 3.2 Why 300 s works — but only with a pre-call feed **[R1]** + +**Rev 1 claimed 300 s "dominates every span" with >3× headroom, from an assumed N=3 and a +single command sequence. That was wrong** (**V12**): the real worst case is 240 s, and rev 1's +margin was ~1.25×, erasable by any work preceding the call in the same handler invocation. + +**D-1 is confirmed at 300 s**, and is made safe by a change to the feed policy rather than to +the timeout: **feed immediately before every call that enters bb_epaper's blocking region** +(W-2). Since the 240 s is a single uninterruptible call, feeding on entry means the watchdog +faces exactly that span and nothing else. + +- Longest uncovered span becomes **240 s** — the single worst libdep call, not a sum. +- Margin **60 s (1.25×)**, and everything before the call is irrelevant because the counter is + freshly reloaded. +- **This margin is thin and must be respected**: any future increase in `BUSY_WAIT` count, + controller count, or per-wait cap eats directly into it. Recorded as a residual in §5, with + T2 measuring the real figure on hardware. + +## 4. Design + +### W-0 — Module shape: portable header, two self-gated implementations + +Not an nRF-only module, and the reason is `#ifdef` count in shared code. Every feed site and +breadcrumb stamp lives in a file that compiles for both targets: `loop()`/`idleDelay` in +[main.cpp](../src/main.cpp) (*"One loop body for both targets"*), and `waitforrefresh` plus the +bb_epaper entry points in [display_service.cpp](../src/display_service.cpp). An nRF-only API +would put `#ifdef TARGET_NRF` around ~20 call sites in shared files — what +[PLAN_UNIFY_NRF_ESP32_LOOP_BLE_2026-07-27.md](PLAN_UNIFY_NRF_ESP32_LOOP_BLE_2026-07-27.md) +worked to remove. + +Follow the transport pattern, whose reasoning `ble_transport.h` already records: *"Exactly one +implementation is linked per build... a plain class rather than an abstract base: virtual +dispatch would cost a vtable and indirect calls for zero benefit"*, and *"The whole file is +gated on TARGET_NRF, so an ESP32 build compiles it to an empty translation unit — no +build_src_filter changes needed."* + +``` +src/watchdog.h portable. No nrf_wdt.h, no esp_task_wdt.h — any TU may include it. +src/watchdog_nrf.cpp whole file #ifdef TARGET_NRF — real +src/watchdog_esp32.cpp whole file #ifdef TARGET_ESP32 — stubs (W-6) +``` + +Free functions; no state worth exposing. + +```c +void odWatchdogBootInit(void); // decode reset reason, evaluate strike counter +bool odWatchdogInSafeMode(void); // [R1] main.cpp gates initDisplay() on this +void odWatchdogArm(void); // once, before the boot panel path (W-3) +void odWatchdogFeed(void); // W-2 feed sites +void odWatchdogBreadcrumb(uint8_t phase); // panel-phase stamps (W-4) +``` + +#### Deliberately NOT in the API + +| Omitted | Why | +|---|---| +| `stop()` / `disable()` | nRF WDT **cannot be stopped once started** (**V11**); ESP32's TWDT can. Exposing it would advertise a capability one target cannot honour | +| runtime timeout parameter | `CRV` must be written **before** `START` and is immutable after. Compile-time only | +| task registration | ESP32's TWDT is per-task; nRF's reload registers are not an analogue. Contract is **one loop task, one watchdog**, stated in the header | + +#### Bonus: this removes `#ifdef`s rather than adding them + +The ESP32 reset decode already exists inline in `main.cpp` under `#ifdef TARGET_ESP32` +([:27-44](../src/main.cpp#L27-L44), [:82-84](../src/main.cpp#L82-L84)). Moving it into +`watchdog_esp32.cpp` is a net `#ifdef` reduction in `main.cpp`. + +### W-1 — Watchdog configuration (nRF) + +Use the Nordic HAL (`nrf_wdt.h`): header-only inline functions already on the include path, no +driver to enable (**V3**), and it supplies `NRF_WDT_RR_VALUE` (**V9**) instead of a magic +constant. + +- `CRV = (OPENDISPLAY_NRF_WDT_S × 32768) − 1`. At 300 s: `9,830,399` (`0x0095FFFF`). +- **[R1] Validate the flag at compile time**, since it is a public build knob feeding a + hardware register: `static_assert` that it is either `0` (disabled) or within + `[60, 3600]` seconds. The lower bound keeps it above §3.1's spans; the upper stays well + inside `CRV`'s 32-bit ceiling (~131,072 s). Rev 1's "no overflow guard needed" was valid + only for the literal 300. +- `RREN = RR0` only — one reload register, one feeder. +- `CONFIG.SLEEP = 1` — keep counting while the CPU sleeps; `idleDelay` feeds every ≤100 ms + chunk. Without it, a device that hangs while idle is never recovered. +- `CONFIG.HALT = 0` — do not count while halted by a debugger. +- **[R1] Check `RUNSTATUS` before configuring.** There is no way to disarm or reconfigure a + running WDT: the register block exposes `TASKS_START` and `RR[8]` only — **no `TASKS_STOP`, + no `ENABLE`** (`nrf52840.h:2044-2062`; contrast SPIM/TWI/UART, which have `ENABLE` at + `0x500`) — and `CRV`/`RREN`/`CONFIG` are latched at `START`, so later writes are ignored. + + Consequence: if the **bootloader** left the WDT running, our configuration is silently + discarded and we inherit its timeout — possibly far shorter than 300 s — while believing we + set our own. `odWatchdogArm()` must read `RUNSTATUS` first, and if the WDT is already + running, **log loudly and skip configuration** rather than pretend. Feeding must then still + happen (the inherited dog is real). Confirm behaviour on hardware in T7. + +### W-2 — Feed policy + +**Principle: only feed from a site whose execution proves forward progress.** + +| Site | Why | +|---|---| +| `loop()` top, beside `epdSessionTick()` ([main.cpp:896](../src/main.cpp#L896)) | primary liveness proof | +| `idleDelay()` chunk loop ([main.cpp:1031-1041](../src/main.cpp#L1031-L1041)) | a long idle wait is healthy | +| `waitforrefresh()` poll loop ([display_service.cpp:857](../src/display_service.cpp#L857)) | a 126 s refresh is healthy | +| **[R1] immediately before each bb_epaper entry point** — the **15** call sites in `display_service.cpp` — `bbepRefresh` ×3, `bbepSendCMDSequence` ×3, `bbepWakeUp` ×3, `bbepSleep` ×1, `bbepFill` ×2 (consecutive), and **`bbepInitIO` ×3**. Two counting errors were caught in review: `bbepFill` appears twice, and `bbepInitIO` was omitted entirely even though it sends `pInitFull` internally — twice on a split-buffer panel — making it a ~240 s span in its own right: `bbepWakeUp` ([:365](../src/display_service.cpp#L365), [:476](../src/display_service.cpp#L476), [:500](../src/display_service.cpp#L500)), `bbepSendCMDSequence` ([:366](../src/display_service.cpp#L366), [:479](../src/display_service.cpp#L479), [:503](../src/display_service.cpp#L503)), `bbepSleep` ([:451](../src/display_service.cpp#L451)), `bbepRefresh` ([:562](../src/display_service.cpp#L562), [:2503](../src/display_service.cpp#L2503), [:3340](../src/display_service.cpp#L3340)), `bbepFill` ([:3368](../src/display_service.cpp#L3368)) | **This is what makes 300 s safe** (§3.2). Reloading on entry means the dog faces the single 240 s call, not that call plus everything before it | + +**Explicitly NOT fed from:** + +- **Any ISR, timer, or SoftDevice callback.** An interrupt-fed watchdog verifies the interrupt + controller is alive, not the program. Non-negotiable. +- **`nrfx_spim`'s or `Wire`'s spins** — unreachable without forking the package, and *we do not + want to*: those spins are the fault. + +**[R1] Corrected property statement.** Rev 1 claimed "every wait we have bounded feeds the dog; +every wait we have not, does not." That is false — `bbepWaitBusy` is bounded (5/30 s) and gets +no feed. The accurate statement is: **every span we can reach is fed at its boundary; spans we +cannot reach must individually fit inside the timeout.** V12 is the largest such span. + +**[R1] A stuck BUSY pin does not trip the watchdog** and is not meant to: `waitforrefresh` +keeps feeding for its 6,000 iterations, returns `false`, and `loop()` resumes. That is correct +behaviour — a failed refresh is an error to report, not a wedge to reset. T4 must therefore +distinguish "BUSY stuck" (no reset expected) from a non-returning call inside one iteration +(reset expected). + +### W-3 — Boot-loop containment **[R1] — redesigned** + +Rev 1's design was **internally contradictory** and is replaced wholesale. Its faults: + +1. It armed the watchdog *after* the boot panel path, so a wedge *in* that path could never + produce a `DOG` reset — yet it then claimed a "panel-safe-mode escape after 3 of them." + Strikes could never accumulate from the very failure the escape existed for. +2. It cleared the strike counter on "first successful refresh." Every ordinary boot performs a + successful boot refresh, so each recurring runtime wedge would clear the previous strike and + the count would never reach 3. +3. In safe mode no refresh occurs, so "first successful refresh" could never fire and the + device could never leave safe mode. +4. It put a persistent counter and an overwritten breadcrumb in the same 8-bit register with no + bit allocation, so a breadcrumb write would destroy the counter. + +**Redesigned:** + +- **Arm before the boot panel path**, immediately after `odWatchdogBootInit()`. Boot wedges are + now covered, which is what makes the strike counter meaningful. **V8** removed the clock + reason for arming late, so nothing prevents this. The `bootdiag` `while (!Serial)` gate at + [main.cpp:58](../src/main.cpp#L58) still precedes any sane arm point and stays uncovered. +- **Clear the counter on sustained uptime, not on panel success.** Clear once the device has + run `WDT_HEALTHY_MS` (propose **10 minutes**, ≥2× the timeout) since boot with the watchdog + armed. This is panel-independent, so it works identically in safe mode — solving faults 2 + and 3 together. Strikes accumulate only when resets come *fast*, which is exactly the + boot-loop condition safe mode exists for; a device that survives 10 minutes between wedges is + not boot-looping and should keep retrying the panel. +- **Safe mode is self-exiting.** After 10 healthy minutes in safe mode the counter clears, so + the next reset boots normally and retries the panel. Worst case is a bounded oscillation — + 3 fast resets, a long safe-mode period, one retry — rather than a permanent brick. +- **[R1] Explicit GPREGRET2 bit allocation** (8 bits, `nrf52840_bitfields.h`): + + | Bits | Field | + |---|---| + | 7:6 | validity tag `0b10` — distinguishes a real value from cold-boot garbage | + | 5:4 | strike counter, 0–3, saturating | + | 3:0 | breadcrumb phase, 0–15 | + + Breadcrumb writes are read-modify-write over bits 3:0 only — + `sd_power_gpregret_clr(1, 0x0F)` then `sd_power_gpregret_set(1, phase & 0x0F)`. Two SVCs, not + atomic; safe because only the loop task writes it. Document that in the header. +- **[R1] Risk (V6):** GPREGRET2 is unused by application and framework source, but the + installed **bootloader image was not inspected**. If the bootloader writes it, the validity + tag causes a stale value to be discarded rather than misread — the counter resets, degrading + containment but not causing a wrong action. Verify on hardware (T7). +- **[R1] No MSD status bit.** Rev 1 promised to surface safe mode in the manufacturer data. + Status bit 3 is reserved and "must be 0" in `include/opendisplay_structs.h`, which is + **vendored byte-for-byte** from `opendisplay-protocol`; per CLAUDE.md such a change must + originate in the canonical repo. Out of scope here — safe mode is reported via the boot log + only. Surfacing it on the wire is follow-up work in the protocol repo. + +### W-4 — Observability + +**[R1] Reset-reason decode must use `readResetReason()`, not the peripheral.** Per **V4** the +core has already read and cleared `RESETREAS` before `setup()`. Rev 1's +`sd_power_reset_reason_get()` design would have read zero on every boot and silently reported +"power-on" for every watchdog reset — defeating the entire purpose of step 1. Decode the saved +word's `RESETPIN / DOG / SREQ / LOCKUP / OFF / DIF` bits. No clearing is needed or possible; +the core already did it, which also means bits do **not** accumulate across boots. + +**Breadcrumb.** One-byte phase code in GPREGRET2 bits 3:0, stamped at panel-phase transitions +(`IDLE`, `ACQUIRE_COLD`, `ACQUIRE_WARM`, `INIT_SEQ`, `FILL`, `STREAM`, `REFRESH_WAIT`, +`RELEASE`, `FORCE_OFF`) — 9 values, inside the 16 the field allows. Logged next to the reset +reason at boot. + +Payoff: a freeze stops being "it wedged somewhere" and becomes +`reset=DOG breadcrumb=INIT_SEQ`, naming the wedged wait directly. + +**`TIMEOUT` ISR (D-3).** Fires ~61 µs (2 LFCLK cycles) before the reset — enough to stamp a +final breadcrumb, not enough to write flash or drain a UART. Strictly best-effort; the +boot-side decode is the mechanism we rely on. + +### W-5 — Build-flag control + +`-DOPENDISPLAY_NRF_WDT_S=300` in `[env:nrf52840custom]`, `0` = disabled, validated per W-1. + +- Default **on**. A watchdog that ships disabled is not a watchdog. +- The three other nRF envs inherit via `${env:nrf52840custom.build_flags}` — no separate + entries. `CONFIG.HALT = 0` keeps breakpoints safe in the debug env (**D-4**). +- `bootdiag` is safe: its `while (!Serial)` gate precedes the arm point (W-3). +- **No ESP32 flag** — `watchdog_esp32.cpp` is stubbed (W-6). + +### W-6 — The ESP32 stub **[R1]** + +- `odBootReasonLog()` / `odWatchdogBootInit()` — **real**: the existing `resetReasonName()` + + `esp_reset_reason()` logic relocated from `main.cpp`. +- `odWatchdogArm()` — no-op that logs, **once**, the *accurate* state: the IDF task watchdog + is enabled at 5 s but **no task that would catch a wedged `loop()` is subscribed** (**V1**). +- `odWatchdogFeed()`, `odWatchdogBreadcrumb()` — empty. `odWatchdogInSafeMode()` returns false. + +Rev 1 planned to log "no watchdog armed", which **V1** shows would be false on S3 and classic +ESP32. The accurate message is more useful anyway: it names the specific gap +(`CHECK_IDLE_TASK_CPU0` vs `ARDUINO_RUNNING_CORE=1`) that a future implementation must close, +which is a one-line `esp_task_wdt_add(NULL)` on the loop task plus the feed sites this plan +already wires up. + +## 5. What this does not fix + +- **It does not fix any unbounded wait.** `nrfx_spim.c:598` and the six `Wire_nRF52` spins are + untouched. A permanent hang becomes a periodic reset — better and observable, but the device + still drops its link, loses transfer state, and pays a cold bring-up. +- **Recovery is slow, by choice.** Up to 5 minutes dead before reset; with **D-2**'s threshold + of 3, up to ~15 minutes to reach safe mode. Right trade for e-paper, where five minutes late + is invisible but resetting a healthy device mid-refresh is a visible regression. +- **[R1] The 240 s / 300 s margin is thin (1.25×).** Any growth in `BUSY_WAIT` count, + controller count, or the multicolour cap eats it directly. If a future panel exceeds it, + healthy devices get reset mid-refresh — the main way this change can do harm. T2 measures the + real figure; treat V12 as a number to re-check whenever a panel is added. +- **[R1] DFU can take a `DOG` reset — accepted, out of scope.** Per **V13**, `enterDFUMode()` + jumps to the bootloader without a system reset, and per **V11** the watchdog cannot be + stopped, so it keeps counting into a bootloader that will not feed it. A DFU session lasting + >300 s from the jump will be reset. Assessment: the reset re-enters the bootloader (GPREGRET + id 0 still holds `0xB1`), so the expected outcome is an interrupted transfer the host must + retry, not a brick — **unless** the reset lands mid-flash-write, which is not analysed here. + The fix, if it is ever wanted, is to replace the direct jump with `NVIC_SystemReset()`, + matching the core's own `enterUf2Dfu()` (`wiring.c:76-80`). **[R2] Caveat:** whether a + running nRF52840 WDT survives a non-power-on reset could not be established from any source + in this workspace. If it survives, `NVIC_SystemReset()` would **not** help and only a power + cycle stops it. The implementation is written to be correct either way (W-1's unconditional + inherit-detection); `RUNSTATUS` logged at boot settles it empirically. Explicitly descoped. + + **[R1] This residual may not exist at all.** Many Nordic/Adafruit bootloaders feed the WDT in + their main loop precisely because the application may have armed one. The installed + bootloader is a flashed binary, absent from these sources, so this could not be verified. + **T9 answers it empirically** — if DFU survives, the residual is void. +- **It says nothing about brownout.** POFCON is not enabled (`grep POFCON src/` → nothing). +- **It does not verify the panel rail power-cycles on reset** (was D-6, descoped). +- **ESP32 gains no watchdog.** The stub's boot log makes that explicit rather than assumed. + +## 6. Decisions — all resolved + +| ID | Decision | Resolution | +|---|---|---| +| **D-1** | Timeout value | ✅ **300 s**, re-confirmed after **V12** revealed a 240 s worst case. Made safe by W-2's pre-call feed, not by the timeout alone. Margin 1.25× — residual in §5 | +| **D-2** | Panel-safe-mode threshold | ✅ **3 consecutive `DOG` resets**, with the redesigned clear rule in W-3 | +| **D-3** | `TIMEOUT` ISR for a final breadcrumb? | ⚠️ **Deferred — NOT implemented.** Phase-transition breadcrumbs are stamped eagerly at every panel-phase entry, so the retained value is already correct when the reset lands; the ISR would only add ~61 µs of redundancy. Revisit if a real failure shows a phase gap | +| **D-4** | Watchdog in the debug envs? | ✅ **Enabled**, inherited; `bootdiag` included (its Serial gate precedes the arm point) | +| **D-5** | Ship observability before arming? | ✅ **Yes — split.** Step 1 lands W-0/W-4/W-6 with nothing armed | +| **D-6** | Does the panel rail drop on a WDT reset? | ✅ **Descoped** — §5 | +| **D-7 [R1]** | Fix the DFU jump so the watchdog cannot reset a DFU session? | ✅ **No — out of scope by decision.** Recorded as an accepted residual in §5 with its severity assessment | + +## 7. Test plan + +| # | Test | Expected | +|---|---|---| +| T1 | Normal operation, many push cycles | No reset; `DOG` never appears in the decode | +| T2 **[R1]** | Full refresh on the **slowest supported panel** (7-colour split-buffer if available), cold, full-frame | No reset, and **log the measured span** — this validates V12's 240 s and the real margin against 300 s. The single most important test | +| T3 | Injected infinite loop in `loop()` behind a debug command | Reset within timeout; boot logs `reset=DOG` + breadcrumb | +| T4 **[R1]** | (a) BUSY held asserted through a refresh; (b) non-returning call inside one `waitforrefresh` iteration | (a) **no** reset — `waitforrefresh` returns false and `loop()` resumes; (b) reset. Distinguishing these is the point | +| T5 | Injected wedge in the SPIM path (transfer with SPIM disabled) | Reset with `breadcrumb=STREAM`/`INIT_SEQ` — the target case | +| T6 | Long `idleDelay` at max `sleep_timeout_ms`, battery | No reset — validates `CONFIG.SLEEP=1` + the idle feed | +| T7 **[R1]** | Force 3 fast consecutive `DOG` resets; then let the device run >10 min. Log `RUNSTATUS` at every boot | Safe mode entered, device advertises and is DFU-reachable; counter clears after the healthy window; next reset boots normally. Also confirms (a) GPREGRET2 survives a `DOG` reset and the bootloader does not clobber it (**V6**), and (b) `RUNSTATUS` reads *not running* at boot — i.e. the bootloader did not leave a WDT armed that would silently override our config (W-1) | +| T8 | Debugger breakpoint held >5 min | No reset — validates `CONFIG.HALT=0` | +| T9 **[R1]** | Enter DFU and idle in the bootloader >5 min | **Outcome unknown — that is the point of the test.** If the bootloader feeds the WDT, no reset and the **D-7** residual is void. If it does not, a reset is expected; confirm the device re-enters DFU rather than bricking. Either way, record the result against §5 | +| T10 | `bootdiag`, no USB host, left >5 min | No reset — the Serial gate must stay outside coverage | + +## 8. Files touched + +| File | Change | +|---|---| +| `src/watchdog.h` | **new** — portable interface, five free functions (W-0). No vendor headers | +| `src/watchdog_nrf.cpp` | **new** — `#ifdef TARGET_NRF`. HAL config, feed, `readResetReason()` decode, GPREGRET2 bit-allocated counter + breadcrumb, safe-mode state | +| `src/watchdog_esp32.cpp` | **new** — `#ifdef TARGET_ESP32`. Real boot decode relocated from `main.cpp`; accurate `Arm()` log; empty feed/breadcrumb (W-6) | +| `src/main.cpp` | feed at `loop()` top and in `idleDelay`; `odWatchdogBootInit()` + `odWatchdogArm()` before the boot panel path; gate `initDisplay()` on `odWatchdogInSafeMode()`. **Net `#ifdef` reduction** — `resetReasonName()` and its call site move out | +| `src/display_service.cpp` | feed in `waitforrefresh` **and before all 15 bb_epaper entry points** (W-2); breadcrumb stamps. No `#ifdef`s | +| `platformio.ini` | `-DOPENDISPLAY_NRF_WDT_S=300` in `[env:nrf52840custom]`; three other nRF envs inherit | +| `docs/TIMER_AND_WATCHDOG_INVENTORY_2026-07-26.md` | §1.1/§1.2 are **wrong** per **V1** — the ESP32 TWDT is enabled, just not watching `loop()`. Correct both | +| `docs/PLAN_PHASE2_BOUND_WAITS_2026-07-26.md` | note D-L/D-K residuals are now recoverable (not fixed) | + +## 9. Sequencing + +1. **W-0 + W-4 + W-6 — module and observability, nothing armed** (**D-5**). Create the header + and both implementations; relocate the ESP32 decode; add the nRF `readResetReason()` decode + and the GPREGRET2 breadcrumb; wire all feed sites so they exist and compile while `Arm()` is + inert. Zero brick risk. **Independently valuable**: answers whether field units are resetting + or hanging, and lands the `main.cpp` `#ifdef` cleanup regardless of the rest. +2. **T2 first, then W-1 + W-2** — measure the real worst-case refresh *before* arming, since + §3.2's margin is only 1.25×. Then arm at 300 s. +3. **W-3** — arm-before-boot-panel-path, GPREGRET2 bit allocation, strike counter with the + uptime-based clear, safe mode. + +Steps 2 and 3 must land together: step 2 alone is the configuration with the boot-loop hazard +W-3 exists to contain. + +## 10. Review corrections (rev 1 → rev 2) + +Rev 1 was reviewed adversarially; findings were re-verified against source before acceptance. + +**Blocking errors, all confirmed:** +1. **V4** — `RESETREAS` is cleared by the core before `setup()`; rev 1's SoftDevice-API design + would have read zero forever and reported every watchdog reset as a power-on. +2. **V12 / §3.2** — worst case is 240 s (4 `BUSY_WAIT` × 2 controllers × 30 s), not the assumed + ≤90 s. Rev 1's ">3× headroom" was wrong; 300 s is only safe with W-2's pre-call feed. +3. **W-3** — rev 1's containment was self-contradictory in four distinct ways (§W-3); redesigned. +4. **V13 / D-7** — DFU jumps to the bootloader without a reset, so rev 1's T9 ("the system reset + clears the WDT") was factually wrong. Now an accepted residual. + +**Corrected but non-blocking:** V1 (ESP32 TWDT *is* enabled — conclusion held, reason wrong), +V2/V3 (imprecise inventory), V6 (bootloader not inspected), V7 (`board_build.ldscript` exists), +V8 (LFCLK already started before `setup()`), W-1 (flag needs validation), W-2 (property +statement was false; stuck-BUSY behaviour clarified), W-6 (stub message would have been false). + +**Verified correct and unchanged:** V5, V9, V10, V11, and §3.1's ~126 s `waitforrefresh` +arithmetic. diff --git a/platformio.ini b/platformio.ini index 41bc6ea..10984be 100644 --- a/platformio.ini +++ b/platformio.ini @@ -84,6 +84,22 @@ build_flags = ; build. If it silently stops doing so we are back on SPIM3 with no error. Check ; with: pio run -e nrf52840custom -v 2>&1 | grep -c 'SPI_32MHZ_INTERFACE=1.*SPI\.cpp' -DSPI_32MHZ_INTERFACE=1 + ; Hardware watchdog timeout, seconds. 0 disables; valid range is 60..3600 + ; (enforced by a static_assert in src/watchdog_nrf.cpp). + ; + ; 300 s is NOT a comfortable margin over normal operation -- it is sized against + ; the single longest span the firmware cannot instrument: a REFRESH_FULL on a + ; 7-colour split-buffer panel sends the init sequence to BOTH controllers, and + ; each of its 4 BUSY_WAIT entries can take 30 s, i.e. ~240 s inside one + ; bbepRefresh() call. What makes 300 s safe is not the number but the feed + ; immediately before every bb_epaper entry point in display_service.cpp, so the + ; watchdog faces that one call rather than that call plus everything preceding + ; it. Margin is therefore ~1.25x, and any growth in BUSY_WAIT count, controller + ; count or the multicolour cap eats into it directly -- re-check when adding a + ; panel. See docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md sections 3.2 and W-2. + ; + ; The three other nRF envs inherit this via ${env:nrf52840custom.build_flags}. + -DOPENDISPLAY_NRF_WDT_S=300 platform = https://github.com/maxgerhardt/platform-nordicnrf52 framework = arduino board_build.variants_dir = variants @@ -154,7 +170,7 @@ lib_deps = bitbank2/FastEPD@^2.2.0 build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + -DOPENDISPLAY_ENABLE_WIFI ; LAN transport + ROM tinfl. Set ONLY on -DBOARD_HAS_PSRAM envs (src/wifi_service.h) #-DOPENDISPLAY_ZLIB_WINDOW_BITS=15 -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DBOARD_HAS_PSRAM @@ -184,7 +200,7 @@ lib_deps = bitbank2/FastEPD@^2.2.0 build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + -DOPENDISPLAY_ENABLE_WIFI ; LAN transport + ROM tinfl. Set ONLY on -DBOARD_HAS_PSRAM envs (src/wifi_service.h) #-DOPENDISPLAY_ZLIB_WINDOW_BITS=15 -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DBOARD_HAS_PSRAM @@ -213,7 +229,7 @@ lib_deps = bitbank2/FastEPD@^2.2.0 build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + -DOPENDISPLAY_ENABLE_WIFI ; LAN transport + ROM tinfl. Set ONLY on -DBOARD_HAS_PSRAM envs (src/wifi_service.h) #-DOPENDISPLAY_ZLIB_WINDOW_BITS=15 -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DBOARD_HAS_PSRAM @@ -242,7 +258,7 @@ platform = https://github.com/pioarduino/platform-espressif32/releases/download/ framework = arduino build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + -DOPENDISPLAY_ENABLE_WIFI ; LAN transport + ROM tinfl. Set ONLY on -DBOARD_HAS_PSRAM envs (src/wifi_service.h) -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DBOARD_HAS_PSRAM -DARDUINO_USB_MODE=0 @@ -291,7 +307,8 @@ platform = https://github.com/pioarduino/platform-espressif32/releases/download/ framework = arduino build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + ; -DOPENDISPLAY_ENABLE_WIFI deliberately absent: no PSRAM here (see src/wifi_service.h). + ; BLE-only, and the inflate engine falls back to uzlib. #-DOPENDISPLAY_ZLIB_WINDOW_BITS=15 -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DARDUINO_USB_MODE=1 @@ -311,7 +328,8 @@ extra_scripts = post:scripts/esp32c6_nimble_mempool_link.py build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + ; -DOPENDISPLAY_ENABLE_WIFI deliberately absent: no PSRAM here (see src/wifi_service.h). + ; BLE-only, and the inflate engine falls back to uzlib. #-DOPENDISPLAY_ZLIB_WINDOW_BITS=15 -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DARDUINO_USB_MODE=1 @@ -331,7 +349,8 @@ platform = https://github.com/pioarduino/platform-espressif32/releases/download/ framework = arduino build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + ; -DOPENDISPLAY_ENABLE_WIFI deliberately absent: no PSRAM here (see src/wifi_service.h). + ; BLE-only, and the inflate engine falls back to uzlib. #-DOPENDISPLAY_ZLIB_WINDOW_BITS=15 -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DARDUINO_USB_MODE=1 @@ -355,7 +374,7 @@ lib_deps = bitbank2/FastEPD@^2.2.0 build_flags = -DTARGET_ESP32 - -DOPENDISPLAY_ENABLE_WIFI ; WiFi/LAN transport: S3 + C6 + C3 (classic ESP32 excluded) + -DOPENDISPLAY_ENABLE_WIFI ; LAN transport + ROM tinfl. Set ONLY on -DBOARD_HAS_PSRAM envs (src/wifi_service.h) -DOPENDISPLAY_ZLIB_USE_HEAP_WINDOW=1 -DBOARD_HAS_PSRAM -DARDUINO_USB_MODE=1 diff --git a/src/boot_screen.cpp b/src/boot_screen.cpp index 32608f6..20f974e 100644 --- a/src/boot_screen.cpp +++ b/src/boot_screen.cpp @@ -1,4 +1,5 @@ #include "boot_screen.h" +#include "watchdog.h" #include #include #include @@ -510,6 +511,7 @@ static bool bootLayoutFit(uint16_t w, uint16_t h, uint16_t h_full, int blockH, i // and stream it. planeRow scratch lives just past the 2bpp row in staticRowBuffer // (200B + 100B for an 800px panel, well within the 680B buffer). static void writeGray4PlaneRow(const uint8_t* row2bpp, int pitch2bpp, int planePitch, uint16_t w, int bitSel) { + odWatchdogBreadcrumb(OD_WDT_PHASE_STREAM); uint8_t* planeRow = staticRowBuffer + pitch2bpp; memset(planeRow, 0x00, planePitch); for (uint16_t x = 0; x < w; x++) { @@ -1031,6 +1033,7 @@ bool writeBootScreenWithQr() { } else if (e1004Stream) { e1004_write_stream_bytes(row + (size_t)halfPass * e1004HalfPitch, e1004HalfPitch); } else { + odWatchdogBreadcrumb(OD_WDT_PHASE_STREAM); bbepWriteData(&bbep, row, pitch); } #else @@ -1039,6 +1042,7 @@ bool writeBootScreenWithQr() { } else if (e1004Stream) { e1004_write_stream_bytes(row + (size_t)halfPass * e1004HalfPitch, e1004HalfPitch); } else { + odWatchdogBreadcrumb(OD_WDT_PHASE_STREAM); bbepWriteData(&bbep, row, pitch); } #endif @@ -1058,6 +1062,7 @@ bool writeBootScreenWithQr() { memset(row, 0x00, pitch); bbepSetAddrWindow(&bbep, 0, 0, w, h); bbepStartWrite(&bbep, PLANE_1); + odWatchdogBreadcrumb(OD_WDT_PHASE_STREAM); for (uint16_t y = 0; y < h; y++) bbepWriteData(&bbep, row, pitch); } } diff --git a/src/display_service.cpp b/src/display_service.cpp index 23b637f..348c8e1 100644 --- a/src/display_service.cpp +++ b/src/display_service.cpp @@ -15,6 +15,7 @@ #include "link_owner.h" #include "session_guard.h" #include "touch_input.h" +#include "watchdog.h" #include "uzlib.h" #if defined(TARGET_ESP32) && defined(OPENDISPLAY_FASTEPD) #include "display_fastepd.h" @@ -203,6 +204,8 @@ static void prepareEpdRailForBoot() { static void e1004InitPanel(void) { const DisplayConfig& d = globalConfig.displays[0]; bbepSetCS2(&bbep, e1004_cs2_pin()); + odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + odWatchdogFeed(); // bbepInitIO sends pInitFull internally (~240 s worst case) bbepInitIO(&bbep, d.dc_pin, d.reset_pin, d.busy_pin, d.cs_pin, d.data_pin, d.clk_pin, 8000000); } @@ -361,8 +364,12 @@ static void initBbepPanelSession() { return; } #endif + odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + odWatchdogFeed(); // bbepInitIO sends pInitFull internally (~240 s worst case) bbepInitIO(&bbep, d.dc_pin, d.reset_pin, d.busy_pin, d.cs_pin, d.data_pin, d.clk_pin, 8000000); + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepWakeUp(&bbep); + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepSendCMDSequence(&bbep, bbep.pInitFull); epdAlignCustomPartialRamMode(); delay(200); @@ -439,6 +446,7 @@ static void pwrmgmLockGive(void) { // power off without re-taking the non-recursive lock. static void epdSessionForceOffLocked(void) { if (pwrmgmState == PWR_OFF) return; // idempotent + odWatchdogBreadcrumb(OD_WDT_PHASE_FORCE_OFF); od_log_info("[EPD session] force off"); if (epdSessionUsesFastepd()) { #if defined(TARGET_ESP32) && defined(OPENDISPLAY_FASTEPD) @@ -448,19 +456,39 @@ static void epdSessionForceOffLocked(void) { fastepd_mark_hw_deinitialized(); #endif } else { + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepSleep(&bbep, 1); delay(50); } pwrmgm(false); // -> PWR_OFF, clears deadline epdPlanesPrepared = false; + // Panel work is finished. Without this, a later wedge in BLE/WiFi/command + // handling would boot reporting breadcrumb=FORCE_OFF and point the next + // investigation at the panel teardown that had actually already completed. + odWatchdogBreadcrumb(OD_WDT_PHASE_IDLE); } // Bring the panel up for a transfer/refresh. Returns true iff it was COLD (rail // was off) — callers may need to (re)open the address window regardless. static bool epdSessionAcquire(bool partialInit) { + // Safe mode: three consecutive watchdog resets say the panel path is what + // keeps wedging, so refuse to drive it at all. Skipping initDisplay() at boot + // is NOT sufficient on its own -- a client can connect and push an image, and + // that reaches here directly. + // + // Refusing leaves the session in PWR_OFF, so the transfer fails through the + // existing refresh-failure path rather than through new error handling. It is + // reported late and generically; a clean immediate NACK needs a "device in + // safe mode" code, which must originate in ../opendisplay-protocol and is out + // of scope for this branch. + if (odWatchdogInSafeMode()) { + od_log_warn("[EPD session] acquire REFUSED - watchdog safe mode"); + return false; + } pwrmgmLockTake(); bool cold; if (pwrmgmState == PWR_OFF) { + odWatchdogBreadcrumb(OD_WDT_PHASE_ACQUIRE_COLD); od_log_info("[EPD session] acquire: COLD bring-up"); pwrmgm(true); // -> PWR_ACTIVE (guarded; real transition) if (!epdSessionUsesFastepd()) { @@ -472,10 +500,14 @@ static bool epdSessionAcquire(bool partialInit) { } else #endif { + odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + odWatchdogFeed(); // bbepInitIO sends pInitFull internally (~240 s worst case) bbepInitIO(&bbep, d.dc_pin, d.reset_pin, d.busy_pin, d.cs_pin, d.data_pin, d.clk_pin, 8000000); + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepWakeUp(&bbep); const uint8_t* initSeq = partialInit ? (bbep.pInitPart ? bbep.pInitPart : bbep.pInitFull) : bbep.pInitFull; + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepSendCMDSequence(&bbep, initSeq); epdAlignCustomPartialRamMode(); epdSessionInitWasPartial = partialInit; @@ -484,6 +516,7 @@ static bool epdSessionAcquire(bool partialInit) { cold = true; } else { // WARM re-acquire (or, defensively, an already-ACTIVE re-entry). + odWatchdogBreadcrumb(OD_WDT_PHASE_ACQUIRE_WARM); od_log_info(pwrmgmState == PWR_ACTIVE ? "[EPD session] acquire: already ACTIVE (defensive)" : "[EPD session] acquire: WARM re-acquire"); pwrmgmState = PWR_ACTIVE; @@ -497,9 +530,12 @@ static bool epdSessionAcquire(bool partialInit) { } else #endif { + odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepWakeUp(&bbep); const uint8_t* initSeq = partialInit ? (bbep.pInitPart ? bbep.pInitPart : bbep.pInitFull) : bbep.pInitFull; + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepSendCMDSequence(&bbep, initSeq); epdAlignCustomPartialRamMode(); epdSessionInitWasPartial = partialInit; @@ -517,6 +553,7 @@ static bool epdSessionAcquire(bool partialInit) { static void epdSessionRelease(bool refreshSuccess) { pwrmgmLockTake(); if (pwrmgmState == PWR_OFF) { pwrmgmLockGive(); return; } // nothing to release + odWatchdogBreadcrumb(OD_WDT_PHASE_RELEASE); uint32_t window = epdKeepAliveWindowMs(); if (window == 0 || !refreshSuccess) { od_log_info(refreshSuccess ? "[EPD session] release: keep-alive disabled, powering off" @@ -528,6 +565,7 @@ static void epdSessionRelease(bool refreshSuccess) { // Controller stays AWAKE (no bbepSleep; is_awake stays 1); rail/SPI stay up. od_log_info("[EPD session] release: panel warm-idle, off in %u ms", (unsigned)window); } + odWatchdogBreadcrumb(OD_WDT_PHASE_IDLE); // see the note in ForceOffLocked pwrmgmLockGive(); } @@ -557,8 +595,10 @@ static bool refreshBootScreenFull() { od_log_warn("Boot screen render failed"); return false; } + odWatchdogBreadcrumb(OD_WDT_PHASE_BOOT_REFRESH); od_log_info("EPD refresh: FULL (boot)"); touchSuspendForEpdRefresh(); + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepRefresh(&bbep, REFRESH_FULL); return waitforrefresh(60); } @@ -850,11 +890,21 @@ bool waitforrefresh(int timeout){ od_log_info("Refresh completed inside bb_epaper"); return true; } + odWatchdogBreadcrumb(OD_WDT_PHASE_REFRESH_WAIT); // Poll at 10 ms (was 100 ms) so a ~0.5 s refresh returns up to ~90 ms sooner. // BUSY asserts within µs of MASTER_ACTIVATE, so the i==0 "never went busy" // error check stays valid at a 10 ms first poll. Loop bound scales x10 // (timeout*100 iterations of 10 ms); dot cadence every 50 iters keeps ~0.5 s/dot. for (size_t i = 0; i < (size_t)(timeout * 100); i++){ + // A refresh legitimately runs for a long time here -- the loop bound is + // ~126 s, NOT `timeout` seconds, because each iteration costs delay(10) + // plus bbepIsBusy()'s own delay(10)+delay(1). Feeding per iteration is + // what keeps that healthy wait from being mistaken for a wedge. + // + // This does NOT create a blind spot for a stuck BUSY line: that case keeps + // iterating, exhausts the bound, returns false, and loop() resumes -- a + // failed refresh is an error to report, not a hang to reset through. + odWatchdogFeed(); delay(10); if(i % 50 == 0) od_log_raw("."); if(!bbepIsBusy(&bbep)){ @@ -2045,6 +2095,9 @@ static inline bool directWriteIsGray4(void) { // is the running total across both planes, so the compressed and uncompressed paths // share this one plane-split implementation. static void streamGray4Bytes(const uint8_t* buf, uint32_t len) { + // Panel data path. Repeats are filtered inside odWatchdogBreadcrumb(), so a + // per-call stamp here costs one comparison. + odWatchdogBreadcrumb(OD_WDT_PHASE_STREAM); const uint32_t planeBytes = (((uint32_t)directWriteWidth + 7u) / 8u) * directWriteHeight; uint32_t off = 0; while (off < len && directWriteBytesWritten < 2u * planeBytes) { @@ -2065,6 +2118,9 @@ static void streamGray4Bytes(const uint8_t* buf, uint32_t len) { } static void directWriteSinkBytes(uint8_t* data, uint32_t len) { + // Panel data path. Repeats are filtered inside odWatchdogBreadcrumb(), so a + // per-call stamp here costs one comparison. + odWatchdogBreadcrumb(OD_WDT_PHASE_STREAM); #ifdef BBEP_T133A01 if (e1004_panel_used()) { if (e1004GeometryOk) e1004_sink_bytes(data, len); @@ -2500,6 +2556,7 @@ static void directWriteFinishAndRefresh(uint8_t* data, uint16_t len, uint8_t end #ifdef BBEP_T133A01 if (e1004_panel_used()) e1004_end_plane(); #endif + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepRefresh(&bbep, refreshMode); refreshSuccess = waitforrefresh(60); // No bbepSleep here: cleanupDirectWriteState(false) releases the session, @@ -3228,6 +3285,8 @@ static void partial_set_addr_window(BBEPDISP *pBBEP, int x, int y, int cx, int c } static bool partial_consume_bytes(uint8_t* data, uint32_t len) { + // Per-frame, but repeats are filtered inside odWatchdogBreadcrumb(). + odWatchdogBreadcrumb(OD_WDT_PHASE_STREAM); if (partialCtx.compressed) { if (len > UINT32_MAX - partialCtx.bytes_received) return false; } else { @@ -3355,6 +3414,7 @@ static bool partial_trigger_refresh(int refreshMode) { bbepWriteCmd(&bbep, SSD1608_MASTER_ACTIVATE); return waitforrefresh(60); } + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepRefresh(&bbep, refreshMode); return waitforrefresh(60); } @@ -3383,7 +3443,10 @@ static void partial_prepare_panel_ram(void) { partialCtx.width == globalConfig.displays[0].pixel_width && partialCtx.height == globalConfig.displays[0].pixel_height; if (!fullFrame) { + odWatchdogBreadcrumb(OD_WDT_PHASE_FILL); + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepFill(&bbep, BBEP_WHITE, PLANE_1); + odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepFill(&bbep, BBEP_WHITE, PLANE_0); od_log_debug("[+%ums] after fills (ran: sub-rect)", (unsigned)(millis() - t0)); } else { diff --git a/src/main.cpp b/src/main.cpp index 3352ae0..9167824 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -12,6 +12,7 @@ #include "link_owner.h" #include "session_guard.h" #include "od_log.h" +#include "watchdog.h" #if defined(TARGET_ESP32) && defined(OPENDISPLAY_LOG_UART) #include @@ -25,24 +26,10 @@ static HardwareSerial LogSerialPort(1); #endif #ifdef TARGET_ESP32 -// Distinguishes a hidden mid-cycle reset (PANIC/WDT/BROWNOUT/SW) from a real -// power-on or deep-sleep wake; any reset here clears the wake cause, so the -// next boot takes the NORMAL BOOT branch and redraws the boot screen. -static const char* resetReasonName(esp_reset_reason_t reason) { - switch (reason) { - case ESP_RST_POWERON: return "POWERON"; - case ESP_RST_EXT: return "EXT"; - case ESP_RST_SW: return "SW"; - case ESP_RST_PANIC: return "PANIC"; - case ESP_RST_INT_WDT: return "INT_WDT"; - case ESP_RST_TASK_WDT: return "TASK_WDT"; - case ESP_RST_WDT: return "WDT"; - case ESP_RST_DEEPSLEEP: return "DEEPSLEEP"; - case ESP_RST_BROWNOUT: return "BROWNOUT"; - case ESP_RST_SDIO: return "SDIO"; - default: return "UNKNOWN"; - } -} +// resetReasonName() used to live here. It moved to watchdog_esp32.cpp so both +// targets reach their reset-reason decode through one portable call +// (odWatchdogBootInit), rather than ESP32 having an inline #ifdef block and nRF +// having nothing at all. See src/watchdog.h. // Defined with the sleep helpers below loop()'s activity poller; setup() logs // the window length when arming the button-wake hold. @@ -128,10 +115,12 @@ void setup() { // Set only by the ESP32 wake-cause check below; NRF has no deep-sleep wake path. bool is_deep_sleep_wake = false; bool woke_by_button = false; + // Decode why we booted, on BOTH targets. On nRF this also reads the retained + // breadcrumb, so a watchdog reset can name the panel phase that wedged. Must + // run after od_log_init() (above) or the line is emitted into a dark port, and + // before odWatchdogArm(). + odWatchdogBootInit(); #ifdef TARGET_ESP32 - esp_reset_reason_t reset_reason = esp_reset_reason(); - const char* resetReasonStr = resetReasonName(reset_reason); - od_log_info("Reset reason: %s (%d)", resetReasonStr, (int)reset_reason); esp_sleep_wakeup_cause_t wakeup_reason = esp_sleep_get_wakeup_cause(); is_deep_sleep_wake = (wakeup_reason != ESP_SLEEP_WAKEUP_UNDEFINED); if (is_deep_sleep_wake) { @@ -198,7 +187,23 @@ void setup() { #endif } #endif - if (!is_deep_sleep_wake) { + // Arm the hardware watchdog immediately BEFORE the boot panel path, so a wedge + // inside initDisplay() is itself covered -- that is what makes the strike + // counter meaningful (W-3). Nothing earlier may block longer than the timeout; + // the bootdiag `while (!Serial)` gate sits far above this point and stays + // deliberately outside coverage. + // + // Inert until step 2 of the plan arms it; it logs the target's watchdog status + // either way, which is the only place the ESP32 TWDT gap is reported. + odWatchdogArm(); + if (odWatchdogInSafeMode()) { + // Three consecutive watchdog resets: the panel path is what keeps wedging, + // so skip it entirely this boot. BLE still comes up below, which is the + // whole point -- a device in safe mode stays reachable for a config change + // or a DFU instead of being bricked until someone pulls the battery. + od_log_warn("[WDT] safe mode - skipping initDisplay()"); + rebootFlag = 1; + } else if (!is_deep_sleep_wake) { // Arm here rather than at declaration: this branch is the boot screen // redraw, and every real reset (power-on, panic, WDT, SW) clears the // wake cause and lands here. A deep-sleep wake skips it and keeps the @@ -891,6 +896,10 @@ static void platformIdle() { // One loop body for both targets. The per-target policy that genuinely differs // lives in the two hooks above; everything here is shared. void loop() { + // The primary liveness proof: reaching the top of loop() is what "the program + // is still making progress" means. Every other feed site exists only to keep a + // LEGITIMATE long wait from looking like a wedge. + odWatchdogFeed(); serviceBleEvents(); processLedFlash(); epdSessionTick(); // millis()-poll: power the panel down screen_timeout_seconds after last release @@ -1029,6 +1038,10 @@ void idleDelay(uint32_t delayMs) { const uint32_t CHECK_INTERVAL_MS = 100; uint32_t remainingDelay = delayMs; while (remainingDelay > 0) { + // A long idle wait is healthy, not a wedge. Fed every chunk (<=100 ms), + // which is also what makes WDT CONFIG.SLEEP=1 safe: the CPU sleeps inside + // delay() below, and the watchdog keeps counting through it. + odWatchdogFeed(); ble.tick(); // no-op on ESP32 processButtonEvents(); processTouchInput(); @@ -1216,6 +1229,12 @@ static void configureDisplayPinsLowPower() { } void pwrmgm(bool onoff){ + // Never bring the panel rail up in watchdog safe mode. Powering down is still + // allowed, so a rail left on by a pre-safe-mode boot can still be shut off. + if(onoff && odWatchdogInSafeMode()){ + od_log_warn("Panel power-up refused - watchdog safe mode"); + return; + } if(globalConfig.display_count == 0){ od_log_warn("No display configured"); return; diff --git a/src/watchdog.h b/src/watchdog.h new file mode 100644 index 0000000..64f788c --- /dev/null +++ b/src/watchdog.h @@ -0,0 +1,85 @@ +#pragma once + +#include + +// Portable watchdog + boot-reason interface. +// +// Deliberately includes NO vendor headers -- not nrf_wdt.h, not esp_task_wdt.h -- +// so any translation unit can call it without dragging in an SDK. Exactly one +// implementation is linked per build (watchdog_nrf.cpp or watchdog_esp32.cpp), +// each gated whole-file on its target, so the other compiles to an empty +// translation unit and no build_src_filter change is needed. This mirrors +// BleTransport; see src/ble_transport.h for the same reasoning stated at length. +// +// Free functions rather than a class: unlike BleTransport there is no state worth +// exposing, and every caller wants exactly one global watchdog. +// +// WHY THIS IS PORTABLE WHEN THE WATCHDOG IS NOT +// --------------------------------------------- +// Only nRF arms a hardware watchdog today. But every feed site and every +// breadcrumb stamp lives in code that compiles for BOTH targets -- loop() and +// idleDelay() in main.cpp ("One loop body for both targets"), waitforrefresh() +// and the bb_epaper entry points in display_service.cpp. An nRF-only API would +// mean #ifdef TARGET_NRF around ~20 call sites in shared files, which is what +// docs/PLAN_UNIFY_NRF_ESP32_LOOP_BLE_2026-07-27.md worked to remove. A no-op on +// ESP32 costs nothing and keeps the shared body clean. +// +// WHAT IS DELIBERATELY ABSENT +// --------------------------- +// - stop()/disable(): the nRF52840 WDT CANNOT be stopped once started. Its +// register block has no TASKS_STOP and no ENABLE (contrast SPIM/TWI/UART, +// which have ENABLE at 0x500); only a system reset clears it. Exposing a +// stop() would advertise a capability one target cannot honour. +// - a runtime timeout parameter: CRV must be written BEFORE the start task and +// is latched thereafter. Compile-time only, via OPENDISPLAY_NRF_WDT_S. +// - task registration: ESP32's task watchdog is per-task subscribe/unsubscribe; +// nRF's reload registers are not an analogue. The contract here is ONE LOOP +// TASK, ONE WATCHDOG. +// +// See docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md. + +// --- breadcrumb phases ----------------------------------------------------- +// Stamped at panel-phase transitions and retained across a reset, so the boot +// after a watchdog reset can name the wait that wedged instead of reporting only +// that something did. Values must fit 4 bits (0-15); see the GPREGRET2 layout in +// watchdog_nrf.cpp. +enum OdWatchdogPhase : uint8_t { + OD_WDT_PHASE_IDLE = 0, + OD_WDT_PHASE_ACQUIRE_COLD = 1, + OD_WDT_PHASE_ACQUIRE_WARM = 2, + OD_WDT_PHASE_INIT_SEQ = 3, + OD_WDT_PHASE_FILL = 4, + OD_WDT_PHASE_STREAM = 5, + OD_WDT_PHASE_REFRESH_WAIT = 6, + OD_WDT_PHASE_RELEASE = 7, + OD_WDT_PHASE_FORCE_OFF = 8, + OD_WDT_PHASE_BOOT_REFRESH = 9, + OD_WDT_PHASE__MAX = 15, +}; + +// Decode and log why we booted, and evaluate the consecutive-reset strike +// counter. Call ONCE, early in setup(), after od_log_init() so the line is +// actually emitted. Must run before odWatchdogArm(). +void odWatchdogBootInit(void); + +// True when the strike counter tripped and this boot must skip all panel work. +// Always false until W-3 lands. +bool odWatchdogInSafeMode(void); + +// Arm the hardware watchdog. Irreversible on nRF. Call ONCE, after +// odWatchdogBootInit() and before the boot panel path. No-op where no hardware +// watchdog is used. +void odWatchdogArm(void); + +// Prove forward progress. Cheap enough to call in a tight poll loop; a no-op +// when nothing is armed. +// +// FEED ONLY FROM SITES WHOSE EXECUTION PROVES THE PROGRAM IS ALIVE -- loop(), +// cooperative waits that return, and immediately before entering a bounded +// library call. NEVER from an ISR, timer or stack callback: an interrupt-fed +// watchdog verifies that the interrupt controller is running, not that the +// program is, which is the classic way to build a watchdog that never fires. +void odWatchdogFeed(void); + +// Record the panel phase we are about to enter. Retained across reset. +void odWatchdogBreadcrumb(uint8_t phase); diff --git a/src/watchdog_esp32.cpp b/src/watchdog_esp32.cpp new file mode 100644 index 0000000..50fff92 --- /dev/null +++ b/src/watchdog_esp32.cpp @@ -0,0 +1,81 @@ +// ESP32 implementation of the portable watchdog interface. +// +// The whole file is gated on TARGET_ESP32, so an nRF build compiles it to an +// empty translation unit -- no build_src_filter changes needed across the CI +// environments. +// +// Only odWatchdogBootInit() does real work here: it is the reset-reason decode +// that used to live inline in main.cpp under #ifdef TARGET_ESP32, moved out so +// both targets reach it through one portable call. Arming is a stub. +// +// See docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md (W-6). + +#include "watchdog.h" + +#ifdef TARGET_ESP32 + +#include +#include +#include "od_log.h" + +// Distinguishes a hidden mid-cycle reset (PANIC/WDT/BROWNOUT/SW) from a real +// power-on or deep-sleep wake; any reset here clears the wake cause, so the +// next boot takes the NORMAL BOOT branch and redraws the boot screen. +static const char* resetReasonName(esp_reset_reason_t reason) { + switch (reason) { + case ESP_RST_POWERON: return "POWERON"; + case ESP_RST_EXT: return "EXT"; + case ESP_RST_SW: return "SW"; + case ESP_RST_PANIC: return "PANIC"; + case ESP_RST_INT_WDT: return "INT_WDT"; + case ESP_RST_TASK_WDT: return "TASK_WDT"; + case ESP_RST_WDT: return "WDT"; + case ESP_RST_DEEPSLEEP: return "DEEPSLEEP"; + case ESP_RST_BROWNOUT: return "BROWNOUT"; + case ESP_RST_SDIO: return "SDIO"; + default: return "UNKNOWN"; + } +} + +void odWatchdogBootInit(void) { + esp_reset_reason_t r = esp_reset_reason(); + od_log_info("Reset reason: %s (%d)", resetReasonName(r), (int)r); +} + +bool odWatchdogInSafeMode(void) { + return false; +} + +void odWatchdogArm(void) { + // No hardware watchdog is armed by this firmware on ESP32. + // + // Be precise about what that does and does not mean, because the obvious + // wording ("no watchdog on this target") is FALSE and would mislead whoever + // reads the log next. The IDF task watchdog IS enabled and initialised in the + // shipped sdkconfigs -- CONFIG_ESP_TASK_WDT_EN=y, CONFIG_ESP_TASK_WDT_INIT=y, + // CONFIG_ESP_TASK_WDT_TIMEOUT_S=5 -- but nothing it watches would catch a + // wedged loop(): + // + // - S3 and classic ESP32 set CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU0=y, + // while CONFIG_ARDUINO_RUNNING_CORE=1 puts loopTask on CPU1. A spin in + // loop() starves IDLE1, which nothing is subscribed to. + // - C3 and C6 initialise the task watchdog but subscribe no idle task at all. + // + // Closing this is a one-liner -- esp_task_wdt_add(NULL) on the loop task -- and + // the feed sites it would need are already wired by this module. Deliberately + // not done here: the timeout would be bounded by different spans than nRF's + // (FastEPD refresh, WiFi/TLS handshakes) and needs its own analysis. + // + // Logged once, at boot, so the gap is explicit rather than assumed. + od_log_info("[WDT] no firmware watchdog on ESP32; IDF TWDT is enabled but no " + "task that would catch a wedged loop() is subscribed"); +} + +void odWatchdogFeed(void) { +} + +void odWatchdogBreadcrumb(uint8_t phase) { + (void)phase; +} + +#endif // TARGET_ESP32 diff --git a/src/watchdog_nrf.cpp b/src/watchdog_nrf.cpp new file mode 100644 index 0000000..a00cae4 --- /dev/null +++ b/src/watchdog_nrf.cpp @@ -0,0 +1,386 @@ +// nRF52840 implementation of the portable watchdog interface. +// +// The whole file is gated on TARGET_NRF, so an ESP32 build compiles it to an +// empty translation unit -- no build_src_filter changes needed across the CI +// environments. Nothing outside this file names an nRF WDT type. +// +// See docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md. + +#include "watchdog.h" + +#ifdef TARGET_NRF + +#include +#include +#include +#include +#include +#include "od_log.h" + +// --------------------------------------------------------------------------- +// Reset reason +// --------------------------------------------------------------------------- +// +// CRITICAL: do NOT read NRF_POWER->RESETREAS here, and do NOT call +// sd_power_reset_reason_get(). The Arduino core has ALREADY read and cleared the +// register before setup() runs: +// +// cores/nRF5/wiring.c:37-40 +// _reset_reason = NRF_POWER->RESETREAS; +// NRF_POWER->RESETREAS |= NRF_POWER->RESETREAS; // write-1-to-clear +// +// and exposes the saved word through readResetReason() (wiring.h:32). Reading the +// peripheral ourselves returns zero on every boot, which would silently report +// every watchdog reset as a power-on -- defeating the entire point of this module. +// +// It also means the bits do NOT accumulate across boots: the core clears them each +// time, so no clearing is needed or possible on our side. + +static uint32_t s_resetReason = 0; + +static void logResetReason(uint32_t r) { + // RESETREAS is a bitfield, not an enum: several causes can be latched at once. + // Print every set bit rather than the first match. + // + // ALL eight nRF52840 causes are listed. An earlier revision omitted VBUS, NFC + // and LPCOMP, which made any of them print as "POWERON" because the fallback + // keyed on "nothing matched" rather than on r == 0. + static const struct { uint32_t msk; const char* name; } kinds[] = { + { POWER_RESETREAS_RESETPIN_Msk, "RESETPIN" }, + { POWER_RESETREAS_DOG_Msk, "DOG" }, + { POWER_RESETREAS_SREQ_Msk, "SREQ" }, + { POWER_RESETREAS_LOCKUP_Msk, "LOCKUP" }, + { POWER_RESETREAS_OFF_Msk, "OFF" }, + { POWER_RESETREAS_LPCOMP_Msk, "LPCOMP" }, + { POWER_RESETREAS_DIF_Msk, "DIF" }, + { POWER_RESETREAS_NFC_Msk, "NFC" }, + { POWER_RESETREAS_VBUS_Msk, "VBUS" }, + }; + if (r == 0) { + // A cold start latches nothing. This is the ONLY power-on signature. + od_log_info("[WDT] reset reason: POWERON (0x00000000)"); + return; + } + char buf[96]; + size_t n = 0; + uint32_t seen = 0; + buf[0] = '\0'; + for (unsigned i = 0; i < sizeof(kinds) / sizeof(kinds[0]); i++) { + if (!(r & kinds[i].msk)) continue; + seen |= kinds[i].msk; + int w = snprintf(buf + n, sizeof(buf) - n, "%s%s", n ? "|" : "", kinds[i].name); + // snprintf returns the length it WOULD have written; clamp so a truncating + // write cannot push n past the buffer. + if (w <= 0) break; + n += (size_t)w; + if (n >= sizeof(buf) - 1) { n = sizeof(buf) - 1; break; } + } + // Nonzero with no recognised bit is a real anomaly, not a power-on. Say so. + if (r & ~seen) { + snprintf(buf + n, sizeof(buf) - n, "%sUNKNOWN", n ? "|" : ""); + } + od_log_info("[WDT] reset reason: %s (0x%08lX)", buf, (unsigned long)r); +} + +// --------------------------------------------------------------------------- +// Retained state: GPREGRET2 +// --------------------------------------------------------------------------- +// +// GPREGRET (id 0) is NOT available -- device_control.cpp:868-869 uses it for the +// DFU handshake (0xB1). GPREGRET2 (id 1) is free and retained across a watchdog or +// soft reset (cleared only by power-on/brownout). +// +// It is 8 bits, and it has to carry two things, so the layout is explicit: +// +// bit 7 6 | 5 4 | 3 2 1 0 +// tag | cnt | phase +// +// 7:6 validity tag, always 0b10. Distinguishes a value we wrote from +// cold-boot garbage or another writer; a bad tag means "discard". +// 5:4 consecutive-DOG strike counter, 0-3, saturating (W-3, not yet used). +// 3:0 breadcrumb phase, OdWatchdogPhase. +// +// Without this allocation a breadcrumb write would destroy the strike counter. +// +// *** WHY THERE ARE TWO ACCESS PATHS *** +// +// sd_power_gpregret_{get,set,clr} are numbered from SOC_SVC_BASE_NOT_AVAILABLE +// (nrf_soc.h:65,164-166) -- "SVCs that are not available when the SoftDevice is +// disabled". odWatchdogBootInit() runs early in setup(), whereas ble.begin() +// enables the SoftDevice much later, so at boot those SVCs cannot be used. +// +// When the SoftDevice is DISABLED, POWER is not a protected peripheral and +// NRF_POWER->GPREGRET2 is directly readable/writable -- which is also cheaper and +// atomic (one store, versus a get + clr + set SVC sequence). +// When the SoftDevice is ENABLED, POWER is protected and the SVCs are mandatory. +// +// sd_softdevice_is_enabled() is numbered from SDM_SVC_BASE (nrf_sdm.h:89,195), +// which IS available either way, so it is a safe discriminator. + +#define OD_WDT_G2_TAG_MASK 0xC0u +#define OD_WDT_G2_TAG_VALUE 0x80u /* 0b10 << 6 */ +#define OD_WDT_G2_CNT_MASK 0x30u +#define OD_WDT_G2_CNT_SHIFT 4 +#define OD_WDT_G2_PHASE_MASK 0x0Fu + +static bool sdEnabled(void) { + uint8_t on = 0; + if (sd_softdevice_is_enabled(&on) != NRF_SUCCESS) return false; + return on != 0; +} + +// Both accessors report failure rather than swallowing it: a silently dead +// breadcrumb would be worse than no breadcrumb, because the boot log would still +// print a (stale or zero) phase and invite a wrong conclusion. +static bool g2Read(uint8_t* out) { + if (!sdEnabled()) { + *out = (uint8_t)(NRF_POWER->GPREGRET2 & 0xFFu); + return true; + } + uint32_t v = 0; + uint32_t rc = sd_power_gpregret_get(1, &v); + if (rc != NRF_SUCCESS) return false; + *out = (uint8_t)(v & 0xFFu); + return true; +} + +static bool g2Write(uint8_t v) { + if (!sdEnabled()) { + NRF_POWER->GPREGRET2 = v; // single atomic store; no SVC, no clr/set race + return true; + } + // The SoftDevice offers no store, only masked clear and set. + uint32_t rc = sd_power_gpregret_clr(1, 0xFFu); + if (rc != NRF_SUCCESS) return false; + if (v) rc = sd_power_gpregret_set(1, v); + return rc == NRF_SUCCESS; +} + +static bool g2UpdateField(uint8_t mask, uint8_t value) { + uint8_t cur = 0; + if (!g2Read(&cur)) return false; + if ((cur & OD_WDT_G2_TAG_MASK) != OD_WDT_G2_TAG_VALUE) { + cur = OD_WDT_G2_TAG_VALUE; // stale/garbage: reinitialise + } + return g2Write((uint8_t)((cur & (uint8_t)~mask) | (value & mask))); +} + +// --------------------------------------------------------------------------- +// Public interface +// --------------------------------------------------------------------------- + +// --- W-3: boot-loop containment ------------------------------------------ +// +// A watchdog that resets a device which wedges during BOOT turns one hang into an +// endless reset cycle: the device never advertises, is unreachable over BLE/DFU, +// and flattens its battery faster than if it had simply hung. That is strictly +// worse than the bug it is meant to fix, so the strike counter exists to escape it. +// +// Strikes accumulate ONLY when resets arrive fast. The counter is cleared after +// OD_WDT_HEALTHY_MS of continuous uptime, which is deliberately NOT tied to panel +// success: +// +// - Clearing on "first successful refresh" would never accumulate, because every +// ordinary boot performs a successful boot refresh and would wipe the previous +// strike. +// - It would also make safe mode permanent, because safe mode performs no +// refresh and so could never satisfy the clear condition. +// +// An uptime rule solves both at once and is panel-independent, so it behaves +// identically in safe mode. A device that survives ten minutes between wedges is +// not boot-looping and should keep retrying the panel. +#define OD_WDT_SAFE_MODE_STRIKES 3u +#define OD_WDT_HEALTHY_MS (10UL * 60UL * 1000UL) /* >= 2x the timeout */ + +static bool s_safeMode = false; +static bool s_strikesToClear = false; +static uint32_t s_bootMs = 0; + +static void strikesSet(uint8_t n) { + if (n > 3) n = 3; + (void)g2UpdateField(OD_WDT_G2_CNT_MASK, (uint8_t)(n << OD_WDT_G2_CNT_SHIFT)); +} + +void odWatchdogBootInit(void) { + s_bootMs = millis(); + s_resetReason = readResetReason(); + logResetReason(s_resetReason); + + const bool wasDog = (s_resetReason & POWER_RESETREAS_DOG_Msk) != 0; + uint8_t strikes = 0; + + uint8_t g2 = 0; + if (!g2Read(&g2)) { + od_log_warn("[WDT] GPREGRET2 unreadable - breadcrumb and strike count unavailable"); + } else if ((g2 & OD_WDT_G2_TAG_MASK) == OD_WDT_G2_TAG_VALUE) { + strikes = (uint8_t)((g2 & OD_WDT_G2_CNT_MASK) >> OD_WDT_G2_CNT_SHIFT); + od_log_info("[WDT] breadcrumb from previous run: phase=%u strikes=%u", + (unsigned)(g2 & OD_WDT_G2_PHASE_MASK), (unsigned)strikes); + } else { + od_log_info("[WDT] no retained breadcrumb (cold start or first boot)"); + if (!g2Write(OD_WDT_G2_TAG_VALUE)) { + od_log_warn("[WDT] GPREGRET2 unwritable - breadcrumbs disabled"); + } + } + + if (wasDog) { + if (strikes < 3) strikes++; + strikesSet(strikes); + od_log_warn("[WDT] previous boot ended in a watchdog reset (strike %u/%u)", + (unsigned)strikes, (unsigned)OD_WDT_SAFE_MODE_STRIKES); + } else if (strikes != 0) { + // Any non-DOG reset means the fast-reset cycle was broken by something + // else (power cycle, pin reset, deliberate reboot). Start clean. + strikes = 0; + strikesSet(0); + } + + s_safeMode = (strikes >= OD_WDT_SAFE_MODE_STRIKES); + if (s_safeMode) { + od_log_error("[WDT] SAFE MODE: %u consecutive watchdog resets - skipping ALL panel " + "work this boot so the device stays reachable over BLE/DFU. " + "Clears after %lu s of healthy uptime.", + (unsigned)strikes, (unsigned long)(OD_WDT_HEALTHY_MS / 1000UL)); + } + s_strikesToClear = (strikes != 0); + + odWatchdogBreadcrumb(OD_WDT_PHASE_IDLE); +} + +// Called from the loop-top feed. Clears the strike counter once the device has +// demonstrably survived, so safe mode is self-exiting and a slow-recurring fault +// never accumulates toward it. +static void strikesClearIfHealthy(void) { + if (!s_strikesToClear) return; + if ((uint32_t)(millis() - s_bootMs) < OD_WDT_HEALTHY_MS) return; + s_strikesToClear = false; + strikesSet(0); + od_log_info("[WDT] %lu s of healthy uptime - strike counter cleared", + (unsigned long)(OD_WDT_HEALTHY_MS / 1000UL)); +} + +bool odWatchdogInSafeMode(void) { + return s_safeMode; +} + +// --------------------------------------------------------------------------- +// The watchdog itself +// --------------------------------------------------------------------------- +// +// Timeout is compile-time ONLY: CRV must be written before the start task and is +// latched thereafter, so there is no runtime knob to expose. + +#ifndef OPENDISPLAY_NRF_WDT_S +#define OPENDISPLAY_NRF_WDT_S 300 +#endif + +#if OPENDISPLAY_NRF_WDT_S != 0 +// Lower bound: must exceed every legitimate blocking span in the firmware -- the +// largest is a ~240 s bbepRefresh() on a 7-colour split-buffer panel (plan V12). +// Upper bound: CRV is 32 bits at 32768 Hz, so ~131072 s is representable; 3600 s +// keeps a wide margin and rejects a mistyped value that would silently disable +// recovery for hours. +static_assert(OPENDISPLAY_NRF_WDT_S >= 60 && OPENDISPLAY_NRF_WDT_S <= 3600, + "OPENDISPLAY_NRF_WDT_S must be 0 (disabled) or 60..3600 seconds"); +#endif + +static bool s_inherited = false; // a watchdog we found running, not one we started +#if OPENDISPLAY_NRF_WDT_S != 0 +static bool s_armed = false; +#endif + +void odWatchdogArm(void) { + // Inherit-detection runs on EVERY build, including OPENDISPLAY_NRF_WDT_S=0. + // + // Whether a running nRF52840 WDT survives a non-power-on reset (soft reset, + // DOG reset, pin reset) is NOT established by any source available in this + // workspace, and the two possibilities have very different consequences: + // + // - If it does NOT survive, this branch simply never fires and costs a + // single register read at boot. + // - If it DOES survive, then a build with the watchdog disabled -- reached by + // DFU, a reflash, or NVIC_SystemReset from a build that had it enabled -- + // would inherit a live watchdog it never feeds, and reset forever until + // someone physically removes power. That is a brick. + // + // Feeding whatever we find is correct under both, so do that rather than pick. + // T7 logs RUNSTATUS at boot and will settle the question empirically. + if (nrf_wdt_started(NRF_WDT)) { + od_log_warn("[WDT] ALREADY RUNNING at boot (not started by this call). " + "CRV=%lu (%lus) RREN=0x%lX CONFIG=0x%lX - cannot be stopped or " + "reconfigured; feeding it as-is.", + (unsigned long)NRF_WDT->CRV, + (unsigned long)((NRF_WDT->CRV + 1UL) / 32768UL), + (unsigned long)NRF_WDT->RREN, + (unsigned long)NRF_WDT->CONFIG); + s_inherited = true; + odWatchdogFeed(); + return; + } +#if OPENDISPLAY_NRF_WDT_S == 0 + od_log_warn("[WDT] disabled at build time (OPENDISPLAY_NRF_WDT_S=0)"); +#else + // Order is load-bearing: CRV, RREN and CONFIG all latch at TASKS_START. + nrf_wdt_reload_value_set(NRF_WDT, ((uint32_t)OPENDISPLAY_NRF_WDT_S * 32768UL) - 1UL); + nrf_wdt_reload_request_enable(NRF_WDT, NRF_WDT_RR0); // one reload register, + // one feeder, one task + // RUN_SLEEP: keep counting while the CPU sleeps (idleDelay's delay() chunks), + // but pause while halted by a debugger, so a breakpoint is not a reset. + nrf_wdt_behaviour_set(NRF_WDT, NRF_WDT_BEHAVIOUR_RUN_SLEEP); + + s_armed = true; + odWatchdogFeed(); // start the first period from a known state + nrf_wdt_task_trigger(NRF_WDT, NRF_WDT_TASK_START); // irreversible + od_log_info("[WDT] armed: %us", (unsigned)OPENDISPLAY_NRF_WDT_S); +#endif +} + +void odWatchdogFeed(void) { + strikesClearIfHealthy(); +#if OPENDISPLAY_NRF_WDT_S != 0 + if (!s_armed && !s_inherited) return; +#else + if (!s_inherited) return; // nothing of ours is armed; only feed an inherited dog +#endif + // Every ENABLED reload register must be written before the counter reloads -- + // RREN is an AND, not an OR. Our own arm path enables RR0 alone, but an + // INHERITED watchdog (one the bootloader started; see odWatchdogArm) may have + // any subset enabled, and feeding only RR0 would then never reload it. Walk + // RREN instead of assuming. + // + // nrf_wdt_reload_request_set writes NRF_WDT_RR_VALUE (0x6E524635); the magic + // value is why a wild pointer or stray memset cannot accidentally pet the dog. + for (uint8_t i = 0; i <= (uint8_t)NRF_WDT_RR7; i++) { + nrf_wdt_rr_register_t rr = (nrf_wdt_rr_register_t)i; + if (nrf_wdt_reload_request_is_enabled(NRF_WDT, rr)) { + nrf_wdt_reload_request_set(NRF_WDT, rr); + } + } +} + +void odWatchdogBreadcrumb(uint8_t phase) { + // Skip the register work when the phase has not actually changed. A stamp + // costs up to three SVCs once the SoftDevice is enabled (get + clr + set), so + // without this the streaming stamps -- which sit on per-frame and per-row + // paths -- would be far too expensive to place where they are most useful. + // + // Kept in sync with the register by odWatchdogBootInit(), which stamps IDLE + // explicitly after establishing the tag, so the cache never starts out lying. + static uint8_t s_lastPhase = 0xFF; + static bool s_failLogged = false; + phase &= OD_WDT_G2_PHASE_MASK; + if (phase == s_lastPhase) return; + // Advance the cache ONLY on success. Updating it first would suppress the + // retry after a failed write, leaving the cache claiming a phase the register + // never received -- and a failed clr+set can even leave the byte cleared. + if (!g2UpdateField(OD_WDT_G2_PHASE_MASK, phase)) { + if (!s_failLogged) { // latched: this sits on per-frame paths + s_failLogged = true; + od_log_warn("[WDT] GPREGRET2 write failed - breadcrumb may be stale"); + } + return; + } + s_lastPhase = phase; +} + +#endif // TARGET_NRF From 4b54a88a646525c98a12e5ba3147934389f3f964 Mon Sep 17 00:00:00 2001 From: David Lee <247393336+davelee98@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:15:15 -0400 Subject: [PATCH 2/2] fix(nrf): watchdog debug instrumentation and reset-reason logging fix Timeout dropped from 300s to 120s, which is now below the ~240s worst-case REFRESH_FULL span on a 7-colour split-buffer panel -- a healthy refresh on that panel class will trip the watchdog mid-refresh. Known gap, documented in platformio.ini; re-check before shipping to such a panel. Adds two new breadcrumb phases (IDLE_OFF/IDLE_WARM replacing the shared IDLE) so a freeze while the panel session is idle can be told apart from one during keep-alive, plus four more (PWRMGM_AXP2101/RAIL/PINS/WIRE) instrumenting pwrmgm()'s previously-blind power-up path, added after a watchdog reset landed there with no breadcrumb to explain why. A phaseName() lookup makes the retained phase human-readable in the boot log instead of a bare integer. WDT-DEBUG-tagged od_log_debug lines pair with each EPD-session and pwrmgm() breadcrumb (stamped first, since the log call itself can hang on the same USB CDC mutex delay() depends on) -- grep "WDT-DEBUG" to remove the whole set later. Also adds a bounded (2s cap), debug-build-only wait for the USB CDC host to reconnect before the first log line: without it, the reset-reason and retained-breadcrumb lines -- the whole point of this feature -- reliably lose the race against USB re-enumeration after a reset and are silently discarded by od_log's dark-port check, which doesn't count them as drops. --- docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md | 24 ++++++++++ platformio.ini | 22 ++++----- src/display_service.cpp | 44 ++++++++++++++++- src/main.cpp | 48 +++++++++++++++++++ src/watchdog.h | 17 ++++++- src/watchdog_nrf.cpp | 30 +++++++++++- 6 files changed, 169 insertions(+), 16 deletions(-) diff --git a/docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md b/docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md index 526c08c..d51da8d 100644 --- a/docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md +++ b/docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md @@ -10,6 +10,30 @@ reset-reason decode relocates out of `main.cpp`. **Goal:** recover the device when an *unbounded* wait — in `loop()`, or below it in a vendored driver we cannot instrument — wedges the panel permanently. +--- +**STATUS UPDATE (2026-08-03): timeout changed to 120 s, below this plan's D-1 value.** +`OPENDISPLAY_NRF_WDT_S` was dropped from the 300 s this plan derives and validates (§3.2, +§10 D-1) to **120 s**. This is a deliberate, confirmed choice made outside this plan's +analysis, not a correction to it — everything below about the 240 s worst case, the 1.25× +margin at 300 s, and W-2's pre-call feed policy is still accurate background, but the +headline numbers ("300 s", "1.25× margin") no longer describe the shipped value. + +**Consequence: the margin this plan relies on is gone.** At 120 s, a healthy +`REFRESH_FULL` on the 7-colour split-buffer panel (§3.1's ~240 s worst case) will trip the +watchdog *mid-refresh* on a device that isn't wedged. T2 (§8) — measuring that panel's real +span — is now a prerequisite for shipping to it, not a confirmation exercise; the "possible +sizing error" residual in §10 is elevated from unlikely to expected. Do not ship this +timeout to a 7-colour split-buffer panel without re-deriving it. + +**Also since rev 2:** two idle breadcrumb phases (`IDLE_OFF`/`IDLE_WARM`) replaced the +single shared `OD_WDT_PHASE_IDLE` used at `epdSessionForceOffLocked()`/`epdSessionRelease()`, +and four more (`PWRMGM_AXP2101`/`RAIL`/`PINS`/`WIRE`) were added inside `pwrmgm()` itself +(`main.cpp`) after a watchdog reset landed there — `pwrmgm()` had no breadcrumb coverage in +the original design. All 16 phase values in the 4-bit field are now in use (`watchdog.h`). +See the retained-breadcrumb reset-reason logging fix below for a related gap that was +losing the very reset-reason line this plan's boot log depends on. +--- + ## 1. What this closes Two accepted residuals and one new finding converge on the same gap. diff --git a/platformio.ini b/platformio.ini index 10984be..3df7b64 100644 --- a/platformio.ini +++ b/platformio.ini @@ -87,19 +87,19 @@ build_flags = ; Hardware watchdog timeout, seconds. 0 disables; valid range is 60..3600 ; (enforced by a static_assert in src/watchdog_nrf.cpp). ; - ; 300 s is NOT a comfortable margin over normal operation -- it is sized against - ; the single longest span the firmware cannot instrument: a REFRESH_FULL on a - ; 7-colour split-buffer panel sends the init sequence to BOTH controllers, and - ; each of its 4 BUSY_WAIT entries can take 30 s, i.e. ~240 s inside one - ; bbepRefresh() call. What makes 300 s safe is not the number but the feed - ; immediately before every bb_epaper entry point in display_service.cpp, so the - ; watchdog faces that one call rather than that call plus everything preceding - ; it. Margin is therefore ~1.25x, and any growth in BUSY_WAIT count, controller - ; count or the multicolour cap eats into it directly -- re-check when adding a - ; panel. See docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md sections 3.2 and W-2. + ; KNOWN GAP: 120 s is BELOW the single longest span the firmware cannot + ; instrument -- a REFRESH_FULL on a 7-colour split-buffer panel sends the init + ; sequence to BOTH controllers, and each of its 4 BUSY_WAIT entries can take + ; 30 s, i.e. ~240 s inside one bbepRefresh() call. The per-entry-point feed in + ; display_service.cpp (see odWatchdogFeed() call sites) still bounds the dog to + ; that one call rather than that call plus everything preceding it, but at + ; 120 s a HEALTHY refresh on that panel class will trip the watchdog mid-refresh. + ; Chosen anyway on 2026-08-03 (see conversation); re-check before shipping to any + ; 7-colour split-buffer panel. See docs/PLAN_NRF_HARDWARE_WATCHDOG_2026-08-01.md + ; sections 3.2 and W-2 for the original 300 s derivation. ; ; The three other nRF envs inherit this via ${env:nrf52840custom.build_flags}. - -DOPENDISPLAY_NRF_WDT_S=300 + -DOPENDISPLAY_NRF_WDT_S=120 platform = https://github.com/maxgerhardt/platform-nordicnrf52 framework = arduino board_build.variants_dir = variants diff --git a/src/display_service.cpp b/src/display_service.cpp index 348c8e1..8253dc0 100644 --- a/src/display_service.cpp +++ b/src/display_service.cpp @@ -205,6 +205,9 @@ static void e1004InitPanel(void) { const DisplayConfig& d = globalConfig.displays[0]; bbepSetCS2(&bbep, e1004_cs2_pin()); odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the hardware + // watchdog work -- safe to delete this block if it's no longer needed. + od_log_debug("[EPD session][WDT] e1004InitPanel: INIT_SEQ (dual-controller)"); odWatchdogFeed(); // bbepInitIO sends pInitFull internally (~240 s worst case) bbepInitIO(&bbep, d.dc_pin, d.reset_pin, d.busy_pin, d.cs_pin, d.data_pin, d.clk_pin, 8000000); } @@ -365,6 +368,9 @@ static void initBbepPanelSession() { } #endif odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the hardware + // watchdog work -- safe to delete this block if it's no longer needed. + od_log_debug("[EPD session][WDT] initBbepPanelSession: INIT_SEQ"); odWatchdogFeed(); // bbepInitIO sends pInitFull internally (~240 s worst case) bbepInitIO(&bbep, d.dc_pin, d.reset_pin, d.busy_pin, d.cs_pin, d.data_pin, d.clk_pin, 8000000); odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) @@ -447,6 +453,9 @@ static void pwrmgmLockGive(void) { static void epdSessionForceOffLocked(void) { if (pwrmgmState == PWR_OFF) return; // idempotent odWatchdogBreadcrumb(OD_WDT_PHASE_FORCE_OFF); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the hardware + // watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] FORCE_OFF: pwrmgmState=%u -> tearing down", (unsigned)pwrmgmState); od_log_info("[EPD session] force off"); if (epdSessionUsesFastepd()) { #if defined(TARGET_ESP32) && defined(OPENDISPLAY_FASTEPD) @@ -465,7 +474,12 @@ static void epdSessionForceOffLocked(void) { // Panel work is finished. Without this, a later wedge in BLE/WiFi/command // handling would boot reporting breadcrumb=FORCE_OFF and point the next // investigation at the panel teardown that had actually already completed. - odWatchdogBreadcrumb(OD_WDT_PHASE_IDLE); + // IDLE_OFF (not plain IDLE) so a freeze here is distinguishable at the next + // boot from a freeze during PWR_WARM keep-alive (see epdSessionRelease). + odWatchdogBreadcrumb(OD_WDT_PHASE_IDLE_OFF); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the hardware + // watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] IDLE_OFF: pwrmgmState=%u", (unsigned)pwrmgmState); } // Bring the panel up for a transfer/refresh. Returns true iff it was COLD (rail @@ -489,6 +503,9 @@ static bool epdSessionAcquire(bool partialInit) { bool cold; if (pwrmgmState == PWR_OFF) { odWatchdogBreadcrumb(OD_WDT_PHASE_ACQUIRE_COLD); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the + // hardware watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] ACQUIRE_COLD: partialInit=%d", (int)partialInit); od_log_info("[EPD session] acquire: COLD bring-up"); pwrmgm(true); // -> PWR_ACTIVE (guarded; real transition) if (!epdSessionUsesFastepd()) { @@ -501,6 +518,9 @@ static bool epdSessionAcquire(bool partialInit) { #endif { odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the + // hardware watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] INIT_SEQ (cold): bbepInitIO+bbepWakeUp+CMDSequence"); odWatchdogFeed(); // bbepInitIO sends pInitFull internally (~240 s worst case) bbepInitIO(&bbep, d.dc_pin, d.reset_pin, d.busy_pin, d.cs_pin, d.data_pin, d.clk_pin, 8000000); odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) @@ -517,6 +537,10 @@ static bool epdSessionAcquire(bool partialInit) { } else { // WARM re-acquire (or, defensively, an already-ACTIVE re-entry). odWatchdogBreadcrumb(OD_WDT_PHASE_ACQUIRE_WARM); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the + // hardware watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] ACQUIRE_WARM: pwrmgmState=%u partialInit=%d", + (unsigned)pwrmgmState, (int)partialInit); od_log_info(pwrmgmState == PWR_ACTIVE ? "[EPD session] acquire: already ACTIVE (defensive)" : "[EPD session] acquire: WARM re-acquire"); pwrmgmState = PWR_ACTIVE; @@ -531,6 +555,9 @@ static bool epdSessionAcquire(bool partialInit) { #endif { odWatchdogBreadcrumb(OD_WDT_PHASE_INIT_SEQ); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the + // hardware watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] INIT_SEQ (warm): bbepWakeUp+CMDSequence"); odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) bbepWakeUp(&bbep); const uint8_t* initSeq = partialInit ? (bbep.pInitPart ? bbep.pInitPart : bbep.pInitFull) @@ -554,18 +581,28 @@ static void epdSessionRelease(bool refreshSuccess) { pwrmgmLockTake(); if (pwrmgmState == PWR_OFF) { pwrmgmLockGive(); return; } // nothing to release odWatchdogBreadcrumb(OD_WDT_PHASE_RELEASE); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the hardware + // watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] RELEASE: refreshSuccess=%d", (int)refreshSuccess); uint32_t window = epdKeepAliveWindowMs(); if (window == 0 || !refreshSuccess) { od_log_info(refreshSuccess ? "[EPD session] release: keep-alive disabled, powering off" : "[EPD session] release: refresh failed, powering off"); + // epdSessionForceOffLocked() stamps OD_WDT_PHASE_IDLE_OFF itself; nothing + // to add here or the plain-IDLE stamp below would clobber it. epdSessionForceOffLocked(); } else { pwrmgmState = PWR_WARM; pwrmgmOffDeadlineMs = millis() + window; // Controller stays AWAKE (no bbepSleep; is_awake stays 1); rail/SPI stay up. od_log_info("[EPD session] release: panel warm-idle, off in %u ms", (unsigned)window); + // See the note in ForceOffLocked -- IDLE_WARM, not plain IDLE, so a freeze + // during keep-alive is distinguishable from one during PWR_OFF. + odWatchdogBreadcrumb(OD_WDT_PHASE_IDLE_WARM); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the + // hardware watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] IDLE_WARM: off in %u ms", (unsigned)window); } - odWatchdogBreadcrumb(OD_WDT_PHASE_IDLE); // see the note in ForceOffLocked pwrmgmLockGive(); } @@ -596,6 +633,9 @@ static bool refreshBootScreenFull() { return false; } odWatchdogBreadcrumb(OD_WDT_PHASE_BOOT_REFRESH); + // WDT-DEBUG: EPD session stage instrumentation, added alongside the hardware + // watchdog work -- safe to delete this line if it's no longer needed. + od_log_debug("[EPD session][WDT] BOOT_REFRESH: entering bbepRefresh(REFRESH_FULL)"); od_log_info("EPD refresh: FULL (boot)"); touchSuspendForEpdRefresh(); odWatchdogFeed(); // reload before entering bb_epaper (may block ~240 s) diff --git a/src/main.cpp b/src/main.cpp index 9167824..b6c0548 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -90,6 +90,25 @@ void setup() { // is documented to flap on a healthy link, so a hook there would silently // discard good output. od_log_set_ready_hook([]() -> bool { return (bool)Serial; }); + #if OD_LOG_LEVEL >= OD_LOG_DEBUG + // Bounded wait for a host terminal to reconnect after ANY reset. USB + // re-enumerates from scratch on reset, and without this, the reset-reason and + // breadcrumb lines logged just below (odWatchdogBootInit()) -- the whole point + // of the watchdog work -- race the host's reconnect and are silently discarded + // by the dark-port check in od_emit(). Those drops are NOT counted (see the + // comment there), so they vanish with no trace. + // + // Debug builds only (OD_LOG_LEVEL >= OD_LOG_DEBUG, e.g. nrf52840custom-debug): + // production boots should never pay a boot-time cost for a terminal that isn't + // there. Capped at 2 s even here, unlike OPENDISPLAY_BOOT_DIAG's unbounded + // `while (!Serial)`, and returns immediately once a host is already connected. + { + uint32_t serialWaitStart = millis(); + while (!Serial && (millis() - serialWaitStart) < 2000u) { + delay(10); + } + } + #endif #endif #endif // Immediately after od_log_init(), so the boot lines below are not emitted at a @@ -1260,10 +1279,20 @@ void pwrmgm(bool onoff){ } if(axp2101_found){ if(onoff){ + // WDT-DEBUG: pwrmgm() step instrumentation, added alongside the hardware + // watchdog work -- safe to delete this block if it's no longer needed. + // Breadcrumb stamped BEFORE the debug log: the log call itself reaches + // tud_cdc_write_flush() -> usbd_edpt_claim() -> a WAIT_FOREVER mutex + // (see conversation, 2026-08-03), so it is not guaranteed to return + // either. Stamping first means the phase survives even if the log + // line is what hangs. + odWatchdogBreadcrumb(OD_WDT_PHASE_PWRMGM_AXP2101); + od_log_debug("[pwrmgm][WDT] PWRMGM_AXP2101: entering initAXP2101(bus=%u)", (unsigned)axp2101_bus_id); od_log_info("Powering up AXP2101 PMIC..."); initAXP2101(axp2101_bus_id); } else{ + od_log_debug("[pwrmgm][WDT] AXP2101 power-down"); od_log_info("Powering down AXP2101 PMIC..."); powerDownAXP2101(); Wire.end(); @@ -1281,12 +1310,21 @@ void pwrmgm(bool onoff){ #endif const DisplayConfig& disp = globalConfig.displays[0]; if (onoff) { + // WDT-DEBUG: pwrmgm() step instrumentation -- see the note above on + // breadcrumb-before-log ordering. Safe to delete this whole block (all + // odWatchdogBreadcrumb(OD_WDT_PHASE_PWRMGM_*) + od_log_debug("[pwrmgm][WDT]...") + // pairs below) if it's no longer needed. + odWatchdogBreadcrumb(OD_WDT_PHASE_PWRMGM_RAIL); + od_log_debug("[pwrmgm][WDT] PWRMGM_RAIL: pwr_pin=%u", (unsigned)globalConfig.system_config.pwr_pin); if (globalConfig.system_config.pwr_pin != 0xFF) { digitalWrite(globalConfig.system_config.pwr_pin, HIGH); delay(800); } else { od_log_warn("Power pin not set"); } + od_log_debug("[pwrmgm][WDT] PWRMGM_RAIL: delay(800) returned"); + odWatchdogBreadcrumb(OD_WDT_PHASE_PWRMGM_PINS); + od_log_debug("[pwrmgm][WDT] PWRMGM_PINS: fastepd_driver_spi=%d", (int)fastepd_driver_spi); if (!fastepd_driver_spi) { if (disp.reset_pin != 0xFF) { pinMode(disp.reset_pin, OUTPUT); @@ -1319,17 +1357,27 @@ void pwrmgm(bool onoff){ } delay(200); } + od_log_debug("[pwrmgm][WDT] PWRMGM_PINS: pin setup + delay done"); + odWatchdogBreadcrumb(OD_WDT_PHASE_PWRMGM_WIRE); + od_log_debug("[pwrmgm][WDT] PWRMGM_WIRE: entering initOrRestoreWireForOpenDisplay()"); initOrRestoreWireForOpenDisplay(); + od_log_debug("[pwrmgm][WDT] PWRMGM_WIRE: returned"); } else { + // WDT-DEBUG: pwrmgm() power-down step instrumentation. No spare breadcrumb + // phase values remain (all 16 are used), so this path is debug-log-only -- + // safe to delete if it's no longer needed. + od_log_debug("[pwrmgm][WDT] power-down: SPI.end()"); if (!fastepd_driver_spi) { SPI.end(); } // Keep I2C alive when sensors/touch use data_bus[0] (e.g. reTerminal MISC_I2C on GPIO0/1). if (!openDisplayI2cBusConfigured()) { + od_log_debug("[pwrmgm][WDT] power-down: Wire.end()"); Wire.end(); invalidateOpenDisplayWire(); } if (globalConfig.system_config.pwr_pin != 0xFF) { + od_log_debug("[pwrmgm][WDT] power-down: configureDisplayPinsLowPower()"); configureDisplayPinsLowPower(); digitalWrite(globalConfig.system_config.pwr_pin, LOW); } diff --git a/src/watchdog.h b/src/watchdog.h index 64f788c..09b81d0 100644 --- a/src/watchdog.h +++ b/src/watchdog.h @@ -44,7 +44,8 @@ // that something did. Values must fit 4 bits (0-15); see the GPREGRET2 layout in // watchdog_nrf.cpp. enum OdWatchdogPhase : uint8_t { - OD_WDT_PHASE_IDLE = 0, + OD_WDT_PHASE_IDLE = 0, // pre-session: stamped once at boot, before + // the first epdSessionAcquire/Release/ForceOff OD_WDT_PHASE_ACQUIRE_COLD = 1, OD_WDT_PHASE_ACQUIRE_WARM = 2, OD_WDT_PHASE_INIT_SEQ = 3, @@ -54,6 +55,20 @@ enum OdWatchdogPhase : uint8_t { OD_WDT_PHASE_RELEASE = 7, OD_WDT_PHASE_FORCE_OFF = 8, OD_WDT_PHASE_BOOT_REFRESH = 9, + // pwrmgmState is plain RAM, not retained across a reset, so without a + // distinct phase per idle sub-state a freeze during either one reports the + // same generic OD_WDT_PHASE_IDLE and the two are indistinguishable after the + // fact. These name which idle state the session was actually left in. + OD_WDT_PHASE_IDLE_OFF = 10, // session fully powered down (PWR_OFF) + OD_WDT_PHASE_IDLE_WARM = 11, // panel kept awake for keep-alive (PWR_WARM) + // pwrmgm(true)'s rail bring-up sequence, uninstrumented until the 2026-08-03 + // freeze (reset ~120s after ACQUIRE_COLD, never reaching INIT_SEQ -- the wedge + // was somewhere inside pwrmgm() itself). These name which of its four + // sub-steps was entered last. Uses the last 4 of the 16 available phase values. + OD_WDT_PHASE_PWRMGM_AXP2101 = 12, // before initAXP2101() (I2C PMIC bring-up) + OD_WDT_PHASE_PWRMGM_RAIL = 13, // before pwr_pin HIGH + delay(800) + OD_WDT_PHASE_PWRMGM_PINS = 14, // before panel GPIO setup + delay(100/200) + OD_WDT_PHASE_PWRMGM_WIRE = 15, // before initOrRestoreWireForOpenDisplay() OD_WDT_PHASE__MAX = 15, }; diff --git a/src/watchdog_nrf.cpp b/src/watchdog_nrf.cpp index a00cae4..97aab25 100644 --- a/src/watchdog_nrf.cpp +++ b/src/watchdog_nrf.cpp @@ -82,6 +82,31 @@ static void logResetReason(uint32_t r) { od_log_info("[WDT] reset reason: %s (0x%08lX)", buf, (unsigned long)r); } +// Names OdWatchdogPhase values (watchdog.h) for the boot-time breadcrumb log. +// Kept as a plain table, same pattern as logResetReason()'s kinds[] above, so a +// new phase added to the enum is one line here, not a guess at the call site. +static const char* phaseName(uint8_t phase) { + switch (phase) { + case OD_WDT_PHASE_IDLE: return "IDLE"; + case OD_WDT_PHASE_ACQUIRE_COLD: return "ACQUIRE_COLD"; + case OD_WDT_PHASE_ACQUIRE_WARM: return "ACQUIRE_WARM"; + case OD_WDT_PHASE_INIT_SEQ: return "INIT_SEQ"; + case OD_WDT_PHASE_FILL: return "FILL"; + case OD_WDT_PHASE_STREAM: return "STREAM"; + case OD_WDT_PHASE_REFRESH_WAIT: return "REFRESH_WAIT"; + case OD_WDT_PHASE_RELEASE: return "RELEASE"; + case OD_WDT_PHASE_FORCE_OFF: return "FORCE_OFF"; + case OD_WDT_PHASE_BOOT_REFRESH: return "BOOT_REFRESH"; + case OD_WDT_PHASE_IDLE_OFF: return "IDLE_OFF"; + case OD_WDT_PHASE_IDLE_WARM: return "IDLE_WARM"; + case OD_WDT_PHASE_PWRMGM_AXP2101: return "PWRMGM_AXP2101"; + case OD_WDT_PHASE_PWRMGM_RAIL: return "PWRMGM_RAIL"; + case OD_WDT_PHASE_PWRMGM_PINS: return "PWRMGM_PINS"; + case OD_WDT_PHASE_PWRMGM_WIRE: return "PWRMGM_WIRE"; + default: return "UNKNOWN"; + } +} + // --------------------------------------------------------------------------- // Retained state: GPREGRET2 // --------------------------------------------------------------------------- @@ -214,8 +239,9 @@ void odWatchdogBootInit(void) { od_log_warn("[WDT] GPREGRET2 unreadable - breadcrumb and strike count unavailable"); } else if ((g2 & OD_WDT_G2_TAG_MASK) == OD_WDT_G2_TAG_VALUE) { strikes = (uint8_t)((g2 & OD_WDT_G2_CNT_MASK) >> OD_WDT_G2_CNT_SHIFT); - od_log_info("[WDT] breadcrumb from previous run: phase=%u strikes=%u", - (unsigned)(g2 & OD_WDT_G2_PHASE_MASK), (unsigned)strikes); + uint8_t phase = (uint8_t)(g2 & OD_WDT_G2_PHASE_MASK); + od_log_info("[WDT] breadcrumb from previous run: phase=%s (%u) strikes=%u", + phaseName(phase), (unsigned)phase, (unsigned)strikes); } else { od_log_info("[WDT] no retained breadcrumb (cold start or first boot)"); if (!g2Write(OD_WDT_G2_TAG_VALUE)) {