Skip to content

fix(control): a driver that cannot actuate gets its autonomous default - #800

Merged
frahlg merged 2 commits into
masterfrom
harvest/driver-failure-default
Aug 4, 2026
Merged

fix(control): a driver that cannot actuate gets its autonomous default#800
frahlg merged 2 commits into
masterfrom
harvest/driver-failure-default

Conversation

@frahlg

@frahlg frahlg commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

AGENTS.md states the law plainly: a failed/stale driver receives its autonomous default mode. FTW does the stale half well. The failed half was not done at all.

Two cases reached no default. They are the same condition seen from opposite ends of the wire, so they get one mechanism and one latch.

(a) A device fault never reached the default. A driver that flags DeviceFault keeps polling, so WatchdogScan — which keys on LastSuccess alone — saw nothing wrong and produced no transition. The driver left onlineBats, received no command as a result, and held its last accepted setpoint for as long as the fault lasted. Before this PR, DeviceFault had no reader anywhere in go/cmd/ftw outside home_link.go. DeviceFault means "cannot actuate", which is precisely the condition the autonomous default exists for. Three shipped drivers raise it: ferroamp.lua, sungrow.lua, pixii.lua.

(b) A command-refusing driver stayed "healthy". A v1 driver that answers every poll and rejects every command had its error logged and discarded by sendDriverCommand. It kept Status=ok, stayed inside onlineBats and inside the MPC fleet, and the power the plan counted on but never got silently became grid import. #741 gave the registry a per-command restoreAfterCommand, so the device does now get a default write after each refusal — but nothing told the rest of the system to stop counting on it, so the loop ran on forever: dispatch, refuse, default, dispatch, refuse, default, once per control tick.

First commit: one offline surface, not two

Asked separately, before building anything on top.

  • DriverHealth.SetOffline()dead in production. Zero runtime callers; WatchdogScan set h.Status = StatusOffline inline and only tests reached the method. Wired, so staleness has one writer, with a doc comment naming the caller that owns it.
  • Store.DriverHealthMut()not dead. It is the documented test-setup escape hatch ("Runtime code should use the Store RecordDriver* helpers"), used by ~50 tests. Kept.

No behaviour change in that commit.

The mechanism

driverActuationTracker (go/cmd/ftw/driver_failure_default.go) copies the shape of staleSiteDefaultTracker: send the declared default exactly once per transition, re-arm on recovery. Both (a) and (b) are inputs to it.

Exclusion needs no new path. Both conditions land in telemetry as DeviceFault, so IsOnline() — the predicate ComputeDispatch and onlineFleetParams already share — drops the driver from dispatch and from the plan, and every existing operator surface (/api/health "fault", the driver inventory, the support report) reports it without a line changed.

DeviceFault becomes derived from two sources that cannot overwrite each other. This is the one piece of new structure, and it earns its place: a driver re-asserts its own view on every poll — sungrow.lua calls set_device_fault(false, "") each time it does not see a fault — so a single shared boolean would have let a driver that believes the device is fine clear a core-set fault on its next poll, flipping the driver in and out of the MPC fleet for as long as the refusals lasted. driverFault is the driver's verdict, commandFault is core's; DeviceFault is their OR, and the driver's reason wins when both are set because it saw the device.

Refusals: three in a row, matching RecordError's existing degrade threshold — one rejected Modbus write is a normal event on a busy device, three in a row at control cadence is not. One command is let through every five minutes so an inverter that rejects writes through a firmware restart comes back without an operator; a single fresh refusal puts it straight back out.

Not counted as refusals, because each is a fault another mechanism already owns and double-booking it would push a merely slow driver out of control: ErrObserveOnly (the registry refused on the driver's behalf and never touched the device), ErrControlBlocked (the registry is already holding the driver in its default and retrying with backoff), and deadline/cancel (a wedged driver stops emitting telemetry too, and the staleness watchdog walks it to its default — the same reasoning sendDriverCommand's timeout handling already carries from #791).

What this trades

New commands now reach hardware on a path that previously sent none. A driver reporting a device fault, or one that has refused three commands, now receives driver_default_mode where before it received nothing at all.

That is the intended direction, and the trade is explicit: one write of the driver's own declared safe state, against a battery holding a stale setpoint into a fault indefinitely. A Ferroamp in Fault Mode that was last told to discharge at 5 kW keeps that setpoint until somebody restarts something; a Sungrow that stopped taking writes keeps whatever it last accepted while the plan spends energy it is not delivering. The default is the driver's own declaration, not core's guess at one, and observe-only drivers receive nothing — not even this.

A driver that provably rejects the default release is reported as default held, not as a failure (set_self_consumption in sungrow.lua returns true for an inverter that was never under control). Nothing here escalates on it: only dispatch commands feed the refusal counter, never the default release itself.

Not in scope

A driver stuck returning ErrControlBlocked — because its own default keeps failing — is still counted by the MPC fleet while the registry retries it with backoff. Same "silently counted" shape through a third door. Left out deliberately: the registry owns that state machine, and #798 is currently changing exactly that path. Worth a follow-up once #798 lands.

Verification

  • go test ./internal/control/ -run TestGoldenCorpusno record moved.
  • make verify clean (pre-commit gate on both commits); go test -race on cmd/ftw, telemetry, control, mpc, drivers.
  • Each new test was checked against a mutation of the code it covers: removing the exclusion fails the (b) tests, removing the latch fails the (a) test, collapsing the two fault sources fails the poll/command-fault test.
  • No overlap with any open PR's files. fix(control): keep autonomous defaults ahead of stale commands #798 touches only registry.go; it decides how a default gets through, this decides when one is owed.

Tests

  • driver_failure_default_test.go — device fault reaches the default once per transition and re-arms; a stale driver is left to the watchdog; observe-only receives nothing; refusals exclude and default once; the retry window re-admits, re-excludes on a fresh refusal, and clears on an accepted command; non-refusal errors never exclude; a removed driver is forgotten and never resurrected.
  • internal/control — a battery that refuses commands gets no dispatch target and the load is reallocated to its healthy sibling.
  • internal/mpc — a refusing battery is dropped from onlineFleetParams: its capacity and its SoC leave the plan.
  • internal/telemetry — a driver's own poll cannot clear a core-set command fault; the two sources are independent; a command fault never creates a health record.

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9b72b7aae4

ℹ️ 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".

Comment on lines +237 to +240
case errors.Is(err, drivers.ErrObserveOnly):
return false
case errors.Is(err, drivers.ErrControlBlocked):
return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Count refusals that also report blocked recovery

When a command is refused and the registry's automatic default restoration also fails, restoreAfterCommand returns a joined error that matches both the underlying refusal and ErrControlBlocked. This early case therefore discards the initial refusal, and every later dispatch returns bare ErrControlBlocked and is discarded as well. If the driver keeps polling, it never reaches the refusal threshold, remains online in dispatch and MPC, and the plan continues relying on a device for which control is blocked indefinitely.

Useful? React with 👍 / 👎.

Comment thread go/cmd/ftw/main.go
}
payload, _ := json.Marshal(map[string]any{"action": "battery", "power_w": t.TargetW})
sendDriverCommand(ctx, reg, "driver send", t.Driver, payload, driverCmdTimeout)
actuation.dispatchCommand(ctx, reg, "driver send", t.Driver, payload, driverCmdTimeout, tickNow)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Track refusal outcomes from EV and PV dispatch too

The tracker is wired only around the storage finalTargets loop. In the inspected control tick, loadpoint commands still go through lpController.TickWithDispatch using reg.Send, and PV curtailment still calls sendDriverCommand directly, so chargers and PV-only drivers that answer polls but reject every command remain online and continue contributing to plans indefinitely. Route those dispatch results through the same tracker (or expose an equivalent outcome hook) so the advertised unable-to-actuate behavior applies to every controlled driver.

Useful? React with 👍 / 👎.

frahlg and others added 2 commits August 4, 2026 17:53
DriverHealth.SetOffline had no runtime caller: WatchdogScan set
h.Status = StatusOffline inline and the method was reached only from
tests. Two ways to take a driver offline, one of them dead in
production, is one too many for the surface the next change lands on.

Wire the watchdog through SetOffline so staleness has a single writer,
and say in the doc comment which caller owns it.

DriverHealthMut is the other candidate and is not dead: it is the
documented test-setup escape hatch ("Runtime code should use the Store
RecordDriver* helpers") and ~50 tests use it. It stays.

No behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AGENTS.md states the law plainly: a failed/stale driver receives its
autonomous default mode. Stale was covered. Failed was not.

Two cases reached no default, and they are the same condition seen from
opposite ends of the wire.

A driver that flags DeviceFault keeps polling, so WatchdogScan — which
keys on LastSuccess alone — saw nothing wrong and produced no
transition. The driver left the dispatch set, received no command, and
held its last setpoint for as long as the fault lasted. DeviceFault had
no reader anywhere in go/cmd/ftw.

A driver that answers every poll and rejects every command stayed
Status=ok. It stayed inside onlineBats and inside the MPC fleet, its
error logged and discarded, and the power the plan counted on but never
got became grid import.

One tracker covers both, shaped after staleSiteDefaultTracker: send the
declared default exactly once per transition, re-arm on recovery. Both
conditions land in telemetry as DeviceFault, so IsOnline() — the
predicate ComputeDispatch and the MPC fleet already share — drops the
driver from both without a new exclusion path.

DeviceFault becomes derived from two sources that cannot overwrite each
other. Without the split, a driver that believes the device is fine
would clear a core-set fault on its next poll and the two would flip the
driver in and out of the fleet for as long as the refusals lasted.

Refusals are counted only for dispatch commands, three in a row, with
one command let through every five minutes so a device that recovers on
its own comes back without an operator. Deadlines, observe_only and
control-blocked are not refusals — each is a fault another mechanism
already owns, and the staleness watchdog already walks a wedged driver
to its default.

Commands now reach hardware on a path that previously sent none: a
faulted driver receives driver_default_mode. That is the trade — one
write of the driver's own declared safe state against a battery holding
a stale setpoint into a fault, indefinitely. A driver that rejects the
default release is reported as held, not failed, and nothing escalates
on it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@frahlg
frahlg force-pushed the harvest/driver-failure-default branch from 9b72b7a to 2724cce Compare August 4, 2026 15:57
@frahlg
frahlg merged commit 7a1252a into master Aug 4, 2026
13 checks passed
@frahlg
frahlg deleted the harvest/driver-failure-default branch August 4, 2026 15:59
frahlg added a commit that referenced this pull request Aug 5, 2026
… command (#805)

#800 established the law — a driver that cannot actuate gets its
autonomous default and stops being counted on — and wired exactly one
path into it. The reviewer named the gap and its author confirmed it:
only the storage loop filed an outcome. PV curtail discarded its error
outright and the loadpoint controller only logged one.

So the bug #800 fixed for batteries was still live for the other two.
A wallbox that answers every poll and refuses every setpoint held the
current it last accepted while the plan went on booking the charge and
the surplus reserve went on being held back from the battery for an EV
that was not drawing. An inverter that refused every cap kept exporting
into a negative price while the plan booked the saving.

Route both into the same tracker. The wiring is small; what took the
work was deciding which command on each path is the one whose refusal
means core cannot put power where it asked. Getting that wrong excludes
a healthy device, which is worse than the bug.

PV curtail: the cap counts, the release does not. A `curtail_disable`
is core letting go of the inverter, so refusing it proves nothing about
the device — the same reading #800 gives a rejected set_self_consumption
in sungrow.lua. It would also seal shut: ComputePVCurtail emits a
release the moment a driver drops offline, so a counted refusal would
let an excluded inverter hold itself out on its own exclusion.

Loadpoint: the periodic ev_set_current counts. Four other sends do not,
and each for its own reason. The 0 W standdown is core withdrawing
under a stale site meter, a transition the staleness tracker already
owns. `charge_start` goes to the bound vehicle driver, and a parked car
refuses it whenever it is asleep — counting that would take the car's
SoC out of the plan for napping, and wakeVehicleAuto already backs off
on its own. The ev_pause/ev_resume contactor cycle is documented as
free for any charger implementing those actions, so a charger that
implements neither returns an error and is behaving correctly; it also
runs on its own goroutine, which the tracker is not built to take. The
operator's force-start and refresh are not dispatch at all.

Nothing about what counts as a refusal changes: ErrObserveOnly,
ErrControlBlocked and deadline/cancel remain faults another mechanism
owns. Observe-only drivers still receive no command of any kind.

Deliberately still out of scope, as in #800: a refusal joined with
ErrControlBlocked is not counted, so such a driver stays in the MPC
fleet. #798 owns that registry state machine.

Risk direction, stated the way #800 did it: commands now reach hardware
on paths that previously sent none. An excluded PV inverter receives
its driver's declared default where it previously received nothing.
Both shipped EV chargers declare a no-op default — easee_cloud.lua and
ctek.lua both say a wallbox has no autonomous equivalent and should
hold its last current — so an excluded charger receives no new write at
all, and the exclusion is pure accounting.

Golden corpus: no record moved, as predicted. This changes command
outcome accounting, not dispatch arithmetic, and the replay never goes
through the control tick.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant