Leopoldo Rodríguez
914a3fd2a9
Verified findings: millis()-rollover recovery-gate collapse (uint32-modelled), fan-tach false-fault on 2-wire fans, PROD silicon undriven (CHRG_EN/STAT/FAULT, BYP_CTRL/K1, NTC PB1, gauge). v3.0 scope in doc section 4. Bumps firmware submodule to ea1d84c (v2.2+v2.3 source + tools/hostsim harness). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GSKKes8GsS4YHAMWJ44tCm
204 lines
13 KiB
Markdown
204 lines
13 KiB
Markdown
---
|
||
title: Firmware Review — blackpillrtkcode_v2 v2.3 → EinarBoard 1.0 production
|
||
tags: [project-drogheda, firmware, watchdog, review, einarboard, production]
|
||
project: Project Drogheda
|
||
author: Helgen CTO (review run 2026-07-24)
|
||
date: 2026-07-24
|
||
status: Active — review of v2.3 as production candidate + v3.0 build spec
|
||
supersedes: extends firmware-review.md (v1→v2, 2026-06-19); does not replace it
|
||
---
|
||
|
||
# Firmware Review — `blackpillrtkcode_v2.ino` v2.3 as the EinarBoard 1.0 production firmware
|
||
|
||
**Question under review:** v2.3 (2026-06-22) was written for the Blackpill *proto* wiring.
|
||
EinarBoard 1.0 is fabbed and in service. Is v2.3 correct, and what must change to make it the
|
||
production ("on-PCB") software?
|
||
|
||
**Bottom line:** v2.3's recovery doctrine is sound and was re-verified programmatically
|
||
(backoff ladder, SPS-thrash guard, RTK-loss grace all behave exactly as specified). But it has
|
||
**one verified field-impacting bug** (millis()-rollover collapses the recovery backoff every
|
||
49.7 days of uptime), **one wrong-for-the-shipped-hardware subsystem** (3-wire fan tach on a
|
||
2-wire fan → permanent false `fan_fault` + floating EXTI), and **it drives none of the
|
||
EinarBoard-only silicon** (LTC4040 charge management, Modo-B relay, NTC, gauge). Flashed today
|
||
it is *safe but blind*: the hardware defaults (PA8 100 k pull-down = charging enabled with the
|
||
charger's own NTC guard; PB5 low = K1 NC = corrections direct) were designed to tolerate
|
||
exactly this firmware. The production build is a **v3.0** — scope in §4.
|
||
|
||
Severity: 🔴 field-impacting · 🟠 robustness · 🟡 cleanup. Authority for hardware facts:
|
||
[[watchdog-electrical-design-state]] (canonical) → [[watchdog-production-schematic-delta]] §2/§4/§5
|
||
→ EinarBoard harness truth 2026-07-23 (DFR1015 removed pre-fab, audit H3).
|
||
|
||
---
|
||
|
||
## 1. Findings — bugs in v2.3 as written
|
||
|
||
### 🔴 F1. `millis()` rollover collapses the recovery gate (verified, uint32 model)
|
||
|
||
`beginPowerCycle()` arms the backoff as an **absolute deadline**, and RUNNING checks it with an
|
||
**absolute compare**:
|
||
|
||
```cpp
|
||
nextRecoveryAfter = millis() + POWER_CYCLE_TIME + delayMs; // wraps at 2^32
|
||
...
|
||
if (millis() >= nextRecoveryAfter) beginPowerCycle(...); // not rollover-safe
|
||
```
|
||
|
||
On the target `unsigned long` is 32-bit → `millis()` wraps every **49.7 days**. These units run
|
||
24/7; every unit reaches rollover around day 50. Any wait window that straddles the wrap
|
||
overflows to a *small* deadline, so the gate reads open — and every re-arm inside the pre-wrap
|
||
window overflows again. A LIMP unit (always inside a 30-min window) is hit **every** rollover.
|
||
|
||
**Modelled** (`tools/hostsim/rollover_model.cpp`, uint32 arithmetic, LIMP unit, wedged stack):
|
||
gaps go `1806 s → 91 s ×~20 → 1806 s` — i.e. ~20 back-to-back power-cycles (rail-off 6 s +
|
||
boot 25 s + NMEA-timeout 60 s apart) in the last ~30 min before wrap. Each one is a tracker
|
||
disconnect: this is precisely the flapping v2 was built to kill, resurrected two times per
|
||
quarter per unit. Mid-ladder (non-LIMP) units get the same collapse if a backoff window
|
||
straddles the wrap, and `recoveryCount` inflates into a false LIMP latch.
|
||
|
||
**Fix (small):** store the window start + length and compare elapsed time — subtraction is
|
||
wrap-safe: `waiting && (millis() - recoveryWaitStart) >= recoveryWaitLen`. All other timers in
|
||
the file already use the subtraction idiom correctly; this is the only absolute compare.
|
||
(The `linkLostSince == 0` / `pwrEdgeMs == 0` sentinels are benign — 0 recurs for 1 ms every
|
||
49.7 days at worst.)
|
||
|
||
### 🔴 F2. Fan-fault machinery is wrong for the shipped fans (2-wire, no tach)
|
||
|
||
Canonical open item (2026-06-24): **fans are 2-wire, PB10 tach = DNP provision** — "drop the
|
||
PB10 tach health check and infer fan-fault from temperature." v2.3 still ships the 3-wire
|
||
logic, so on EinarBoard:
|
||
|
||
- `fanHealth()` boosts to 100 % for 300 ms every 30 s whenever the fan is commanded, counts
|
||
**0 edges** (no tach exists) → `fanFault` **latches true on every unit, forever** →
|
||
`fan_fault=1` in every TLM/$PHLG → alarm fatigue that discredits the very telemetry channel
|
||
the fleet-health effort depends on. Plus an audible periodic full-speed blip.
|
||
- Worse, PB10 is configured `INPUT` + EXTI FALLING assuming the board's 10 k pull-up — which is
|
||
**DNP** on EinarBoard. A floating EXTI pin can fire on noise → garbage `fanRpm` values and a
|
||
randomly *masked* fault. Either failure mode corrupts the field.
|
||
|
||
**Fix:** gate the tach path behind a build flag (off for EinarBoard), report fan fault by
|
||
temperature trend (PWM ≥ threshold for N min while control temp still rising), and if the tach
|
||
path is ever kept, `INPUT_PULLDOWN`.
|
||
|
||
### 🔴 F3. The EinarBoard-only silicon is not driven at all (the actual "production software" gap)
|
||
|
||
Not bugs — missing scope. But flashing v2.3 leaves half the new board inert, including the two
|
||
levers that answer the RCA's #1 failure (hot battery) and the ICAVE observability goal:
|
||
|
||
| Pin | Net (delta §2) | Hardware state under v2.3 | Consequence |
|
||
|---|---|---|---|
|
||
| PA8 | `CHRG_EN` (LTC4040 CHGOFF) | Hi-Z; 100 k pull-down → **charging enabled** | Safe (charger's own cell-NTC still guards) but: **no temp-qualified charge cutoff** (the firmware half of the RCA #1 answer), and **no safety-timer restart** — a deep-discharged 12 Ah pack needs ≈5 h @ 2.42 A, which exceeds the LTC4040's internal timer, so without the documented `CHRG_EN` toggle-when-cool logic the pack stays **chronically undercharged** after any deep discharge (delta §4 checklist item). |
|
||
| PB3 / PB4 | `CHRG_STAT` / `CHRG_FAULT` | unread | Charging / NTC-fault state invisible in TLM/$PHLG. A unit just died in service (23-Jul short incident); charge-path observability is exactly what the fleet screen needs. |
|
||
| PB5 | `BYP_CTRL` (K1 driver) | low → K1 NC = direct uplink | Safe by design — but the MCU **never inserts itself**, so `$PHLG` never reaches the server. Modo B is the *permanent* uplink architecture since the 2026-07-02 carrier decision; the v2.3 header note "v3 keeps the modem on the F9P socket" and `ENABLE_PHLG_RELAY 0` are **stale**. Live fleet health — the point of the board — is off. |
|
||
| PB1 | `NTC_TEMP` (ADC1_IN9) | unread | No hot-spot (battery/modem) temperature; it is specified to feed the charge cutoff and fan control. |
|
||
| PB12–15 | `BATLED1–4` | unconfigured | Battery gauge dark. Hardware populated; the install/operation manual documents it; installers will read "dead battery." |
|
||
|
||
### 🟠 F4. Blocking sensor read stalls the loop — UART overrun risk, and a Modo-B blocker
|
||
|
||
`readSHT40()` sits in `delay(10)` (plus ~1 ms of blocking I²C traffic at 100 kHz) every
|
||
`THERMAL_PERIOD` (2 s). At 115 200 baud, 10 ms ≈ **115 bytes**; the STM32duino default UART RX
|
||
ring is **64 bytes** unless built with `-DSERIAL_RX_BUFFER_SIZE`. A GNSS epoch burst colliding
|
||
with the read overflows the ring → lost/garbled sentences. For supervision this is survivable
|
||
(next epoch refreshes `lastNmeaMs`), but once the Modo-B relay is enabled the **uplink GGA to
|
||
the caster** shares this loop — dropped bytes there corrupt the NTRIP VRS feed.
|
||
|
||
**Fix (prerequisite for enabling the relay):** make SHT40 a two-phase non-blocking read
|
||
(trigger measurement, harvest ≥10 ms later on the next service pass) and build with
|
||
`SERIAL_RX_BUFFER_SIZE 256` (and TX likewise for the relay). I²C stuck-bus: STM32duino ops have
|
||
internal timeouts (won't trip the 8 s IWDG), but add 9-clock bus recovery at init as hygiene.
|
||
|
||
### 🟠 F5. Thermal derate ignores the battery
|
||
|
||
`thermalDerate` derives from **MCU die temp only** (≥75 °C), while the fan curve correctly uses
|
||
`thermalCtlTemp()` = hottest of air/probe/MCU. The RCA #1 failure is a **hot battery** — derate
|
||
(and the F3 charge cutoff) should key off the control temperature including `tBat`/NTC, with
|
||
per-sensor limits (a 75 °C die is fine; a 60 °C cell is not).
|
||
|
||
### 🟡 F6. GGA parser: empty-quality field and no checksum validation (verified)
|
||
|
||
- Quality field empty (`...,W,,08,...`) → `fixQuality = ',' - '0' = `**−4** (verified in sim).
|
||
Semantics survive (−4 < 4 → unhealthy) but telemetry prints −4. Clamp to 0–8.
|
||
- A GGA with a **bad checksum is accepted** (verified) — line noise during power transitions can
|
||
set bogus fix/sat for one epoch. The XOR is already implemented for `$PHLG`; validate inbound too.
|
||
|
||
### 🟡 F7. Docs/comments contradict the fabbed board
|
||
|
||
`docs/PINOUT.md` still describes PA5 as "conditioned via DFR1015" — **DFR1015 was removed
|
||
pre-fab** (audit H3); the real path is raw 12–24 V → Rig 22 k / Rigb 10 k divider + BAT54 clamp
|
||
(harness truth 2026-07-23). PA4 is described as "PowerBoost rail" (it taps **VEH5V**). The v2.3
|
||
header's Modo-B note is stale (F3). Also note: `INPUT_PULLDOWN` on PA5 puts the internal
|
||
~30–50 k in parallel with Rigb 10 k → divider sits ≈3.2 V at 12 V ignition — still a clean
|
||
logic HIGH, but document it and don't "clean up" the pulldown without redoing that math.
|
||
|
||
### 🟡 F8. SLEEP is not low-power
|
||
|
||
Brownout "SLEEP" keeps the MCU spinning at full clock (~tens of mA). On EinarBoard the MCU runs
|
||
from the *backed-up* rail, so a parked machine drains the 12 Ah pack in roughly two weeks of
|
||
firmware overhead alone. Intent (stay alive to wake on vehicle power) is served today; an STM32
|
||
STOP-mode + EXTI/RTC-wake pass would turn weeks into months. Later rev, not a v3.0 blocker.
|
||
|
||
### 🟡 F9. Minor
|
||
|
||
`newDataReceived` is written, never read (ANSI-TUI leftover). `$PHLG` `nmea_age` grows
|
||
monotonically in SLEEP (cosmetic). `PIN_5V_SENSE` name predates the VEH5V rename.
|
||
|
||
---
|
||
|
||
## 2. What v2.3 gets right (verified — keep all of it)
|
||
|
||
- **Recovery doctrine intact and exact** (sim): backoff gaps 66/126/246/486/966 s → LIMP
|
||
retries every 1806 s; **SPS-only unit is never power-cycled** (guard 5a — 0 cycles in 30 min
|
||
of healthy fix=1); RTK-achieved-then-lost recovers at lost+120 s grace.
|
||
- Non-blocking main loop with IWDG petted once per pass; power-off is the safe state; PA2
|
||
floated on rail-off so GPIO can't back-power the stack.
|
||
- No `String`/heap; fixed 110-byte NMEA buffer with overflow guard; `$PHLG` checksum correct
|
||
(21 fields, verified) and append-only field discipline maintained COLS↔TLM↔$PHLG.
|
||
- EMA-filtered ADC, hysteresis + debounce on power/ignition, 5 s brownout confirmation,
|
||
reset-cause captured before flags clear, sensors-absent fallbacks throughout.
|
||
- The relay pump design (forward verbatim, inject only after ≥8 ms line idle) is the right
|
||
shape for Modo B — it just needs F4's prerequisites before it can be enabled.
|
||
|
||
## 3. Verification performed (2026-07-24)
|
||
|
||
Harness committed at `04-Source-Code/firmware/tools/hostsim/` (run `./run.sh`):
|
||
|
||
- **Stub-compile** of the unmodified sketch: `g++ -std=c++14 -Wall -Wextra` — clean, zero warnings.
|
||
- **Scenario sims** (fake millis/ADC/UART): `ladder`, `sps`, `rtkloss`, `gga`, `phlg` — results
|
||
as cited in §1/§2.
|
||
- **Rollover** modelled separately in **uint32_t** (`rollover_model.cpp`) because host
|
||
`unsigned long` is 64-bit and cannot reproduce target wrap: as-written gate → 91 s cycle
|
||
storm before wrap; wrap-safe rewrite → uniform 1806 s across it.
|
||
- Host sim validates **logic only**, not the STM32 target — the standing rule is unchanged:
|
||
**bench-flash before field.**
|
||
|
||
## 4. v3.0 — the actual production build (proposed scope, in order)
|
||
|
||
1. **Safe fixes on the current doctrine** — F1 wrap-safe recovery window; F2 fan-fault by
|
||
temperature trend + tach behind a flag; F6 checksum + quality clamp; F9 cleanups.
|
||
2. **Board-target split** — one source, `TARGET_EINARBOARD` / `TARGET_PROTO` flag:
|
||
charge-control polarity (**PROD `CHRG_EN` HIGH = charge OFF** vs proto `CHRG_RUN` inverted),
|
||
SHT40+TMP102 vs AHT10, relay on/off, tach off. Kills the class of "flashed the proto build
|
||
on a production unit" errors.
|
||
3. **Charge management block** (the RCA #1 firmware half): read PB1 NTC (β≈3950/Steinhart) +
|
||
TMP102; drive PA8 cutoff from the hottest cell-relevant temp; safety-timer restart
|
||
(`CHRG_EN` toggle when cool ∧ `BAT_SENSE` low ∧ not charging); PB3/PB4 into EVT + new
|
||
TLM/$PHLG fields (append-only); derate from control temp (F5).
|
||
4. **Battery gauge** PB12–15 from `vBatF` with hysteresis (suggest 3.55/3.70/3.85/3.95 V for
|
||
the 4.00 V-charged 1S pack — bench-tune; also sanity-check BAT_CRITICAL/RECOVERY on the
|
||
Hyvel cell).
|
||
5. **Modo-B enablement** — after F4 prerequisites (non-blocking SHT40, 256-byte UART rings):
|
||
`BYP_CTRL` HIGH only in RUNNING (K1 inserts the MCU), LOW + PA9 floated in
|
||
SLEEP/POWER_CYCLE/BOOTING; `ENABLE_PHLG_RELAY 1`; verify GGA-uplink integrity on the bench
|
||
(byte-exact forward, injection only in idle gaps). K1's NC path remains the hardware
|
||
guarantee if firmware dies — do not weaken it.
|
||
6. **Docs**: rewrite `docs/PINOUT.md` for EinarBoard truth (F7); extend LOGGING-SPEC + backend
|
||
`PHLG_FIELDS` for the new columns.
|
||
|
||
Gate: host-sim regressions green (`tools/hostsim`) → bench-flash on a spare EinarBoard →
|
||
soak with the LTC4040 backup-transition + charge-cutoff test (the standing fab-gate test) →
|
||
then fleet.
|
||
|
||
---
|
||
*Related: [[firmware-review]] (v1→v2) · [[watchdog-electrical-design-state]] ·
|
||
[[watchdog-production-schematic-delta]] · `docs/LOGGING-SPEC.md` · CHANGELOG.md ·
|
||
`tools/hostsim/` (this review's harness).*
|