fix(sungrow): a zero-power battery command needs evidence after all

[frahlg]

Takes back #42's exemption. sungrow 1.5.5 → 1.5.6, FTW v2 target 1.3.4 → 1.3.5.

#42 let a zero-power battery command through the guard #40 added, on three grounds. None of them holds.

"Zero hands the device back to itself: forced mode off"

It does not. set_battery_idle writes 13049 = 2 — forced mode on — and pins the battery at 0 W under the host's control. Its own comment, two lines above and untouched by #42, says so: hold the battery at 0W without handing control back to the inverter's autonomous self-consumption mode. The write that returns an inverter to self-consumption is mode 0, in driver_default_mode.

FTW's own two tests distinguish them. TestSungrowDefaultModeReturnsToSelfConsumption asserts 13049 == 0; TestSungrowZeroBatteryCommandForcesIdle asserts 13049 == 2. Run against 1.5.4, the driver reports it itself:

a zero-watt battery command was accepted on a device that has confirmed nothing,
writing [{addr:13049 value:2} {addr:13050 value:204} {addr:13051 value:0}]

#44 states the same fact from the other side, in the reason it gives for not gating set_self_consumption: "since 1.5.4 a zero-watt battery command is accepted on every family and writes EMS mode 2."

"It arrives from the lifecycle rather than the planner"

Not on FTW. The only producer of {"action":"battery","power_w":0} is the dispatch loop in go/cmd/ftw/main.go — planner targets, or the self-tune override that holds every other driver at 0.

Every path that has to release a device without waiting for a poll goes through Registry.SendDefault to driver_default_mode: shutdown (runLoop's stop case), lease expiry (leaseC), the telemetry watchdog, and the stale-site-meter standdown, whose staleSiteDefaultTracker covers every registered driver including one added while the gate is closed. That entrypoint is not gated. driver_init writes self-consumption as well, before the run loop exists to accept a command — and on the control v2 path FTW calls driver_default_mode_v2 itself once driver_init returns.

So "a battery with no way to be told to stop" was not reachable.

"On a string inverter the write fails at the Modbus layer and costs nothing"

The incident says otherwise. first_write_error returns false if any of the three writes errors, so had the SG12RT rejected them the driver would have stopped there and reported failure. It went on to the read-back and reported success — which is only reachable if the writes were taken. Some firmware accepts a write to a register it does not implement.

What the exemption cost

Mode 2 and a setpoint back on an SG inverter that implements neither, with the verifying read-back failing on that same device, so the host records an applied setpoint and renews the lease on a battery that is not there. That is the SG12RT bug, through the one action nobody thought to guard.

Worse in one respect than before #40: under 1.5.4 a zero command could put an SG inverter into forced mode, and no battery command could take it out again, because non-zero stays refused. Only default mode clears it.

Tests

Four cases, replacing #42's two:

• zero is refused before the first poll, on both copies — polls=0, the order Registry.Add actually produces
• zero writes 13049=2,13050=204,13051=0 on a confirmed hybrid — the same three FTW asserts, which is the proof that zero takes the inverter over rather than releasing it
• the release is never gated, on both copies
• driver_init reaches self-consumption on its own

The last two are load-bearing: they are why refusing is safe. Both are proven to fail when the behaviour is put back — gate driver_default_mode and both it and #44's boundary test fail; exempt zero again and the first fails.

#44 keeps its conclusion and loses its premise

Not gating set_self_consumption is still right, for a reason that does not depend on the exemption: an inverter can arrive already holding mode 2 — a container that died mid-force, an installer app, a version that wrote it before restarting — and a hybrid shipped under a device-type code classify_device_type has not been taught lands in the "string" branch too. test_the_release_is_still_reachable_on_a_string_inverter now seeds that state rather than creating it with a zero command, and covers driver_init as well as the watchdog and cleanup. The known_to_have_no_battery comment and the SG-RT / SG-CX manifest notes are updated to match.

Keeps #42's host_mock.lua fix, which was right and independent of the decision.

Verified

make check — 3110 passed, 1361 skipped.

FTW's entire Go suite passes with 1.5.6 dropped into drivers/sungrow.lua, including TestSungrowZeroBatteryCommandForcesIdle. The matching FTW change is already on srcfl/ftw#704's branch: name the device at 4999 and call Init before the command, which is the order production always has, plus a paired test that the refusal is deliberate and the release still works on the same unproven device.

The pin on srcfl/ftw#704 should move to this commit rather than 12f40d1 once this merges.

🤖 Generated with Claude Code

[frahlg]

Flagging for human review rather than merging — this PR now directly contradicts a decision already merged to main.

The conflict: This branch was cut from 12f40d1 (before #42 landed). #42, "fix(sungrow): a zero-power battery command is never refused" (merged as edb32ff, ~2h after this PR opened), changed driver_command so power_w == 0 bypasses the model_family/battery_confirmed guard entirely and is always accepted — with its own pytest cases pinning exactly that behavior.

This PR's whole thesis is the opposite: it adds tests asserting a zero-watt command must be refused (ACCEPTED == "false", CODE == "no_battery") before the first poll. Those new tests (test_zero_watts_is_still_a_battery_command, plus the zero_command_before_any_poll helper) will fail against the driver as it exists on main right now.

It's also a real git conflict, not just a logical one — mergeable_state is dirty and a local test-merge of this branch into current main conflicts in drivers/tests/test_sungrow_model_family.py (both sides edited the same "zero watts before the first poll" test block from different starting points).

Why I'm not resolving this myself: both sides make a genuine safety-tradeoff argument (this PR: never write mode 2 + a setpoint to an unconfirmed device; #42: never let a stop command be gated, because a device stuck in a forced state is worse). That's a design call about battery-control safety, not something to pick between mechanically. Someone should decide which behavior is correct, then either:

• close this PR if fix(sungrow): a zero-power battery command is never refused #42's resolution (already on main) is the intended final answer, or
• rebase this PR onto current main and update it to argue against/undo fix(sungrow): a zero-power battery command is never refused #42 explicitly if the refusal is actually meant to stand.

No CI checks have run on this PR (get_status/get_check_runs both come back empty), so this hasn't even been machine-verified against current main yet. No other action taken on this PR — not merging, not closing.

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd1663334b

ℹ️ 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]

Require the v2 release to report defaulted

When the v2 mode write succeeds but its readback fails, driver_default_mode_v2 returns {status = "failed"} while this expression still prints true; because register 13049 was already written to zero, the EMS-mode assertion also passes. The test therefore misses a failed structured result on the safety-release path, so compare the status explicitly with "defaulted".

AGENTS.md reference: AGENTS.md:L17-L18

Useful? React with 👍 / 👎.

[frahlg]

Scheduled review pass: this PR can't merge as-is, and the reason isn't mechanical.

main already carries #42 (fix(sungrow): a zero-power battery command is never refused), which changed driver_command to:

if power_w ~= 0 and model_family ~= "hybrid" and not battery_confirmed then

i.e. a zero-watt battery command is now unconditionally accepted, exempt from the family/battery-confirmation guard, specifically so it can run before the first poll.

This PR's new tests (test_zero_watts_is_still_a_battery_command, test_the_release_is_never_gated, etc.) assert the opposite: that a zero-watt command must be refused before the first poll on a device with no confirmed family, with code no_battery. That's not a merge-conflict-in-the-diff problem (though it is also that, in drivers/tests/test_sungrow_model_family.py) — it's a direct contradiction of behavior #42 already shipped on a driver with control: true.

This branch (claude/festive-elbakyan-1cd864) was cut from 12f40d1, before #42 merged, so it argues against #40's guard without being aware #42 already resolved the same srcfl/ftw#704 stall by exempting zero rather than refusing it.

Given this is battery-command safety semantics on a live control-capable driver, I'm not resolving this by picking a side autonomously. Could a maintainer confirm which behavior is intended — #42's "zero always lands" or this PR's "zero refused until a poll confirms the device"? Whichever is correct, the other needs to be reverted or this PR closed as superseded.

---

Generated by Claude Code

[frahlg]

Superseded by #45, which takes this PR's position (a zero-power battery command needs evidence, same as here) combined with #44's startup gate, and lands on top of #44 instead of predating it. Closing this one in favor of #45.

---

Generated by Claude Code