fix(sungrow): a zero-power battery command is never refused

CHANGELOG.md

@@ -7,6 +7,10 @@ Driver versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html
 
 ## [Unreleased]
 
+### 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
+- **The Lua mock raised where the host returns.** `host_mock.lua` answered a failing `modbus_read` with `error()`, but FTW's binding pushes `nil` plus a message and returns — it never raises. Every driver wraps the call in `pcall` and works either way, which is why nobody noticed. Code written correctly for the real host did not: the v2 target's `v2_read_register` checks the returned error, and under the mock it threw instead, taking the whole command down rather than reporting a structured failure. The mock now returns what the host returns. Nothing broke — 3108 tests pass — which is the point: it had been lying for free
+
 ### Fixed
 - **pixii** 2.1.3 — **State of charge stopped being reported, silently.** Routing every read through one helper in 2.1.2 rewrote guards of the form `if ok_x and x_regs then`, but three sites in pixii carried a third term — `if ok_soc and soc_regs and soc_regs[1] ~= nil then` — and were left alone. `ok_soc` stopped existing. In Lua an undeclared name is a global, an unassigned global is `nil`, and `nil` in a condition is false, so the branch never ran again. SoC, apparent power and reactive power vanished from the emit while the battery record kept being sent, so nothing that counted records noticed. Caught by FTW's Go suite, `TestPixiiTroubleshootingStatusAndSetpoint`, and nothing in this repository
 - `drivers/tests/test_no_undefined_locals.py` holds every driver and package target to the rule the toolchain cannot: a name read but never declared. `luac -p` compiles it, the sandbox allows it, and the driver goes on emitting with one field missing. It found the three pixii sites and no others, and it is proven to fail when the bug is put 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.3 | ftw-core | observe_only_profile_blocked | 1.3.2 | 1.3.0 | exact_model_firmware_required | — | not_assessed | no |
-| sungrow | 1.5.3 | blixt-l1 | not_assessed | — | — | not_recorded | — | not_assessed | no |
+| sungrow | 1.5.4 | ftw-core | observe_only_profile_blocked | 1.3.3 | 1.3.0 | exact_model_firmware_required | — | not_assessed | no |
+| sungrow | 1.5.4 | 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.3"
+            version: "1.5.4"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1733,7 +1733,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.3"
+            version: "1.5.4"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1744,7 +1744,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.3"
+            version: "1.5.4"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1755,7 +1755,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.3"
+            version: "1.5.4"
             ders: [battery, meter, pv]
             control: true
         firmware_versions: ""
@@ -1766,7 +1766,7 @@ manufacturers:
         protocols:
           - protocol: modbus
             driver: "sungrow"
-            version: "1.5.3"
+            version: "1.5.4"
             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.3",
+  version      = "1.5.4",
   protocols    = { "modbus" },
   capabilities = { "meter", "pv", "battery", "pv-curtail" },
   description  = "Sungrow SH-series hybrid inverters with LFP battery, via Modbus TCP.",
@@ -722,7 +722,20 @@ function driver_command(action, power_w, cmd)
     if action == "init" then
         return true
     elseif action == "battery" then
-        if model_family ~= "hybrid" and not battery_confirmed 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.
+        --
+        -- 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
             local why = model_family == "string"
                 and "this model has no battery registers"
                 or "no battery register has answered on this device"

drivers/tests/lua_harness/host_mock.lua

@@ -197,7 +197,7 @@ function host.modbus_read(addr, count, kind)
 
     local read_error = host._modbus_read_fail_addresses[addr] or host._modbus_read_error
     if read_error then
-        error(tostring(read_error))
+        return nil, tostring(read_error)
     end
 
     local short_count = host._modbus_read_short_counts[addr]

drivers/tests/test_sungrow_model_family.py

@@ -392,28 +392,30 @@ def test_string_inverter_still_takes_a_curtail_command() -> None:
 BOTH_DRIVERS = [pytest.param(DRIVER, id="catalog"), pytest.param(FTW_V2, id="ftw-v2")]
 
 
-def battery_command_on(driver: Path) -> str:
+def battery_command_on(driver: Path, power_w: int = 1000,
+                       polls: int = 4) -> str:
     """Send one battery setpoint through whichever ABI this file speaks."""
     if driver == FTW_V2:
         call = '''
 local result = driver_command_v2({
     command = "battery.set_power",
     runtime_action = "battery",
-    inputs = {power_w = 1000},
+    inputs = {power_w = POWER_W},
 })
 print("ACCEPTED " .. tostring(result.status ~= "rejected"))
 print("CODE " .. tostring(result.code or "none"))
 '''
     else:
         call = '''
-local accepted, refusal = driver_command("battery", 1000, {})
+local accepted, refusal = driver_command("battery", POWER_W, {})
 print("ACCEPTED " .. tostring(accepted == true))
 print("CODE " .. tostring(type(refusal) == "table" and refusal.code or "none"))
 '''
+    call = call.replace("POWER_W", str(power_w))
     return f'''
 dofile("{driver}")
 driver_init({{}})
-for poll = 1, 4 do pcall(driver_poll) end
+for poll = 1, {polls} do pcall(driver_poll) end
 host._modbus_write_attempts = 0
 {call}
 print("WRITES " .. tostring(host._modbus_write_attempts))
@@ -463,3 +465,63 @@ def test_neither_copy_refuses_a_battery_it_can_confirm(
         f"{driver.name} refused a battery setpoint on {label}. A driver that "
         f"reads a battery every poll and will not command it has taken a "
         f"control off the fleet.")
+
+
+# ---------------------------------------------------------------------------
+# Zero is not a dispatch
+# ---------------------------------------------------------------------------
+#
+# A zero-power battery command is the host handing the device back to itself:
+# forced mode off, setpoint nought. It arrives from the lifecycle rather than
+# the planner, so it can land before the first poll -- when nothing has been
+# confirmed and the guard above would otherwise refuse it.
+#
+# 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 is known about the
+# hardware. On a 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.
+#
+# FTW's TestSungrowZeroBatteryCommandForcesIdle sends exactly this, straight
+# after load. These cases are that test's rule, held here so the two cannot
+# drift.
+
+@pytest.mark.parametrize("driver", BOTH_DRIVERS)
+@pytest.mark.parametrize("device,label", [
+    pytest.param(STRING_INVERTER,
+                 "a model that names itself a string inverter",
+                 id="named-string"),
+    pytest.param(UNIDENTIFIED_STRING,
+                 "a model that names nothing and answers no battery register",
+                 id="unidentified-string"),
+    pytest.param(HEALTHY_HYBRID, "a hybrid", id="hybrid"),
+])
+def test_zero_is_never_refused(driver: Path, device: str, label: str) -> None:
+    out = run_lua(device + battery_command_on(driver, power_w=0))
+
+    # Not refused by the guard is the rule. Whether the write then succeeds is
+    # a separate matter: on a device that does not implement 13049 the
+    # read-back fails and the driver says so, which is the honest outcome. The
+    # thing that must never happen is the command being turned away before it
+    # is tried.
+    assert out["CODE"] != "no_battery", (
+        f"{driver.name} refused a zero-power battery command on {label}. Zero "
+        f"is the safe default -- the host handing the device back to itself. "
+        f"A driver that will not accept 'stop' can leave a battery in a "
+        f"forced state with no way out.")
+
+
+@pytest.mark.parametrize("driver", BOTH_DRIVERS)
+def test_zero_lands_before_the_first_poll(driver: Path) -> None:
+    """The ordering FTW's Go suite exercises: load, then command, no poll.
+
+    Nothing has answered yet, so neither the model family nor the battery is
+    confirmed. This is precisely when the guard must not fire.
+    """
+    out = run_lua(UNIDENTIFIED_STRING
+                  + battery_command_on(driver, power_w=0, polls=0))
+
+    assert out["CODE"] != "no_battery", (
+        f"{driver.name} refused a zero-power command issued before the first "
+        f"poll. Detection has had no chance to run at that point, and a "
+        f"safety idle cannot wait for it.")

index.yaml

@@ -561,13 +561,13 @@ drivers:
     size_bytes: 6158
     sha256: "92163e4356b895e16b0205e849aa42a2dcea0d799f8cc6bcbce60b5e2cd92285"
   - name: "sungrow"
-    version: "1.5.3"
+    version: "1.5.4"
     tier: core
     protocol: modbus
     ders: [battery, meter, pv]
     control: true
-    size_bytes: 43486
-    sha256: "722eeb831ba7b42ef9609367267fea0abcbfb5cf2a11d54da615e6234e219164"
+    size_bytes: 44353
+    sha256: "94f6e4fd483e4dff54fafd79179c4700637662ca8b30bb600127189848cf503f"
   - name: "tesla_vehicle"
     version: "0.2.0"
     tier: core

manifests/sungrow.yaml

@@ -1,5 +1,5 @@
 name: "sungrow"
-version: "1.5.3"
+version: "1.5.4"
 tier: core
 author: "Sourceful Labs AB"
 protocol: modbus
@@ -42,9 +42,9 @@ tested_devices:
     notes: "Commercial/industrial three-phase string inverter. Needs 1.2.1, which stops polling the SH battery registers this family does not answer."
     min_driver_version: "1.2.1"
 min_host_version: "2.0.0"
-size_bytes: 43486
+size_bytes: 44353
 dkb_id: "sungrow_sh_rt"
-sha256: "722eeb831ba7b42ef9609367267fea0abcbfb5cf2a11d54da615e6234e219164"
+sha256: "94f6e4fd483e4dff54fafd79179c4700637662ca8b30bb600127189848cf503f"
 signature: ""
 
 bytecode_sha256: ""

packages/v1/sungrow/package-source.json

@@ -1,7 +1,7 @@
 {
   "schema_version": "sourceful.driver-package-source/v1",
   "package_id": "com.sourceful.driver.sungrow",
-  "version": "1.3.2",
+  "version": "1.3.3",
   "channel": "beta",
   "display_name": "Sungrow observe-only pilot",
   "identity": {

packages/v1/sungrow/targets/ftw.lua

@@ -12,7 +12,7 @@ DRIVER = {
     id = "sungrow",
     name = "Sungrow hybrid and string inverter",
     manufacturer = "Sungrow",
-    version = "1.3.3",
+    version = "1.3.4",
     protocols = { "modbus" },
     capabilities = { "pv", "battery", "meter" },
     description = "Sungrow hybrid and string inverters via Modbus.",
@@ -413,7 +413,14 @@ function driver_command_v2(command)
         -- implement. Checking only for "string" left the gap: an SG inverter
         -- whose device-type register never answered fell through and got the
         -- writes. Matches drivers/lua/sungrow.lua, deliberately.
-        if model_family ~= "hybrid" and not battery_confirmed then
+        --
+        -- Zero is exempt. It is not a dispatch but the host handing the
+        -- device back to itself, it arrives from the lifecycle rather than
+        -- the planner -- so before anything has been confirmed -- and a
+        -- device left in a forced state with no way to say stop is the worse
+        -- outage. On a string inverter the write fails at the Modbus layer
+        -- and costs nothing.
+        if inputs.power_w ~= 0 and model_family ~= "hybrid" and not battery_confirmed then
             return {
                 status = "rejected",
                 code = "no_battery",

support-status.json

@@ -1962,7 +1962,7 @@
     },
     {
       "catalog_source": true,
-      "catalog_version": "1.5.3",
+      "catalog_version": "1.5.4",
       "driver_id": "sungrow",
       "package_id": "com.sourceful.driver.sungrow",
       "targets": {
@@ -1977,7 +1977,7 @@
           "target_conformance": "not_assessed"
         },
         "ftw-core": {
-          "candidate_package_version": "1.3.2",
+          "candidate_package_version": "1.3.3",
           "control_enabled": false,
           "hil": "exact_model_firmware_required",
           "historical_signed_beta_version": "1.3.0",

tests/test_driver_package.py

@@ -111,7 +111,11 @@ def test_pilot_safety_metadata_is_explicit() -> None:
     sungrow = load_json(PILOTS["sungrow"])
     targets = {item["target"]: item for item in sungrow["compatibility"]}
     assert sungrow["read_only"] is True
-    assert sungrow["version"] == "1.3.2"
+    # No version assertion. This test is about what the package promises --
+    # read-only, no commands, no control permissions -- and a version is not
+    # one of those promises. Pinning it here made an unrelated safety test the
+    # thing that failed on every legitimate bump, and the version is already
+    # held to the driver's own by the manifest checks and the package build.
     assert sungrow["commands"] == []
     assert sungrow["capabilities"]["control"] == []
     assert sungrow["permissions"] == ["modbus.read"]