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

CHANGELOG.md

@@ -8,11 +8,12 @@ Driver versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html
 ## [Unreleased]
 
 ### Fixed
+- **sungrow** 1.5.6 and its **FTW v2 target** 1.3.5 — **a zero-power battery command needs the same evidence as any other, after all.** 1.5.4 exempted zero on three grounds and none of them holds. *Zero hands the device back:* it does not — `set_battery_idle` writes EMS mode **2**, forced, and pins the battery at 0 W under the host's control, as its own comment two lines above says; the write that returns an inverter to self-consumption is mode 0, in `driver_default_mode`. FTW's two Go tests distinguish them, one asserting `13049 == 0` and the other `13049 == 2`. *It arrives from the lifecycle:* not on FTW, where the only producer of a battery command is the dispatch loop, while every path that has to release a device — shutdown, lease expiry, the telemetry watchdog, the stale-site-meter standdown — calls `driver_default_mode`, which is not gated; `driver_init` writes self-consumption before the host can deliver a command at all. *The write fails harmlessly on hardware without the registers:* the incident says otherwise — `first_write_error` would have stopped the driver had the SG12RT rejected the writes, and it went on to the read-back and reported success. So refusing withholds nothing that is not reachable by a route which cannot refuse, and the exemption put mode 2 and a setpoint back on an SG inverter that implements neither, with the read-back that would catch it failing on that same device. Worse in one respect than before #40: under 1.5.4 a zero command could put an SG inverter *into* forced mode with no battery command able to take it out, since non-zero stayed refused. Four pytest cases carry the reasoning, two of them load-bearing and both proven to fail when the behaviour is put back — the release is never gated, on either copy, and `driver_init` reaches self-consumption on its own
 - **sungrow** 1.5.5 — **Startup asked a family with no battery about its battery limits.** `configure_power_limits` reads 33046, 33047 and 13057 — charge power, discharge power, and the SoC ceiling and floor — and raises the first two when the unit shipped capped. A model that named itself a string inverter answers none of them, so an SG12RT paid three failed reads on every restart to learn nothing. It now skips them. These run once rather than on every poll, so this is not the SG12RT outage returning; the reason to gate them is that they are free to gate. Each write is already conditional on its read having answered, and none of them clears a forced state
-- **The wider version of this change was wrong, and the test that says so ships with it.** The first draft gated `set_self_consumption` on the same family test, covering `driver_init`'s startup reset, the watchdog, `driver_cleanup` and `deinit` — three writes at startup and three on every watchdog tick, aimed at a block the family has no use for. But since 1.5.4 a zero-watt command is accepted whatever the family, and it writes EMS mode 2. So "the device named itself a string inverter" is not proof there is nothing to release: Sungrow shipping a hybrid under a device-type code `classify_device_type` has not been taught lands in exactly that branch, takes the forced write, and is then left with no path that writes mode 0 back. Measured: mode 2 survived the watchdog and cleanup both. `test_the_release_is_still_reachable_on_a_string_inverter` fails if the gate is put back
+- **The wider version of this change was wrong, and the test that says so ships with it.** The first draft gated `set_self_consumption` on the same family test, covering `driver_init`'s startup reset, the watchdog, `driver_cleanup` and `deinit` — three writes at startup and three on every watchdog tick, aimed at a block the family has no use for. But "the device named itself a string inverter" is not proof there is nothing to release. An inverter can arrive already holding EMS mode 2 — a container that died mid-force, an installer app, a version that wrote it before restarting — and Sungrow shipping a hybrid under a device-type code `classify_device_type` has not been taught lands in that branch as well. Gate the release and such a device is left with no path that writes mode 0 back. Measured: mode 2 survived the watchdog and cleanup both. `test_the_release_is_still_reachable_on_a_string_inverter` fails if the gate is put back
 - The two failed reads left at startup are both at 13049 and both deliberate — `set_self_consumption`'s readback, which must run on every family, and the EMS-state log, which is the one place a device misclassified as `string` while holding mode 2 would show itself
 - Neither package target changes. `packages/v1/sungrow/targets/ftw.lua` maps `curtail` onto 13050 and 13049 rather than the Active Power Limitation pair, so its `driver_default_mode_v2` is the release for a curtail it accepts on a string inverter — the same reasoning again, and that register map belongs to the review `packages/v1/sungrow/PILOT.md` already requires. `packages/v1/sungrow/targets/ftw-observe.lua` is read-only by construction
-- The **SG-RT** and **SG-CX** manifest notes named 1.2.1, which stopped the reads, and said nothing about the writes. They now name 1.5.3, which refuses a battery dispatch on those families, and the deliberate exception 1.5.4 made for zero
+- The **SG-RT** and **SG-CX** manifest notes named 1.2.1, which stopped the reads, and said nothing about the writes. They now name 1.5.3, which refuses a battery dispatch on those families, and record that 1.5.4's exception for zero was taken back in 1.5.6
 
 ### Fixed
 - **sungrow** 1.5.4 and its **FTW v2 target** 1.3.4 — **a zero-power battery command is never refused.** 1.5.3 required the model to have named itself a hybrid or a battery register to have answered, which was right for a dispatch and wrong for zero. Zero is not a dispatch: it is the host handing the device back to itself, forced mode off and setpoint nought. It arrives from the lifecycle rather than the planner, so it lands before the first poll — exactly when nothing has been confirmed. Refusing to write *stop* is a different risk from refusing to write *charge*, because a device left in a forced state stays there. On a genuine string inverter the write fails at the Modbus layer and costs nothing; the outage it prevents is a battery with no way to be told to stop. Non-zero setpoints still need evidence, so #40's hole stays shut

SUPPORT_STATUS.md

@@ -144,8 +144,8 @@ Catalog source is not proof that a target can install or run a driver.
 | solis_string | 1.1.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no |
 | sonnen | 2.0.1 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no |
 | sonnen | 2.0.1 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no |
-| sungrow | 1.5.5 | ftw-core | observe_only_profile_blocked | 1.3.3 | 1.3.0 | exact_model_firmware_required | — | not_assessed | no |
-| sungrow | 1.5.5 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no |
+| sungrow | 1.5.6 | ftw-core | observe_only_profile_blocked | 1.3.3 | 1.3.0 | exact_model_firmware_required | — | not_assessed | no |
+| sungrow | 1.5.6 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no |
 | tesla_vehicle | 0.2.0 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no |
 | tesla_vehicle | 0.2.0 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no |
 | tibber | 1.1.0 | ftw-core | not_assessed | — | — | not_recorded | — | not_assessed | no |

devices.yaml

@@ -1722,29 +1722,29 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.5"
+            version: "1.5.6"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
-        notes: "Commercial/industrial three-phase string inverter. Needs 1.5.3: 1.2.1 stopped polling the SH battery registers this family does not answer, and 1.5.3 refuses a battery dispatch instead of writing them. A zero-watt command is still written on every family from 1.5.4 -- it is the release, and on a model without the registers it fails harmlessly."
+        notes: "Commercial/industrial three-phase string inverter. Needs 1.5.3: 1.2.1 stopped polling the SH battery registers this family does not answer, and 1.5.3 refuses a battery dispatch instead of writing them. 1.5.4 exempted a zero-watt command from that refusal; 1.5.6 takes the exemption back, because zero writes EMS mode 2 rather than releasing the device. The release is driver_default_mode, which is never gated."
       - name: "SG-RT (Three-Phase String)"
         variants: [SG10RT, SG12RT, SG15RT, SG17RT, SG20RT, SG25RT, SG5.0RT, SG6.0RT, SG7.0RT, SG8.0RT]
         regions: [EU]
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.5"
+            version: "1.5.6"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
-        notes: "Three-phase string inverter, PV + meter only (no battery). Needs 1.5.3: 1.2.1 stopped polling the SH battery registers this family does not answer, and 1.5.3 refuses a battery dispatch instead of writing them. A zero-watt command is still written on every family from 1.5.4 -- it is the release, and on a model without the registers it fails harmlessly."
+        notes: "Three-phase string inverter, PV + meter only (no battery). Needs 1.5.3: 1.2.1 stopped polling the SH battery registers this family does not answer, and 1.5.3 refuses a battery dispatch instead of writing them. 1.5.4 exempted a zero-watt command from that refusal; 1.5.6 takes the exemption back, because zero writes EMS mode 2 rather than releasing the device. The release is driver_default_mode, which is never gated."
       - name: "SH-RS (Single-Phase Hybrid)"
         variants: [SH3.0RS, SH3.6RS, SH4.0RS, SH5.0RS, SH6.0RS]
         regions: [EU]
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.5"
+            version: "1.5.6"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1755,7 +1755,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.5"
+            version: "1.5.6"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1766,7 +1766,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.5"
+            version: "1.5.6"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""

drivers/lua/sungrow.lua

@@ -9,7 +9,7 @@ DRIVER = {
   id           = "sungrow-shx",
   name         = "Sungrow SH Hybrid Inverter",
   manufacturer = "Sungrow",
-  version      = "1.5.5",
+  version      = "1.5.6",
   protocols    = { "modbus" },
   capabilities = { "meter", "pv", "battery", "pv-curtail" },
   description  = "Sungrow SH-series hybrid inverters with LFP battery, via Modbus TCP.",
@@ -139,13 +139,13 @@ end
 -- Whether the device has named itself a model with no battery registers.
 --
 -- Read this narrowly. It gates startup *reads* of the battery limit block and
--- nothing else. It must never gate a write that clears a forced state: since
--- 1.5.4 a zero-watt command is accepted on any family, so a device this
--- function calls "string" can still be holding EMS mode 2, and
--- set_self_consumption is the only thing that writes mode 0 back. A device
--- classified here is not proof there is nothing to release -- Sungrow shipping
--- a hybrid under a device-type family classify_device_type has not been taught
--- lands in exactly this branch.
+-- nothing else. It must never gate a write that clears a forced state. A
+-- device this function calls "string" is not proof there is nothing to
+-- release: an inverter can arrive already holding EMS mode 2 -- a container
+-- that died mid-force, an installer app, a previous version -- and
+-- set_self_consumption is the only thing that writes mode 0 back. Sungrow
+-- shipping a hybrid under a device-type family classify_device_type has not
+-- been taught lands in exactly this branch too.
 local function known_to_have_no_battery()
     return model_family == "string"
 end
@@ -747,20 +747,22 @@ function driver_command(action, power_w, cmd)
     if action == "init" then
         return true
     elseif action == "battery" then
-        -- Zero is not a dispatch. It is the host handing the device back to
-        -- itself: forced mode off, setpoint nought. Refusing to write "stop"
-        -- is a different risk from refusing to write "charge" -- a device
-        -- left in a forced state stays there, and the safe default is the
-        -- one path that must never be gated on how much we know about the
-        -- hardware. It also arrives before the first poll, from the
-        -- lifecycle rather than from the planner, which is exactly when
-        -- nothing has been confirmed yet.
+        -- Zero is not exempt, and 1.5.4 was wrong to exempt it. Zero does not
+        -- hand the device back: set_battery_idle writes EMS mode 2 -- forced
+        -- -- and pins the battery at 0 W under the host's control. The write
+        -- that returns an inverter to its own self-consumption is mode 0, and
+        -- it lives in driver_default_mode. Nor does zero arrive from the
+        -- lifecycle: on FTW the only producer of a battery command is the
+        -- dispatch loop, while every path that has to release a device --
+        -- shutdown, lease expiry, the telemetry watchdog, the stale-site-meter
+        -- standdown -- calls driver_default_mode, which is not gated and never
+        -- should be. driver_init writes self-consumption too, before the host
+        -- can deliver a command at all.
         --
-        -- On a genuine string inverter this writes to a register block the
-        -- model does not implement. That write fails at the Modbus layer and
-        -- costs nothing; the outage it protects against is a battery left
-        -- charging with no way to say stop.
-        if power_w ~= 0 and model_family ~= "hybrid" and not battery_confirmed then
+        -- So refusing withholds nothing that is not reachable by a route which
+        -- cannot refuse, and exempting zero put mode 2 and a setpoint back on
+        -- an SG inverter that implements neither.
+        if model_family ~= "hybrid" and not battery_confirmed then
             local why = model_family == "string"
                 and "this model has no battery registers"
                 or "no battery register has answered on this device"