P1AM firmware safety: comms watchdog, correct trip tier, bumpless setpoints

[dieterolson]

Closes #3999. Closes #4001. Closes #4002. Closes #4032. Refs #4009.

Stacked on #4043 (the harness repair) — that PR is what makes any of this testable. Merge #4043 first; this targets its branch and will retarget to main automatically.

Every fix here has a failing host test written first.

#3999 — no dead-man timer on the SCADA link

The heater relay is a coil read and the analog outputs are driven from broker tags. Both held their last commanded value forever once the host died, and the host cannot cover this case because the host is the thing that died.

New CommsWatchdog (firmware/CommsWatchdog.{h,cpp}) drives all analog outputs to 0 %, opens the heater relay, asserts Inhibit, and holds the PID loops after 2 s of silence. Two independent activity signals, because each misses a case the other catches:

• a live Modbus TCP connection — covers host power loss, a killed backend, a pulled cable (all drop the socket);
• a host heartbeat register (holding register 560) — covers a wedged backend holding an idle socket open.

Either re-arms it. It is armed in setup() before the loop, so a PLC that boots into a dead network safes itself rather than sitting energized waiting for a host that is not coming.

The class is deliberately free of Arduino headers — the caller supplies millis(). That is what makes the rollover behaviour testable, and it matters: millis() wraps roughly every 49.7 days, and a naive comparison would disarm the watchdog for another 49 days at the wrap. TestCommsWatchdog asserts the wrap explicitly.

#4001 — the trip fired on the warning band, over unrouted tags, unrecoverably

Three compounding problems:

• Evaluate() compared against low/high, which is the SCADA layer's severity-1 warning tier.
• It evaluated all 32 tags, including unrouted ones sitting at 0.0.
• ClearTrip() had zero call sites and coil 1 was never read, so the latch was unrecoverable short of a power cycle — after which the flash-saved config tripped it again on the first scan.

Since the stock config writes low=5.0 to every tag, deploying it latched the plant off permanently.

Now: Evaluate() trips on hihi/lolo, and skips tags that are not routed as an input or output. The routing predicate is SignalBroker::IsTagRouted — the broker owns routing, so it lives there rather than being duplicated in the interlock. Coil 1 clears the latch and is written back to 0 as a pulse acknowledgement, so the host can see the reset was consumed; Evaluate() re-trips on the next scan if the condition persists, which TestInterlockTripIsRecoverable pins.

#4002 — a wound-up integrator survived the E-stop

SetSetpoint did not touch the integrator, and Compute() ran even while tripped. Zeroing the PID setpoints is the only part of the host E-stop that reaches the plant, so the integral term alone held the analog output at 100 % of its 4-20 mA span for tens of seconds after the operator commanded a stop.

SetSetpoint now clears integral and derivative history on a change — bumpless transfer, which is the right behaviour for ordinary retargeting too, not just E-stop. Hold()/Release() freeze the loops while tripped or blind so recovery starts from a clean integrator instead of slamming the output the moment the trip clears.

#4032 — limits outside the tag domain silently disabled their trip

Broker tags are clamped to [0, 100], so any limit above 100 could never be crossed. An operator entering 900 meaning 900 °C on a percent-scaled tag disabled the interlock with no indication, and the HMI displayed the limit exactly as configured.

SafetyInterlock::IsLimitEffective distinguishes a deliberate ±99999 "never trip" sentinel from an unreachable entry. Evaluate() skips both rather than comparing against a threshold that cannot be crossed, and the host can call the same predicate to reject the configuration at the API boundary — that backend half is tracked separately.

Also: firmware half of #4009

The scan integrated against a hardcoded 0.1f while the real period runs well past that — the same iteration does ~300 register reads, SPI thermocouple reads, and on a config deploy a blocking flash write. Understating dt understates Ki and overstates Kd for exactly the scans that overran. It now uses the measured interval, bounded to [1 ms, 1 s] so a long stall cannot inject a huge integral step or a near-zero derivative divisor.

Note on a changed test

TestSafetyInterlock encoded the old warning-band-trips behaviour (high=75, tag at 80 → trip, on an unrouted tag). That is the defect, not the contract, so the test is updated to the corrected semantics rather than the behaviour being preserved to keep it passing.

Register map

Holding register 560 is new (host heartbeat). configureHoldingRegisters is widened accordingly — the count argument is one past the highest address, so it is now kHostHeartbeatReg + 1. The firmware README register-map table is updated, and documents the trip semantics and watchdog behaviour.

Verification

All 12 host suites pass under g++ 11.4 (WSL Ubuntu-22.04):

TestSignalBroker / TestPIDController / TestSafetyInterlock / TestStorageManager
TestSoftFailRuntimeContracts / TestInterlockTripsOnHihiLoloNotHighLow
TestInterlockIgnoresUnroutedTags / TestInterlockTripIsRecoverable
TestPidResetsIntegralOnSetpointChange / TestPidDoesNotIntegrateWhileTripped
TestCommsWatchdog / TestInterlockLimitDomain
All C++ Core Firmware Tests Passed Successfully!

The four non-Arduino translation units also pass g++ -fsyntax-only -Wall -Wextra. The sketch itself is covered by the arduino-cli gate added in #4043 — it cannot be compiled locally without the board package.

Backend work this unblocks

• The backend must write the heartbeat register each scan, or the watchdog falls back to socket liveness alone.
• The backend should call IsLimitEffective's equivalent at /api/routing to reject unreachable limits.
• clear_estop() can now read the coil back to confirm the reset actually happened, instead of returning True on a write to a coil nothing read.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e07b9284d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Keep the watchdog timeout above the normal polling cadence

The backend starts in PerformanceMode.LIGHTWEIGHT (backend/main.py) with a 2.0-second interval (backend/settings.py), sleeps for that interval only after completing _poll_once, and never writes heartbeat register 560. Consequently, even if each Modbus request re-arms the watchdog, normal processing time and scheduling jitter make the silence exceed this exact 2000 ms timeout; if only new connections re-arm it, the persistent connection fares worse. Default/lightweight operation will therefore repeatedly hold the PIDs and force all outputs safe despite a healthy host. Add timeout margin or implement a faster independent heartbeat.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Initialize scan timing after setup completes

On a boot where setup takes more than one second—which is expected when the serial wait alone can last up to five seconds—lastScanTime is still its global zero value, so the first scan computes dt from the entire boot duration and clamps it to 1 second. A persisted PID with nonzero Ki therefore integrates ten nominal scans on its first execution and can immediately saturate its analog output. Initialize lastScanTime at the end of setup() or treat the first control scan as one nominal interval.

Useful? React with 👍 / 👎.