From 159d7461d8c3b5b6ada8ee28307bef8ba74ebce6 Mon Sep 17 00:00:00 2001 From: admin Date: Mon, 28 Sep 2026 08:48:21 +0200 Subject: [PATCH 1/4] UX-15: setup-only mode, and the wizard fixes F1, F2, meter, step 1 A new install names no inverter. When that is the startup gate's only refusal, the add-on now serves the setup wizard in setup-only mode instead of exiting 78. It builds no controller, MQTT client or meter task, its HomeAssistant is read-only (every service call refuses before sending), Controller refuses a read-only HomeAssistant, and /action answers 409. Any other refusal still exits 78. DECISIONS/UX-15-setup-only-mode.md. Wizard fixes from the UX-13 P4 test: - F1: /config/validate converts what it can, so a conversion error blocks its own step and the live checks still run; save 422 carries steps. - F2: saves send the model the page opened (base); only keys this page changed are written onto the blob stored now. - A meter entity missing from Home Assistant is red on the last step. - Step 1 says the reading went down after a skip that followed a fall. Co-Authored-By: Claude Opus 5.5 --- .../DECISIONS/UX-02-two-config-surfaces.md | 5 + .../DECISIONS/UX-13-setup-wizard.md | 22 +- .../DECISIONS/UX-15-setup-only-mode.md | 110 ++++ goodwe_controller/DOCS.md | 32 +- goodwe_controller/app/hass.py | 20 +- goodwe_controller/app/main.py | 18 +- goodwe_controller/app/messages.py | 13 + goodwe_controller/app/setuponly.py | 90 ++++ goodwe_controller/app/static/app.js | 15 +- goodwe_controller/app/static/index.html | 1 + goodwe_controller/app/static/settings.js | 16 +- goodwe_controller/app/store.py | 7 +- goodwe_controller/app/ui_text/en.json | 4 + goodwe_controller/app/uimodel.py | 25 +- goodwe_controller/app/uipage.py | 42 +- goodwe_controller/app/validate.py | 35 +- goodwe_controller/app/web.py | 53 +- goodwe_controller/test_setup.py | 139 ++++- goodwe_controller/test_setup_only.py | 488 ++++++++++++++++++ goodwe_controller/tools/ux13_wizard_blobs.py | 11 +- 20 files changed, 1075 insertions(+), 71 deletions(-) create mode 100644 goodwe_controller/DECISIONS/UX-15-setup-only-mode.md create mode 100644 goodwe_controller/app/setuponly.py create mode 100644 goodwe_controller/test_setup_only.py diff --git a/goodwe_controller/DECISIONS/UX-02-two-config-surfaces.md b/goodwe_controller/DECISIONS/UX-02-two-config-surfaces.md index 76a57dc..81b9ee1 100644 --- a/goodwe_controller/DECISIONS/UX-02-two-config-surfaces.md +++ b/goodwe_controller/DECISIONS/UX-02-two-config-surfaces.md @@ -148,6 +148,11 @@ exits 78 and the panel goes with it. Recovery is the Configuration tab. That is the reason it stays, and the reason the panel refuses to write a blob the validator rejected: this surface must never be the thing that removes itself. +*Amended by [[UX-15-setup-only-mode]] (2026-09-28):* one refused configuration +keeps the panel: options that name no inverter at all and have no other +refusal, which is what a new install ships. The add-on then serves the page in +setup-only mode and commands nothing. Every other refusal still exits 78. + **The panel shows fewer refusals than the startup gate can, never more.** Two of `validate.py`'s rules compare a configured ceiling against the setpoint device's *own* reported range, which is only knowable by asking Home Assistant. diff --git a/goodwe_controller/DECISIONS/UX-13-setup-wizard.md b/goodwe_controller/DECISIONS/UX-13-setup-wizard.md index 5d7387e..2fa8570 100644 --- a/goodwe_controller/DECISIONS/UX-13-setup-wizard.md +++ b/goodwe_controller/DECISIONS/UX-13-setup-wizard.md @@ -25,8 +25,9 @@ The page opens `#setup` only when it is opened with no route, and only when - **`no_power_request`**: the stored options name no inverter to control. - **`first_start`**: this boot found no `/data/state.json` (`Store.fresh`), the Store has no `setup_done` and no `auto` key, and control is not started. - `auto` is the key Start/Stop writes. The startup gate exits 78 before the - controller writes the Store, so a new install's first good boot is fresh. + `auto` is the key Start/Stop writes. Neither the startup gate's exit 78 nor + setup-only mode (UX-15) writes the Store, so a new install's first good boot + is fresh. "Never started" alone is not enough: both TEST-23 add-ons have `auto_start` off and no `auto` key, and one has run in shadow for weeks (review of PR #146, @@ -37,14 +38,12 @@ started or stopped it, is never sent. Without a Store the answer is no. The wiza `setup_done` (an ISO time) to the Store. A save from the Installer view does not. -**Not decided here, for the owner:** the add-on exits 78 before it serves the -page when the stored options name no power request entity, which is what a -brand-new install ships (config.yaml's `setpoint_entity: ""`). So on a truly -new install the installer fills in "Inverter setpoint entity" on the -Configuration tab first, and the wizard opens on the next page visit. -Serving a setup-only page from a refused configuration would change the CFG-01 -startup gate and UX-02 §4 ("the panel is not available when it is most -needed"), so it is left as an open question. +**Decided since, in [[UX-15-setup-only-mode]] (owner, 2026-09-28):** a +brand-new install ships `setpoint_entity: ""`, and it no longer exits 78. When +naming no inverter is the gate's only refusal, the add-on serves the page in +setup-only mode, which commands nothing, and the wizard opens with +`no_power_request`. Setup-only mode writes no Store file, so the first-run +rule above still holds for an install set up on the Configuration tab. ## 3. The kettle test and the sign checks @@ -96,6 +95,7 @@ fingerprint was not handed out by this process, it falls back to `changed`. it), step blocking, the first-run rule on new and TEST-23 sites, the live checks, the save through the P2 path, the stale fix, and the words. - tools/ux13_wizard_blobs.py produces every blob the wizard can save from a new - install and three TEST-23 sites (88 accepted by the shipped validator). At P4 + install and three TEST-23 sites (88 accepted by the shipped validator). + UX-15 added the shipped defaults as a start (setup-only mode). At P4 all 88 were accepted by Supervisor 2026.08.0's `AppOptions`, offline, 78 keys each. diff --git a/goodwe_controller/DECISIONS/UX-15-setup-only-mode.md b/goodwe_controller/DECISIONS/UX-15-setup-only-mode.md new file mode 100644 index 0000000..7265a50 --- /dev/null +++ b/goodwe_controller/DECISIONS/UX-15-setup-only-mode.md @@ -0,0 +1,110 @@ +# UX-15 — setup-only mode: a new install serves the wizard + +**Status: DECIDED 2026-09-28 (owner decision; UX-15 dev).** Card +6aba0053d27a32cb3f210dcc. Changes the CFG-01 startup gate and closes the open +question in [[UX-13-setup-wizard]] §2. Amends [[UX-02-two-config-surfaces]] §4. + +## 1. The rule + +`config.yaml` ships `setpoint_entity: ""`, so a new install's options are +refused by rule 1 of `validate_startup_config()`: no inverter to control. +Until now that was exit 78 before any page was served, and the installer had +to fill in the entity on the Configuration tab before the wizard could open. + +The add-on now starts in **setup-only mode** when, and only when +(`setuponly.applies()`): + +- the options name no inverter at all: no flat `setpoint_entity` and no + `inverters` entry, and +- rule 1's refusal (`no_power_request_problem()`) is the gate's **only** + refusal. + +In setup-only mode it serves the page, says it is in setup (banner, `/status` +`setup_only: true`, a WARNING log line) and opens the wizard when the page is +opened with no route (`setup_needed` is `no_power_request`). After the +wizard's save and restart the normal gate applies to the saved options. + +## 2. What still exits 78, and why + +Everything else the gate refuses, including: + +- a complete configuration that is refused, e.g. `maintenance_charge_w` above + `max_w`; +- no power request entity **and** any other refusal; +- an `inverters` list whose entry has a blank power request entity. + +These are configurations somebody edited. The Configuration tab stored them, +the log's refusal banner names each problem, and the tab is where they are +fixed (UX-02 §4). Serving the wizard there would put a six-step setup in +front of someone who needs to change one field. A narrow rule also keeps the +setup-only decision to one comparison against one known sentence, so a new +validator rule cannot widen it by accident: any other refusal fails the +equality and exits 78. + +The cost: an installer who first sets, say, a DSMR meter with no entities on +the Configuration tab and leaves the power request entity empty gets exit 78, +not the wizard. The log then names both fields. + +## 3. Why it cannot command + +Setup-only mode builds nothing that commands. `amain()` branches to +`setuponly.serve()` before the MQTT broker, the controller, the meter task and +the tariff task are built, and it returns from there when the Supervisor +stops it. What serves the page is `SetupOnly`, which holds no inverter. + +Three checks make it structural rather than a convention: + +1. **The HomeAssistant is read-only** (`HomeAssistant(read_only=True)`). + `call()` refuses before it sends anything, and every service call goes + through it: `number.set_value` (`set_number()`), notifications, all of + them. The flag is fixed at construction, and there is no setter. +2. **`SetupOnly` refuses a HomeAssistant that can write, and `Controller` + refuses one that cannot.** So neither can be built on the wrong one. +3. **`/action` answers 409** in setup-only mode, before + `handle_action()`. Start, Stop and the battery cycle do nothing. + +No MQTT client is created, so nothing is published. The page's other routes +already read only, or write the add-on's own options (UX-02 §6). + +Setup-only mode writes no Store file. The wizard's save writes `setup_done`, +as in any mode. So a new install that is set up on the Configuration tab +instead still has a fresh Store on its first normal boot, and the first-run +rule (UX-13 §2) sends it to the wizard as before. + +## 4. How it is held + +- `test_setup_only.py` part 1: when it applies, including the three exit-78 + cases above. Part 2: the read-only write path refuses with nothing sent, + and the two constructors refuse. Part 3 boots the real `main()` on the + shipped defaults against a fake Supervisor and Home Assistant over HTTP and + logs every request: the page is served, `/status` says setup-only, Start + and the cycle get 409, and there are 0 service calls and no MQTT request, + also with `auto_start` on and a Store that says control was started. The + wizard's save is then booted: it passes the normal gate, does not open the + wizard again, and every write is 0 W (shadow). Part 4: the refused + configurations exit 78 and serve nothing. +- Mutations, each red: `applies()` always false or always true, the + read-only refusal removed, the Controller's check removed, `amain()` giving + setup-only mode a writable HomeAssistant, `/action`'s 409 removed, and the + `return` after `serve()` removed. +- TEST-23: a real container on the shipped defaults, with the Supervisor + stand-in logging every service call (the PR has the evidence). + +## 5. The wizard fixes in the same card + +- **F1.** `/config/validate` converts what it can of a page model + (`uimodel.ui_to_options_partial()`), so a conversion error blocks its own + step beside its field and the live checks still run on the rest. A refusal + about a key the failed conversion writes is left out, because it would + judge the stored value. The save still refuses such a model (422, with + `steps`). +- **F2.** A save from the page model sends `base`, the model it opened. The + add-on writes only the keys whose value differs from `base`, onto the blob + stored now (`uipage.only_edits()`). So "save again to keep yours" after a + 409 keeps the other surface's changes. A key both surfaces changed takes + this page's value, which is what the second press after the banner means. +- **Meter entity.** On the last step a meter entity Home Assistant does not + have is red, like a missing power request entity, so a skipped kettle test + cannot let it through. In the Installer view it stays a warning. +- **Step 1 after a skip.** When the kettle test saw the reading fall and was + then skipped, step 1 says the reading went down. The skip stays (UX-13 §3). diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index d5367cc..7493bdc 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -3233,19 +3233,35 @@ the last step. sign for you; with a net or import/export meter it tells you what to change. - **The last step** runs the add-on's own start checks and reads Home Assistant: the meter reading, that each power request entity exists, the - battery levels, and whether prices are known. A missing power request entity - is red, so it cannot be saved. + battery levels, and whether prices are known. A missing power request or + meter entity is red, so it cannot be saved. - **Save and restart** saves, then restarts the add-on. Every inverter is set to 0 W while it restarts, and with "Start in shadow mode" on (the default) it stays at 0 W until you turn shadow mode off under Settings. Setup opens by itself on a new install's first start, until Setup finishes or control is started, and whenever the stored settings name no inverter to -control. A site that already ran before is never sent there. The add-on does -not start with no power request entity at all, so on a brand-new install fill -in "Inverter setpoint entity" on the Configuration tab first. +control. A site that already ran before is never sent there. -*For installers: card UX-13, and `DECISIONS/UX-13-setup-wizard.md`.* +A new install names no inverter yet, so the add-on starts **in setup**: it +serves the page and sends nothing to any inverter. Home says "Setup: the +add-on controls nothing until setup is done", and there is no Start button. The +installer's steps: + +1. Install the add-on, turn on **Watchdog** on its Info tab, and start it. + Leave the Configuration tab as it is. +2. Open the add-on's page. Setup opens by itself. +3. Go through the six steps and press **Save and restart**. With "Start in + shadow mode" on, every inverter stays at 0 W after the restart. +4. On Home, check the meter direction and what the add-on would do. Turn + shadow mode off under Settings when it looks right. + +Any other problem in the stored settings still stops the add-on (exit code 78), +and the log names each one. Fix those on the Configuration tab. + +*For installers: cards UX-13 and UX-15, `DECISIONS/UX-13-setup-wizard.md` and +`DECISIONS/UX-15-setup-only-mode.md`. In setup the `status` JSON has +`setup_only: true`, and no MQTT entity is published.* ## Status page @@ -3260,6 +3276,10 @@ inverter stays at 0 W. If the banner is red and says a safety stop stays on, one of the red rows below is a stop that will still be there when shadow mode ends. That row says how to clear it. +In **setup** (a new install that names no inverter yet) the banner says the +add-on controls nothing until setup is done, and the one row says to finish +Setup. See *Setup, for a new install*. + | Row starts with | What it tells you | |---|---| | Control | When control last ran. Red when it has stopped running: the control board then sets the inverter to 0 W. | diff --git a/goodwe_controller/app/hass.py b/goodwe_controller/app/hass.py index a2546c1..2f75fa7 100644 --- a/goodwe_controller/app/hass.py +++ b/goodwe_controller/app/hass.py @@ -51,10 +51,18 @@ class Outage: OUTAGE = Outage() +class ReadOnly(RuntimeError): + """A service call on a read-only HomeAssistant (UX-15 setup-only mode).""" + + class HomeAssistant: - def __init__(self, session: aiohttp.ClientSession, token: str | None = None): + def __init__(self, session: aiohttp.ClientSession, token: str | None = None, + *, read_only: bool = False): self.session = session self.token = token or os.environ.get("SUPERVISOR_TOKEN", "") + # UX-15: setup-only mode reads Home Assistant and calls no service. + # Fixed at construction; `call()` refuses before it sends anything. + self._read_only = bool(read_only) self._last_moan: dict[str, float] = {} if not self.token: _LOG.error("SUPERVISOR_TOKEN missing - is this running as an add-on?") @@ -181,7 +189,17 @@ class HomeAssistant: return None return (_f("min"), _f("max"), _f("step")) + @property + def read_only(self) -> bool: + return self._read_only + async def call(self, domain: str, service: str, data: dict) -> bool: + if self._read_only: + # Every service call, `number.set_value` included, goes through + # here, so setup-only mode has no way to command (DECISIONS UX-15). + _LOG.error("Setup-only mode called no service, because it may not " + "command anything.") + raise ReadOnly(f"{domain}.{service} refused: setup-only mode") try: async with self.session.post( f"{CORE_API}/services/{domain}/{service}", diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 30df409..bdce450 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -87,9 +87,11 @@ from .watchdog import ( AGAINST_METER, AGAINST_OWN_POWER, SIGN_OPTION_DEFAULTS, SignWatchdog, SignWatchdogConfig, loop_is_following, ) +from . import setuponly from . import web OPTIONS_PATH = "/data/options.json" +INGRESS_PORT = 8099 _LOG = logging.getLogger("goodwe") # ⚠️ `UNIT_A` NOW LIVES IN app/inverter.py AND IS RE-EXPORTED HERE, because the @@ -375,6 +377,11 @@ class Controller: def __init__(self, opts: dict, hass: HomeAssistant, store, mqtt_pub, *, stop, peak_db_path: str = "/data/tariff.db", price_history_path: str = "/data/price_history.json"): + # UX-15: a read-only HomeAssistant belongs to setup-only mode, which + # builds no controller. Refused here so no controller can hold one. + if getattr(hass, "read_only", False) is True: + raise ValueError("a controller cannot run on a read-only " + "HomeAssistant (setup-only mode)") self.o = opts self.hass = hass self.store = store @@ -6011,6 +6018,15 @@ async def amain() -> None: _dev_min, _dev_max, _ = await hass.limits(_sp) _LOG.info("setpoint device %s reports range %s…%s W", _sp, _dev_min, _dev_max) _problems = validate_startup_config(opts, _dev_min, _dev_max) + # UX-15: a new install names no inverter. When that is the ONLY + # refusal it serves the setup wizard in setup-only mode, which builds + # nothing that commands and returns when stopped. Any other refusal + # still exits 78 (DECISIONS/UX-15-setup-only-mode.md). + if _problems and setuponly.applies(opts, _problems): + await setuponly.serve( + opts, HomeAssistant(session, read_only=True), store, + port=INGRESS_PORT) + return if _problems: refuse(_problems) raise ConfigRefused(_problems) @@ -6054,7 +6070,7 @@ async def amain() -> None: stop = asyncio.Event() controller = Controller(opts, hass, store, pub, stop=stop) - runner = await web.start(controller, port=8099) + runner = await web.start(controller, port=INGRESS_PORT) loop = asyncio.get_running_loop() for sig in (signal.SIGTERM, signal.SIGINT): diff --git a/goodwe_controller/app/messages.py b/goodwe_controller/app/messages.py index 7fbeada..4750030 100644 --- a/goodwe_controller/app/messages.py +++ b/goodwe_controller/app/messages.py @@ -382,6 +382,16 @@ def banner_stopped() -> str: return "Stopped — the inverters are set to 0 W" +def banner_setup_only() -> str: + """UX-15: setup-only mode, where the add-on serves the page and commands + nothing (app/setuponly.py).""" + return "Setup: the add-on controls nothing until setup is done" + + +def row_setup_only() -> str: + return "No inverter is named yet, so finish Setup and restart the add-on." + + def banner_shadow(latched: bool) -> str: """SAFETY-10: first in the cascade, never displaced by another banner. @@ -861,6 +871,7 @@ HINTS = { "control_stopped": "Control stopped", "settings_saved": "Settings saved, they apply after a restart", "restart_requested": "Restart requested from the settings page", + "setup_only": "In setup, nothing is sent to any inverter", "roster": "Inverter list read: {n} inverters", "maint_history": "Maintenance history did not move over cleanly", "stop_restored": "{unit}: an earlier safety stop is back, it stays at 0 W", @@ -952,6 +963,8 @@ def status_samples() -> list: ("banner_peak_reserve", None, banner_peak_reserve()), ("banner_power_too_high", None, banner_power_too_high()), ("banner_shadow", None, banner_shadow(False)), + ("banner_setup_only", None, banner_setup_only()), + ("row_setup_only", AMBER, row_setup_only()), ("banner_shadow", None, banner_shadow(True)), ("row_controller_starting", GREEN, row_controller_starting()), ("row_controller_ran", GREEN, row_controller_ran(2)), diff --git a/goodwe_controller/app/setuponly.py b/goodwe_controller/app/setuponly.py new file mode 100644 index 0000000..0e0a6b2 --- /dev/null +++ b/goodwe_controller/app/setuponly.py @@ -0,0 +1,90 @@ +"""UX-15: setup-only mode. A new install serves the setup wizard and commands +nothing. The rule and the argument are in DECISIONS/UX-15-setup-only-mode.md. + +`applies()` says when: the startup gate's only refusal is that no inverter is +named. `serve()` then runs the ingress page on a `SetupOnly` stand-in for the +controller. Nothing that can command is built: no Controller, no inverter, no +MQTT, no meter task. Its HomeAssistant is read-only, so a service call refuses +before anything is sent (hass.py), and `/action` refuses too (web.py). +""" + +import asyncio +import contextlib +import logging +import signal +from datetime import datetime, timezone + +from . import messages, web +from .inverter import roster_entries +from .validate import actuator_writers, no_power_request_problem + +_LOG = logging.getLogger("goodwe.setup") + + +def applies(opts: dict, problems) -> bool: + """True when the options name no inverter at all and that is the gate's + only refusal. Anything else refused keeps the exit 78 (CFG-01).""" + return (not actuator_writers(opts) and not roster_entries(opts) + and list(problems) == [no_power_request_problem()]) + + +class SetupOnly: + """What web.build_app() needs from a controller, and nothing that writes. + + It holds no inverter and no publisher. `hass` must be read-only.""" + + setup_only = True + auto = False # control is never started here + grid = None # no meter is read here + inverters = () + + def __init__(self, opts: dict, hass, store): + if getattr(hass, "read_only", False) is not True: + raise ValueError("setup-only mode needs a read-only HomeAssistant") + self.o, self.hass, self.store = opts, hass, store + self.events: list[str] = [] + self.hint = "" + + def log_event(self, msg: str, hint: str | None = None) -> None: + stamp = datetime.now(timezone.utc).strftime("%H:%M:%S") + self.events.insert(0, f"{stamp} {msg}") + del self.events[40:] + if hint is not None: + self.hint = f"{datetime.now().astimezone():%H:%M} {hint}" + + def status_payload(self) -> dict: + return {"setup_only": True, "mode": "setup_only", + "banner": messages.banner_setup_only(), "level": "warn", + "rows": [["Control", "not started, in setup"]], + "checks": [{"ok": False, "warn": True, + "text": messages.row_setup_only()}], + "hint": self.hint, "events": list(self.events)} + + async def handle_action(self, what): + # web.py answers 409 before it gets here. Nothing is done either way. + _LOG.warning("The page asked for an action in setup, so nothing " + "was done.") + return None + + async def shutdown(self) -> None: + """Nothing to wind down: nothing was ever commanded.""" + + +async def serve(opts: dict, hass, store, *, port: int) -> None: + """Serve the page in setup-only mode until the Supervisor stops us.""" + controller = SetupOnly(opts, hass, store) + _LOG.warning("No inverter is named yet, so the add-on is in setup and " + "controls nothing. Open its page to finish setup.") + controller.log_event("setup-only mode: no inverter named, nothing is " + "commanded", hint=messages.hint("setup_only")) + runner = await web.start(controller, port=port) + stop = asyncio.Event() + loop = asyncio.get_running_loop() + for sig in (signal.SIGTERM, signal.SIGINT): + with contextlib.suppress(NotImplementedError): + loop.add_signal_handler(sig, stop.set) + try: + await stop.wait() + finally: + await runner.cleanup() + _LOG.info("stopped") diff --git a/goodwe_controller/app/static/app.js b/goodwe_controller/app/static/app.js index 8c9530c..2aed6b4 100644 --- a/goodwe_controller/app/static/app.js +++ b/goodwe_controller/app/static/app.js @@ -94,11 +94,13 @@ function render() { renderPill(); const structured = S.now && S.plan && !S.home_error; - banner($("#home-banner"), S.level, S.banner, structured ? S.state.sub : "", S.level === "bad" ? "alert" : null); + // UX-15: setup-only mode commands nothing and has no control to offer. + const setup = S.setup_only === true; + banner($("#home-banner"), S.level, S.banner, structured ? S.state.sub : setup ? t("setup_only.description") : "", S.level === "bad" ? "alert" : null); ["#c-now", "#c-plan", "#c-peak", "#c-attn"].forEach((id) => { $(id).hidden = !structured; }); if (structured) { renderNow(); renderPlan(); renderPeak(); renderAttention(); } - renderControl(structured); - renderDetails(!structured); + renderControl(structured, setup); + renderDetails(!structured && !setup); renderBatteries(); } @@ -270,12 +272,15 @@ badge.textContent = n; badge.hidden = !n; badge.classList.toggle("bad", list.some((a) => a.severity === "alarm")); } - function renderControl(structured) { + function renderControl(structured, setup) { const running = structured ? S.control.running : null; const btn = $("#btn-ctl"); btn.textContent = running === false ? t("control.start") : t("control.stop"); btn.className = "btn" + (running === false ? " primary" : ""); - $("#ctl-text").textContent = structured ? S.control.text : ""; + btn.hidden = !!setup; + $("#btn-setup").hidden = !setup; + $("#c-ctl .more").hidden = !!setup; + $("#ctl-text").textContent = structured ? S.control.text : setup ? t("setup_only.control") : ""; $("#hint").textContent = S.hint || ""; const docs = $("#docs-link"); if (structured && S.control.docs_url) { docs.href = S.control.docs_url; docs.hidden = false; } else docs.hidden = true; diff --git a/goodwe_controller/app/static/index.html b/goodwe_controller/app/static/index.html index 3831627..14577db 100644 --- a/goodwe_controller/app/static/index.html +++ b/goodwe_controller/app/static/index.html @@ -72,6 +72,7 @@
+
diff --git a/goodwe_controller/app/static/settings.js b/goodwe_controller/app/static/settings.js index 603dfcb..aa3e9ff 100644 --- a/goodwe_controller/app/static/settings.js +++ b/goodwe_controller/app/static/settings.js @@ -533,10 +533,12 @@ if (input) input.focus({ preventScroll: true }); } // The one save path, for the Installer view and the wizard. After a stale - // refusal the page holds the new token, so a second press saves. + // refusal the page holds the new token, so a second press saves. `base` is + // the model the page opened: only what changed since then is written, onto + // what is stored now (UX-15 F2). async function postSave(extra) { const r = await fetch("config/save", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify(Object.assign({ model: V, fingerprint: FP }, extra || {})) }); + body: JSON.stringify(Object.assign({ model: V, base: ORIG, fingerprint: FP }, extra || {})) }); const b = await r.json().catch(() => ({})); if (r.status === 409 && b.kind === "stale") FP = b.fingerprint || FP; return { r, b }; @@ -624,8 +626,10 @@ if (WZ.kettle && WZ.kettle.sig !== sig) WZ.kettle = null; // another meter: test again const k = WZ.kettle || { sig }; box.append(h("div", { class: "help" }, WS.kettle.description)); - const tone = { ok: "ok", flip: "info", none: "warn", unread: "bad", skipped: "warn" }[k.say] || "bad"; - const words = k.say ? WS.kettle[k.say] : k.before != null ? WS.kettle.before : null; + // A skip after the reading went down still says it went down (UX-15). + const said = k.say === "skipped" && k.wrong ? "skipped_down" : k.say; + const tone = { ok: "ok", flip: "info", none: "warn", unread: "bad", skipped: "warn", skipped_down: "warn" }[said] || "bad"; + const words = said ? WS.kettle[said] : k.before != null ? WS.kettle.before : null; if (words) box.append(h("div", { class: "msg " + (k.say ? tone : "info"), role: "status" }, say(words.description, { w: k.before != null ? watts(k.before, true) : "", d: k.d != null ? watts(k.d) : "" }))); const busy = (b) => { b.disabled = true; }; @@ -715,7 +719,9 @@ if (!b) { toast(WS.check.failed.description); return; } if (WZ.blocked) { toast(WS.check.blocked.description); return; } const shadow = shadowValue() === M.setup.shadow.on; - ask(shadow ? WS.dialog : WS.dialog_live, async () => { + // In setup-only mode nothing runs yet, so no inverter is set to 0 W first. + const dlg = M.setup_only ? (shadow ? WS.dialog_setup : WS.dialog_setup_live) : (shadow ? WS.dialog : WS.dialog_live); + ask(dlg, async () => { wzBanner(); let res; try { res = await postSave({ setup: "done", model: posted() }); } catch (e) { toast(W.toast.failed); return; } diff --git a/goodwe_controller/app/store.py b/goodwe_controller/app/store.py index b3fea7a..c716fd6 100644 --- a/goodwe_controller/app/store.py +++ b/goodwe_controller/app/store.py @@ -581,9 +581,12 @@ def migrate_maint_keys(store, unit: str, roster) -> MigrationReport: return rep +STATE_PATH = "/data/state.json" + + class Store: - def __init__(self, path: str = "/data/state.json"): - self.path = path + def __init__(self, path: str | None = None): + self.path = path or STATE_PATH self.data = dict(DEFAULTS) # UX-13 P4: True when this boot found no state file, i.e. a new /data. # Only the setup wizard's first-run rule reads it (uipage.setup_needed). diff --git a/goodwe_controller/app/ui_text/en.json b/goodwe_controller/app/ui_text/en.json index bd5cb3c..833286e 100644 --- a/goodwe_controller/app/ui_text/en.json +++ b/goodwe_controller/app/ui_text/en.json @@ -79,6 +79,7 @@ }, "toast": {"started": "Control started.", "stopped": "Control stopped.", "done": "Done.", "failed": "That did not work. Try again."}, "offline": {"name": "Cannot reach the add-on", "description": "The page tries again every 3 seconds."}, + "setup_only": {"description": "Nothing is sent to any inverter until setup is saved and the add-on restarts.", "control": "Control starts after setup.", "open": "Open Setup"}, "batteries": {"name": "Batteries", "description": "Battery modes and on and off switches arrive in a later update.", "level": "Level", "now": "Now", "stored": "Stored", "mode": "Mode", "settings": "Change inverter settings"}, "settings": {"name": "Settings", "description": "The settings are grouped by task below.", "open": "Open the settings page"} }, @@ -192,6 +193,7 @@ "self": {"description": "The add-on reads this meter itself, so Home shows its direction after the restart."}, "skipped": {"description": "The meter direction is not tested, so check it on Home after the restart."}, "skipped_wrong": {"description": "The kettle test saw the meter reading go down, so check its direction in step 1 before you start control."}, + "skipped_down": {"description": "The reading went down when the kettle came on, so check the meter's direction before you start control."}, "tested": {"description": "The meter direction was tested with a kettle."}, "needed": {"description": "Do the kettle test first, or skip it."} }, @@ -206,6 +208,8 @@ "shadow": {"name": "Start in shadow mode", "description": "Recommended, because every inverter then stays at 0 W until you turn shadow mode off under Settings."}, "dialog": {"name": "Save and restart?", "description": "Every inverter is set to 0 W while the add-on restarts, and shadow mode keeps them at 0 W after that.", "yes": "Save and restart"}, "dialog_live": {"name": "Save and restart?", "description": "Every inverter is set to 0 W while the add-on restarts, and then the new settings apply.", "yes": "Save and restart"}, + "dialog_setup": {"name": "Save and restart?", "description": "The add-on restarts with these settings, and shadow mode keeps every inverter at 0 W.", "yes": "Save and restart"}, + "dialog_setup_live": {"name": "Save and restart?", "description": "The add-on restarts with these settings, and then they apply.", "yes": "Save and restart"}, "done": {"name": "Setup is done", "description": "The add-on restarts in shadow mode now, so open Home in a minute to see what it would do."}, "done_live": {"name": "Setup is done", "description": "The add-on restarts now, so open Home in a minute to start control."}, "restart_failed": {"name": "Saved, not running yet", "description": "The restart did not go through, so press Restart now to apply the settings."}, diff --git a/goodwe_controller/app/uimodel.py b/goodwe_controller/app/uimodel.py index 08523ed..ad72b97 100644 --- a/goodwe_controller/app/uimodel.py +++ b/goodwe_controller/app/uimodel.py @@ -20,7 +20,8 @@ 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", +__all__ = ["options_to_ui", "ui_to_options", "ui_to_options_partial", + "REGION_PRESETS", "arbitrage_from_batteries", "DeriveError", "COUNT", "GROWN"] @@ -32,6 +33,7 @@ class DeriveError(ValueError): def __init__(self, text, code="", field=""): super().__init__(text) self.code, self.field = code, field + self.keys = () # the option keys the failed derivation writes # The page model's inverter count, which show conditions read, and what adding @@ -337,14 +339,31 @@ def options_to_ui(options: dict) -> dict: 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.""" + out, errors = ui_to_options_partial(model) + if errors: + raise errors[0] + return out + + +def ui_to_options_partial(model: dict): + """(blob, [DeriveError]): every derivation that can be made, and the + stored values (`keep`) for each one that cannot. For the checks only, so a + step can be blocked by its own field and the rest still checked (UX-15 + F1). Never saved: `ui_to_options()` raises instead.""" 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.")} + errors = [] 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: + continue + try: out.update(encode(model, keep)) - return out + except DeriveError as err: + err.keys = keys + errors.append(err) + out.update({k: copy.deepcopy(keep[k]) for k in keys if k in keep}) + return out, errors diff --git a/goodwe_controller/app/uipage.py b/goodwe_controller/app/uipage.py index 42dfd52..e4564b2 100644 --- a/goodwe_controller/app/uipage.py +++ b/goodwe_controller/app/uipage.py @@ -186,7 +186,32 @@ def to_options(model: dict, stored: dict | None = None) -> dict: """The options blob for a posted page model. A value equal to the stored one is written as stored, in the stored key order, so a save that changed nothing writes an identical blob (the browser reads 3.0 as 3).""" - out = U.ui_to_options(model) + return _settle(U.ui_to_options(model), stored) + + +def to_options_partial(model: dict, stored: dict | None = None): + """(blob, [DeriveError]) for the checks: `uimodel.ui_to_options_partial`, + settled like `to_options`. Never saved (UX-15 F1).""" + out, errors = U.ui_to_options_partial(model) + return _settle(out, stored), errors + + +def only_edits(mine: dict, base: dict, stored: dict) -> dict: + """The blob to save: `stored`, with the keys whose value in `mine` (this + page's model) differs from `base` (the model it opened) set to `mine`'s. + A key this page never edited keeps what is stored now, even when another + surface changed it meanwhile (UX-13 P4 test, F2).""" + edits = {k: v for k, v in mine.items() + if k not in base or not _same(v, base[k])} + gone = {k for k in base if k not in mine} + out = {k: copy.deepcopy(edits.get(k, v)) for k, v in stored.items() + if k not in gone} + out.update({k: copy.deepcopy(v) for k, v in edits.items() + if k not in out}) + return out + + +def _settle(out: dict, stored: dict | None) -> dict: stored = stored or {} for k, v in out.items(): if k in stored and _same(v, stored[k]): @@ -202,6 +227,21 @@ def derive_message(err: U.DeriveError) -> dict: "text": text} +def derive_messages(errors, problems, notes, model, keys): + """The page's messages when part of the model could not become options: + each conversion's own sentence beside its field, then the checks of the + rest. A refusal or note about a key the failed conversion writes is left + out, because it judges the stored value, not what the page shows.""" + failed = {k for err in errors for k in err.keys} + by_key, unanchored = problems_by_field(problems, keys) + kept = [t for k, texts in by_key.items() if k not in failed + for t in texts] + list(unanchored) + kept = [p for p in problems if p in kept] + notes = [n for n in notes if not set(n.keys or ()) & failed] + return ([derive_message(err) for err in errors] + + anchors(kept, notes, model, keys)) + + # --------------------------------------------------------------------------- # # Where a sentence goes on the page # --------------------------------------------------------------------------- # diff --git a/goodwe_controller/app/validate.py b/goodwe_controller/app/validate.py index 2a27f6d..525e94b 100644 --- a/goodwe_controller/app/validate.py +++ b/goodwe_controller/app/validate.py @@ -808,6 +808,14 @@ class ConfigRefused(Exception): super().__init__(problems[0] if problems else "configuration refused") +def no_power_request_problem() -> str: + """Rule 1's refusal: no inverter to control. UX-15's setup-only mode + starts when this is the only refusal (app/setuponly.py).""" + return (f"{S('setpoint_entity')} is empty, so the add-on cannot control " + "the inverter. Set it to the ESPHome number entity for the " + "setpoint.") + + 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. @@ -828,9 +836,7 @@ def validate_startup_config(opts: dict, dev_min=None, dev_max=None, # --- 1. an actuator to write to, and AT MOST ONE WRITER PER ACTUATOR ----- writers = actuator_writers(opts) if not writers: - problems.append( - f"{S('setpoint_entity')} is empty, so the add-on cannot control the " - "inverter. Set it to the ESPHome number entity for the setpoint.") + problems.append(no_power_request_problem()) # SAFETY-28. The §1.5 non-goal, graded on the writers rather than on the # count of option keys. Read `actuator_writers()` for why the old check went # green on the very config this one is here to refuse. @@ -1704,6 +1710,14 @@ WATT_ENTITY_KEYS = ( "p1_phase_export_entities", "batt_entity", "peak_forecast_entity") +# The meter entities each `meter_source` reads through Home Assistant. +METER_ENTITY_KEYS = { + "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")} + + 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 @@ -1716,12 +1730,7 @@ def _entities_read(opts: dict) -> list[tuple[str, str, str]]: 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, ()): + for key in METER_ENTITY_KEYS.get(source, ()): add(key, opts.get(key)) entries = roster_entries(opts) if not entries: @@ -1844,7 +1853,7 @@ def setup_checks(opts: dict, *, entities: dict | None, running=None, entity exists in Home Assistant (a write to a missing one is answered 200, SAFETY-77, so only the entity list can tell), the battery levels read, and prices are known. Tier "ok", "info", "warn", or "bad" for a missing power - request entity, which the wizard will not save. Reads nothing itself: + request or meter entity, which the wizard will not save. Reads nothing itself: `entities` is Home Assistant's list by id (None when it did not answer), `running` the options this process started with, `meter_now_w` its meter reading and `prices_known` how many of today's hours have a price. @@ -1855,7 +1864,11 @@ def setup_checks(opts: dict, *, entities: dict | None, running=None, _advise(notes, "bad", ("setpoint_entity",), "Home Assistant is not " "answering, so press Check again once it is back.") else: - _entity_notes(notes, opts, entities, bad=("setpoint_entity",)) + # A missing meter entity is red too (UX-15): with the kettle test + # skipped, nothing else on the way to the save would catch it. + meter_keys = {k for keys in METER_ENTITY_KEYS.values() for k in keys} + _entity_notes(notes, opts, entities, + bad=("setpoint_entity",) + tuple(sorted(meter_keys))) meter = _meter_check(notes, opts, entities, running, meter_now_w) batts = [_card_checks(notes, who, entry, entities) for who, entry in _cards(opts)] diff --git a/goodwe_controller/app/web.py b/goodwe_controller/app/web.py index 730073a..e21029c 100644 --- a/goodwe_controller/app/web.py +++ b/goodwe_controller/app/web.py @@ -31,6 +31,8 @@ of Home Assistant's entity list (app/haentities.py) and writes nothing. UX-13 P4's setup wizard uses the same routes. The one thing it adds that is not options is a Store key, `setup_done`, written when the wizard's save succeeds, so the page stops opening the wizard by itself (uipage.setup_needed). +UX-15's setup-only mode serves these routes from `setuponly.SetupOnly`, which +commands nothing; there `/action` answers 409. """ import io @@ -136,6 +138,10 @@ def build_app(controller, selfcfg=None) -> web.Application: except ValueError: raise web.HTTPBadRequest(text="expected JSON") what = body.get("action") + if getattr(controller, "setup_only", False): + # UX-15: nothing to start or stop until setup is saved. + return web.json_response( + {"ok": False, "error": messages.row_setup_only()}, status=409) result = await controller.handle_action(what) if result is None: raise web.HTTPBadRequest(text=f"unknown action {what!r}") @@ -149,14 +155,23 @@ def build_app(controller, selfcfg=None) -> web.Application: """(body, options, model). UX-13's settings view posts its page `model`, which app/uimodel.py turns into the options blob here, so the validator and the Supervisor only ever see a real blob. The classic - page posts `options`. Raises DeriveError for a model no blob can say.""" + page posts `options`. Raises DeriveError for a model no blob can say. + + With `stored` (a save) and the `base` model the page opened, only the + keys this page changed are written onto what is stored now, so a save + after a stale refusal keeps the other surface's changes (UX-15 F2).""" try: body = await request.json() except ValueError: raise web.HTTPBadRequest(text="expected JSON") model = body.get("model") if isinstance(model, dict): - return body, uipage.to_options(model, stored), model + mine = uipage.to_options(model, stored) + base = body.get("base") + if stored is not None and isinstance(base, dict): + was, _errors = uipage.to_options_partial(base, stored) + mine = uipage.only_edits(mine, was, stored) + return body, mine, model options = body.get("options") if not isinstance(options, dict): raise web.HTTPBadRequest(text="expected an options object") @@ -202,6 +217,8 @@ def build_app(controller, selfcfg=None) -> web.Application: options, store_data(), bool(getattr(controller, "auto", False)), fresh=getattr(getattr(controller, "store", None), "fresh", False) is True), + # UX-15: this process serves setup only and commands nothing. + "setup_only": getattr(controller, "setup_only", False) is True, }) return web.json_response(model) @@ -219,11 +236,19 @@ def build_app(controller, selfcfg=None) -> web.Application: be a second opinion about a rule it does not own. """ try: - _body, options, model = await _proposed(request) - except DeriveError as err: - return web.json_response({"problems": [], "by_key": {}, - "unanchored": [], "notes": [], - "messages": [uipage.derive_message(err)]}) + _body = await request.json() + except ValueError: + raise web.HTTPBadRequest(text="expected JSON") + model = _body.get("model") + errors = [] + if isinstance(model, dict): + # UX-15 F1: a part the page cannot turn into options blocks its + # own step, and everything else is still checked. + options, errors = uipage.to_options_partial(model) + else: + model, options = None, _body.get("options") + if not isinstance(options, dict): + raise web.HTTPBadRequest(text="expected an options object") notes = [] problems, unread = await selfconfig.problems_and_ranges( options, controller.hass, notes=notes) @@ -237,8 +262,10 @@ def build_app(controller, selfcfg=None) -> web.Application: if unread: payload["device_note"] = DEVICE_NOTE % ", ".join(sorted(set(unread))) if model is not None: - payload["messages"] = uipage.anchors(problems, notes, model, - _keys_of(options)) + payload["messages"] = ( + uipage.derive_messages(errors, problems, notes, model, + _keys_of(options)) if errors + else uipage.anchors(problems, notes, model, _keys_of(options))) payload["steps"] = uipage.step_problems(payload["messages"], model) if _body.get("setup"): payload["setup"] = [n._asdict() for n in await _setup_checks(options)] @@ -286,10 +313,14 @@ def build_app(controller, selfcfg=None) -> web.Application: return web.json_response({"error": str(err)}, status=502) try: body, options, model = await _proposed(request, stored_now) - except DeriveError as err: + except DeriveError: + model = posted["model"] + _opts, errors = uipage.to_options_partial(model, stored_now) + messages_ = [uipage.derive_message(e) for e in errors] return web.json_response( {"kind": "invalid", "problems": [], "by_key": {}, - "unanchored": [], "messages": [uipage.derive_message(err)]}, + "unanchored": [], "messages": messages_, + "steps": uipage.step_problems(messages_, model)}, status=422) expect = str(body.get("fingerprint") or "") try: diff --git a/goodwe_controller/test_setup.py b/goodwe_controller/test_setup.py index be86f82..ebcd97c 100644 --- a/goodwe_controller/test_setup.py +++ b/goodwe_controller/test_setup.py @@ -9,7 +9,8 @@ Part 6 the save through the P2 path, and every blob the wizard can produce (tools/ux13_wizard_blobs.py; the Supervisor's own validator runs over the same set in the PR, not in CI, which has no Supervisor image). Part 7 the stale save names only what changed elsewhere (P2 test, finding a). Part 8 the -words. The headless-browser click-through is in the PR. +words. Part 9 UX-15's fixes: F1, F2 with two tabs, a missing meter entity and +step 1 after a skip. The headless-browser click-through is in the PR. Mutating a module this file grades? Read MUTATION.md first (purge __pycache__). """ @@ -51,6 +52,7 @@ DEFAULTS = D.defaults(D.CONFIG) NEW = dict(DEFAULTS, setpoint_entity="number.inv1_power") UI_TEXT = uipage.ui_text() WS = UI_TEXT["settings"]["setup"] +WS_D = UI_TEXT["settings"]["derive"] JS = io.open(os.path.join(HERE, "app", "static", "settings.js"), encoding="utf-8").read() @@ -626,11 +628,15 @@ print(f" {len(_all)} blobs from {len(B.starts())} starts; " check(f"every plan from a new install is accepted by the shipped validator " f"({len(_new)} plans)", len(_new) >= 25 and not [n for n, r in _new if r]) +_empty = [(n, r) for n, _b, r in _all if n.startswith("empty-")] +check(f"every plan from a new install as shipped (no power request entity, " + f"setup-only mode) is accepted ({len(_empty)} plans)", + len(_empty) == len(_new) and not [n for n, r in _empty if r]) check("every plan from the TEST-23 sites is accepted, except where the site " "trades on price with a plan that has no meter or no prices, which the " "last step then lists", all(not r or ("options.arb" in n and "arbitrage_enabled" in r[0]) - for n, _b, r in _all) and len(_ok) >= 80) + for n, _b, r in _all) and len(_ok) >= 100) check("every accepted blob keeps every option key (the Supervisor count, 78)", all(set(b) == set(DEFAULTS) for _n, b, r in _all if not r)) @@ -645,9 +651,11 @@ async def _stale(s, root, sup): model = copy.deepcopy(m["ui"]) model["price_timeout_s"] = 12 # the operator's own edit first = await _post(s, root + "config/save", - {"model": model, "fingerprint": m["fingerprint"]}) + {"model": model, "base": m["ui"], + "fingerprint": m["fingerprint"]}) again = await _post(s, root + "config/save", - {"model": model, "fingerprint": first[1]["fingerprint"]}) + {"model": model, "base": m["ui"], + "fingerprint": first[1]["fingerprint"]}) return first, again @@ -659,12 +667,13 @@ check("the banner's list is the other surface's key only, not the operator's " "own edit", _first_r[0] == 409 and _first_r[1]["changed_fields"] == ["peak_cap_w"] and _first_r[1]["moved"] == ["peak_cap_w"]) -check("`changed` (stored against mine, which the classic page shows as " - "theirs -> mine) is unchanged", - _first_r[1]["changed"] == ["peak_cap_w", "price_timeout_s"]) +check("`changed` (stored against what this page would write) is only the " + "page's own edit, since a save writes only that (UX-15 F2)", + _first_r[1]["changed"] == ["price_timeout_s"]) check("saving again with the new token is the deliberate second press: it " - "stores the page as shown (Discard loads theirs instead)", - _again[0] == 200 and _sup.options["peak_cap_w"] == 3500 + "stores this page's own edit and keeps the other surface's change " + "(UX-15 F2)", + _again[0] == 200 and _sup.options["peak_cap_w"] == 5000 and _sup.options["price_timeout_s"] == 12) @@ -757,6 +766,118 @@ _leak = sorted(k for k in TL.config_surface(D.CONFIG)[0] if re.search( check(f"settings.js, wizard included, names no option key ({_leak})", not _leak) +# --------------------------------------------------------------------------- # +print("\n9. UX-15: F1, F2, a missing meter entity, and step 1 after a skip") +# --------------------------------------------------------------------------- # + + +async def _f1(s, root, sup): + m = await _get(s, root + "config/model?lang=en") + price = copy.deepcopy(m["ui"]) + price.update({"ui.contract": "daynight", "ui.fallback": "daynight", + "ui.day_price": None, "ui.night_price": 0.25}) + none = copy.deepcopy(m["ui"]) + none["inverters"] = [] + none[U.COUNT] = 0 + return {"price": (await _post(s, root + "config/validate", + {"model": price, "setup": True}))[1], + "none": (await _post(s, root + "config/validate", + {"model": none}))[1], + "save": await _post(s, root + "config/save", { + "model": price, "base": m["ui"], "setup": "done", + "fingerprint": m["fingerprint"]})} + + +_r, _sup = asyncio.run(session(Controller(SITE, STATES, Store(), grid=402.0), + SITE, _f1)) +print(f" empty day price: steps {_r['price']['steps']}") +check("F1: a price the page cannot turn into options blocks step 4, beside " + "its own field", + _r["price"]["steps"]["4"] >= 1 + and {"field": "ui.day_price", "card": None, "tier": "bad", + "text": WS_D["no_price"]["description"]} in _r["price"]["messages"]) +check("F1: ...and the last step's live checks still run on the rest", + [x["tier"] for x in _r["price"].get("setup", []) + if "number.inv1_power" in x["text"]] == ["ok"] + and any("402 W" in x["text"] for x in _r["price"]["setup"])) +check("F1: no inverter card blocks step 2, not the last step alone", + _r["none"]["steps"]["2"] >= 1) +check("F1: the save refuses it (422) and says which step to go back to", + _r["save"][0] == 422 and _r["save"][1]["steps"]["4"] >= 1 + and _sup.writes == []) + + +async def _tabs(s, root, sup): + """Two tabs open one blob. A saves a peak limit; B edited only the + timeout, gets the 409, and presses Save again.""" + a = await _get(s, root + "config/model?lang=en") + b = await _get(s, root + "config/model?lang=en") + va = copy.deepcopy(a["ui"]) + va["peak_cap_w"] = 4000 + ra = await _post(s, root + "config/save", { + "model": va, "base": a["ui"], "fingerprint": a["fingerprint"]}) + after_a = copy.deepcopy(sup.options) + vb = copy.deepcopy(b["ui"]) + vb["price_timeout_s"] = 12 + first = await _post(s, root + "config/save", { + "model": vb, "base": b["ui"], "fingerprint": b["fingerprint"]}) + again = await _post(s, root + "config/save", { + "model": vb, "base": b["ui"], "fingerprint": first[1]["fingerprint"]}) + return ra, after_a, first, again + + +(_ra, _after_a, _first_b, _again_b), _sup = asyncio.run( + session(Controller(SITE, STATES), dict(SITE, peak_cap_w=3600), _tabs)) +_moved = sorted(k for k in _sup.options + if _sup.options[k] != _after_a.get(k)) +print(f" A: {_ra[0]}, B: {_first_b[0]} then {_again_b[0]}; B's re-save " + f"moved {_moved}") +check("F2: two tabs. A saves peak 4.0; B, which edited only the timeout, " + "gets the 409 naming the peak limit", + _ra[0] == 200 and _first_b[0] == 409 + and _first_b[1]["changed_fields"] == ["peak_cap_w"]) +check("F2: B's second press keeps A's peak 4.0 and writes only B's own key", + _again_b[0] == 200 and _sup.options["peak_cap_w"] == 4000 + and _sup.options["price_timeout_s"] == 12 + and _moved == ["price_timeout_s"]) +_both = uipage.only_edits({"a": 2, "b": 1, "c": 5}, {"a": 1, "b": 1, "c": 3}, + {"a": 1, "b": 9, "c": 4, "d": 0}) +check("F2: only_edits() keeps every stored key the page did not edit, and a " + "key both edited takes this page's value (the deliberate press)", + _both == {"a": 2, "b": 9, "c": 5, "d": 0} + and list(_both) == ["a", "b", "c", "d"]) + +_gone = tiers(setup_checks(dict(SITE, meter_entity="sensor.gone"), + entities=ROWS)) +check("a meter entity Home Assistant does not have is red on the last step, " + "so the wizard does not save it with the kettle test skipped", + ("bad", "Home Assistant has no entity sensor.gone, which “Grid power " + "entity” (meter_entity) names.") in _gone) +_net_gone = tiers(setup_checks(dict(SITE, meter_source="ha_signed", + p1_net_entity="sensor.gone"), + entities=ROWS)) +check("...for a net or import and export meter too", + [t for t, x in _net_gone if "sensor.gone" in x] == ["bad"]) +from app.validate import config_notes # noqa: E402 +check("...while the Installer view keeps it a warning, which never blocks " + "a save there", + [n.tier for n in config_notes(dict(SITE, meter_entity="sensor.gone"), + entities=ROWS) + if "sensor.gone" in n.text] == ["warn"]) + +check("step 1 after a skip that followed a fall says plainly that the " + "reading went down", + 'k.say === "skipped" && k.wrong ? "skipped_down" : k.say' in JS + and "went down" in WS["kettle"]["skipped_down"]["description"] + and "not tested" not in WS["kettle"]["skipped_down"]["description"]) +check("in setup-only mode the wizard's restart dialog does not claim a 0 W " + "wind-down that nothing runs, and the shadow one says 0 W", + "M.setup_only ? (shadow ? WS.dialog_setup : WS.dialog_setup_live)" in JS + and "0 W" in WS["dialog_setup"]["description"] + and "0 W" not in WS["dialog_setup_live"]["description"]) +check("the page posts the model it opened with every save (F2)", + "model: V, base: ORIG, fingerprint: FP" in JS) + print() if fails: print(f"{len(fails)} FAILED") diff --git a/goodwe_controller/test_setup_only.py b/goodwe_controller/test_setup_only.py new file mode 100644 index 0000000..e880155 --- /dev/null +++ b/goodwe_controller/test_setup_only.py @@ -0,0 +1,488 @@ +"""UX-15: setup-only mode. `python3 test_setup_only.py` + +A new install names no inverter, and instead of exiting 78 it serves the setup +wizard and commands nothing (DECISIONS/UX-15-setup-only-mode.md). +Part 1 checks when it applies. Part 2 the write path refuses in that mode. +Part 3 boots the REAL `app.main.main()` in a child process (this file with +`--driver`) against a fake Supervisor and Home Assistant over HTTP, and logs +every request they get: the shipped defaults serve the wizard with no service +call, also with `auto_start` on, a Store that says control was started and a +Start pressed on the page; the wizard saves; the next boot passes the normal +gate and runs in shadow. Part 4: a refused but complete configuration still +exits 78 and serves nothing. +Takes ~25 s. Mutating a module this file grades? Read MUTATION.md first. +""" + +import asyncio +import copy +import json +import os +import shutil +import signal +import socket +import subprocess +import sys +import tempfile +import time + +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.join(HERE, "tools")) +DEADLINE_S = 60 + +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +def defaults(): + import ux13_demo as D + return D.defaults(D.CONFIG) + + +def state(eid, value, unit="W", cls=None, **attrs): + return {"entity_id": eid, "state": value, + "attributes": dict({"friendly_name": eid, + "unit_of_measurement": unit, + "device_class": cls}, **attrs)} + + +STATES = [state("sensor.grid", "402"), + state("number.inv1_power", "0", min=-2500, max=2500, step=1), + state("sensor.inv1_level", "62", "%", "battery"), + state("sensor.inv1_battery", "0")] + + +# --------------------------------------------------------------------------- # +# The child: the real main(), with only the outside world faked over HTTP. +# --------------------------------------------------------------------------- # +def driver(scenario, work): + """`work` holds options.json, state.json and the files the parent reads. + Every request the fakes get is streamed as `EV [...]` lines.""" + import aiohttp + from aiohttp import web as aioweb + from aiohttp.abc import AbstractResolver + import app.main as M + import app.store + import app.web + import ux13_demo as D + import ux13_wizard_blobs as B + import translations_lint as TL + + def event(*e): + print("EV " + json.dumps(list(e)), flush=True) + + opts = json.load(open(os.path.join(work, "options.json"))) + stored = {"options": copy.deepcopy(opts)} + schema = D.schema_ui(D.CONFIG) + translations = {"en": TL.parse_translation_file( + os.path.join(HERE, "translations", "en.yaml"))} + + # --- the fake Supervisor and Home Assistant --------------------------- + async def h_probe(_r): + event("GET", "/core/api/") + return aioweb.Response(text="API running.") + + async def h_states(_r): + event("GET", "/core/api/states") + return aioweb.json_response(STATES) + + async def h_state(r): + eid = r.match_info["eid"] + event("GET", "/core/api/states/" + eid) + found = next((s for s in STATES if s["entity_id"] == eid), None) + return (aioweb.json_response(found) if found + else aioweb.json_response({"message": "not found"}, status=404)) + + async def h_service(r): + body = await r.json() + event("SERVICE", r.match_info["domain"], r.match_info["service"], body) + return aioweb.json_response([]) + + async def h_mqtt(_r): + event("GET", "/services/mqtt") + return aioweb.json_response({"result": "error"}, status=400) + + async def h_info(_r): + return aioweb.json_response({"data": {"slug": "local_goodwe", + "homeassistant": "2026.9.1"}}) + + async def h_self_info(_r): + return aioweb.json_response({"data": { + "slug": "local_goodwe", "options": stored["options"], + "schema": schema, "translations": translations}}) + + async def h_options(r): + body = await r.json() + stored["options"] = body["options"] + event("OPTIONS") + with open(os.path.join(work, "saved.json"), "w") as f: + json.dump(body["options"], f) + return aioweb.json_response({"result": "ok", "data": {}}) + + async def h_restart(_r): + event("RESTART") + return aioweb.json_response({"result": "ok", "data": {}}) + + async def h_other(r): + event("OTHER", r.method, r.path) + return aioweb.json_response({}, status=404) + + fake = aioweb.Application() + fake.add_routes([ + aioweb.get("/core/api/", h_probe), + aioweb.get("/core/api/states", h_states), + aioweb.get("/core/api/states/{eid}", h_state), + aioweb.post("/core/api/services/{domain}/{service}", h_service), + aioweb.get("/services/mqtt", h_mqtt), + aioweb.get("/info", h_info), + aioweb.get("/addons/self/info", h_self_info), + aioweb.post("/addons/self/options", h_options), + aioweb.post("/addons/self/restart", h_restart), + aioweb.route("*", "/{tail:.*}", h_other)]) + + fake_port = free_port() + + class ToFake(AbstractResolver): + """`http://supervisor` is the fake, on its port.""" + async def resolve(self, host, port=0, family=socket.AF_INET): + return [{"hostname": host, "host": "127.0.0.1", "port": fake_port, + "family": socket.AF_INET, "proto": 0, "flags": 0}] + + async def close(self): + pass + + Real = aiohttp.ClientSession + + def session(*a, **k): + k.setdefault("connector", aiohttp.TCPConnector(resolver=ToFake())) + return Real(*a, **k) + + aiohttp.ClientSession = session + os.environ["SUPERVISOR_TOKEN"] = "test-token" + M.OPTIONS_PATH = os.path.join(work, "options.json") + app.store.STATE_PATH = os.path.join(work, "state.json") + M.INGRESS_PORT = free_port() + kw = M.Controller.__init__.__kwdefaults__ + kw["peak_db_path"] = os.path.join(work, "tariff.db") + kw["price_history_path"] = os.path.join(work, "price_history.json") + + served = [] + real_start = app.web.start + + async def start(controller, port=8099, selfcfg=None): + runner = await real_start(controller, port, selfcfg) + served.append(type(controller).__name__) # listening now + event("SERVED", type(controller).__name__, + getattr(controller.hass, "read_only", None)) + return runner + + app.web.start = start + real_amain = M.amain + root = f"http://127.0.0.1:{M.INGRESS_PORT}/" + + async def ask(s, method, path, body=None): + async with s.request(method, root + path, json=body) as r: + try: + out = await r.json(content_type=None) + except ValueError: + out = None + event("PAGE", method, path, r.status, out) + return r.status, out + + async def scenario_steps(): + async with Real() as s: + await ask(s, "GET", "status") + _st, model = await ask(s, "GET", "config/model?lang=en") + event("STATE_FILE", os.path.exists(app.store.STATE_PATH)) + await ask(s, "POST", "action", {"action": "auto_toggle"}) + if scenario.startswith("setup"): + await ask(s, "POST", "action", {"action": "maint_start"}) + wiz = B.apply(model["ui"], B.BASE) + wiz = B.browser(json.loads(json.dumps(wiz))) + await ask(s, "POST", "config/validate", + {"model": wiz, "setup": True}) + event("STATE_FILE", os.path.exists(app.store.STATE_PATH)) + await ask(s, "POST", "config/save", + {"model": wiz, "base": model["ui"], + "fingerprint": model["fingerprint"], + "setup": "done"}) + await ask(s, "POST", "config/restart", {"confirm": "restart"}) + await asyncio.sleep(3.0) # room for any stray task to write + await ask(s, "GET", "status") + + async def wrapped(): + runner = aioweb.AppRunner(fake) + await runner.setup() + await aioweb.TCPSite(runner, "127.0.0.1", fake_port).start() + task = asyncio.create_task(real_amain()) + try: + t0 = time.monotonic() + while not served and not task.done() \ + and time.monotonic() - t0 < 20: + await asyncio.sleep(0.05) + if task.done(): + event("ENDED_BEFORE_SERVING") + await task + return + await scenario_steps() + os.kill(os.getpid(), signal.SIGTERM) # the Supervisor's stop + await asyncio.wait_for(task, 20) + finally: + await runner.cleanup() + + M.amain = wrapped + M.main() + + +def free_port(): + with socket.socket() as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +def run_child(scenario, options, state=None): + work = tempfile.mkdtemp(prefix="ux15-") + with open(os.path.join(work, "options.json"), "w") as f: + json.dump(options, f) + if state is not None: + with open(os.path.join(work, "state.json"), "w") as f: + json.dump(state, f) + env = dict(os.environ, PYTHONDONTWRITEBYTECODE="1") + try: + p = subprocess.run([sys.executable, "-B", __file__, "--driver", + scenario, work], cwd=HERE, env=env, + capture_output=True, text=True, timeout=DEADLINE_S) + rc, out, err = p.returncode, p.stdout, p.stderr + except subprocess.TimeoutExpired as e: + rc, out, err = None, e.stdout or "", e.stderr or "" + out = out.decode() if isinstance(out, bytes) else out + err = err.decode() if isinstance(err, bytes) else err + ev = [json.loads(ln[3:]) for ln in out.splitlines() if ln.startswith("EV ")] + files = {} + for name in ("saved.json", "state.json"): + path = os.path.join(work, name) + if os.path.exists(path): + files[name] = json.load(open(path)) + shutil.rmtree(work, ignore_errors=True) + return rc, ev, err, files + + +def page(ev, method, path): + """(status, body) of each page request, in order.""" + return [(e[3], e[4]) for e in ev + if e[0] == "PAGE" and e[1] == method and e[2].startswith(path)] + + +def tail(text, n=12): + return "\n".join(" | " + ln for ln in text.splitlines()[-n:]) + + +if len(sys.argv) == 4 and sys.argv[1] == "--driver": + driver(sys.argv[2], sys.argv[3]) + sys.exit(0) + + +# --------------------------------------------------------------------------- # +print("1. when setup-only mode applies") +# --------------------------------------------------------------------------- # +from app import setuponly # noqa: E402 +from app.validate import validate_startup_config # noqa: E402 + +SHIPPED = defaults() +V = validate_startup_config +check("the shipped defaults are refused only for the missing power request " + "entity, and that is setup-only mode", + setuponly.applies(SHIPPED, V(SHIPPED)) and len(V(SHIPPED)) == 1) +_blank_bad = dict(SHIPPED, maintenance_charge_w=2500) +check("no power request entity AND another refusal: not setup-only, exit 78", + len(V(_blank_bad)) >= 2 and not setuponly.applies(_blank_bad, + V(_blank_bad))) +_complete_bad = dict(SHIPPED, setpoint_entity="number.inv1_power", + maintenance_charge_w=2500) +check("a complete configuration that is refused: not setup-only, exit 78", + V(_complete_bad) and not setuponly.applies(_complete_bad, + V(_complete_bad))) +_list_blank = dict(SHIPPED, inverters=[{ + "name": "inverter-a", "setpoint_entity": "", "soc_entity": "sensor.s", + "batt_entity": "sensor.b", "max_w": 2000, "bms_soc_floor": 10}]) +check("an inverter list whose entry has no power request entity: not " + "setup-only (someone edited it), exit 78", + V(_list_blank) and not setuponly.applies(_list_blank, V(_list_blank))) +_ok = dict(SHIPPED, setpoint_entity="number.inv1_power") +check("a startable configuration: nothing refused, the normal start", + V(_ok) == [] and not setuponly.applies(_ok, V(_ok))) + +# --------------------------------------------------------------------------- # +print("\n2. in setup-only mode the write path refuses") +# --------------------------------------------------------------------------- # +from app import hass as H # noqa: E402 +import app.main as M # noqa: E402 + + +class Session: + """Records every request a HomeAssistant would send.""" + + def __init__(self): + self.sent = [] + + def post(self, url, **k): + self.sent.append(("POST", url)) + raise AssertionError("a request was sent") + + def get(self, url, **k): + self.sent.append(("GET", url)) + raise AssertionError("a request was sent") + + +def _refused(coro): + try: + asyncio.run(coro) + except H.ReadOnly: + return True + return False + + +_s = Session() +_ro = H.HomeAssistant(_s, "t", read_only=True) +check("a read-only HomeAssistant refuses number.set_value before it sends " + "anything", _refused(_ro.set_number("number.inv1_power", 0.0)) + and _s.sent == []) +check("...and any other service call, e.g. a notification", + _refused(_ro.call("persistent_notification", "create", + {"message": "x"})) and _s.sent == []) +try: + _ro.read_only = False + _flipped = True +except AttributeError: + _flipped = False +check("...and read-only cannot be switched off afterwards", + not _flipped and _ro.read_only is True) +_rw = H.HomeAssistant(Session(), "t") +try: + asyncio.run(_rw.set_number("number.inv1_power", 0.0)) + _sent = False +except AssertionError: + _sent = True +check("control: a normal HomeAssistant does send the write", _sent + and _rw.read_only is False) +try: + M.Controller(dict(_ok), _ro, None, None, stop=asyncio.Event()) + _built = True +except ValueError: + _built = False +check("no controller can be built on a read-only HomeAssistant", not _built) +try: + setuponly.SetupOnly(SHIPPED, _rw, None) + _took = True +except ValueError: + _took = False +check("setup-only mode refuses a HomeAssistant that can write", not _took) +_so = setuponly.SetupOnly(SHIPPED, _ro, None) +check("setup-only mode holds no inverter and never says control is started", + list(_so.inverters) == [] and _so.auto is False + and _so.status_payload()["setup_only"] is True) + +# --------------------------------------------------------------------------- # +print("\n3. the real main(): the shipped defaults serve the wizard, 0 writes") +# --------------------------------------------------------------------------- # +_runs = { + "setup": (dict(SHIPPED), None), + # A started-control attempt three ways: auto_start on, a Store that says + # control was started, and the page's own Start (the driver presses it). + "setup_started": (dict(SHIPPED, auto_start=True), {"auto": True}), +} +SAVED = None +for name, (options, st) in _runs.items(): + rc, ev, err, files = run_child(name, options, st) + services = [e for e in ev if e[0] == "SERVICE"] + print(f" {name}: exit {rc}, {len(ev)} requests logged, " + f"{len(services)} service calls") + check(f"{name}: the process served the page in setup-only mode on a " + "read-only HomeAssistant, and exited 0 when stopped", + rc == 0 and ["SERVED", "SetupOnly", True] in ev) + if rc != 0: + print(tail(err)) + check(f"{name}: 0 service calls to Home Assistant (no set_value, no " + "notification), and MQTT was never asked for", + services == [] and ["GET", "/services/mqtt"] not in ev) + _status = page(ev, "GET", "status") + check(f"{name}: /status says setup_only, with the setup banner", + _status and all(b and b.get("setup_only") is True + and b.get("mode") == "setup_only" + and b.get("banner", "").startswith("Setup") + for _c, b in _status)) + _model = page(ev, "GET", "config/model") + check(f"{name}: the page model opens the wizard (no power request) and " + "says setup-only", + _model and _model[0][1]["setup_needed"] == "no_power_request" + and _model[0][1]["setup_only"] is True) + _acts = page(ev, "POST", "action") + check(f"{name}: Start and the battery cycle are refused with 409", + len(_acts) == 2 and all(c == 409 for c, _b in _acts)) + check(f"{name}: setup-only mode writes no Store file before the wizard " + "saves, so the first-run rule still sees a new install", + [e for e in ev if e[0] == "STATE_FILE"][:2] + == [["STATE_FILE", st is not None]] * 2) + _val = page(ev, "POST", "config/validate") + check(f"{name}: the wizard's live checks run and find nothing red", + _val and _val[0][0] == 200 + and not [m for m in _val[0][1]["messages"] if m["tier"] == "bad"] + and not [m for m in _val[0][1]["setup"] if m["tier"] == "bad"]) + _save = page(ev, "POST", "config/save") + check(f"{name}: the wizard saves (200) and asks for the restart", + _save and _save[0][0] == 200 and ["RESTART"] in ev + and "saved.json" in files) + check(f"{name}: the Store gets setup_done from the save", + bool(files.get("state.json", {}).get("setup_done"))) + if name == "setup": + SAVED, SAVED_STATE = files.get("saved.json"), files.get("state.json") + +check("the saved blob passes the normal gate: no refusal, not setup-only", + SAVED is not None and V(SAVED) == [] + and not setuponly.applies(SAVED, V(SAVED)) + and SAVED.get("shadow_mode") == "on") + +print("\n after the restart, on the saved settings:") +rc, ev, err, files = run_child("after", SAVED or {}, SAVED_STATE) +services = [e for e in ev if e[0] == "SERVICE"] +writes = [e[3].get("value") for e in services + if e[1:3] == ["number", "set_value"]] +print(f" after: exit {rc}, {len(services)} service calls, set_value " + f"values {sorted(set(writes))}") +check("the next boot passes the normal gate: a Controller serves the page, " + "and it exits 0 when stopped", rc == 0 and ["SERVED", "Controller", + False] in ev) +if rc != 0: + print(tail(err)) +check("...where Start is accepted (200), unlike in setup-only mode", + [c for c, _b in page(ev, "POST", "action")] == [200]) +_status = page(ev, "GET", "status") +check("...its /status is not setup-only", _status and all( + b and "setup_only" not in b for _c, b in _status)) +_model = page(ev, "GET", "config/model") +check("...and the wizard does not open again: setup_done is in the Store", + _model and _model[0][1]["setup_needed"] is None + and _model[0][1]["setup_only"] is False) +check("...and it runs in shadow, even after Start: every service call is " + "number.set_value 0 W", writes and len(writes) == len(services) + and all(w == 0.0 for w in writes)) + +# --------------------------------------------------------------------------- # +print("\n4. a refused configuration that is not only missing setup: exit 78") +# --------------------------------------------------------------------------- # +for name, options in (("refused", _complete_bad), ("refused_blank", _blank_bad)): + rc, ev, err, _files = run_child(name, options) + print(f" {name}: exit {rc}") + check(f"{name}: exit 78 and nothing served, no service call", + rc == 78 and not [e for e in ev if e[0] in ("SERVED", "SERVICE")] + and ["ENDED_BEFORE_SERVING"] in ev) + +print() +if fails: + print(f"{len(fails)} FAILED") + sys.exit(1) +print("all passed") diff --git a/goodwe_controller/tools/ux13_wizard_blobs.py b/goodwe_controller/tools/ux13_wizard_blobs.py index 8b74ee7..d3f7d71 100644 --- a/goodwe_controller/tools/ux13_wizard_blobs.py +++ b/goodwe_controller/tools/ux13_wizard_blobs.py @@ -11,7 +11,7 @@ The wizard edits the page model one step at a time and saves it through uipage.to_options(), like the Installer view. `blobs()` walks each step's choices (meter source and sign, one to three inverters, solar rows, every contract and price source, region, peak limit, shadow mode) from a new -install and from the TEST-23 sites, and keeps what the shipped validator +install as shipped, one with a power request entity, and the TEST-23 sites, and keeps what the shipped validator accepts, because the wizard saves nothing it refuses. test_setup.py checks the set; `--supervisor` runs it through Supervisor 2026.08.0's `AppOptions` offline, as the P2 test did. It touches nothing: no network, no Supervisor. @@ -177,13 +177,14 @@ def apply(model, plan): def starts(): - """(name, stored options) the wizard opens on: a new install (config.yaml's - defaults and one power request entity, typed in the Configuration tab so - the add-on starts) and the TEST-23 sites.""" + """(name, stored options) the wizard opens on: a new install as shipped + (config.yaml's defaults, served in setup-only mode since UX-15), one with a + power request entity typed in the Configuration tab, and the TEST-23 + sites.""" sys.path.insert(0, os.path.join(HERE, "tools")) import ux13_demo as D new = dict(D.defaults(D.CONFIG), setpoint_entity="number.inv1_power") - out = [("new", new)] + out = [("empty", D.defaults(D.CONFIG)), ("new", new)] for f in sorted(glob.glob(os.path.join(HERE, "scenarios", "ux13", "test23", "*.json")))[:3]: out.append((os.path.basename(f)[:-5], dict(D.defaults(D.CONFIG), -- 2.54.0 From 63e7e8a1331629b3e2f45abf0a1f6d42d42ebba5 Mon Sep 17 00:00:00 2001 From: admin Date: Mon, 28 Sep 2026 08:54:26 +0200 Subject: [PATCH 2/4] UX-15: mutations fail named checks, not crashes Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/test_setup.py | 15 +++++++++------ goodwe_controller/test_setup_only.py | 10 ++++++++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/goodwe_controller/test_setup.py b/goodwe_controller/test_setup.py index ebcd97c..4de85cf 100644 --- a/goodwe_controller/test_setup.py +++ b/goodwe_controller/test_setup.py @@ -28,7 +28,7 @@ HERE = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, os.path.join(HERE, "tools")) logging.disable(logging.WARNING) # the validator's own log lines -from aiohttp import ClientSession, web as aioweb # noqa: E402 +from aiohttp import ClientSession, ContentTypeError, web as aioweb # noqa: E402 import translations_lint as TL # noqa: E402 import ux13_demo as D # noqa: E402 @@ -264,7 +264,10 @@ async def _get(s, url): async def _post(s, url, body): async with s.post(url, json=body) as r: - return r.status, await r.json() + try: + return r.status, await r.json() + except (ValueError, ContentTypeError): # e.g. a plain-text 500 + return r.status, {} STATES = [state("sensor.grid", "402"), state("number.inv1_power", "0"), @@ -790,18 +793,18 @@ async def _f1(s, root, sup): _r, _sup = asyncio.run(session(Controller(SITE, STATES, Store(), grid=402.0), SITE, _f1)) -print(f" empty day price: steps {_r['price']['steps']}") +print(f" empty day price: steps {_r['price'].get('steps')}") check("F1: a price the page cannot turn into options blocks step 4, beside " "its own field", - _r["price"]["steps"]["4"] >= 1 + _r["price"].get("steps", {}).get("4", 0) >= 1 and {"field": "ui.day_price", "card": None, "tier": "bad", - "text": WS_D["no_price"]["description"]} in _r["price"]["messages"]) + "text": WS_D["no_price"]["description"]} in _r["price"].get("messages", [])) check("F1: ...and the last step's live checks still run on the rest", [x["tier"] for x in _r["price"].get("setup", []) if "number.inv1_power" in x["text"]] == ["ok"] and any("402 W" in x["text"] for x in _r["price"]["setup"])) check("F1: no inverter card blocks step 2, not the last step alone", - _r["none"]["steps"]["2"] >= 1) + _r["none"].get("steps", {}).get("2", 0) >= 1) check("F1: the save refuses it (422) and says which step to go back to", _r["save"][0] == 422 and _r["save"][1]["steps"]["4"] >= 1 and _sup.writes == []) diff --git a/goodwe_controller/test_setup_only.py b/goodwe_controller/test_setup_only.py index e880155..3da2d5e 100644 --- a/goodwe_controller/test_setup_only.py +++ b/goodwe_controller/test_setup_only.py @@ -339,10 +339,14 @@ class Session: def _refused(coro): + """True only for the read-only refusal; a request that reached the + session (AssertionError there) or no refusal at all is False.""" try: asyncio.run(coro) except H.ReadOnly: return True + except Exception: # noqa: BLE001 + return False return False @@ -372,8 +376,10 @@ check("control: a normal HomeAssistant does send the write", _sent try: M.Controller(dict(_ok), _ro, None, None, stop=asyncio.Event()) _built = True -except ValueError: - _built = False +except ValueError as err: + _built = "read-only" not in str(err) +except Exception: # noqa: BLE001 + _built = True # it got past the check and failed on something else check("no controller can be built on a read-only HomeAssistant", not _built) try: setuponly.SetupOnly(SHIPPED, _rw, None) -- 2.54.0 From 30cba1e58d21a64e29716c254b9ba11fd59ae80a Mon Sep 17 00:00:00 2001 From: Claude review agent Date: Mon, 28 Sep 2026 15:06:12 +0200 Subject: [PATCH 3/4] UX-15 review: shorter setup banner (PR #147) The Home banner title was ten words; UX-08 asks for about six. Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/DOCS.md | 11 +++++------ goodwe_controller/app/messages.py | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 7493bdc..8a8712b 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -3244,9 +3244,8 @@ control is started, and whenever the stored settings name no inverter to control. A site that already ran before is never sent there. A new install names no inverter yet, so the add-on starts **in setup**: it -serves the page and sends nothing to any inverter. Home says "Setup: the -add-on controls nothing until setup is done", and there is no Start button. The -installer's steps: +serves the page and sends nothing to any inverter. Home says "Setup: nothing +is controlled yet", and there is no Start button. The installer's steps: 1. Install the add-on, turn on **Watchdog** on its Info tab, and start it. Leave the Configuration tab as it is. @@ -3276,9 +3275,9 @@ inverter stays at 0 W. If the banner is red and says a safety stop stays on, one of the red rows below is a stop that will still be there when shadow mode ends. That row says how to clear it. -In **setup** (a new install that names no inverter yet) the banner says the -add-on controls nothing until setup is done, and the one row says to finish -Setup. See *Setup, for a new install*. +In **setup** (a new install that names no inverter yet) the banner says +nothing is controlled yet, and the one row says to finish Setup. See *Setup, +for a new install*. | Row starts with | What it tells you | |---|---| diff --git a/goodwe_controller/app/messages.py b/goodwe_controller/app/messages.py index 4750030..32bb31d 100644 --- a/goodwe_controller/app/messages.py +++ b/goodwe_controller/app/messages.py @@ -385,7 +385,7 @@ def banner_stopped() -> str: def banner_setup_only() -> str: """UX-15: setup-only mode, where the add-on serves the page and commands nothing (app/setuponly.py).""" - return "Setup: the add-on controls nothing until setup is done" + return "Setup: nothing is controlled yet" def row_setup_only() -> str: -- 2.54.0 From 39e6c260fe14f2b7af7559683f3ca0b987b97f42 Mon Sep 17 00:00:00 2001 From: Claude review agent Date: Mon, 28 Sep 2026 15:15:51 +0200 Subject: [PATCH 4/4] UX-15 review: DECISIONS states the TEST-23 run that happened (PR #147) It said the PR had TEST-23 evidence, but the PR says TEST-23 was not run. Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/DECISIONS/UX-15-setup-only-mode.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/goodwe_controller/DECISIONS/UX-15-setup-only-mode.md b/goodwe_controller/DECISIONS/UX-15-setup-only-mode.md index 7265a50..d733a4c 100644 --- a/goodwe_controller/DECISIONS/UX-15-setup-only-mode.md +++ b/goodwe_controller/DECISIONS/UX-15-setup-only-mode.md @@ -87,8 +87,12 @@ rule (UX-13 §2) sends it to the wizard as before. read-only refusal removed, the Controller's check removed, `amain()` giving setup-only mode a writable HomeAssistant, `/action`'s 409 removed, and the `return` after `serve()` removed. -- TEST-23: a real container on the shipped defaults, with the Supervisor - stand-in logging every service call (the PR has the evidence). +- TEST-23 (PR #147 review, 2026-09-28): the add-on image on the shipped + defaults and an empty `/data` made 0 service calls and no MQTT request in + 3.5 minutes of setup-only mode, counted in both the stand-in and the rig + shim logs. The wizard save then passed Supervisor 2026.08.0's `AppOptions`, + and after the restart it ran in shadow: 11 of 11 writes were + `set_value 0.0`. ## 5. The wizard fixes in the same card -- 2.54.0