fix(sungrow): stop writing an EMS block the inverter has already refused

CHANGELOG.md

@@ -7,6 +7,15 @@ Driver versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html
 
 ## [Unreleased]
 
+### Fixed
+- **sungrow** 1.5.7 — **The driver kept writing a register block the inverter had already refused.** An SG12RT in the field logged `self-consumption reset write failed: modbus exception function=0x06` once per watchdog tick, indefinitely. `driver_init`'s startup reset, the watchdog, `driver_cleanup` and the `deinit` action all write 13049-13051, and nothing counted how often the device said no. The EMS block now follows the rule the register reads have followed since #36: absence has to be proved, three refusals prove it, and the paths that write unprompted stop. Measured on the reported device — writes per watchdog tick went `3,3,3,3,3,3,3,3` to `3,3,0,0,0,0,0,0`, and the warn lines over eight ticks from 9 to 3
+- **Deliberately not gated on `model_family`, which is the obvious fix, the one the field thread proposed, and the wrong one.** The startup reset exists because the inverter can be holding a forced state this driver did not set: a container that died mid-force, an older driver version, iSolarCloud or another EMS on the same bus. None of those care what `classify_device_type` decided, and a hybrid shipped under a device-type code the driver has not been taught is classified `string`. Skipping the release on the label leaves exactly that inverter forced with nothing left to write mode 0 back. What separates the cases is not the label but whether the device took the write: one that refuses it cannot be holding a forced state, one that accepts it keeps its release
+- It also stops the question being re-argued. Whether a zero-watt command reaches the EMS block has now been settled in both directions inside one day — 1.5.4 opened it, 1.5.6 closed it again. This rule does not depend on that answer, and did not change when it flipped
+- A command always attempts, however many refusals came before, and its outcome still feeds the count, so one write that lands puts the release back at once. The count lives in the process, so a restart always attempts the startup reset — the case that reset exists for — exactly as a restart re-probes a read
+- While the driver is still attempting, a refused write is reported as the failure it is; reporting success over a write that did not land is the silent-failure shape #164 is about. What stops is the failure repeating forever, so the watchdog settles instead of escalating against an inverter that was never under control
+- `drivers/tests/test_sungrow_model_family.py` carries the reported device as a fixture — device type 9268, no meter wired, no hybrid block, every EMS write refused. The tests separate the three candidate designs: with no bound three fail, with a `model_family` gate five fail including the stranded-inverter case, and only the refusal count passes all of them
+- **The other half of that field report needs no driver change.** The same device shows `driver_poll: 2 of 9 modbus reads failed` on every poll and never comes online, which is what puts it offline. #36 already bounded those reads: reproduced against 1.4.0 they fail `2/9` on every poll forever, and against `main` they settle to `0/7` by the fourth. The site runs 1.4.0 and its update panel reads "up to date", so what it needs is FTW's pin moved, not a patch
+
 ### Fixed
 - **sungrow** 1.5.6 — **the zero-power refusal is restored. 1.5.4 was wrong, on a premise that did not survive reading the driver.** The case for waving zero through was that it hands the device back to itself, so refusing it could strand an inverter in a forced state. Neither half holds. `set_battery_idle` writes EMS mode **2** — forced — with command `0xCC` and setpoint 0: it pins the battery at zero under the host's control rather than releasing it. The release is `driver_default_mode`, which FTW reaches through `SendDefault` on shutdown, lease expiry and the watchdog, never through `driver_command`, and which no guard touches. So refusing withholds nothing that is not reachable by a route that cannot refuse — while accepting put mode 2 and a setpoint back on an SG inverter that implements neither, and reported success, because the read-back that would catch it fails on that device too. That is the SG12RT bug through the one action nobody guarded
 - The two sungrow copies disagree about what zero writes — the catalog driver forces idle at mode 2, the FTW v2 target hands the inverter back at mode 0 and clears the lease — and they use different register maps throughout. That belongs to the register-map review `packages/v1/sungrow/PILOT.md` already requires before the v2 target ships; its `verification_status` is `experimental` and the signed package builds from `targets/ftw-observe.lua`, so nothing there reaches hardware today. Recorded because untested drift between these two files is how Pixii's 40288 came back

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.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 |
+| sungrow | 1.5.7 | ftw-core | observe_only_profile_blocked | 1.3.3 | 1.3.0 | exact_model_firmware_required | — | not_assessed | no |
+| sungrow | 1.5.7 | 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,7 +1722,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.6"
+            version: "1.5.7"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1733,7 +1733,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.6"
+            version: "1.5.7"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1744,7 +1744,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.6"
+            version: "1.5.7"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1755,7 +1755,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.6"
+            version: "1.5.7"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1766,7 +1766,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.6"
+            version: "1.5.7"
             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.6",
+  version      = "1.5.7",
   protocols    = { "modbus" },
   capabilities = { "meter", "pv", "battery", "pv-curtail" },
   description  = "Sungrow SH-series hybrid inverters with LFP battery, via Modbus TCP.",
@@ -150,6 +150,65 @@ local function known_to_have_no_battery()
     return model_family == "string"
 end
 
+----------------------------------------------------------------------------
+-- The EMS control block, 13049-13051
+--
+-- The write-side twin of the register probe below, and it needs the same
+-- rule: absence has to be proved, and once proved, stop paying for it.
+--
+-- An SG string inverter rejects every write here with a Modbus exception. The
+-- driver retried three of them at startup and three more on every watchdog
+-- tick for the life of the session -- an SG12RT logged "self-consumption reset
+-- write failed" once per tick, indefinitely, while it was already offline for
+-- an unrelated reason.
+--
+-- Deliberately NOT gated on model_family, which is the obvious fix and the
+-- wrong one. The startup reset exists because the inverter can be holding a
+-- forced state this driver did not set: a container that died mid-force, an
+-- older driver version, iSolarCloud or another EMS on the same bus. None of
+-- those care what classify_device_type decided, and a hybrid shipped under a
+-- device-type code this driver has not been taught is classified "string".
+-- Skipping the release on the label would leave exactly that inverter forced,
+-- with nothing left that writes mode 0 back.
+--
+-- What separates the two cases is not the label but whether the device took
+-- the write. One that rejects it cannot be holding a forced state, so there is
+-- nothing to release. One that accepts it can, and keeps its release.
+--
+-- This also stops the question being re-argued. Whether a zero-watt command
+-- reaches the EMS block has now been settled in both directions inside a
+-- single day -- 1.5.4 opened it, 1.5.6 closed it again. The rule here does not
+-- depend on that answer.
+--
+-- Three failures rather than one for the same reason the read probe uses
+-- three: a busy bus is not proof. A restart clears the count, so the startup
+-- reset is always attempted at least once -- which is the case it exists for.
+local EMS_WRITE_ATTEMPTS = 3
+local ems_write_failures = 0
+
+-- True while the EMS block is still worth writing unprompted.
+local function ems_block_worth_writing()
+    return ems_write_failures < EMS_WRITE_ATTEMPTS
+end
+
+-- Record what the device did with one EMS operation. Counts the operation,
+-- not each of its three registers: the trio goes out together and fails
+-- together, so one rejected trio is one piece of evidence.
+local function note_ems_write(write_err)
+    if write_err == nil then
+        ems_write_failures = 0
+        return
+    end
+    ems_write_failures = ems_write_failures + 1
+    if ems_write_failures == EMS_WRITE_ATTEMPTS then
+        host.log("info", "Sungrow: the EMS control block rejected "
+            .. ems_write_failures .. " writes in a row; treating it as absent "
+            .. "and no longer writing it unprompted")
+    end
+end
+
+----------------------------------------------------------------------------
+
 -- A register that answers on one model and not another. Absence has to be
 -- proved: a single timeout or a busy bus must not silence a register for the
 -- rest of the session, so only a run of failures counts.
@@ -806,6 +865,10 @@ function set_battery_power(power_w)
     local cmd_err = host.modbus_write(13050, want_cmd)   -- 2. charge/discharge cmd
     local power_err = host.modbus_write(13051, watts)    -- 3. power setpoint
     local write_err = first_write_error(mode_err, cmd_err, power_err)
+    -- A command always attempts, however many times the block has refused
+    -- before: only the unprompted paths give up. But the outcome still counts,
+    -- and a success here clears the count and puts the release back.
+    note_ems_write(write_err)
     if write_err then
         host.log("warn", "Sungrow: force command write failed: " .. write_err)
         return false
@@ -846,6 +909,9 @@ function set_battery_idle()
     local cmd_err = host.modbus_write(13050, 0xCC)   -- stop forced charge/discharge
     local power_err = host.modbus_write(13051, 0)    -- zero power setpoint
     local write_err = first_write_error(mode_err, cmd_err, power_err)
+    -- Reached by a zero-watt command, which 1.5.4 accepts on every family.
+    -- It still attempts; the count only decides the unprompted paths.
+    note_ems_write(write_err)
     if write_err then
         host.log("warn", "Sungrow: idle write failed: " .. write_err)
         return false
@@ -868,14 +934,29 @@ function set_battery_idle()
 end
 
 -- Return to self-consumption mode (safe default)
+--
+-- Four paths reach this with no command in sight: driver_init's startup reset,
+-- the watchdog, driver_cleanup and the deinit action. The watchdog is the one
+-- that runs on a timer forever, so this is where a device that refuses the
+-- write has to be let go of.
 function set_self_consumption()
+    -- Nothing to hand back on a device that has refused this block three
+    -- times: a write it rejects cannot have latched a forced state. Report the
+    -- default as held rather than failed -- the watchdog reads a false as the
+    -- safe state being unreachable and escalates, which is the wrong answer
+    -- for an inverter that was never under control to begin with.
+    if not ems_block_worth_writing() then
+        return true
+    end
+
     -- Stop first, clear the stale power setpoint, then hand control back to
     -- the inverter. This makes the post-restart state deterministic instead
     -- of leaving e.g. mode=0/cmd=CC/power=880W latched in diagnostics.
     local cmd_err = host.modbus_write(13050, 0xCC)
     local power_err = host.modbus_write(13051, 0)
     local mode_err = host.modbus_write(13049, 0)
     local write_err = first_write_error(cmd_err, power_err, mode_err)
+    note_ems_write(write_err)
     if write_err then
         host.log("warn", "Sungrow: self-consumption reset write failed: " .. write_err)
         return false