From 0b20f53cf8fe1b757f925e17eb23d2649dfb9c0b Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Sat, 7 Nov 2026 12:20:56 +0100 Subject: [PATCH] SMA-2 round 2: census keeps ACTUATOR_WRITE_KEYS, setters, NaN/inf port refused, tests Co-Authored-By: Claude --- goodwe_controller/CHANGELOG.md | 2 +- .../DECISIONS/SMA-2-driver-seam.md | 15 +- goodwe_controller/app/inverter.py | 25 ++++ goodwe_controller/app/validate.py | 17 ++- goodwe_controller/test_sma2_driver.py | 135 +++++++++++++++++- 5 files changed, 185 insertions(+), 9 deletions(-) diff --git a/goodwe_controller/CHANGELOG.md b/goodwe_controller/CHANGELOG.md index ef51a31..63bf561 100644 --- a/goodwe_controller/CHANGELOG.md +++ b/goodwe_controller/CHANGELOG.md @@ -2,7 +2,7 @@ ### Not in a release yet -- Each inverter is now reached through a driver, so another make can be added later. Nothing changes for a GoodWe site, and the inverter list accepts five new optional settings that nothing reads yet ([[SMA-2]]). +- Each inverter is now reached through a driver, so another make can be added later. Nothing changes for a GoodWe site. The inverter list also accepts five new optional settings that nothing reads yet, and the schema no longer requires a setpoint entity per entry: a GoodWe entry without one is still refused at start ([[SMA-2]]). - Home now draws today's charge from the grid for every hour the add-on will really charge, not only the run under way ([[UX-77]]). - When Home Assistant passes readings on late, control no longer swings on them: it waits until the battery shows its last power request, and the peak limit no longer asks for full power on them. The page says so, and if the readings stay too late every inverter goes to 0 W ([[SAFETY-128]]). - With a 5 s meter, readings 30 s late are now noticed within 100 seconds, not 3 minutes ([[SAFETY-129]]). diff --git a/goodwe_controller/DECISIONS/SMA-2-driver-seam.md b/goodwe_controller/DECISIONS/SMA-2-driver-seam.md index 8981c42..f904173 100644 --- a/goodwe_controller/DECISIONS/SMA-2-driver-seam.md +++ b/goodwe_controller/DECISIONS/SMA-2-driver-seam.md @@ -25,11 +25,16 @@ from the driver yet either: with only GoodWe they would change nothing. **The seam is not TCP-shaped.** The census asks each entry's driver for its actuator id: the setpoint entity for GoodWe, `host:port:unit_id` for a driver reached over a socket, so two units behind one gateway are two actuators and -one unit named twice is the SAFETY-28 refusal. The driver binds its Home +one unit named twice is the SAFETY-28 refusal. The census still counts every +`setpoint_entity` key as before and adds the driver's own actuator to it (parity +with 23ff9d0 on 349 option shapes is a test). The driver binds its Home Assistant on first use by the Controller, so a unit added later is wired too. -**Schema.** `setpoint_entity` is optional per entry (rule 9(d) still requires -it for GoodWe, with the same sentence), beside `driver` (only `goodwe_esphome`), +**Schema.** The brief says "`setpoint_entity` becomes optional" (SMA-2, "Schema per +inverter entry"; SMA-9 section 1.1): it is the GoodWe driver's own key, so a +driver reached by host and port has none. `setpoint_entity` is optional per entry +in config.yaml; rule 9(d) still refuses a GoodWe entry without it, with the same +sentence (`test_sma2_driver.py` part 6), beside `driver` (only `goodwe_esphome`), `host`, `port`, `unit_id`, `maintenance`. The last four are checked for shape and not read. The Supervisor shape probe was re-run (section K). @@ -37,3 +42,7 @@ and not read. The Supervisor shape probe was re-run (section K). call are byte-identical at N=1 and N=2; the TEL-07 announce diff is 0/0/0; a fake driver beside a GoodWe unit allocates, grades, drops and publishes per unit; a call site that goes round the seam is caught. + +**Per-entry keys the page does not draw** are listed in `uilayout.ROSTER_HIDDEN`; +test_uilayout and test_sma2_driver fail for a key that is neither a card field +nor hidden, and a save through the page keeps them (a test). diff --git a/goodwe_controller/app/inverter.py b/goodwe_controller/app/inverter.py index 64a7003..d5fa66a 100644 --- a/goodwe_controller/app/inverter.py +++ b/goodwe_controller/app/inverter.py @@ -366,6 +366,31 @@ class Inverter: def batt_invert(self) -> bool: return bool(getattr(self.driver, "batt_invert", False)) + # Assigning one writes through to the driver, as the attributes did before + # SMA-2 (nothing in the repo assigns them today; this keeps it working). + # A driver that has no such field refuses, rather than losing the value. + def _set_driver_field(self, name, value): + if not hasattr(self.driver, name): + raise AttributeError( + f"{type(self.driver).__name__} has no {name}") + setattr(self.driver, name, value) + + @setpoint_entity.setter + def setpoint_entity(self, v): + self._set_driver_field("setpoint_entity", str(v or "")) + + @soc_entity.setter + def soc_entity(self, v): + self._set_driver_field("soc_entity", str(v or "")) + + @batt_entity.setter + def batt_entity(self, v): + self._set_driver_field("batt_entity", str(v or "")) + + @batt_invert.setter + def batt_invert(self, v): + self._set_driver_field("batt_invert", bool(v)) + # -- store --------------------------------------------------------------- def key(self, name: str) -> str: """This unit's namespaced Store key for `name`. See store.unit_key().""" diff --git a/goodwe_controller/app/validate.py b/goodwe_controller/app/validate.py index d9e1371..9d9dec4 100644 --- a/goodwe_controller/app/validate.py +++ b/goodwe_controller/app/validate.py @@ -328,6 +328,7 @@ WHAT IS DELIBERATELY *NOT* REFUSED, because it is untidy rather than unsafe: """ import logging +import math import re from typing import NamedTuple @@ -466,6 +467,15 @@ def _collect_writers(node, path: str, out: list) -> None: cls = drivers.driver_class(node) for k, v in node.items(): here = f"{path}.{k}" if path else str(k) + # Every key in ACTUATOR_WRITE_KEYS declares a writer, as before + # SMA-2 and whatever the entry's driver; PLUS the entry driver's + # own writer key. The second is added to the first, never in place + # of it (census parity with 23ff9d0 is graded in test_sma2_driver). + if k in ACTUATOR_WRITE_KEYS: + entity = str(v or "").strip() + if entity: + out.append((here, entity)) + continue if k == cls.writer_key: actuator = cls.actuator_id_of(node) if actuator: @@ -793,9 +803,12 @@ def _whole(raw, lo: int, hi: int): return None try: val = float(raw) - except (TypeError, ValueError): + # NaN and infinity: int() raises on them, so refuse them first. + if not math.isfinite(val) or val != int(val): + return None + except (TypeError, ValueError, OverflowError): return None - return int(val) if val == int(val) and lo <= val <= hi else None + return int(val) if lo <= val <= hi else None def _driver_problems(entry: dict, who: str) -> list[str]: diff --git a/goodwe_controller/test_sma2_driver.py b/goodwe_controller/test_sma2_driver.py index 7629308..e98b296 100644 --- a/goodwe_controller/test_sma2_driver.py +++ b/goodwe_controller/test_sma2_driver.py @@ -514,11 +514,13 @@ check("another driver's actuator is host:port:unit: two units behind one " "gateway are two actuators, and one unit named twice is one", set(cen) == {"gw:502:1", "gw:502:2", "gw:503:1"} and cen["gw:502:1"] == ["inverters[0].host", "inverters[3].host"]) -check("a GoodWe key on a fake entry declares no writer there, and a `host` on " - "a GoodWe entry declares none", +check("a GoodWe key on a fake entry STILL declares a writer (the pre-SMA-2 " + "census kept, the driver's actuator added to it), and a `host` on a " + "GoodWe entry declares none", actuator_writers({"inverters": [entry(driver="fake", host="gw", setpoint_entity="number.x")]}) - == {"gw:502:1": ["inverters[0].host"]} + == {"number.x": ["inverters[0].setpoint_entity"], + "gw:502:1": ["inverters[0].host"]} and actuator_writers({"inverters": [entry(setpoint_entity="number.x", host="gw")]}) == {"number.x": ["inverters[0].setpoint_entity"]}) @@ -600,6 +602,133 @@ check("setpoint_entity is optional per entry, beside driver, host, port, " check("config.yaml lists exactly the selectable drivers", decl["driver"] == f"list({'|'.join(SELECTABLE)})?") +# ============================================================================== +print() +print("6. round 2: census parity, setters, rule 9(d), NaN ports, hidden keys") + +import copy, itertools, math # noqa: E402 +from app import uilayout as L, uimodel # noqa: E402 +from app.validate import ACTUATOR_WRITE_KEYS # noqa: E402 + + +def old_census(opts): + """23ff9d0's census, copied: every ACTUATOR_WRITE_KEYS key, wherever it is.""" + found = [] + + def walk(node, path): + if isinstance(node, dict): + for k, v in node.items(): + here = f"{path}.{k}" if path else str(k) + if k in ACTUATOR_WRITE_KEYS: + e = str(v or "").strip() + if e: + found.append((here, e)) + continue + walk(v, here) + elif isinstance(node, (list, tuple)): + for i, v in enumerate(node): + walk(v, f"{path}[{i}]") + walk(opts, "") + cen = {} + for where, e in found: + cen.setdefault(e.lower(), []).append(where) + return cen + + +check("ACTUATOR_WRITE_KEYS is exactly ('setpoint_entity',): the parity below " + "is about that key", tuple(ACTUATOR_WRITE_KEYS) == ("setpoint_entity",)) +SP = ["number.a", "Number.A", " number.b ", "", None, 5, ["x"]] +shapes = [] +for flat, e0, e1 in itertools.product(SP, SP, SP): + shapes.append({"setpoint_entity": flat, "inverters": [ + {"name": "a", "setpoint_entity": e0}, {"name": "b", "setpoint_entity": e1}]}) +shapes += [{"x": {"y": [{"setpoint_entity": "number.z"}]}, "setpoint_entity": "n.q"}, + {"inverters": "junk"}, {"inverters": [{"driver": "nope", + "setpoint_entity": "n.k"}]}, + {"inverters": [{"driver": "goodwe_esphome", "setpoint_entity": "n.k"}]}, + {}, {"inverters": []}] +bad = [i for i, o in enumerate(shapes) if actuator_writers(o) != old_census(o)] +check(f"CENSUS PARITY: on {len(shapes)} flat/roster/odd option shapes the " + "census equals 23ff9d0's, keys, order and all", not bad) +mixed = {"setpoint_entity": "number.a", "inverters": [ + {"name": "a", "setpoint_entity": "number.a"}, + {"name": "f", "driver": "fake", "host": "gw", "port": 502, "unit_id": 1, + "setpoint_entity": "number.ignored"}, + {"name": "g", "setpoint_entity": "number.g"}]} +new_c, old_c = actuator_writers(mixed), old_census(mixed) +check("...and on a mixed config (a fake-driver entry too) the census is a " + "SUPERSET of the pre-SMA-2 one: every old writer is still counted, " + "plus the driver's own actuator", + all(new_c.get(k) and set(v) <= set(new_c[k]) for k, v in old_c.items()) + and "gw:502:1" in new_c) + +gi = Inverter("s", setpoint_entity="number.s", soc_entity="sensor.a", + batt_entity="sensor.b") +gi.setpoint_entity, gi.soc_entity, gi.batt_entity, gi.batt_invert = ( + "number.t", "sensor.c", "sensor.d", True) +check("SETTERS: assigning setpoint_entity/soc_entity/batt_entity/batt_invert " + "on an Inverter writes through to its GoodWe driver, as it did before", + (gi.driver.setpoint_entity, gi.driver.soc_entity, gi.driver.batt_entity, + gi.driver.batt_invert) == ("number.t", "sensor.c", "sensor.d", True) + and gi.has_writer() and gi.driver.actuator_id() == "number.t") + + +def _assign(): + inv_f.setpoint_entity = "x" + + +try: + _assign() + _refused = False +except AttributeError: + _refused = True +check("...and on a driver that has no such field it refuses, not loses the value", + _refused) + +nogood = validate_startup_config(dict(BASEOPTS, inverters=[ + {k: v for k, v in GOOD.items() if k != "setpoint_entity"}])) +check("RULE 9(d): a GoodWe entry with no setpoint_entity is refused, by name " + "and by position, with the sentence it always had", + any(p.startswith("inverters[0] (u) has no") and "setpoint entity" in p.lower() + and "Set its ESPHome number entity, or remove the entry." in p + for p in nogood) + and nogood != base_problems) +check("...also with `driver: goodwe_esphome` said out loud", + any("has no" in p for p in validate_startup_config(dict( + BASEOPTS, inverters=[{k: v for k, v in dict(GOOD, driver="goodwe_esphome").items() + if k != "setpoint_entity"}])))) + +nan, inf = float("nan"), float("inf") +for key in ("port", "unit_id"): + for bad_v, nm in ((nan, "NaN"), (inf, "inf"), (-inf, "-inf")): + try: + got = prob(**{key: bad_v}) + ok_ = bool(named(got, key)) + except Exception as e: # noqa: BLE001 + got, ok_ = repr(e), False + check(f"{key} = {nm} is refused by key and does not raise", ok_) + +keys_cfg = set(decl) +placed = {k.split(".", 1)[1] for _g, _p, f in L.fields() + for k in [f.key] if k.startswith("inverters.")} +check("ROSTER_HIDDEN: every per-entry key in config.yaml is either a card " + "field or hidden, never both and never neither", + all((k in placed) != (k in L.ROSTER_HIDDEN) for k in keys_cfg) + and not (placed & set(L.ROSTER_HIDDEN)) + and set(L.ROSTER_HIDDEN) <= keys_cfg) +check("...and a per-entry key that is neither would be flagged (test_uilayout " + "grades the same rule with a made-up key)", + not all((k in placed) != (k in L.ROSTER_HIDDEN) + for k in keys_cfg | {"brand_new_key"})) +ent = dict(name="a", setpoint_entity="number.a", max_w=3000, bms_soc_floor=10, + soc_entity="sensor.s", batt_entity="sensor.b", driver="goodwe_esphome", + host="h", port=502, unit_id=3, maintenance=False) +opts_rt = {"inverters": [ent, dict(ent, name="b", setpoint_entity="number.b")]} +back = uimodel.ui_to_options(uimodel.options_to_ui(copy.deepcopy(opts_rt))) +check("the page model carries the hidden keys through a save untouched " + "(this is how ROSTER_HIDDEN keys survive the settings page)", + back.get("inverters") == opts_rt["inverters"]) + del REGISTRY["fake"] # THE SEAM MUTANTS THIS FILE WAS RUN AGAINST (twelve, each one edit to a copy of