diff --git a/goodwe_controller/DECISIONS/UX-13-one-layout-table.md b/goodwe_controller/DECISIONS/UX-13-one-layout-table.md new file mode 100644 index 0000000..b23019f --- /dev/null +++ b/goodwe_controller/DECISIONS/UX-13-one-layout-table.md @@ -0,0 +1,69 @@ +# UX-13 — one layout table, and why UI code may now name an option key + +**Status: DECIDED 2026-09-27 (owner approved the UX-13 design and mockup).** +Card 6ab979852f3eca02fb40f69e. Amends [[UX-02]] §3.1. Design note and +mockup: `goodwe-session/UX-13-design.md`, `UX-13-mockup.html`. + +## 1. What UX-02 said, and why it no longer fits + +UX-02 built the configuration panel from the Supervisor's schema, so +`app/configui.py` contains "no option key, no label and no rule", and +test_selfconfig.py part 5 checks that the page script names no key. That kept +a second copy of the option surface from drifting, and it is why the panel is +a flat list of fields in schema order. + +UX-13 needs things a schema cannot say: groups and sub-groups, a slider in kW +for a value stored in W, a picker for an entity, a field shown only for one +meter source, locked measured values, a Simple view and a setup wizard. It +also needs page fields that are not options at all: one inverter card that is +stored as six flat keys, solar rows that become `pv_entity` or `pv_entities`, +a contract and two prices that become `static_profile` JSON, and a region +that becomes the levies figure. None of that can be read off the schema, so +some code has to name option keys. + +## 2. The decision + +Option keys may appear in UI code in exactly two files: + +- **`app/uilayout.py`**, the layout table: screen → group → part → field, with + control, unit, display scale, step, entity kind, when it is shown, whether it + is on the Simple view or a wizard step, and whether it applies now or after a + restart. Every option in `config.yaml` is a field, `DERIVED` or `HIDDEN`, + exactly once. +- **`app/uimodel.py`**, the derivations the table names: `options_to_ui()` and + `ui_to_options()`. It may name only the keys it writes. + +What UX-02 made authoritative stays authoritative: + +- **Ranges** come from the schema (`uilayout.range_of()`); the table writes + none. A unit holds no digit. +- **Labels and help** come from `translations/*.yaml` by key. Page-only fields, + group titles and choices are in `app/ui_text/*.json`, held to STYLE by + test_messages.py. +- **Rules** stay in `validate.py`. It gains a warning tier (`validate.Note`, + "warn" or "info") for the page. A note never blocks a save, and passing the + notes list does not change the refusals returned. +- **The stored blob** is still the one truth. The page model carries the stored + values of every derived key (`keep`). A derivation whose page fields are + unchanged writes those values back untouched, so a blob the page cannot + express survives a save of another field. + +## 3. How it is held + +- test_uilayout.py fails when `config.yaml` gains an option, or an inverter + field, that the table does not place. It also checks that uimodel.py names + only the keys it writes and that web.py and haentities.py name none. +- test_uimodel.py round-trips options → page model → JSON → options for every + validator call the suite made at 652097a (`scenarios/ux13/`), the TEST-23 + option files and config.yaml's defaults. All must be identical. +- `tools/ux13_validator_calls.py compare` runs those calls through the old and + new validator. At P0: 261 calls, 158 refused, no refusal changed. +- test_selfconfig.py part 5 still checks the old page script; P1 moves the page + into `app/static/` and the same check applies there. + +## 4. What this costs + +The table is a second place that knows the option surface, which UX-02 set +out to avoid. The cost is bounded by the coverage test: an option added to +config.yaml and not placed here fails CI, rather than silently missing from +the page. Changing a key's type or range needs no change here. diff --git a/goodwe_controller/app/haentities.py b/goodwe_controller/app/haentities.py new file mode 100644 index 0000000..d6f0784 --- /dev/null +++ b/goodwe_controller/app/haentities.py @@ -0,0 +1,87 @@ +"""UX-13: Home Assistant's entities, for the settings page's entity picker. + +A read of `GET /api/states` through the add-on's own session (hass.states()), +cached for 30 s and trimmed to what a picker shows. Nothing here writes, so +UX-02 §6 holds. When Home Assistant does not answer, the last good list is +served with its age, or an empty one that says why: the route never fails. +""" + +import asyncio +import time + +CACHE_S = 30.0 +DOWN_TEXT = "Home Assistant is not answering, so the entity list is empty." +STALE_TEXT = "Home Assistant is not answering, so this list may be old." +_POWER_UNITS = ("mW", "W", "kW", "MW", "GW", "TW") +_ENERGY_UNITS = ("Wh", "kWh", "MWh", "GWh") + + +def row(state: dict) -> dict | None: + """One state object -> {entity_id, name, state, unit, device_class}.""" + if not isinstance(state, dict) or not isinstance(state.get("entity_id"), str): + return None + attrs = state.get("attributes") if isinstance(state.get("attributes"), dict) else {} + return {"entity_id": state["entity_id"], + "name": str(attrs.get("friendly_name") or state["entity_id"]), + "state": state.get("state"), + "unit": attrs.get("unit_of_measurement"), + "device_class": attrs.get("device_class")} + + +def matches(r: dict, kind: str) -> bool: + """Is `r` what a picker of this kind offers first? `kind` is one of + uilayout.ENTITY_KINDS; "" or "any" offers everything.""" + unit, cls = r.get("unit"), r.get("device_class") + if kind in ("power", "power_w"): + return cls == "power" or unit in _POWER_UNITS + if kind == "energy": + return cls == "energy" or unit in _ENERGY_UNITS + if kind == "pct": + return unit == "%" + if kind == "number": + return r["entity_id"].startswith(("number.", "input_number.")) + return True + + +class EntityCache: + """The last entity list read from Home Assistant, at most CACHE_S old.""" + + def __init__(self, hass, clock=time.monotonic): + self.hass = hass + self.clock = clock + self.rows: dict | None = None + self.read_at: float | None = None + self._lock = asyncio.Lock() + + async def get(self) -> tuple[dict | None, float | None]: + """(rows by entity id, age in s), or (None, None) if never read.""" + async with self._lock: # one HA read however many pickers ask + now = self.clock() + if self.read_at is None or now - self.read_at >= CACHE_S: + states = await self.hass.states() + if states is not None: + rows = (row(s) for s in states) + self.rows = {r["entity_id"]: r for r in rows if r} + self.read_at = now + if self.read_at is None: + return None, None + return self.rows, now - self.read_at + + def peek(self) -> dict | None: + """The cached rows if they are fresh, without asking Home Assistant.""" + if self.read_at is None or self.clock() - self.read_at >= CACHE_S: + return None + return self.rows + + async def payload(self, kind: str = "") -> dict: + """The route's JSON: matching entities first, then the rest.""" + rows, age = await self.get() + if rows is None: + return {"available": False, "text": DOWN_TEXT, "entities": []} + ordered = sorted(rows.values(), key=lambda r: r["entity_id"]) + first = [dict(r, match=True) for r in ordered if matches(r, kind)] + rest = [dict(r, match=False) for r in ordered if not matches(r, kind)] + out = {"available": True, "age_s": round(age, 1), "entities": first + rest} + if age >= CACHE_S: + out["text"] = STALE_TEXT + return out diff --git a/goodwe_controller/app/hass.py b/goodwe_controller/app/hass.py index da1dd33..3835e1d 100644 --- a/goodwe_controller/app/hass.py +++ b/goodwe_controller/app/hass.py @@ -18,6 +18,7 @@ SUPERVISOR_API = "http://supervisor" BAD_STATES = ("unknown", "unavailable", "none", "") # What the Supervisor's proxy answers while Home Assistant restarts. UNAVAILABLE = (502, 503, 504) +STATES_TIMEOUT_S = 10 # UX-13's entity list, like every other read here class Outage: @@ -113,6 +114,32 @@ class HomeAssistant: return None return obj + async def states(self) -> list | None: + """Every entity's state object (`GET /api/states`), or None when Home + Assistant does not answer. A read only (UX-13 entity picker).""" + try: + async with self.session.get( + f"{CORE_API}/states", headers=self._headers, + timeout=STATES_TIMEOUT_S + ) as resp: + if resp.status in UNAVAILABLE: + OUTAGE.down("api", f"error {resp.status}", + f"read all states: {resp.status}") + return None + OUTAGE.up("api") + resp.raise_for_status() + obj = await resp.json() + except aiohttp.ClientConnectionError as err: + OUTAGE.down("api", "no connection", f"read all states: {err}") + return None + except (aiohttp.ClientError, TimeoutError, ValueError) as err: + self._moan("read:*", "Reading the entity list failed: %s", err) + return None + if not isinstance(obj, list): + self._moan("shape:*", "The entity list is not a list: %.80r", obj) + return None + return obj + async def number(self, entity_id: str, invert: bool = False) -> float | None: """Numeric state, or None. Never substitutes a default.""" obj = await self.state(entity_id) diff --git a/goodwe_controller/app/messages.py b/goodwe_controller/app/messages.py index 4c8bcdb..df26ea7 100644 --- a/goodwe_controller/app/messages.py +++ b/goodwe_controller/app/messages.py @@ -1099,6 +1099,18 @@ LABELS = { "unit_sign_disagree_w": "Per-inverter wrong-direction threshold", "slew_w": "Largest change per cycle", "shadow_mode": "Shadow mode", + # UX-13: named by the settings page's warnings (validate.Note). + "integrator_max_w": "Catch-up limit", + "maintenance_discharge_w": "Draw-down rate", + "peak_cap_w": "Peak limit", + "peak_forecast_entity": "15-minute demand forecast entity", + "meter_entity": "Grid power entity", + "soc_entity": "Battery level entity", + "batt_entity": "Battery power entity", + "pv_entity": "Solar production entity", + "pv_entities": "Solar production entities, per inverter", + "p1_phase_import_entities": "Import entities per phase", + "p1_phase_export_entities": "Export entities per phase", } diff --git a/goodwe_controller/app/selfconfig.py b/goodwe_controller/app/selfconfig.py index 5f3b852..7c56b78 100644 --- a/goodwe_controller/app/selfconfig.py +++ b/goodwe_controller/app/selfconfig.py @@ -288,7 +288,7 @@ async def problems_for(opts: dict, hass=None) -> list: return problems -async def problems_and_ranges(opts: dict, hass=None): +async def problems_and_ranges(opts: dict, hass=None, notes=None): """`problems_for()`, plus WHICH setpoint entities did not report a range. ⚠️ THE SECOND HALF EXISTS SO THE PAGE CANNOT LOOK GREEN FOR A REASON THE @@ -303,6 +303,8 @@ async def problems_and_ranges(opts: dict, hass=None): has just typed a new `setpoint_entity` must be told about THAT entity, and a route that asked the running controller instead would report on the old one. That was the first shape of this and it disagreed with itself. + + UX-13: a list passed as `notes` collects the validator's warnings too. """ flat = str(opts.get("setpoint_entity", "") or "").strip() @@ -329,7 +331,7 @@ async def problems_and_ranges(opts: dict, hass=None): # disagreement is itself the finding - so it is reported rather than # swallowed. Returning `[]` here would turn a crash into a silent # acceptance, which is worse than the 500 this replaced. - found = list(validate_startup_config(opts)) + found = list(validate_startup_config(opts, notes=notes)) if not found: found = [f"the configuration could not be composed ({type(exc).__name__}: " f"{exc}), and the validator did not name a reason. This is a " @@ -347,7 +349,8 @@ async def problems_and_ranges(opts: dict, hass=None): if flat and unit.setpoint_entity.strip().lower() == flat.lower(): dev_min, dev_max = unit.dev_min, unit.dev_max break - problems = list(validate_startup_config(opts, dev_min, dev_max)) + problems = list(validate_startup_config(opts, dev_min, dev_max, + notes=notes)) problems += validate_unit_ranges(units, skip_entity=flat) unread = [unit.setpoint_entity for unit in units if unit.setpoint_entity and unit.dev_min is None diff --git a/goodwe_controller/app/ui_text/en.json b/goodwe_controller/app/ui_text/en.json new file mode 100644 index 0000000..d3244fb --- /dev/null +++ b/goodwe_controller/app/ui_text/en.json @@ -0,0 +1,38 @@ +{ + "groups": { + "meter": {"name": "Meter", "description": "Where the grid reading comes from."}, + "inverters": {"name": "Inverters and batteries", "description": "The inverters the add-on controls, and your solar."}, + "tariff": {"name": "Tariff and prices", "description": "What you pay and are paid per kWh."}, + "safety": {"name": "Safety limits", "description": "The peak limit, starting and stopping, and the safety stops."}, + "maintenance": {"name": "Maintenance", "description": "The monthly battery cycle and the log."}, + "tuning": {"name": "Control tuning", "description": "For support only, because these were measured on real hardware."} + }, + "parts": { + "meter.timing": {"name": "Timing"}, + "inverters.solar": {"name": "Solar"}, + "tariff.selling": {"name": "Charging and selling on price"}, + "tariff.feed": {"name": "Price feed details"}, + "safety.peak": {"name": "Peak limit"}, + "safety.start": {"name": "Starting and stopping"}, + "safety.stops": {"name": "Safety stops", "description": "Measured on real hardware, so leave them unchanged."}, + "maintenance.support": {"name": "Support"} + }, + "fields": { + "solar": {"name": "Solar inverters", "description": "Each solar inverter's power sensor, which the add-on records but does not use for control."}, + "contract": {"name": "Your contract", "description": "How your supplier prices each kWh you take from the grid."}, + "region": {"name": "Region", "description": "Fills in the grid costs, levies and VAT for the year shown, so check them on your bill."}, + "fallback": {"name": "Price when the feed is down", "description": "Used when the price source does not answer."}, + "fixed_price": {"name": "Price per kWh, all in", "description": "Energy, supplier costs, grid costs, levies and VAT together."}, + "day_price": {"name": "Day price, weekdays 07:00 to 22:00", "description": "All in, as on your bill."}, + "night_price": {"name": "Night and weekend price", "description": "All in, as on your bill."} + }, + "choices": { + "contract": {"none": "No prices", "fixed": "Fixed price", "daynight": "Day and night", "dynamic": "Dynamic, hourly", "custom": "Other time windows"}, + "fallback": {"none": "None", "fixed": "One price", "daynight": "Day and night", "custom": "Other time windows"}, + "region": {"flanders": "Flanders", "own": "I enter my own figure"} + }, + "applies": { + "restart": {"name": "Applies after a restart"}, + "now": {"name": "Applies right away"} + } +} diff --git a/goodwe_controller/app/uilayout.py b/goodwe_controller/app/uilayout.py new file mode 100644 index 0000000..23fec4d --- /dev/null +++ b/goodwe_controller/app/uilayout.py @@ -0,0 +1,295 @@ +"""UX-13: the new page's layout, written once - screen, group, field. + +UI code names option keys only here and in uimodel.py, which holds the +derivations this table lists (DECISIONS/UX-13-one-layout-table.md, amending +UX-02 §3). Everything else stays where UX-02 put it: ranges in config.yaml's +schema, rules in validate.py. test_uilayout.py fails CI when an option in +config.yaml has no place here. + +A field's `key` is an option key, `inverters.` for one inverter in the +list, or `ui.` for a page-only control that uimodel.py turns into options. +The key is also its text key: label and help are translations/*.yaml's entry +for an option, and app/ui_text's `fields.` for a `ui.` field. +""" + +from dataclasses import dataclass + +RESTART = "restart" # stored options apply after a restart (UX-02 §5) +NOW = "now" # runtime state (OPT-11's battery switches), none yet + +# Controls are names, not string literals, so a key is never followed by a +# constant (test_defaults_pin.py reads `f("key", )` as a fallback). +CHOICE, ENTITY, ENTITY_LIST, TOGGLE, SEGMENTED, SLIDER = ( + "choice", "entity", "entity_list", "toggle", "segmented", "slider") +NUMBER, TEXT, SELECT, LOCKED, INVERTER_LIST, SOLAR_LIST = ( + "number", "text", "select", "locked", "inverter_list", "solar_list") +CONTROLS = (CHOICE, ENTITY, ENTITY_LIST, TOGGLE, SEGMENTED, SLIDER, NUMBER, + TEXT, SELECT, LOCKED, INVERTER_LIST, SOLAR_LIST) +# What an entity picker offers first. `power_w` is a power sensor the add-on +# reads as watts with no unit conversion, so validate.py warns on a kW sensor. +ENTITY_KINDS = ("power_w", "power", "energy", "pct", "number", "any") +SHOW_OPS = ("==", "!=", "in", ">", "<") + + +@dataclass(frozen=True) +class Field: + key: str + control: str + unit: str = "" + scale: int = 1 # shown = stored / scale (the peak limit in kW) + step: float = 0 # slider or number step, stored units; 0 = 1 + kind: str = "" # entity picker filter, one of ENTITY_KINDS + show: tuple = () # ((name, op, value), ...), all must hold + applies: str = RESTART + simple: bool = False # also on Settings, Simple + wizard: int = 0 # setup wizard step 1-6, 0 = not in the wizard + + @property + def text(self) -> str: + """Where the label and help live: `translations` or `ui_text`.""" + return "ui_text" if self.key.startswith("ui.") else "translations" + + +@dataclass(frozen=True) +class Part: + id: str # "" is the group's main part + fields: tuple + collapsed: bool = False + + +@dataclass(frozen=True) +class Group: + id: str + parts: tuple + support_only: bool = False # collapsed, with a "for support" warning + + +def _src(*values): + return (("meter_source", "in", values),) + + +_DYNAMIC = (("ui.contract", "==", "dynamic"),) +_MAINT = (("maintenance_enabled", "==", True),) +_ONE = (("inverter_count", "==", 1),) +_SEVERAL = (("inverter_count", ">", 1),) +_PHASES = (("meter_phases", ">", 1),) + +# The per-inverter card. Its keys are `inverters.` in config.yaml. +ROSTER_FIELDS = ( + Field("inverters.name", TEXT, wizard=2), + Field("inverters.setpoint_entity", ENTITY, kind="number", wizard=2), + Field("inverters.max_w", SLIDER, "W", step=100, wizard=2), + Field("inverters.bms_soc_floor", SLIDER, "%", wizard=2), + Field("inverters.soc_entity", ENTITY, kind="pct", wizard=2), + Field("inverters.batt_entity", ENTITY, kind="power_w", wizard=2), + Field("inverters.batt_invert", TOGGLE, wizard=2), + Field("inverters.capacity_kwh", NUMBER, "kWh", step=0.1, wizard=2), +) + +GROUPS = ( + Group("meter", ( + Part("", ( + Field("meter_source", CHOICE, wizard=1), + Field("meter_entity", ENTITY, kind="power_w", show=_src("off"), + wizard=1), + Field("meter_invert", TOGGLE, show=_src("off"), wizard=1), + Field("p1_net_entity", ENTITY, kind="power_w", + show=_src("ha_signed"), wizard=1), + Field("p1_import_entity", ENTITY, kind="power_w", + show=_src("ha_dsmr"), wizard=1), + Field("p1_export_entity", ENTITY, kind="power_w", + show=_src("ha_dsmr"), wizard=1), + Field("p1_host", TEXT, show=_src("homewizard_local"), wizard=1), + Field("meter_mqtt_topic", TEXT, show=_src("mqtt_p1"), wizard=1), + Field("meter_phases", SEGMENTED, wizard=1), + Field("p1_phase_net_entities", ENTITY_LIST, kind="power_w", + show=_src("ha_signed") + _PHASES, wizard=1), + Field("p1_phase_import_entities", ENTITY_LIST, kind="power_w", + show=_src("ha_dsmr") + _PHASES, wizard=1), + Field("p1_phase_export_entities", ENTITY_LIST, kind="power_w", + show=_src("ha_dsmr") + _PHASES, wizard=1), + )), + Part("timing", ( + Field("meter_max_age_s", SLIDER, "s", step=5), + Field("meter_poll_s", SLIDER, "s", + show=_src("homewizard_local")), + ), collapsed=True), + )), + Group("inverters", ( + Part("", (Field("inverters", INVERTER_LIST, wizard=2),)), + # pv.py reads each entity's own unit, so a kW sensor is fine here. + Part("solar", (Field("ui.solar", SOLAR_LIST, kind="power", + wizard=3),)), + )), + Group("tariff", ( + Part("", ( + Field("ui.contract", SEGMENTED, wizard=4), + Field("price_provider", SELECT, show=_DYNAMIC, wizard=4), + Field("price_formula", TEXT, + show=_DYNAMIC + (("price_provider", "==", "custom_formula"),), + wizard=4), + Field("price_csv_path", TEXT, + show=_DYNAMIC + (("price_provider", "==", "manual_csv"),), + wizard=4), + Field("ui.region", SELECT, wizard=4), + Field("price_taxes_eur_kwh", NUMBER, "€/kWh", step=0.00001, + wizard=4), + Field("price_markup_eur_kwh", NUMBER, "€/kWh", step=0.001, + show=_DYNAMIC, wizard=4), + Field("export_price_eur_kwh", NUMBER, "€/kWh", step=0.001, + wizard=4), + Field("price_export_formula", TEXT), + Field("ui.fallback", SEGMENTED, show=_DYNAMIC, wizard=4), + Field("ui.fixed_price", NUMBER, "€/kWh", step=0.01, + show=(("ui.fallback", "==", "fixed"),), wizard=4), + Field("ui.day_price", NUMBER, "€/kWh", step=0.01, + show=(("ui.fallback", "==", "daynight"),), wizard=4), + Field("ui.night_price", NUMBER, "€/kWh", step=0.01, + show=(("ui.fallback", "==", "daynight"),), wizard=4), + )), + Part("selling", ( + # OPT-11: derived from the batteries' selling switches once battery + # modes exist (uimodel.arbitrage_from_batteries). A toggle until then. + Field("arbitrage_enabled", TOGGLE), + Field("cycle_cost_threshold_eur", NUMBER, "€/kWh", step=0.01), + Field("arbitrage_soc_floor", SLIDER, "%"), + Field("arbitrage_soc_ceiling", SLIDER, "%"), + )), + Part("feed", ( + Field("price_feed_url", TEXT, show=_DYNAMIC), + Field("price_horizon_h", SLIDER, "h", show=_DYNAMIC), + Field("price_timeout_s", SLIDER, "s", show=_DYNAMIC), + Field("price_now_entity", ENTITY, kind="any"), + Field("price_avg_entity", ENTITY, kind="any"), + ), collapsed=True), + )), + Group("safety", ( + Part("peak", ( + Field("peak_cap_w", SLIDER, "kW", scale=1000, step=100, + simple=True, wizard=5), + Field("peak_forecast_entity", ENTITY, kind="power_w", wizard=5), + Field("peak_soc_floor", LOCKED, "%"), + )), + Part("start", ( + Field("shadow_mode", SEGMENTED, simple=True, wizard=6), + Field("auto_start", TOGGLE), + Field("estop_fitted", TOGGLE), + Field("stale_input_s", SLIDER, "s"), + Field("meter_freeze_s", SLIDER, "s", step=30, + show=_src("homewizard_local")), + )), + Part("stops", ( + Field("sign_disagree_w", LOCKED, "W", step=10, show=_ONE), + Field("sign_disagree_s", LOCKED, "s", step=5, show=_ONE), + Field("unit_sign_disagree_w", LOCKED, "W", step=10, + show=_SEVERAL), + Field("unit_sign_disagree_s", LOCKED, "s", step=5, + show=_SEVERAL), + Field("site_sign_disagree_w", LOCKED, "W", step=10, + show=_SEVERAL), + Field("site_sign_disagree_s", LOCKED, "s", step=5, + show=_SEVERAL), + Field("circulation_w", LOCKED, "W", step=10, show=_SEVERAL), + Field("circulation_hold_s", LOCKED, "s", step=5, show=_SEVERAL), + Field("delivery_hold_s", LOCKED, "s", step=5, show=_SEVERAL), + ), collapsed=True), + )), + Group("maintenance", ( + Part("", ( + Field("maintenance_enabled", TOGGLE), + Field("maintenance_interval_days", SLIDER, "days", show=_MAINT), + Field("maintenance_start_hour", SELECT, show=_MAINT), + Field("maintenance_discharge_w", SLIDER, "W", step=100, + show=_MAINT), + Field("maintenance_charge_w", SLIDER, "W", step=100, + show=_MAINT), + Field("maintenance_soc_floor", SLIDER, "%", show=_MAINT), + Field("maintenance_soc_target", SLIDER, "%", show=_MAINT), + Field("maintenance_hold_min", SLIDER, "min", step=5, + show=_MAINT), + )), + Part("support", (Field("log_level", SELECT),)), + )), + Group("tuning", ( + Part("", ( + Field("gain", LOCKED, step=0.05), + Field("slew_w", LOCKED, "W", step=50), + Field("deadband_w", LOCKED, "W"), + Field("target_grid_w", LOCKED, "W"), + Field("step_w", LOCKED, "W"), + Field("saturation_w", LOCKED, "W", step=10), + Field("saturation_cycles", LOCKED), + Field("integrator_max_w", LOCKED, "W", step=100), + Field("heartbeat_s", LOCKED, "s"), + Field("cycle_floor_s", LOCKED, "s"), + )), + ), support_only=True), +) + +# Option keys the page never shows. uimodel.py writes each from page fields. +DERIVED = { + "setpoint_entity": "inverters", # one inverter, stored flat + "soc_entity": "inverters", + "batt_entity": "inverters", + "batt_invert": "inverters", + "max_w": "inverters", + "bms_soc_floor": "inverters", + "pv_entity": "ui.solar", # one solar row, stored flat + "pv_entities": "ui.solar", + "pv_invert_entities": "ui.solar", # the rows' "reads negative" toggles + "static_profile": "ui.fallback", # built from the price fields + "price_taxes_profile": "ui.region", # a preset writes it, "own" keeps it +} +# Option keys the page never shows and never writes: passed through as stored. +HIDDEN: dict = {} + + +def fields(): + """Every field once, in page order, roster fields after `inverters`.""" + for group in GROUPS: + for part in group.parts: + for field in part.fields: + yield group, part, field + if field.control == INVERTER_LIST: + for sub in ROSTER_FIELDS: + yield group, part, sub + + +def field(key: str) -> Field | None: + return next((f for _g, _p, f in fields() if f.key == key), None) + + +def visible(fld: Field, model: dict) -> bool: + """Whether `fld` is shown for this page model (uimodel.options_to_ui).""" + for name, op, value in fld.show: + have = model.get(name) + if op == "==" and have != value: + return False + if op == "!=" and have == value: + return False + if op == "in" and have not in value: + return False + try: + if op == ">" and not float(have) > value: + return False + if op == "<" and not float(have) < value: + return False + except (TypeError, ValueError): + return False + return True + + +def range_of(key: str, schema_ui: list): + """(min, max) from the Supervisor's schema (`info["schema"]`), or None. + + Never typed here: config.yaml is the one place a range is written. A range + arrives as lengthMin/lengthMax (tools/supervisor_selfapi_probe.py).""" + path = key.split(".") + nodes = schema_ui or [] + for step in path[:-1]: + parent = next((n for n in nodes if n.get("name") == step), None) + nodes = (parent or {}).get("schema") or [] + node = next((n for n in nodes if n.get("name") == path[-1]), None) + if not node or "lengthMin" not in node or "lengthMax" not in node: + return None + return node["lengthMin"], node["lengthMax"] diff --git a/goodwe_controller/app/uimodel.py b/goodwe_controller/app/uimodel.py new file mode 100644 index 0000000..f5f9c5a --- /dev/null +++ b/goodwe_controller/app/uimodel.py @@ -0,0 +1,326 @@ +"""UX-13: stored options <-> the page model, for the keys the page derives. + +`options_to_ui()` turns the stored blob into what the page edits: always a list +of inverter cards, solar rows with a "reads negative" toggle, a contract and +simple prices instead of `static_profile` JSON, and a region preset instead of +the levies figure. `ui_to_options()` turns it back, so the validator and the +Supervisor always see the real blob. + +The rule that makes the round trip exact: the model carries the stored values +of the derived keys (`keep`). A derivation whose page fields still read what +`keep` decodes to writes `keep` back untouched, and only an edited one is +re-encoded. So a blob the page cannot express (a `static_profile` with odd +windows, a malformed roster) survives a save of some other field unchanged. +See DECISIONS/UX-13-one-layout-table.md. +""" + +import copy +import json + +from .price import FLEMISH_TAXES_COMPONENTS_EUR_KWH +from .static_tariff import ProfileInvalid, parse_static_profile + +__all__ = ["options_to_ui", "ui_to_options", "REGION_PRESETS", + "arbitrage_from_batteries", "DeriveError"] + + +class DeriveError(ValueError): + """The page model asks for something no options blob can say.""" + + +# --------------------------------------------------------------------------- # +# Region presets for "Grid costs, levies and VAT". Year-stamped, updated each +# January (owner, 2026-09-27). Only presets with a sourced figure are listed: +# Flanders is app/price.py's Fluvius Antwerpen sum (TARIFF-18). Wallonia and +# Brussels have a day and a night band (config.yaml) and join with sourced +# figures; until then those sites choose "own". +# --------------------------------------------------------------------------- # +REGION_PRESETS = { + "flanders": { + "year": 2026, + "taxes_eur_kwh": round(sum(v for _n, v in + FLEMISH_TAXES_COMPONENTS_EUR_KWH), 5), + "taxes_profile": "", + }, +} +OWN = "own" + +_ROSTER_FLAT = ("setpoint_entity", "soc_entity", "batt_entity", "batt_invert", + "max_w", "bms_soc_floor") +_TOL = 1e-9 + + +def _blank(v) -> bool: + return v is None or (isinstance(v, str) and not v.strip()) + + +def _num(v): + try: + return None if _blank(v) or isinstance(v, bool) else float(v) + except (TypeError, ValueError): + return None + + +# --- 1. one inverter (flat keys) or the `inverters` list -------------------- # +def _roster_decode(o): + entries = o.get("inverters") + if entries is None or entries == "" or entries == []: + card = {k: o.get(k) for k in _ROSTER_FLAT} + card.update(name="", capacity_kwh=None) + return {"inverter_shape": "flat", "inverters": [card]} + if isinstance(entries, list) and all(isinstance(e, dict) for e in entries): + return {"inverter_shape": "list", + "inverters": [dict(e) for e in entries]} + # Not something a card can show: edited in the Configuration tab only. + return {"inverter_shape": "raw", "inverters": None} + + +def _roster_encode(m, keep): + out = {} + shape, cards = m.get("inverter_shape"), m.get("inverters") + if shape == "raw": + raise DeriveError("the inverter list is edited in the Configuration tab") + if not cards: + raise DeriveError("at least one inverter is needed") + if shape == "flat" and len(cards) == 1: + card = cards[0] + if not _blank(card.get("name")) or not _blank(card.get("capacity_kwh")): + raise DeriveError("one inverter stored flat has no name or capacity") + for k in _ROSTER_FLAT: + if card.get(k) is not None or k in keep: + out[k] = card.get(k) + out["inverters"] = [] + return out + out["inverters"] = [{k: v for k, v in c.items() if v is not None} + for c in cards] + if shape == "flat": + # CFG-04 rule 9e: a flat power request entity beside a list is refused. + # The other flat keys stay as stored and are not read. + out["setpoint_entity"] = "" + return out + + +# --- 2. solar rows: pv_entity / pv_entities / pv_invert_entities ------------ # +def _strs(v): + return [str(x).strip() for x in (v if isinstance(v, list) else []) + if not _blank(x)] + + +def _solar_rows(o): + listed = _strs(o.get("pv_entities")) + if listed: + return listed, True + one = "" if _blank(o.get("pv_entity")) else str(o.get("pv_entity")).strip() + return ([one] if one else []), False + + +def _solar_decode(o): + rows, _listed = _solar_rows(o) + flipped = set(_strs(o.get("pv_invert_entities"))) + return {"ui.solar": [{"entity": e, "reversed": e in flipped} + for e in rows]} + + +def _solar_encode(m, keep): + out = {} + rows = [r for r in (m.get("ui.solar") or []) + if not _blank(r.get("entity"))] + entities = [str(r["entity"]).strip() for r in rows] + old_rows, was_list = _solar_rows(keep) + if len(entities) <= 1 and not was_list: + out["pv_entity"] = entities[0] if entities else "" + if "pv_entities" in keep: + out["pv_entities"] = [] + else: + out["pv_entities"] = entities + if "pv_entity" in keep: + out["pv_entity"] = "" # TEL-12: the list replaces it + # Flips of entities that were never a row stay as stored (pv.stray_inverts + # reports them). Flips of removed rows go with the row. + extras = [x for x in _strs(keep.get("pv_invert_entities")) + if x not in old_rows and x not in entities] + flips = [e for e, r in zip(entities, rows) if r.get("reversed")] + extras + if flips or "pv_invert_entities" in keep: + out["pv_invert_entities"] = flips + return out + + +# --- 3. contract and static_profile ----------------------------------------- # +def _fixed_json(price) -> str: + return json.dumps({"periods": [ + {"name": "all", "from": "00:00", "to": "24:00", "days": "mon-sun", + "eur_per_kwh": price}]}) + + +def _daynight_json(day, night) -> str: + return json.dumps({"periods": [ + {"name": "day", "from": "07:00", "to": "22:00", "days": "mon-fri", + "eur_per_kwh": day}, + {"name": "night", "from": "22:00", "to": "07:00", "days": "mon-fri", + "eur_per_kwh": night}, + {"name": "night", "from": "00:00", "to": "24:00", "days": "sat,sun", + "eur_per_kwh": night}]}) + + +def _shape(text): + """The periods of a profile without their names, for comparing, or None.""" + try: + prof = parse_static_profile(text) + except ProfileInvalid: + return None + if prof is None: + return () + return tuple(sorted((p.start_min, p.end_min, tuple(sorted(p.days)), + p.eur_per_kwh) for p in prof.periods)) + + +def _profile_decode(raw): + """("none"|"fixed"|"daynight"|"custom", fixed, day, night).""" + if isinstance(raw, str) and _blank(raw): + return "none", None, None, None + if not isinstance(raw, str): + return "custom", None, None, None + got = _shape(raw) + if not got: + return ("none" if got == () else "custom"), None, None, None + if len(got) == 1 and got == _shape(_fixed_json(got[0][3])): + return "fixed", got[0][3], None, None + day = next((p[3] for p in got if p[:2] == (420, 1320)), None) + night = next((p[3] for p in got if p[:2] == (1320, 420)), None) + if (day is not None and night is not None + and got == _shape(_daynight_json(day, night))): + return "daynight", None, day, night + return "custom", None, None, None + + +def _tariff_decode(o): + kind, fixed, day, night = _profile_decode(o.get("static_profile", "")) + provider = o.get("price_provider") + dynamic = not _blank(provider) and provider != "off" + return {"ui.contract": "dynamic" if dynamic else kind, + "price_provider": provider, "ui.fallback": kind, + "ui.fixed_price": fixed, "ui.day_price": day, + "ui.night_price": night} + + +def _tariff_encode(m, keep): + out = {} + contract = m.get("ui.contract") + if contract == "dynamic": + if _blank(m.get("price_provider")) or m.get("price_provider") == "off": + raise DeriveError("a dynamic contract needs a price source") + out["price_provider"] = m["price_provider"] + kind = m.get("ui.fallback") + else: + out["price_provider"] = "off" + kind = contract + if kind == "none": + out["static_profile"] = "" + elif kind == "fixed": + out["static_profile"] = _fixed_json(_price_of(m, "ui.fixed_price")) + elif kind == "daynight": + out["static_profile"] = _daynight_json(_price_of(m, "ui.day_price"), + _price_of(m, "ui.night_price")) + elif kind == "custom": + if "static_profile" not in keep: + raise DeriveError("odd tariff windows are edited in the " + "Configuration tab") + out["static_profile"] = keep["static_profile"] + else: + raise DeriveError(f"unknown contract {kind!r}") + return out + + +def _price_of(m, name): + value = _num(m.get(name)) + if value is None: + raise DeriveError(f"{name} needs a price") + return value + + +# --- 4. region preset -> price_taxes_eur_kwh / price_taxes_profile ---------- # +def _region_decode(o): + taxes, profile = o.get("price_taxes_eur_kwh"), o.get("price_taxes_profile", "") + region = OWN + for rid, p in REGION_PRESETS.items(): + same = _num(taxes) is not None and abs(_num(taxes) - p["taxes_eur_kwh"]) < _TOL + if same and str(profile or "").strip() == p["taxes_profile"]: + region = rid + return {"ui.region": region, "price_taxes_eur_kwh": taxes} + + +def _region_encode(m, keep): + region = m.get("ui.region") + if region == OWN: + return {"price_taxes_eur_kwh": m.get("price_taxes_eur_kwh")} + if region not in REGION_PRESETS: + raise DeriveError(f"unknown region {region!r}") + p = REGION_PRESETS[region] + out = {"price_taxes_eur_kwh": p["taxes_eur_kwh"]} + if p["taxes_profile"] or "price_taxes_profile" in keep: + out["price_taxes_profile"] = p["taxes_profile"] + return out + + +# --- 5. arbitrage_enabled: the OPT-11 hook ---------------------------------- # +def arbitrage_from_batteries(m): + """OPT-11 hook. With battery modes on, `arbitrage_enabled` is true when any + battery may sell. Battery modes are not built yet, so this returns None and + the toggle is stored as the page has it: no behaviour change.""" + return None + + +def _arbitrage_decode(o): + return {"arbitrage_enabled": o.get("arbitrage_enabled")} + + +def _arbitrage_encode(m, keep): + derived = arbitrage_from_batteries(m) + value = m.get("arbitrage_enabled") if derived is None else derived + return {} if value is None and "arbitrage_enabled" not in keep \ + else {"arbitrage_enabled": value} + + +# (option keys it writes, decode, encode). Each is independent of the others. +DERIVATIONS = ( + (("inverters",) + _ROSTER_FLAT, _roster_decode, _roster_encode), + (("pv_entity", "pv_entities", "pv_invert_entities"), _solar_decode, + _solar_encode), + (("price_provider", "static_profile"), _tariff_decode, _tariff_encode), + (("price_taxes_eur_kwh", "price_taxes_profile"), _region_decode, + _region_encode), + (("arbitrage_enabled",), _arbitrage_decode, _arbitrage_encode), +) +_OWNED = {k for keys, _d, _e in DERIVATIONS for k in keys} +_MODEL_ONLY = ("keep", "inverter_shape", "inverter_count", "region_year") + + +def options_to_ui(options: dict) -> dict: + """The page model for a stored blob. Pure: reads nothing else.""" + model = {k: copy.deepcopy(v) for k, v in options.items() + if k not in _OWNED} + for _keys, decode, _encode in DERIVATIONS: + model.update(decode(options)) + model["keep"] = {k: copy.deepcopy(options[k]) for k in _OWNED + if k in options} + cards = model.get("inverters") + model["inverter_count"] = len(cards) if isinstance(cards, list) else None + preset = REGION_PRESETS.get(model["ui.region"]) + model["region_year"] = preset["year"] if preset else None + return model + + +def ui_to_options(model: dict) -> dict: + """The options blob for a page model. Raises DeriveError on a model no blob + can express, so the page says so instead of saving something else.""" + keep = model.get("keep") or {} + out = {k: copy.deepcopy(v) for k, v in model.items() + if k not in _MODEL_ONLY and k not in _OWNED + and not k.startswith("ui.")} + for keys, decode, encode in DERIVATIONS: + was = decode(keep) + if all(model.get(name) == value for name, value in was.items()): + out.update({k: copy.deepcopy(keep[k]) for k in keys if k in keep}) + else: + out.update(encode(model, keep)) + return out diff --git a/goodwe_controller/app/validate.py b/goodwe_controller/app/validate.py index f006133..5177739 100644 --- a/goodwe_controller/app/validate.py +++ b/goodwe_controller/app/validate.py @@ -321,8 +321,9 @@ WHAT IS DELIBERATELY *NOT* REFUSED, because it is untidy rather than unsafe: """ import logging +from typing import NamedTuple -from . import messages +from . import messages, pv from .arbitrage import enabled as arbitrage_on from .control import SLEW_RAIL_FRAC, LOOP_TICK_S, PLANT_SETTLE_S from .custom_formula import ( @@ -330,7 +331,8 @@ from .custom_formula import ( parse_export_formula, parse_price_formula, stale_card) from .inverter import roster_entries from .maintenance import MAINT_OPTION_DEFAULTS -from .p1 import is_enabled, validate_meter_options +from .p1 import ( + SOURCE_HA, SOURCE_HA_SIGNED, is_enabled, validate_meter_options) from .peakguard import PEAK_OPTION_DEFAULTS from .price import ( PRICE_OPTION_DEFAULTS, PROVIDERS, RETIRED_PROVIDERS, @@ -367,6 +369,22 @@ class _WarnOnce(logging.Filter): WARN_ONCE = _WarnOnce() _LOG.addFilter(WARN_ONCE) + +class Note(NamedTuple): + """UX-13: a warning or info for the settings page. Never a refusal: the + add-on starts. `keys` are the options it is about, so the page can show it + beside them.""" + tier: str # "warn" or "info" + keys: tuple + text: str + + +def _advise(notes, tier: str, keys, text: str) -> None: + """Add a Note when the caller collects them (the settings page).""" + if notes is not None: + notes.append(Note(tier, tuple(keys), text)) + + # ⚠️ A MIRROR OF A FIRMWARE CONSTANT, AND IT CANNOT BE IMPORTED. The firmware # threshold is a compile-time YAML substitution baked into the ESP32 image # (`max_age_s: "30"`, master/goodwe-master-stage4-failsafe.yaml), so nothing at @@ -789,7 +807,8 @@ class ConfigRefused(Exception): super().__init__(problems[0] if problems else "configuration refused") -def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str]: +def validate_startup_config(opts: dict, dev_min=None, dev_max=None, + notes: list | None = None) -> list[str]: """Return the list of reasons this configuration must not be started. Empty list means go. `dev_min`/`dev_max` are the setpoint device's own @@ -799,6 +818,9 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] Pure and side-effect free apart from two warnings, so it can be called from a test or from sim/inject.py without a Home Assistant anywhere near it. + + UX-13: pass a list as `notes` to also get each logged warning as a `Note` + for the settings page. The refusals returned do not depend on it. """ problems: list[str] = [] @@ -1127,7 +1149,7 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] f"site. Set it to 0 or to at most {bound:.0f} W.") # --- 7. two price sources on one site ------------------------------------ - problems.extend(_price_problems(opts)) + problems.extend(_price_problems(opts, notes)) # --- 8. an EMS energy floor the BMS does not already enforce ------------- # ⚠️ THE FALLBACKS ARE peakguard.PEAK_OPTION_DEFAULTS, NOT LITERALS, and @@ -1173,6 +1195,9 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] "integrator_max_w %.0f W is above max_w %.0f W. This is allowed, " "but it only adds overshoot, so 0 is recommended.", integ_w, max_w) + _advise(notes, "warn", ("integrator_max_w",), + f"{S('integrator_max_w')} is above {S('max_w')}, which only " + "adds overshoot, so set it to 0.") # ⚠️ THE SIBLING OF RULE 6, AND NOT THE SAME CASE - see the docstring. The # drain phase OWNS the actuator (`Claim.set` at P_MAINTENANCE), and a SET is @@ -1197,6 +1222,10 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] "maintenance_discharge_w %.0f W is above max_w%s %.0f W, so " "draining runs at %.0f W. Lower it to %.0f W or raise max_w.", disch_w, maint_where, maint_cap, maint_cap, maint_cap) + _advise(notes, "warn", ("maintenance_discharge_w",), + f"{S('maintenance_discharge_w')} is above " + f"{S('max_w', maint_where)}, so the battery empties at " + f"{messages.watts(maint_cap)} and the cycle takes longer.") _LOG.debug( "Not refused: the drain phase sets the setpoint, the arbiter " "clamps it to %.0f W and names the clamp, and the drain phase has " @@ -1205,7 +1234,7 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None) -> list[str] return problems -def _price_problems(opts: dict) -> list[str]: +def _price_problems(opts: dict, notes: list | None = None) -> list[str]: """Rule 7 (TARIFF-02): the price configuration, refused where it is untrue. ⚠️ NOTE WHAT IS *NOT* HERE. `price_provider: "off"` is not refused and never @@ -1423,6 +1452,9 @@ def _price_problems(opts: dict) -> list[str]: "%s has negative levies %s EUR/kWh, which are subtracted from " "the price. This is allowed, but it is usually a sign error.", TAXES_PROFILE_OPTION, negative) + _advise(notes, "warn", (TAXES_PROFILE_OPTION,), + f"{S(TAXES_PROFILE_OPTION)} has negative figures, which " + "lower the price, and that is usually a sign error.") # ⚠️ TARIFF-15: THE FORMULA, GRADED AGAINST THE OPERATOR'S OWN CARD, AT # STARTUP RATHER THAN AT 03:00. @@ -1478,6 +1510,10 @@ def _price_problems(opts: dict) -> list[str]: "price_formula is filled in, but price_provider is %r, so the " "formula is not used. Set price_provider to 'custom_formula' " "or clear price_formula.", pid) + _advise(notes, "warn", ("price_formula", "price_provider"), + f"{S('price_formula')} is not used, because " + f"{S('price_provider')} is not 'custom_formula', so clear " + "it or choose that source.") if spec is not None and pid == "custom_formula": taxes = _number(opts, "price_taxes_eur_kwh", PRICE_OPTION_DEFAULTS["price_taxes_eur_kwh"]) @@ -1491,6 +1527,8 @@ def _price_problems(opts: dict) -> list[str]: # exactly right and a site refused over one is a site that # cannot start. _LOG.warning("%s", aged) + _advise(notes, "warn", ("price_formula",), + _card_note("price_formula", spec)) # ⚠️ TARIFF-16: THE INJECTION LEG, REFUSED AT STARTUP ON THE SAME RULE. # @@ -1571,6 +1609,9 @@ def _price_problems(opts: dict) -> list[str]: "%.4g, offset %+.5f EUR/kWh).", EXPORT_FORMULA_OPTION, xspec.passthrough, xspec.offset_eur_kwh) + _advise(notes, "info", (EXPORT_FORMULA_OPTION,), + f"{S(EXPORT_FORMULA_OPTION)} has no card block, so " + "only the shape of its numbers is checked.") _LOG.debug( "Accepted because some suppliers (Eneco) print the " "injection formula with no price beside it. If your card " @@ -1582,6 +1623,8 @@ def _price_problems(opts: dict) -> list[str]: xaged = stale_card(xspec) if xaged: _LOG.warning("%s", xaged) + _advise(notes, "warn", (EXPORT_FORMULA_OPTION,), + _card_note(EXPORT_FORMULA_OPTION, xspec)) # ⚠️ A WARNING, NOT A REFUSAL, AND THE ASYMMETRY IS DELIBERATE. §11.4 says a # site with no price data falls back to the static profile; a dynamic site @@ -1593,6 +1636,9 @@ def _price_problems(opts: dict) -> list[str]: "price_provider is %r but static_profile is empty, so a price " "feed outage leaves no tariff at all. Fill in static_profile as a " "fallback.", pid) + _advise(notes, "warn", ("static_profile",), + f"{S('static_profile')} is empty, so the site has no tariff " + "at all while the price feed is down.") # ⚠️ OPT-04. THE THREE WAYS `arbitrage_enabled: true` CAN BE A LIE, refused # rather than logged, because a strategy that is armed in the UI and cannot @@ -1644,6 +1690,85 @@ def _price_problems(opts: dict) -> list[str]: return problems +def _card_note(option: str, spec) -> str: + return (f"{S(option)} uses a tariff card dated {spec.card_month}, so copy " + "the new figures if your contract changed.") + + +# UX-13: entity options whose number the add-on takes as watts, with no unit +# conversion. pv.py converts each solar entity's own unit, so those are not here. +WATT_ENTITY_KEYS = ( + "meter_entity", "p1_net_entity", "p1_import_entity", "p1_export_entity", + "p1_phase_net_entities", "p1_phase_import_entities", + "p1_phase_export_entities", "batt_entity", "peak_forecast_entity") + + +def _entities_read(opts: dict) -> list[tuple[str, str, str]]: + """(key, inverter name or "", entity id) for each entity the add-on reads + with these options. Keys the chosen meter source or roster shape ignores + are left out, so a stale default is not reported.""" + out = [] + + def add(key, value, where=""): + for e in (value if isinstance(value, list) else [value]): + if isinstance(e, str) and e.strip(): + out.append((key, where, e.strip())) + + source = str(opts.get("meter_source") or "off").strip() + by_source = {"off": ("meter_entity",), + SOURCE_HA_SIGNED: ("p1_net_entity", "p1_phase_net_entities"), + SOURCE_HA: ("p1_import_entity", "p1_export_entity", + "p1_phase_import_entities", + "p1_phase_export_entities")} + for key in by_source.get(source, ()): + add(key, opts.get(key)) + entries = roster_entries(opts) + if not entries: + for key in ("setpoint_entity", "soc_entity", "batt_entity"): + add(key, opts.get(key)) + for i, entry in enumerate(entries): + if isinstance(entry, dict): + where = _unit_label(opts, i).strip().strip("()") + for key in ("setpoint_entity", "soc_entity", "batt_entity"): + add(key, entry.get(key), where) + solar = [e for e in (opts.get("pv_entities") or []) if str(e or "").strip()] + add("pv_entities" if solar else "pv_entity", list(pv.entities(opts))) + for key in ("peak_forecast_entity", "price_now_entity", "price_avg_entity"): + add(key, opts.get(key)) + return out + + +def config_notes(opts: dict, *, entities: dict | None = None, + month_peak_w=None) -> list[Note]: + """UX-13: warnings that need live data, for the settings page. Never a + refusal, and nothing here changes what `validate_startup_config()` says. + + `entities` is Home Assistant's entity list by id (app/haentities.py), or + None when it could not be read, in which case those checks are skipped. + `month_peak_w` is this month's peak so far, or None. + """ + notes: list[Note] = [] + if entities is not None: + for key, where, entity in _entities_read(opts): + row = entities.get(entity) + if row is None: + _advise(notes, "warn", (key,), + f"Home Assistant has no entity {entity}, which " + f"{S(key, where)} names.") + continue + unit = str(row.get("unit") or "") + if key in WATT_ENTITY_KEYS and unit in pv.WATTS_PER and unit != "W": + _advise(notes, "warn", (key,), + f"{entity} reads in {unit}, but {S(key, where)} is " + "taken as watts, so pick a sensor in W.") + cap = _number(opts, "peak_cap_w", None) + if month_peak_w is not None and cap is not None and cap < month_peak_w: + _advise(notes, "warn", ("peak_cap_w",), + f"This month's peak is already {messages.watts(month_peak_w)}, " + f"so a lower {S('peak_cap_w')} saves nothing until next month.") + return notes + + def _number(opts: dict, key, default): raw = opts.get(key, default) if raw is None or raw == "": diff --git a/goodwe_controller/app/web.py b/goodwe_controller/app/web.py index a774fa3..aa117a9 100644 --- a/goodwe_controller/app/web.py +++ b/goodwe_controller/app/web.py @@ -24,7 +24,8 @@ routes below read `GET /addons/self/info`, run the SHIPPED validator, write mode or reaches `controller.handle_action()`. The set of things an operator can do to the inverter through this panel is exactly what `/action` offered before this card - test_selfconfig.py part 7 asserts that by listing the routes and the -actions and comparing them with the shipped set. +actions and comparing them with the shipped set. UX-13's `ha/entities` is a GET +of Home Assistant's entity list (app/haentities.py) and writes nothing. """ import logging @@ -32,6 +33,8 @@ import logging from aiohttp import web from . import messages, selfconfig +from .haentities import EntityCache +from .validate import config_notes from .config_page import CONFIG_PAGE from .configui import changed_keys, field_model, problems_by_field @@ -140,6 +143,7 @@ def build_app(controller, selfcfg=None) -> web.Application: if selfcfg is None: selfcfg = selfconfig.SelfConfig(controller.hass.session, controller.hass.token) + entities = EntityCache(controller.hass) async def index(_request): return web.Response(text=PAGE, content_type="text/html") @@ -216,11 +220,16 @@ def build_app(controller, selfcfg=None) -> web.Application: be a second opinion about a rule it does not own. """ _body, options = await _proposed(request) + notes = [] problems, unread = await selfconfig.problems_and_ranges( - options, controller.hass) + options, controller.hass, notes=notes) by_key, unanchored = problems_by_field(problems, _keys_of(options)) + # UX-13: warnings and info beside the refusals. Never block a save. + # The entity checks use the picker's cached list and never wait on HA. + notes += config_notes(options, entities=entities.peek()) payload = {"problems": problems, "by_key": by_key, - "unanchored": unanchored} + "unanchored": unanchored, + "notes": [n._asdict() for n in notes]} if unread: payload["device_note"] = DEVICE_NOTE % ", ".join(sorted(set(unread))) return web.json_response(payload) @@ -290,6 +299,12 @@ def build_app(controller, selfcfg=None) -> web.Application: hint=messages.hint("restart_requested")) return web.json_response({"ok": True}) + async def ha_entities(request): + """UX-13: Home Assistant's entities for the picker. A read only, and + never an error: when Home Assistant is down the list says so.""" + kind = str(request.query.get("kind", "") or "")[:16] + return web.json_response(await entities.payload(kind)) + app = web.Application() app.add_routes([ web.get("/", index), @@ -300,6 +315,7 @@ def build_app(controller, selfcfg=None) -> web.Application: web.post("/config/validate", config_validate), web.post("/config/save", config_save), web.post("/config/restart", config_restart), + web.get("/ha/entities", ha_entities), ]) return app diff --git a/goodwe_controller/scenarios/ux13/test23/options.arb.json b/goodwe_controller/scenarios/ux13/test23/options.arb.json new file mode 100644 index 0000000..e0a3ec6 --- /dev/null +++ b/goodwe_controller/scenarios/ux13/test23/options.arb.json @@ -0,0 +1,73 @@ +{ + "arbitrage_enabled": true, + "arbitrage_soc_ceiling": 95, + "arbitrage_soc_floor": 20, + "auto_start": false, + "batt_entity": "sensor.test23_battery_power", + "batt_invert": false, + "bms_soc_floor": 10, + "circulation_hold_s": 75, + "circulation_w": 700, + "cycle_cost_threshold_eur": 0.1, + "cycle_floor_s": 15, + "deadband_w": 15, + "delivery_hold_s": 185, + "estop_fitted": false, + "export_price_eur_kwh": 0.04, + "gain": 0.6, + "heartbeat_s": 10, + "integrator_max_w": 0, + "inverters": [], + "log_level": "info", + "maintenance_charge_w": 2000, + "maintenance_discharge_w": 2000, + "maintenance_enabled": false, + "maintenance_hold_min": 120, + "maintenance_interval_days": 28, + "maintenance_soc_floor": 11, + "maintenance_soc_target": 99, + "maintenance_start_hour": 10, + "max_w": 2000, + "meter_entity": "sensor.p1_meter_active_power", + "meter_freeze_s": 300, + "meter_invert": false, + "meter_max_age_s": 30, + "meter_mqtt_topic": "", + "meter_phases": 1, + "meter_poll_s": 5, + "meter_source": "ha_signed", + "p1_export_entity": "", + "p1_host": "", + "p1_import_entity": "", + "p1_net_entity": "sensor.test23_p1_meter_active_power", + "p1_phase_export_entities": [], + "p1_phase_import_entities": [], + "p1_phase_net_entities": [], + "peak_cap_w": 3500, + "peak_forecast_entity": "", + "peak_soc_floor": 20, + "price_avg_entity": "", + "price_csv_path": "/share/goodwe_prices.csv", + "price_feed_url": "https://api.energy-charts.info/price?bzn=BE", + "price_horizon_h": 48, + "price_markup_eur_kwh": 0.03, + "price_now_entity": "", + "price_provider": "generic_dayahead", + "price_taxes_eur_kwh": 0.06, + "price_timeout_s": 10, + "saturation_cycles": 3, + "saturation_w": 500, + "setpoint_entity": "number.test23_setpoint_w", + "sign_disagree_s": 180, + "sign_disagree_w": 300, + "site_sign_disagree_s": 325, + "site_sign_disagree_w": 300, + "slew_w": 0, + "soc_entity": "sensor.test23_soc", + "stale_input_s": 15, + "static_profile": "", + "step_w": 10, + "target_grid_w": -10, + "unit_sign_disagree_s": 85, + "unit_sign_disagree_w": 250 +} \ No newline at end of file diff --git a/goodwe_controller/scenarios/ux13/test23/options.baseline.json b/goodwe_controller/scenarios/ux13/test23/options.baseline.json new file mode 100644 index 0000000..768a884 --- /dev/null +++ b/goodwe_controller/scenarios/ux13/test23/options.baseline.json @@ -0,0 +1,73 @@ +{ + "arbitrage_enabled": false, + "arbitrage_soc_ceiling": 95, + "arbitrage_soc_floor": 20, + "auto_start": false, + "batt_entity": "", + "batt_invert": false, + "bms_soc_floor": 10, + "circulation_hold_s": 75, + "circulation_w": 700, + "cycle_cost_threshold_eur": 0.1, + "cycle_floor_s": 15, + "deadband_w": 15, + "delivery_hold_s": 185, + "estop_fitted": false, + "export_price_eur_kwh": 0.04, + "gain": 0.6, + "heartbeat_s": 10, + "integrator_max_w": 0, + "inverters": [], + "log_level": "info", + "maintenance_charge_w": 2000, + "maintenance_discharge_w": 2000, + "maintenance_enabled": false, + "maintenance_hold_min": 120, + "maintenance_interval_days": 28, + "maintenance_soc_floor": 11, + "maintenance_soc_target": 99, + "maintenance_start_hour": 10, + "max_w": 2000, + "meter_entity": "sensor.p1_meter_active_power", + "meter_freeze_s": 300, + "meter_invert": false, + "meter_max_age_s": 30, + "meter_mqtt_topic": "", + "meter_phases": 1, + "meter_poll_s": 5, + "meter_source": "off", + "p1_export_entity": "", + "p1_host": "", + "p1_import_entity": "", + "p1_net_entity": "", + "p1_phase_export_entities": [], + "p1_phase_import_entities": [], + "p1_phase_net_entities": [], + "peak_cap_w": 3500, + "peak_forecast_entity": "", + "peak_soc_floor": 20, + "price_avg_entity": "", + "price_csv_path": "/share/goodwe_prices.csv", + "price_feed_url": "", + "price_horizon_h": 48, + "price_markup_eur_kwh": 0.03, + "price_now_entity": "", + "price_provider": "off", + "price_taxes_eur_kwh": 0.06, + "price_timeout_s": 10, + "saturation_cycles": 3, + "saturation_w": 500, + "setpoint_entity": "", + "sign_disagree_s": 180, + "sign_disagree_w": 300, + "site_sign_disagree_s": 325, + "site_sign_disagree_w": 300, + "slew_w": 0, + "soc_entity": "", + "stale_input_s": 15, + "static_profile": "", + "step_w": 10, + "target_grid_w": -10, + "unit_sign_disagree_s": 85, + "unit_sign_disagree_w": 250 +} \ No newline at end of file diff --git a/goodwe_controller/scenarios/ux13/test23/options.doc1-roster.json b/goodwe_controller/scenarios/ux13/test23/options.doc1-roster.json new file mode 100644 index 0000000..551b715 --- /dev/null +++ b/goodwe_controller/scenarios/ux13/test23/options.doc1-roster.json @@ -0,0 +1,98 @@ +{ + "arbitrage_enabled": false, + "arbitrage_soc_ceiling": 95, + "arbitrage_soc_floor": 20, + "auto_start": false, + "batt_entity": "sensor.test23_battery_power", + "batt_invert": false, + "bms_soc_floor": 10, + "circulation_hold_s": 75, + "circulation_w": 700, + "cycle_cost_threshold_eur": 0.1, + "cycle_floor_s": 15, + "deadband_w": 15, + "delivery_hold_s": 185, + "estop_fitted": false, + "export_price_eur_kwh": 0.04, + "gain": 0.6, + "heartbeat_s": 10, + "integrator_max_w": 0, + "inverters": [ + { + "batt_entity": "sensor.goodwe_a_battery_power", + "bms_soc_floor": 10, + "capacity_kwh": 15.36, + "max_w": 5000, + "name": "inverter-a", + "setpoint_entity": "number.goodwe_a_setpoint_w", + "soc_entity": "sensor.goodwe_a_soc" + }, + { + "batt_entity": "sensor.goodwe_b_battery_power", + "bms_soc_floor": 10, + "capacity_kwh": 20.48, + "max_w": 4600, + "name": "inverter-b", + "setpoint_entity": "number.goodwe_b_setpoint_w", + "soc_entity": "sensor.goodwe_b_soc" + } + ], + "log_level": "info", + "maintenance_charge_w": 2000, + "maintenance_discharge_w": 2000, + "maintenance_enabled": false, + "maintenance_hold_min": 120, + "maintenance_interval_days": 28, + "maintenance_soc_floor": 11, + "maintenance_soc_target": 99, + "maintenance_start_hour": 10, + "max_w": 2000, + "meter_entity": "sensor.p1_meter_active_power", + "meter_freeze_s": 300, + "meter_invert": false, + "meter_max_age_s": 30, + "meter_mqtt_topic": "", + "meter_phases": 1, + "meter_poll_s": 5, + "meter_source": "ha_signed", + "p1_export_entity": "", + "p1_host": "", + "p1_import_entity": "", + "p1_net_entity": "sensor.test23_p1_meter_active_power", + "p1_phase_export_entities": [], + "p1_phase_import_entities": [], + "p1_phase_net_entities": [], + "peak_cap_w": 3500, + "peak_forecast_entity": "", + "peak_soc_floor": 20, + "price_avg_entity": "", + "price_csv_path": "/share/goodwe_prices.csv", + "price_export_formula": "", + "price_feed_url": "https://api.energy-charts.info/price?bzn=BE", + "price_formula": "", + "price_horizon_h": 48, + "price_markup_eur_kwh": 0.03, + "price_now_entity": "", + "price_provider": "generic_dayahead", + "price_taxes_eur_kwh": 0.06, + "price_taxes_profile": "", + "price_timeout_s": 10, + "pv_entities": [], + "pv_entity": "", + "saturation_cycles": 3, + "saturation_w": 500, + "setpoint_entity": "", + "shadow_mode": "auto", + "sign_disagree_s": 180, + "sign_disagree_w": 300, + "site_sign_disagree_s": 325, + "site_sign_disagree_w": 300, + "slew_w": 0, + "soc_entity": "sensor.test23_soc", + "stale_input_s": 15, + "static_profile": "", + "step_w": 10, + "target_grid_w": -10, + "unit_sign_disagree_s": 85, + "unit_sign_disagree_w": 250 +} \ No newline at end of file diff --git a/goodwe_controller/scenarios/ux13/test23/options.json b/goodwe_controller/scenarios/ux13/test23/options.json new file mode 100644 index 0000000..5a388aa --- /dev/null +++ b/goodwe_controller/scenarios/ux13/test23/options.json @@ -0,0 +1,73 @@ +{ + "arbitrage_enabled": false, + "arbitrage_soc_ceiling": 95, + "arbitrage_soc_floor": 20, + "auto_start": false, + "batt_entity": "sensor.test23_battery_power", + "batt_invert": false, + "bms_soc_floor": 10, + "circulation_hold_s": 75, + "circulation_w": 700, + "cycle_cost_threshold_eur": 0.1, + "cycle_floor_s": 15, + "deadband_w": 15, + "delivery_hold_s": 185, + "estop_fitted": false, + "export_price_eur_kwh": 0.04, + "gain": 0.6, + "heartbeat_s": 10, + "integrator_max_w": 0, + "inverters": [], + "log_level": "info", + "maintenance_charge_w": 2000, + "maintenance_discharge_w": 2000, + "maintenance_enabled": false, + "maintenance_hold_min": 120, + "maintenance_interval_days": 28, + "maintenance_soc_floor": 11, + "maintenance_soc_target": 99, + "maintenance_start_hour": 10, + "max_w": 2000, + "meter_entity": "sensor.p1_meter_active_power", + "meter_freeze_s": 300, + "meter_invert": false, + "meter_max_age_s": 30, + "meter_mqtt_topic": "", + "meter_phases": 1, + "meter_poll_s": 5, + "meter_source": "ha_signed", + "p1_export_entity": "", + "p1_host": "", + "p1_import_entity": "", + "p1_net_entity": "sensor.test23_p1_meter_active_power", + "p1_phase_export_entities": [], + "p1_phase_import_entities": [], + "p1_phase_net_entities": [], + "peak_cap_w": 3500, + "peak_forecast_entity": "", + "peak_soc_floor": 20, + "price_avg_entity": "", + "price_csv_path": "/share/goodwe_prices.csv", + "price_feed_url": "https://api.energy-charts.info/price?bzn=BE", + "price_horizon_h": 48, + "price_markup_eur_kwh": 0.03, + "price_now_entity": "", + "price_provider": "generic_dayahead", + "price_taxes_eur_kwh": 0.06, + "price_timeout_s": 10, + "saturation_cycles": 3, + "saturation_w": 500, + "setpoint_entity": "number.test23_setpoint_w", + "sign_disagree_s": 180, + "sign_disagree_w": 300, + "site_sign_disagree_s": 325, + "site_sign_disagree_w": 300, + "slew_w": 0, + "soc_entity": "sensor.test23_soc", + "stale_input_s": 15, + "static_profile": "", + "step_w": 10, + "target_grid_w": -10, + "unit_sign_disagree_s": 85, + "unit_sign_disagree_w": 250 +} \ No newline at end of file diff --git a/goodwe_controller/scenarios/ux13/test23/options.s3.json b/goodwe_controller/scenarios/ux13/test23/options.s3.json new file mode 100644 index 0000000..18354a9 --- /dev/null +++ b/goodwe_controller/scenarios/ux13/test23/options.s3.json @@ -0,0 +1,73 @@ +{ + "arbitrage_enabled": true, + "arbitrage_soc_ceiling": 95, + "arbitrage_soc_floor": 20, + "auto_start": true, + "batt_entity": "sensor.test23_battery_power", + "batt_invert": false, + "bms_soc_floor": 10, + "circulation_hold_s": 75, + "circulation_w": 700, + "cycle_cost_threshold_eur": 0.1, + "cycle_floor_s": 15, + "deadband_w": 15, + "delivery_hold_s": 185, + "estop_fitted": false, + "export_price_eur_kwh": 0.04, + "gain": 0.6, + "heartbeat_s": 10, + "integrator_max_w": 0, + "inverters": [], + "log_level": "info", + "maintenance_charge_w": 2000, + "maintenance_discharge_w": 2000, + "maintenance_enabled": false, + "maintenance_hold_min": 120, + "maintenance_interval_days": 28, + "maintenance_soc_floor": 11, + "maintenance_soc_target": 99, + "maintenance_start_hour": 10, + "max_w": 2000, + "meter_entity": "sensor.p1_meter_active_power", + "meter_freeze_s": 300, + "meter_invert": false, + "meter_max_age_s": 30, + "meter_mqtt_topic": "", + "meter_phases": 1, + "meter_poll_s": 5, + "meter_source": "ha_signed", + "p1_export_entity": "", + "p1_host": "", + "p1_import_entity": "", + "p1_net_entity": "sensor.test23_p1_meter_active_power", + "p1_phase_export_entities": [], + "p1_phase_import_entities": [], + "p1_phase_net_entities": [], + "peak_cap_w": 15000, + "peak_forecast_entity": "", + "peak_soc_floor": 20, + "price_avg_entity": "", + "price_csv_path": "/share/goodwe_prices.csv", + "price_feed_url": "", + "price_horizon_h": 48, + "price_markup_eur_kwh": 0.03, + "price_now_entity": "", + "price_provider": "manual_csv", + "price_taxes_eur_kwh": 0.06, + "price_timeout_s": 10, + "saturation_cycles": 3, + "saturation_w": 500, + "setpoint_entity": "number.test23_setpoint_w", + "sign_disagree_s": 180, + "sign_disagree_w": 300, + "site_sign_disagree_s": 325, + "site_sign_disagree_w": 300, + "slew_w": 0, + "soc_entity": "sensor.test23_soc", + "stale_input_s": 15, + "static_profile": "", + "step_w": 10, + "target_grid_w": -10, + "unit_sign_disagree_s": 85, + "unit_sign_disagree_w": 250 +} \ No newline at end of file diff --git a/goodwe_controller/scenarios/ux13/validator-calls-652097a.jsonl.gz b/goodwe_controller/scenarios/ux13/validator-calls-652097a.jsonl.gz new file mode 100644 index 0000000..a019bc5 Binary files /dev/null and b/goodwe_controller/scenarios/ux13/validator-calls-652097a.jsonl.gz differ diff --git a/goodwe_controller/test_haentities.py b/goodwe_controller/test_haentities.py new file mode 100644 index 0000000..a562b97 --- /dev/null +++ b/goodwe_controller/test_haentities.py @@ -0,0 +1,209 @@ +"""UX-13 P0: the entity list for the picker. `python3 test_haentities.py` + +`GET ha/entities` reads Home Assistant's states through the add-on's own +session (hass.states()) and must never fail the page: part 1 drives the real +HomeAssistant client against a local server that answers 502, garbage, the +wrong shape, nothing at all, or too slowly. Part 2 checks the 30 s cache and +what is served while Home Assistant is down. Part 3 serves the real route. +Mutating a module this file grades? Read MUTATION.md first (purge __pycache__). +""" + +import asyncio +import json +import sys +import time + +from aiohttp import ClientSession, web as aioweb + +from app import hass as H, haentities as E, web + +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +STATES = [ + {"entity_id": "sensor.grid", "state": "812", + "attributes": {"friendly_name": "Grid power", "unit_of_measurement": "W", + "device_class": "power"}}, + {"entity_id": "sensor.soc", "state": "54", + "attributes": {"friendly_name": "Battery level", "unit_of_measurement": "%", + "device_class": "battery"}}, + {"entity_id": "number.setpoint", "state": "0", "attributes": {}}, + {"entity_id": "sensor.pv_kw", "state": "1.2", + "attributes": {"unit_of_measurement": "kW"}}, + {"no_entity_id": True}, +] + + +async def serve(handler): + app = aioweb.Application() + app.router.add_get("/api/states", handler) + runner = aioweb.AppRunner(app) + await runner.setup() + site = aioweb.TCPSite(runner, "127.0.0.1", 0) + await site.start() + return runner, site._server.sockets[0].getsockname()[1] + + +async def read_with(handler, timeout_s=10): + runner, port = await serve(handler) + H.CORE_API, H.STATES_TIMEOUT_S = f"http://127.0.0.1:{port}/api", timeout_s + try: + async with ClientSession() as session: + t0 = time.monotonic() + got = await H.HomeAssistant(session, "token").states() + read_with.took = time.monotonic() - t0 + return got + finally: + await runner.cleanup() + + +async def ok(_r): + return aioweb.json_response(STATES) + + +async def bad_gateway(_r): + return aioweb.Response(status=502) + + +async def garbage(_r): + return aioweb.Response(text="", content_type="application/json") + + +async def not_a_list(_r): + return aioweb.json_response({"message": "hi"}) + + +async def slow(_r): + await asyncio.sleep(2) + return aioweb.json_response(STATES) + + +async def closed_port(): + H.CORE_API = "http://127.0.0.1:9/api" + async with ClientSession() as session: + return await H.HomeAssistant(session, "token").states() + + +print("1. hass.states() answers None, never an exception") +check("a normal answer is the state list", asyncio.run(read_with(ok)) == STATES) +for name, handler in (("502 while HA restarts", bad_gateway), + ("a body that is not JSON", garbage), + ("JSON that is not a list", not_a_list)): + check(f"{name} -> None", asyncio.run(read_with(handler)) is None) +check("an answer slower than the timeout -> None, at the timeout", + asyncio.run(read_with(slow, timeout_s=0.3)) is None + and read_with.took < 1.0) +check("no server at all -> None", asyncio.run(closed_port()) is None) +check("the timeout for the list is the same 10 s as every other read", + "STATES_TIMEOUT_S = 10 " in open(H.__file__, encoding="utf-8").read()) + + +class FakeHass: + def __init__(self): + self.answer, self.reads = STATES, 0 + + async def states(self): + self.reads += 1 + await asyncio.sleep(0.01) + return self.answer + + +class Clock: + def __init__(self): + self.t = 1000.0 + + def __call__(self): + return self.t + + +print("\n2. the cache, and Home Assistant being down") +_h, _c = FakeHass(), Clock() +_cache = E.EntityCache(_h, clock=_c) + + +async def burst(): + return await asyncio.gather(*(_cache.payload("power") for _ in range(5))) + + +_p = asyncio.run(burst()) +check("five pickers opening at once cost one read of Home Assistant", + _h.reads == 1 and all(x == _p[0] for x in _p)) +_ids = [r["entity_id"] for r in _p[0]["entities"]] +check("power entities come first, then the rest, and a broken state is left " + f"out ({_ids})", _ids == ["sensor.grid", "sensor.pv_kw", + "number.setpoint", "sensor.soc"] + and [r["match"] for r in _p[0]["entities"]] == [True, True, False, False]) +check("each row carries id, name, state, unit and device class", + _p[0]["entities"][0] == {"entity_id": "sensor.grid", "name": "Grid power", + "state": "812", "unit": "W", + "device_class": "power", "match": True}) +check("the kinds filter as the pickers need", + [r["entity_id"] for r in _p[0]["entities"] + if E.matches(r, "pct")] == ["sensor.soc"] + and [r["entity_id"] for r in _p[0]["entities"] + if E.matches(r, "number")] == ["number.setpoint"]) +_c.t += 10 +asyncio.run(_cache.payload()) +check("within 30 s the cached list is served", _h.reads == 1) +check("peek() hands the validator the fresh list without a read", + set(_cache.peek()) == set(_ids) and _h.reads == 1) +_h.answer = None +_c.t += 30 +_down = asyncio.run(_cache.payload()) +check("Home Assistant down after a good read: the old list, marked old", + _h.reads == 2 and _down["available"] and _down["text"] == E.STALE_TEXT + and len(_down["entities"]) == 4 and _down["age_s"] >= 30) +check("...and peek() then gives nothing, so no warning rests on an old list", + _cache.peek() is None) +_h2 = FakeHass() +_h2.answer = None +_empty = asyncio.run(E.EntityCache(_h2).payload("power")) +check("Home Assistant down from the start: an empty list that says why", + _empty == {"available": False, "text": E.DOWN_TEXT, "entities": []}) + + +print("\n3. the route") + + +class FakeController: + def __init__(self, hass): + self.hass, self.o = hass, {} + + +async def route(hass, query=""): + app = web.build_app(FakeController(hass), selfcfg=object()) + runner = aioweb.AppRunner(app) + await runner.setup() + site = aioweb.TCPSite(runner, "127.0.0.1", 0) + await site.start() + port = site._server.sockets[0].getsockname()[1] + try: + async with ClientSession() as s: + async with s.get(f"http://127.0.0.1:{port}/ha/entities{query}") as r: + return r.status, await r.json() + finally: + await runner.cleanup() + + +_st, _body = asyncio.run(route(FakeHass(), "?kind=pct")) +check("GET ha/entities?kind=pct answers 200 with the matching entity first", + _st == 200 and _body["entities"][0]["entity_id"] == "sensor.soc") +_st, _body = asyncio.run(route(_h2)) +check("with Home Assistant down it still answers 200, with the reason", + _st == 200 and _body["available"] is False and _body["entities"] == []) +check("the words shown are short and plain (STYLE: one sentence)", + all(len(t.split()) <= 12 and t.count(".") == 1 + for t in (E.DOWN_TEXT, E.STALE_TEXT))) +print(json.dumps(_body)) + +print() +if fails: + print(f"{len(fails)} FAILED") + sys.exit(1) +print("all passed") diff --git a/goodwe_controller/test_messages.py b/goodwe_controller/test_messages.py index 1bf14d9..e82176c 100644 --- a/goodwe_controller/test_messages.py +++ b/goodwe_controller/test_messages.py @@ -26,6 +26,7 @@ Run `python3 -B test_messages.py --violations` to print what part 3 sees. import ast import hashlib +import json import os import re import sys @@ -388,6 +389,9 @@ LIMITS = { # kind: (words, keys ok, entity ids ok) "log": (25, True, True), "option-name": (6, False, False), "option-desc": (30, True, True), + "note": (25, True, True), # UX-13: a settings-page warning + "ui-name": (6, False, False), # UX-13: app/ui_text, a name or choice + "ui-desc": (25, False, False), # UX-13: app/ui_text, one-line help } LOG_CALL = re.compile(r"(?:^|\.)(?:_LOG|log|logger|LOG)\.(warning|error|critical|exception)$") @@ -453,6 +457,10 @@ def collect(): if arg is not None: for n, t in strings_in(arg): out.append(("log", f"{f.name}:{n.lineno}", t)) + elif name.endswith("_advise") and node.args: + for n, t in strings_in(node.args[-1], expand=True): + if len(words(t)) > 1: + out.append(("note", f"{f.name}:{n.lineno}", t)) elif re.search(r"problems\.(append|extend)$", name): for a in node.args: for n, t in strings_in(a, expand=True): @@ -475,6 +483,16 @@ def collect(): if isinstance(v.get("fields"), dict): walk(v["fields"], f"{path}{key}.") walk(tr.get("configuration"), "") + # UX-13: the new page's own words, held to the option texts' rule. + for f in sorted((APP / "ui_text").glob("*.json")): + def walk_ui(node, path): + for key, v in node.items(): + if isinstance(v, dict): + walk_ui(v, f"{path}{key}.") + elif isinstance(v, str): + out.append(("ui-desc" if key == "description" else + "ui-name", f"{f.name}:{path}{key}", v)) + walk_ui(json.loads(f.read_text(encoding="utf-8")), "") return out @@ -485,6 +503,8 @@ def problems_of(kind, text): p.append(f"over {lim} words ({len(words(text))})") if ";" in text: p.append("semicolon") + if kind in ("note", "ui-desc") and sentences(text) > 1: + p.append(f"over 1 sentence ({sentences(text)})") return p + text_problems(text, keys=keys, entities=ents) diff --git a/goodwe_controller/test_pv.py b/goodwe_controller/test_pv.py index 6f73277..1768e4b 100644 --- a/goodwe_controller/test_pv.py +++ b/goodwe_controller/test_pv.py @@ -697,13 +697,21 @@ print(f" {len(APP)} modules under app/; TEL-11 material is in: {_touch}") check(f"PRECONDITION: the scan found the whole package, not a subset - " f"{len(APP)} modules", len(APP) >= 25 and "control.py" in [f.name for f in APP]) -check("⭐ EXACTLY THREE MODULES CAN SEE THE MATERIAL: `pv.py` (the reader), " - "`main.py` (the read and the two operator surfaces) and `mqtt.py` (the " - "publication). `control.py`, `arbiter.py`, `allocator.py`, `peakguard.py`, " - "`arbitrage.py`, `maintenance.py`, `clamp.py`, `freeze.py`, " - "`watchdog.py`, `delivery.py` and `circulation.py` do not mention it in " - "any spelling, comments included", - _touch == ["main.py", "mqtt.py", "pv.py"]) +# UX-13 adds the settings surface: labels (messages.py), a settings warning +# (validate.py) and the page's layout and derivation (uilayout.py, uimodel.py). +# None of them decides anything, so the decision modules stay excluded. +_DECIDERS = ("control.py", "arbiter.py", "allocator.py", "peakguard.py", + "arbitrage.py", "maintenance.py", "clamp.py", "freeze.py", + "watchdog.py", "delivery.py", "circulation.py") +check("⭐ ONLY THE READER, THE PUBLISHER AND THE SETTINGS SURFACE SEE THE " + "MATERIAL: `pv.py` (the reader), `main.py` (the read and the two operator " + "surfaces), `mqtt.py` (the publication), and `messages.py`, " + "`validate.py`, `uilayout.py` and `uimodel.py` (settings). " + f"{', '.join(_DECIDERS)} do not mention it in any spelling, comments " + "included", + _touch == ["main.py", "messages.py", "mqtt.py", "pv.py", "uilayout.py", + "uimodel.py", "validate.py"] + and not set(_DECIDERS) & set(_touch)) # habit 4 in the instrument: a scanner that cannot go red is not a scanner. check("...and the scan CAN go red - a module body with the material in it is " "caught, so the green above is the tree speaking and not the scanner " diff --git a/goodwe_controller/test_selfconfig.py b/goodwe_controller/test_selfconfig.py index 10d6608..81bd773 100644 --- a/goodwe_controller/test_selfconfig.py +++ b/goodwe_controller/test_selfconfig.py @@ -636,12 +636,18 @@ _app = web.build_app(FakeController(), FakeSupervisor()) _routes = sorted({(r.method, r.get_info().get("path")) for r in _app.router.routes() if r.method != "HEAD"}) print(" " + " ".join(f"{m} {p}" for m, p in _routes)) -check("the configuration surface adds five routes and touches none of the " - "three that were there", +check("the configuration surface adds five routes and UX-13 one GET, and " + "none touches the three that were there", _routes == [("GET", "/"), ("GET", "/config"), ("GET", "/config/model"), - ("GET", "/status"), ("POST", "/action"), - ("POST", "/config/restart"), ("POST", "/config/save"), - ("POST", "/config/validate")]) + ("GET", "/ha/entities"), ("GET", "/status"), + ("POST", "/action"), ("POST", "/config/restart"), + ("POST", "/config/save"), ("POST", "/config/validate")]) +_ents_src = io.open(os.path.join(HERE, "app", "haentities.py"), + encoding="utf-8").read() +check("UX-13's entity list only reads: haentities.py calls hass.states() and " + "no service, setter or action", + "hass.states()" in _ents_src + and not re.search(r"\.(call|set_number|handle_action|post)\(", _ents_src)) _selfconfig_src = io.open(os.path.join(HERE, "app", "selfconfig.py"), encoding="utf-8").read() for forbidden in ("set_number", "write_setpoint", "handle_action", @@ -692,6 +698,10 @@ async def _e2e(): async with s.post(f"{root}/config/validate", json={"options": base(setpoint_entity="")}) as r: out["invalid"] = await r.json() + async with s.post(f"{root}/config/validate", + json={"options": base(integrator_max_w=3000)} + ) as r: + out["warned"] = await r.json() # ⚠️ THE COUNTS ARE SNAPSHOTTED AT EACH STAGE. Asserting # `sup.writes == []` after the whole sequence would be asserting # about the LAST request, not this one - the first draft did, and @@ -749,6 +759,11 @@ check("...and it honours the browser's language per key, falling back to " check("POST config/validate returns the validator's sentences, anchored", E["invalid"]["problems"] and "setpoint_entity" in E["invalid"]["by_key"]) +check("UX-13: POST config/validate also returns the validator's warnings, " + "beside no refusal", + E["warned"]["problems"] == [] and [n["keys"] for n in + E["warned"]["notes"]] + == [["integrator_max_w"]] and E["warned"]["notes"][0]["tier"] == "warn") check("a save of an invalid blob answers 422 and the Supervisor got nothing", E["save_invalid"][0] == 422 and E["save_invalid"][1]["kind"] == "invalid" diff --git a/goodwe_controller/test_uilayout.py b/goodwe_controller/test_uilayout.py new file mode 100644 index 0000000..5d04ec5 --- /dev/null +++ b/goodwe_controller/test_uilayout.py @@ -0,0 +1,202 @@ +"""UX-13 P0: the layout table places every option once. `python3 test_uilayout.py` + +app/uilayout.py is the one place UI code names an option key +(DECISIONS/UX-13-one-layout-table.md). This file fails CI when config.yaml +gains an option the table does not place, and checks each row against the +sources UX-02 keeps authoritative: config.yaml for ranges, translations and +app/ui_text for words, validate.py for which entities are read as watts. +Mutating a module this file grades? Read MUTATION.md first (purge __pycache__). +""" + +import io +import json +import os +import re +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.join(HERE, "tools")) + +import translations_lint as TL # noqa: E402 +from app import uilayout as L, uimodel, validate # noqa: E402 + +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +CONFIG = io.open(os.path.join(HERE, "config.yaml"), encoding="utf-8").read() +ORDER, NESTED = TL.config_surface(CONFIG) +TYPES = dict(re.findall(r"^ ([a-z_0-9]+): *(\S.*?) *$", + CONFIG[CONFIG.index("\nschema:"):], re.M)) +EN = TL.parse_translation_file(os.path.join(HERE, "translations", "en.yaml")) +UI_TEXT = json.load(io.open(os.path.join(HERE, "app", "ui_text", "en.json"), + encoding="utf-8")) +ROWS = list(L.fields()) +KEYS = [f.key for _g, _p, f in ROWS] + + +def coverage_problems(order, nested): + """Every way the table and config.yaml's schema disagree.""" + out = [] + placed = [k for k in KEYS if not k.startswith(("ui.", "inverters."))] + for k in order: + n = placed.count(k) + (k in L.DERIVED) + (k in L.HIDDEN) + if n != 1: + out.append(f"{k} is placed {n} times") + out += [f"{k} is not in config.yaml" for k in + set(placed) | set(L.DERIVED) | set(L.HIDDEN) if k not in order] + sub = [k.split(".", 1)[1] for k in KEYS if k.startswith("inverters.")] + out += [f"inverters.{k} has no card field" for k in nested.get("inverters", []) + if sub.count(k) != 1] + out += [f"inverters.{k} is not in config.yaml" for k in sub + if k not in nested.get("inverters", [])] + return out + + +print("1. every option has exactly one place") +_p = coverage_problems(ORDER, NESTED) +_fields = sum(1 for k in ORDER if k in KEYS) +print(f" {len(ORDER)} options: {_fields} fields, {len(L.DERIVED)} derived, " + f"{len(L.HIDDEN)} hidden. Problems: {_p}") +check(f"config.yaml's {len(ORDER)} options are each a field, derived or hidden, " + "once", not _p and len(ORDER) >= 78) +check("a new option without a place fails this check", + coverage_problems(ORDER + ["new_option_w"], NESTED) + == ["new_option_w is placed 0 times"]) +check("...and so does a new inverter field", + coverage_problems(ORDER, {"inverters": NESTED["inverters"] + ["mode"]}) + == ["inverters.mode has no card field"]) +check("no key is both a field and derived", + not set(KEYS) & (set(L.DERIVED) | set(L.HIDDEN))) +check("each derived key names the page field that writes it", + all(any(f.key == how for _g, _p2, f in ROWS) + for how in L.DERIVED.values())) + +print("\n2. each row is well formed") +check("every control, entity kind and show operator is a known one", + all(f.control in L.CONTROLS and (not f.kind or f.kind in L.ENTITY_KINDS) + and all(op in L.SHOW_OPS for _n, op, _v in f.show) + for _g, _p2, f in ROWS)) +_names = set(ORDER) | set(KEYS) | {"inverter_count"} +check("every show condition reads an option, a page field or inverter_count", + all(n in _names for _g, _p2, f in ROWS for n, _o, _v in f.show)) +check("every entity control has a kind, and no other control has one", + all((f.control in (L.ENTITY, L.ENTITY_LIST, L.SOLAR_LIST)) == bool(f.kind) + for _g, _p2, f in ROWS)) +check("everything applies after a restart: options are read at start, and " + "OPT-11's runtime switches are not built yet", + all(f.applies == L.RESTART for _g, _p2, f in ROWS)) +check("no unit states a number, so no range is written here", + all(not re.search(r"\d", f.unit) for _g, _p2, f in ROWS)) + + +_SCHEMA = CONFIG[CONFIG.index("\nschema:"):] +_ROSTER = re.search(r"^ inverters:\n((?: {4,}.*\n)+)", _SCHEMA, re.M).group(1) +ROSTER_TYPES = dict(re.findall(r"^ {4,}(?:- )?([a-z_0-9]+): *(\S+)", _ROSTER, + re.M)) + + +def _type(key): + if key.startswith("inverters."): + return ROSTER_TYPES.get(key.split(".", 1)[1], "") + return TYPES.get(key, "") + + +_ranged = [f.key for _g, _p2, f in ROWS + if f.control in (L.SLIDER, L.NUMBER, L.LOCKED) + and not f.key.startswith("ui.")] +_unranged = [k for k in _ranged if not re.match(r"(int|float)\(", _type(k))] +check(f"every slider, number and locked value has a range in config.yaml " + f"({len(_ranged)} of them, without: {_unranged})", not _unranged) +_SUFFIX = (("_eur_kwh", "€/kWh"), ("_eur", "€/kWh"), ("_kwh", "kWh"), + ("_w", "W"), ("_s", "s"), ("_h", "h"), ("_min", "min"), + ("_days", "days"), ("_floor", "%"), ("_ceiling", "%"), + ("_target", "%")) + + +def _unit_problem(f): + want = next((u for suf, u in _SUFFIX if f.key.endswith(suf)), None) + if want is None or f.control not in (L.SLIDER, L.NUMBER, L.LOCKED): + return False + return f.unit != want and not (want == "W" and f.unit == "kW" + and f.scale == 1000) + + +_mism = [(f.key, f.unit) for _g, _p2, f in ROWS if _unit_problem(f)] +check(f"each unit matches its key's suffix (mismatched: {_mism})", not _mism) +check("the peak limit is shown in kW and stored in W", + L.field("peak_cap_w").unit == "kW" and L.field("peak_cap_w").scale == 1000) +_watt = sorted({f.key.split(".")[-1] for _g, _p2, f in ROWS + if f.kind == "power_w"}) +check(f"the pickers that expect watts are exactly the entities validate.py " + f"reads as watts ({_watt})", + _watt == sorted(validate.WATT_ENTITY_KEYS)) +check("range_of() reads a Supervisor-shaped schema, nested ones too", + L.range_of("max_w", [{"name": "max_w", "lengthMin": 100, + "lengthMax": 5000}]) == (100, 5000) + and L.range_of("inverters.max_w", [{"name": "inverters", "schema": [ + {"name": "max_w", "lengthMin": 100, "lengthMax": 5000}]}]) + == (100, 5000) + and L.range_of("p1_host", [{"name": "p1_host"}]) is None) + +print("\n3. the words come from translations and app/ui_text") +_conf = EN["configuration"] +_no_tr = [k for k in KEYS if not k.startswith("ui.") and not ( + k in _conf if "." not in k + else k.split(".")[1] in _conf["inverters"]["fields"])] +check(f"every option field has an en.yaml name (missing: {_no_tr})", not _no_tr) +_no_ui = [k for k in KEYS if k.startswith("ui.") + and k[3:] not in UI_TEXT["fields"]] +check(f"every page-only field has words in ui_text (missing: {_no_ui})", + not _no_ui) +_titles = [g.id for g in L.GROUPS if g.id not in UI_TEXT["groups"]] + [ + f"{g.id}.{p.id}" for g in L.GROUPS for p in g.parts + if p.id and f"{g.id}.{p.id}" not in UI_TEXT["parts"]] +check(f"every group and part has a title (missing: {_titles})", not _titles) +_contract = {"none", "fixed", "daynight", "dynamic", "custom"} +check("every contract, fallback and region the model can hold has words", + set(UI_TEXT["choices"]["contract"]) == _contract + and set(UI_TEXT["choices"]["fallback"]) == _contract - {"dynamic"} + and set(UI_TEXT["choices"]["region"]) + == set(uimodel.REGION_PRESETS) | {uimodel.OWN}) +check("both 'applies' labels have words", + set(UI_TEXT["applies"]) == {L.RESTART, L.NOW}) + +print("\n4. visibility follows the page model") +_m = uimodel.options_to_ui({"meter_source": "ha_signed", "meter_phases": 3, + "inverters": [], "maintenance_enabled": False}) +check("a signed meter shows its net entity, and three phases its net list", + L.visible(L.field("p1_net_entity"), _m) + and L.visible(L.field("p1_phase_net_entities"), _m) + and not L.visible(L.field("meter_entity"), _m) + and not L.visible(L.field("p1_phase_import_entities"), _m)) +check("one inverter shows the one-inverter stops, not the per-inverter ones", + L.visible(L.field("sign_disagree_w"), _m) + and not L.visible(L.field("unit_sign_disagree_w"), _m)) +check("maintenance details hide while maintenance is off", + not L.visible(L.field("maintenance_charge_w"), _m)) + +print("\n5. UI code names option keys in two files only") +_KEY = re.compile(r"""["']([a-z0-9]+(?:_[a-z0-9]+)+)["']""") +_src = io.open(os.path.join(HERE, "app", "uimodel.py"), encoding="utf-8").read() +_owned = set(L.DERIVED) | {"inverters", "price_provider", + "price_taxes_eur_kwh", "arbitrage_enabled"} +_extra = sorted({k for k in _KEY.findall(_src) if k in ORDER} - _owned) +check(f"uimodel.py names only the keys its derivations write (others: " + f"{_extra})", not _extra) +_leak = sorted({f"{name}:{k}" for name in ("haentities.py", "web.py") + for k in _KEY.findall(io.open(os.path.join(HERE, "app", name), + encoding="utf-8").read()) + if k in ORDER}) +check(f"haentities.py and web.py name no option key ({_leak})", not _leak) + +print() +if fails: + print(f"{len(fails)} FAILED") + sys.exit(1) +print("all passed") diff --git a/goodwe_controller/test_uimodel.py b/goodwe_controller/test_uimodel.py new file mode 100644 index 0000000..1267d62 --- /dev/null +++ b/goodwe_controller/test_uimodel.py @@ -0,0 +1,212 @@ +"""UX-13 P0: stored options <-> page model, exactly. `python3 test_uimodel.py` + +app/uimodel.py derives the keys the page does not show (the flat inverter, +the solar keys, static_profile, the levies figure, arbitrage_enabled). Part 1 +round-trips every options blob the test suite hands the validator +(scenarios/ux13/, recorded by tools/ux13_validator_calls.py at 652097a), the +TEST-23 option files and config.yaml's defaults: options -> model -> JSON -> +options must be identical. Parts 2-6 edit one page field and check the blob. +Mutating a module this file grades? Read MUTATION.md first (purge __pycache__). +""" + +import glob +import gzip +import io +import json +import logging +import os +import re +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +logging.disable(logging.WARNING) # the validator's own log lines + +from app import uimodel as U # noqa: E402 +from app.price import (FLEMISH_TAXES_COMPONENTS_EUR_KWH, # noqa: E402 + PRICE_OPTION_DEFAULTS) +from app.static_tariff import parse_static_profile # noqa: E402 +from app.validate import validate_startup_config # noqa: E402 + +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +def trip(options): + return U.ui_to_options(json.loads(json.dumps(U.options_to_ui(options)))) + + +def edit(options, **fields): + model = json.loads(json.dumps(U.options_to_ui(options))) + for k, v in fields.items(): + model[k.replace("__", ".")] = v + return U.ui_to_options(model) + + +def raises(fn): + try: + fn() + except U.DeriveError: + return True + return False + + +def defaults(): + """config.yaml's `options:` block, read without PyYAML (not in CI).""" + text = io.open(os.path.join(HERE, "config.yaml"), encoding="utf-8").read() + block = text[text.index("\noptions:"):text.index("\nschema:")] + out = {} + for key, raw in re.findall(r"^ ([a-z_0-9]+):(.*)$", block, re.M): + raw = raw.split(" #")[0].strip() + try: + out[key] = json.loads(raw) + except ValueError: + out[key] = raw + return out + + +DEFAULTS = defaults() +SITE = dict(DEFAULTS, setpoint_entity="number.goodwe_setpoint", + soc_entity="sensor.soc", batt_entity="sensor.batt") + +print("1. every known blob survives the round trip unchanged") +_calls = [json.loads(line) for line in gzip.open(os.path.join( + HERE, "scenarios", "ux13", "validator-calls-652097a.jsonl.gz"), "rt")] +_test23 = sorted(glob.glob(os.path.join(HERE, "scenarios", "ux13", "test23", + "*.json"))) +_blobs = ([("config.yaml", DEFAULTS)] + + [(c["source"], c["opts"]) for c in _calls + if isinstance(c["opts"], dict)] + + [(os.path.basename(f), json.load(open(f))) for f in _test23]) +_bad = [] +for _src, _o in _blobs: + try: + _back = trip(_o) + except Exception as e: # noqa: BLE001 + _bad.append(f"{_src}: {type(e).__name__} {e}") + continue + if json.dumps(_back, sort_keys=True) != json.dumps(_o, sort_keys=True): + _bad.append(f"{_src}: {sorted(set(_back) ^ set(_o)) or 'values'}") +print(f" {len(_calls)} validator calls, {len(_test23)} TEST-23 files, " + f"config.yaml defaults") +check(f"all {len(_blobs)} are identical after options -> model -> JSON -> " + f"options (not: {_bad[:3]})", not _bad and len(_calls) >= 200 + and len(_test23) >= 5 and len(DEFAULTS) >= 78) +check("config.yaml was read whole: the defaults hold every option", + "inverters" in DEFAULTS and DEFAULTS["inverters"] == [] + and DEFAULTS["price_taxes_eur_kwh"] == 0.11736) +_m = U.options_to_ui(DEFAULTS) +check("the page model never shows a derived key: it keeps them aside", + not {"setpoint_entity", "pv_entity", "pv_entities", "static_profile", + "price_taxes_profile"} & set(_m) + and set(_m["keep"]) >= {"static_profile", "pv_entity", "max_w"}) +check("a blob that lacks keys gains none", trip({"max_w": 900}) == {"max_w": 900}) + +print("\n2. one inverter or a list") +_one = edit(SITE, inverters=[dict(U.options_to_ui(SITE)["inverters"][0], + max_w=1500)]) +check("editing the one inverter writes the flat keys and keeps the list empty", + _one["max_w"] == 1500 and _one["inverters"] == [] + and _one["setpoint_entity"] == "number.goodwe_setpoint") +_card = U.options_to_ui(SITE)["inverters"][0] +_two = edit(SITE, inverters=[ + dict(_card, name="inverter-a", capacity_kwh=None), + dict(_card, name="inverter-b", setpoint_entity="number.goodwe_b", + soc_entity="sensor.soc_b", batt_entity="sensor.batt_b")]) +check("a second inverter writes the list and clears the flat power request " + "entity, which the validator refuses beside a list", + len(_two["inverters"]) == 2 and _two["setpoint_entity"] == "" + and "capacity_kwh" not in _two["inverters"][0] + and not validate_startup_config(_two)) +_listed = dict(SITE, setpoint_entity="", inverters=[ + {"name": "garage", "setpoint_entity": "number.g", "max_w": 2000, + "bms_soc_floor": 10}]) +_l1 = edit(_listed, inverters=[dict(_listed["inverters"][0], max_w=1800)]) +check("a list of one stays a list, so the inverter keeps its name and stored " + "state", _l1["inverters"] == [dict(_listed["inverters"][0], max_w=1800)]) +check("a name on the flat inverter is refused rather than dropped", + raises(lambda: edit(SITE, inverters=[dict(_card, name="garage")]))) +_raw = dict(SITE, inverters="garage") +check("a roster no card can show is kept when another field is saved, and " + "cannot be edited here", + edit(_raw, gain=0.5)["inverters"] == "garage" + and raises(lambda: edit(_raw, inverters=[_card]))) + +print("\n3. solar rows") +check("one row writes pv_entity", + edit(DEFAULTS, ui__solar=[{"entity": "sensor.pv", "reversed": False}]) + ["pv_entity"] == "sensor.pv") +_pv2 = edit(dict(DEFAULTS, pv_entity="sensor.a"), ui__solar=[ + {"entity": "sensor.a", "reversed": False}, + {"entity": "sensor.b", "reversed": True}]) +check("two rows write pv_entities, clear pv_entity and flip the reversed one", + _pv2["pv_entities"] == ["sensor.a", "sensor.b"] and _pv2["pv_entity"] == "" + and _pv2["pv_invert_entities"] == ["sensor.b"]) +_stray = dict(DEFAULTS, pv_entities=["sensor.a", "sensor.b"], + pv_invert_entities=["sensor.b", "sensor.old"]) +_pv1 = edit(_stray, ui__solar=[{"entity": "sensor.a", "reversed": False}]) +check("removing a reversed row drops its flip, and a flip that was never a row " + "stays as stored", _pv1["pv_entities"] == ["sensor.a"] + and _pv1["pv_invert_entities"] == ["sensor.old"]) + +print("\n4. contract and prices") +_fixed = edit(DEFAULTS, ui__contract="fixed", ui__fixed_price=0.31) +check("a fixed price writes a profile the tariff parser reads as one price", + _fixed["price_provider"] == "off" + and [p.eur_per_kwh for p in parse_static_profile( + _fixed["static_profile"]).periods] == [0.31] + and U.options_to_ui(_fixed)["ui.contract"] == "fixed") +_dn = edit(SITE, ui__contract="daynight", ui__day_price=0.34, + ui__night_price=0.26) +check("day and night write a profile that starts and reads back the same", + not validate_startup_config(_dn) + and U.options_to_ui(_dn)["ui.day_price"] == 0.34 + and U.options_to_ui(_dn)["ui.night_price"] == 0.26) +_dyn = edit(_dn, ui__contract="dynamic", price_provider="generic_dayahead") +check("a dynamic contract keeps the day and night profile as its fallback", + _dyn["price_provider"] == "generic_dayahead" + and _dyn["static_profile"] == _dn["static_profile"]) +check("a dynamic contract without a price source is refused here", + raises(lambda: edit(DEFAULTS, ui__contract="dynamic"))) +_odd = json.dumps({"periods": [ + {"from": "00:00", "to": "12:00", "days": "mon-sun", "eur_per_kwh": 0.2}, + {"from": "12:00", "to": "24:00", "days": "mon-sun", "eur_per_kwh": 0.3}]}) +_om = U.options_to_ui(dict(DEFAULTS, static_profile=_odd)) +check("odd windows show as 'custom' and survive saving another field", + _om["ui.contract"] == "custom" + and edit(dict(DEFAULTS, static_profile=_odd), gain=0.5) + ["static_profile"] == _odd) + +print("\n5. region presets") +_fl = U.REGION_PRESETS["flanders"] +check("the Flanders preset is the sum of app/price.py's components, and is the " + "shipped default", _fl["taxes_eur_kwh"] == round(sum( + v for _n, v in FLEMISH_TAXES_COMPONENTS_EUR_KWH), 5) + == PRICE_OPTION_DEFAULTS["price_taxes_eur_kwh"] == DEFAULTS[ + "price_taxes_eur_kwh"]) +check("every preset carries its year", all( + isinstance(p["year"], int) and p["year"] >= 2026 + for p in U.REGION_PRESETS.values())) +_own = dict(DEFAULTS, price_taxes_eur_kwh=0.06) +check("a figure that is no preset reads as 'own', and choosing Flanders " + "writes the preset", U.options_to_ui(_own)["ui.region"] == U.OWN + and edit(_own, ui__region="flanders")["price_taxes_eur_kwh"] == 0.11736) +check("'own' stores the typed figure", + edit(DEFAULTS, ui__region=U.OWN, price_taxes_eur_kwh=0.13) + ["price_taxes_eur_kwh"] == 0.13) + +print("\n6. arbitrage_enabled: the OPT-11 hook changes nothing yet") +check("the hook answers None while battery modes do not exist", + U.arbitrage_from_batteries(U.options_to_ui(DEFAULTS)) is None) +check("so the toggle is stored as the page has it", + edit(DEFAULTS, arbitrage_enabled=True)["arbitrage_enabled"] is True) + +print() +if fails: + print(f"{len(fails)} FAILED") + sys.exit(1) +print("all passed") diff --git a/goodwe_controller/test_validate.py b/goodwe_controller/test_validate.py index 853bff5..ea9c5df 100644 --- a/goodwe_controller/test_validate.py +++ b/goodwe_controller/test_validate.py @@ -1808,6 +1808,47 @@ check("an UNREADABLE shadow_mode resolves to SHADOW, not to live - the " and shadow.engaged({"shadow_mode": ""}, control_started=True) is True and shadow.mode({"shadow_mode": "garbage"}) == "on") +# --------------------------------------------------------------------------- # +print("\nUX-13 - the warning tier: notes for the settings page, never refusals") +from app.validate import Note, WATT_ENTITY_KEYS, config_notes # noqa: E402 + +_warned = base(integrator_max_w=3000, maintenance_discharge_w=2500, + price_provider="generic_dayahead", static_profile="") +_notes = [] +_with = validate_startup_config(_warned, -5000, 5000, notes=_notes) +check("a config with three logged warnings starts, and gives three notes, " + "each filed under the setting it names", + _with == [] and validate_startup_config(_warned, -5000, 5000) == [] + and sorted(n.keys for n in _notes) == [ + ("integrator_max_w",), ("maintenance_discharge_w",), + ("static_profile",)] + and all(n.tier == "warn" and f"({n.keys[0]})" in n.text for n in _notes)) +_refused = base(setpoint_entity="", integrator_max_w=3000) +_n2 = [] +check("collecting notes changes no refusal", + validate_startup_config(_refused, notes=_n2) + == validate_startup_config(_refused) != [] and len(_n2) == 1) +_rows = {"number.goodwe_setpoint": {"unit": None}, + "sensor.soc": {"unit": "%"}, "sensor.batt": {"unit": "kW"}} +_cn = config_notes(base(), entities=_rows) +check("a battery power sensor in kW is warned about: the add-on takes it as " + "watts", [n.keys for n in _cn] == [("batt_entity",)] + and "kW" in _cn[0].text) +_cn = config_notes(base(meter_entity="sensor.grid"), entities={}) +check("an entity Home Assistant does not have is named, and only the ones " + "this meter source and roster shape read", + sorted(n.keys[0] for n in _cn) == ["batt_entity", "meter_entity", + "setpoint_entity", "soc_entity"]) +check("with no entity list (Home Assistant down) those checks are skipped", + config_notes(base(), entities=None) == []) +check("a peak limit below this month's peak is warned about", + [n.keys for n in config_notes(base(peak_cap_w=2500), + month_peak_w=2900)] == [("peak_cap_w",)] + and config_notes(base(peak_cap_w=3500), month_peak_w=2900) == []) +check("pv.py converts solar units itself, so no solar key is read as watts", + not [k for k in WATT_ENTITY_KEYS if k.startswith("pv_")] + and isinstance(_cn[0], Note)) + # --------------------------------------------------------------------------- # print() if fails: diff --git a/goodwe_controller/tools/ux13_validator_calls.py b/goodwe_controller/tools/ux13_validator_calls.py new file mode 100644 index 0000000..bcd75d1 --- /dev/null +++ b/goodwe_controller/tools/ux13_validator_calls.py @@ -0,0 +1,106 @@ +"""UX-13: record every `validate_startup_config()` call the test suite makes, +and compare two validators over those calls. + + python3 tools/ux13_validator_calls.py record OUT.jsonl.gz + python3 tools/ux13_validator_calls.py compare OLD_APP_PARENT OUT.jsonl.gz + +`record` runs each test_*.py with the validator wrapped (in this directory, so +run it where the suite runs: the CI image). `compare` runs the recorded calls +through the validator under OLD_APP_PARENT (a directory holding an `app/`, +e.g. from `git archive goodwe_controller/app`) and through this tree's, +and prints every call whose refusals differ. P0 of UX-13 used it to show that +adding the warning tier changed no refusal. scenarios/ux13/ holds the record +test_uimodel.py round-trips. +""" + +import gzip +import json +import os +import subprocess +import sys + +HERE = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +_WRAP = r""" +import json, runpy, sys +sys.path.insert(0, ".") +from app import validate +real = validate.validate_startup_config +out = open(sys.argv[1], "a") +def rec(opts, dev_min=None, dev_max=None, *a, **kw): + try: + out.write(json.dumps({"opts": opts, "dev_min": dev_min, + "dev_max": dev_max}) + "\n") + except (TypeError, ValueError): + pass + return real(opts, dev_min, dev_max, *a, **kw) +validate.validate_startup_config = rec +sys.argv = sys.argv[2:] +try: + runpy.run_path(sys.argv[0], run_name="__main__") +except SystemExit: + pass +""" + + +def record(out_path): + tmp = out_path + ".part" + open(tmp, "w").close() + rows, seen = [], set() + for name in sorted(f for f in os.listdir(HERE) + if f.startswith("test_") and f.endswith(".py")): + before = sum(1 for _ in open(tmp)) + subprocess.run([sys.executable, "-B", "-c", _WRAP, tmp, name], + cwd=HERE, stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL) + with open(tmp) as fh: + for line in list(fh)[before:]: + call = json.loads(line) + key = json.dumps(call, sort_keys=True) + if key not in seen: + seen.add(key) + rows.append(dict(call, source=name)) + os.remove(tmp) + with gzip.open(out_path, "wt") as fh: + for row in rows: + fh.write(json.dumps(row, sort_keys=True) + "\n") + print(f"{len(rows)} distinct validator calls -> {out_path}") + + +def _run(app_parent, path): + code = r""" +import gzip, json, logging, sys +logging.disable(logging.CRITICAL) +sys.path.insert(0, sys.argv[1]) +from app.validate import validate_startup_config as v +out = [] +for line in gzip.open(sys.argv[2], "rt"): + c = json.loads(line) + try: + out.append(v(c["opts"], c["dev_min"], c["dev_max"])) + except Exception as e: + out.append("raised " + type(e).__name__) +print(json.dumps(out)) +""" + got = subprocess.run([sys.executable, "-B", "-c", code, app_parent, path], + capture_output=True, text=True, check=True) + return json.loads(got.stdout) + + +def compare(old_parent, path): + old, new = _run(old_parent, path), _run(HERE, path) + differ = [i for i, (a, b) in enumerate(zip(old, new)) if a != b] + print(f"{len(old)} calls, {sum(1 for r in old if r)} refused by the old " + f"validator, {len(differ)} with different refusals") + for i in differ: + print(f" call {i}:\n old {old[i]}\n new {new[i]}") + return 1 if differ else 0 + + +if __name__ == "__main__": + if len(sys.argv) == 3 and sys.argv[1] == "record": + record(sys.argv[2]) + elif len(sys.argv) == 4 and sys.argv[1] == "compare": + sys.exit(compare(sys.argv[2], sys.argv[3])) + else: + sys.exit(__doc__)