From 21d8a944bc4892e268441aa533bb67c7902bc85b Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Tue, 13 Oct 2026 21:37:36 +0200 Subject: [PATCH 1/4] MAINT-11: start or stop the maintenance cycle per battery maint_start and maint_abort take an optional unit. With two or more batteries each Batteries card has Start cycle and Stop cycle buttons. One slot per site stays: a start while a battery cycles is refused and names it (no queue). A battery that is switched off, an unknown unit, and a stop on a battery that is not cycling are refused with a plain message. A request with no unit is the old call. The scheduler and the phase machine are unchanged. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC --- .../DECISIONS/MAINT-11-cycle-per-battery.md | 34 ++ goodwe_controller/DOCS.md | 5 + goodwe_controller/app/main.py | 41 ++ goodwe_controller/app/messages.py | 15 + goodwe_controller/app/static/app.js | 29 +- goodwe_controller/app/ui_text/en.json | 1 + goodwe_controller/app/ui_text/nl.json | 1 + goodwe_controller/app/web.py | 10 + goodwe_controller/test_maint11.py | 386 ++++++++++++++++++ 9 files changed, 521 insertions(+), 1 deletion(-) create mode 100644 goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md create mode 100644 goodwe_controller/test_maint11.py diff --git a/goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md b/goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md new file mode 100644 index 0000000..730c5b4 --- /dev/null +++ b/goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md @@ -0,0 +1,34 @@ +# MAINT-11 - start or stop the maintenance cycle per battery + +**Status: for review.** Tests: `test_maint11.py`. No option added. + +**The fault.** At two or more batteries the page had one "start a battery cycle" +button, and `_maint_button()` chose the battery. An operator could not cycle a +chosen one (inverter B, new after storage, needed its first cycle). + +## Rule + +1. `maint_start` and `maint_abort` take an optional `unit`. With no unit they + run the old code unchanged. The Batteries screen sends the unit from a + per-battery Start cycle and Stop cycle button, at two or more batteries only. +2. **A start while any battery holds the maintenance slot is refused, not + queued.** The message names the battery that is cycling. One slot per site + stays the rule, and the scheduler is untouched. +3. Also refused, with a plain message: an unknown unit, a start on a battery + that is switched off (battery modes on), and a stop on a battery that is not + cycling. Nothing changes on a refusal. +4. A stop on the cycling battery is today's abort: the cycle ends, then every + inverter goes to 0 W. + +## Why refuse and not queue + +A queued start is a cycle that begins hours later, unseen, after the operator +has moved on. A refusal is immediate and says why. The scheduler already +queues by waiting time, so nothing is lost. + +## What this does not cover + +- Today's bare button never refused a battery that is refusing to give power + (SAFETY-73); its drain waits. The per-battery start does the same. +- A start on the battery that is already cycling is refused as "busy", where + the bare button restarts its drain. The bare button is unchanged. diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 5cca6f4..3736fde 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -3413,6 +3413,11 @@ day's price runs. An Off battery that the peak limit still needs says so, with the power it gives: "Off, but gives 0.5 kW for the peak limit." +With two or more batteries, each card also has **Start cycle** and **Stop +cycle**, to run the monthly cycle on that battery now. Only one battery cycles +at a time: a start while another battery is cycling, or on a battery that is +switched off, is refused and the message names the battery. + **Settings** opens on the **Simple** view, or on the view you last picked: the peak limit, the batteries' switches and modes, and shadow mode, each marked "Applies right away" or "Applies after a restart". **Installer** shows diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 73ff21d..2b569aa 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -2777,6 +2777,43 @@ class Controller: out["sell"] = pm.sell return out + async def _maint_unit_action(self, what, unit, now) -> dict: + """MAINT-11: start or abort the maintenance cycle of ONE named battery. + + Returns `{"error", "code", "unit"}` for a refusal and changes nothing. + The site keeps its one maintenance slot: a start while any battery + holds it is refused (no queue), see DECISIONS/MAINT-11. The phase + machine is not touched beyond `force_start()` and `abort()`, the same + two calls the bare button makes. + """ + inv = next((i for i in self.inverters if i.name == unit), None) + if inv is None: + return self._maint_refusal("unit") + machine = self.maints[self.inverters.index(inv)] + if what == "maint_start": + holder = next((i for i, m in zip(self.inverters, self.maints) + if m.phase in POWER_MOVING), None) + if holder is not None: + return self._maint_refusal("busy", holder.name) + if self.modes_on and not self.pack_mode(inv).use: + return self._maint_refusal("off", inv.name) + for msg in machine.force_start(now): + self.log_event(msg, hint=messages.hint( + "maintenance", phase=machine.phase)) + return {"unit": inv.name, "phase": machine.phase} + if machine.phase not in POWER_MOVING: + return self._maint_refusal("idle", inv.name) + for msg in machine.abort(now, "operator"): + self.log_event(msg, hint=messages.hint("maintenance", + phase=machine.phase)) + await self.command_site_zero() + return {"unit": inv.name, "phase": machine.phase} + + @staticmethod + def _maint_refusal(code, unit="") -> dict: + return {"error": messages.maint_refused(code, unit), "code": code, + "unit": unit} + def mode_values(self) -> dict: """OPT-11 P2: each battery's mode and switch, for their HA state topics. Read from the store, so they are right after a restart. Empty @@ -7513,6 +7550,10 @@ class Controller: # would read as the stop button not working. await self.command_site_zero() return {"auto": self.auto} + if what in ("maint_start", "maint_abort") and unit is not None: + # MAINT-11: the operator named the battery. A request with no unit + # falls through to the branches below, unchanged. + return await self._maint_unit_action(what, unit, now) if what == "maint_start": # ⚠️ WHICH PACK, BECAUSE THERE ARE NOW SEVERAL AND THE BUTTON HAS # ONE LABEL. `_maint_button()` answers it: the pack that holds the diff --git a/goodwe_controller/app/messages.py b/goodwe_controller/app/messages.py index 5dd84ea..25523b7 100644 --- a/goodwe_controller/app/messages.py +++ b/goodwe_controller/app/messages.py @@ -1022,6 +1022,21 @@ def pack_refused(why: str) -> str: return PACK_REFUSED[why] +# MAINT-11: why a start or stop of ONE battery's cycle was refused. `{unit}` is +# the battery the sentence is about (for "busy", the one that is cycling). The +# page shows the same sentences in Dutch from app/ui_text, keyed by the code. +MAINT_REFUSED = { + "unit": "There is no battery with that name.", + "busy": "Only one battery can cycle at a time, and {unit} is cycling now.", + "off": "{unit} is switched off, so it cannot start a cycle.", + "idle": "{unit} is not running a cycle.", +} + + +def maint_refused(why: str, unit: str = "") -> str: + return MAINT_REFUSED[why].format(unit=unit) + + def hint(key: str, **kw) -> str: """The plain words for one event. `w` is formatted as watts, `sw` as watts with their sign (positive gives power, negative charges).""" diff --git a/goodwe_controller/app/static/app.js b/goodwe_controller/app/static/app.js index 06f201a..868d9db 100644 --- a/goodwe_controller/app/static/app.js +++ b/goodwe_controller/app/static/app.js @@ -336,6 +336,32 @@ finally { packBusy = false; } await refresh(true); } + // MAINT-11: start or stop ONE battery's cycle. Two or more batteries only; + // the add-on refuses a second cycle and says which battery is cycling. + async function cycleAct(what, b) { + if (packBusy) return; + packBusy = true; + try { + const r = await fetch("action", { method: "POST", headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ action: what, unit: b.name }) }); + const j = await r.json().catch(() => ({})); + const own = j.code && t("batteries.cycle." + j.code); + if (!r.ok) toast(own ? say(own.description, { name: j.unit || b.name }) : (j.error || t("toast.failed"))); + else toast(say(t("batteries.cycle.toast." + (what === "maint_start" ? "started" : "stopped")), { name: b.name })); + } catch (e) { toast(t("toast.failed")); } + finally { packBusy = false; } + await refresh(true); + } + function cycleButtons(b, i) { + const ask1 = (key, what) => () => { + const w = t("batteries.cycle." + key); + ask({ name: say(w.name, { name: b.name }), description: say(w.description, { name: b.name }), yes: w.yes }, + () => cycleAct(what, b)); + }; + return h("div", { class: "cyc", style: "display:flex;gap:8px;flex-wrap:wrap;margin-top:10px" }, + h("button", { type: "button", class: "btn", id: "bcs-" + i, onclick: ask1("start_dialog", "maint_start") }, t("batteries.cycle.start")), + h("button", { type: "button", class: "btn danger", id: "bca-" + i, onclick: ask1("abort_dialog", "maint_abort") }, t("batteries.cycle.abort"))); + } function onSwitch(b, i, where, lid) { const sw = h("button", { type: "button", class: "switch", role: "switch", id: "bsw-" + where + i, "aria-checked": String(b.use), "aria-labelledby": lid, @@ -370,7 +396,8 @@ h("dt", {}, t("batteries.level")), h("dd", {}, lvl + stored), h("dt", {}, t("batteries.now")), h("dd", {}, b.now), h("dt", {}, t("batteries.today")), h("dd", {}, todayLines(b)), - h("dt", {}, t("batteries.why")), h("dd", { class: "muted" }, b.why || "–"))); + h("dt", {}, t("batteries.why")), h("dd", { class: "muted" }, b.why || "–")), + S.batteries.length > 1 ? cycleButtons(b, i) : null); } function simpleRow(b, i) { const lid = "bsname-" + i; diff --git a/goodwe_controller/app/ui_text/en.json b/goodwe_controller/app/ui_text/en.json index 7d26926..b5f07ee 100644 --- a/goodwe_controller/app/ui_text/en.json +++ b/goodwe_controller/app/ui_text/en.json @@ -92,6 +92,7 @@ "off": {"name": "Off", "description": "Rests, and helps only when the peak limit needs it."} }, "today": "Today", "why": "Why", + "cycle": {"start": "Start cycle", "abort": "Stop cycle", "start_dialog": {"name": "Start a cycle on {name}?", "description": "{name} empties, refills and holds full, which takes a few hours.", "yes": "Start the cycle"}, "abort_dialog": {"name": "Stop the cycle on {name}?", "description": "Every inverter is set to 0 W for a moment, then control carries on.", "yes": "Stop the cycle"}, "toast": {"started": "Cycle started on {name}.", "stopped": "Cycle stopped on {name}."}, "unit": {"description": "There is no battery with that name."}, "busy": {"description": "Only one battery can cycle at a time, and {name} is cycling now."}, "off": {"description": "{name} is switched off, so it cannot start a cycle."}, "idle": {"description": "{name} is not running a cycle."}}, "modes_off": {"name": "Battery modes are off"}, "how": {"name": "How the modes work", "description": "Which battery gets solar first, and when each gives power.", "solar_first": {"description": "Solar first gets spare solar before the other batteries, and covers the house at any hour."}, diff --git a/goodwe_controller/app/ui_text/nl.json b/goodwe_controller/app/ui_text/nl.json index 69f7a42..dfddd81 100644 --- a/goodwe_controller/app/ui_text/nl.json +++ b/goodwe_controller/app/ui_text/nl.json @@ -89,6 +89,7 @@ "off": {"name": "Uit", "description": "Rust, en helpt alleen als de piekgrens haar nodig heeft."} }, "today": "Vandaag", "why": "Waarom", + "cycle": {"start": "Cyclus starten", "abort": "Cyclus stoppen", "start_dialog": {"name": "Cyclus starten op {name}?", "description": "{name} loopt leeg, laadt weer op en blijft vol, wat enkele uren duurt.", "yes": "Cyclus starten"}, "abort_dialog": {"name": "Cyclus stoppen op {name}?", "description": "Elke omvormer gaat even naar 0 W, daarna gaat de regeling verder.", "yes": "Cyclus stoppen"}, "toast": {"started": "Cyclus gestart op {name}.", "stopped": "Cyclus gestopt op {name}."}, "unit": {"description": "Er is geen batterij met die naam."}, "busy": {"description": "Er kan maar één batterij tegelijk een cyclus doen, en {name} is nu bezig."}, "off": {"description": "{name} staat uit en kan dus geen cyclus starten."}, "idle": {"description": "{name} doet nu geen cyclus."}}, "modes_off": {"name": "Batterijmodi staan uit"}, "how": {"name": "Zo werken de modi", "description": "Welke batterij eerst zonne-energie krijgt, en wanneer elke batterij levert.", "solar_first": {"description": "Eerst zonne-energie krijgt het overschot vóór de andere batterijen, en dekt het huis op elk uur."}, diff --git a/goodwe_controller/app/web.py b/goodwe_controller/app/web.py index ea953fa..f3dc2d4 100644 --- a/goodwe_controller/app/web.py +++ b/goodwe_controller/app/web.py @@ -60,6 +60,8 @@ _LOG = logging.getLogger("goodwe.web") # OPT-11: the per-battery actions, which name a battery and a value. PACK_ACTIONS = ("pack_use", "pack_mode", "pack_sell") +# MAINT-11: start or stop one battery's maintenance cycle (the unit is optional). +MAINT_ACTIONS = ("maint_start", "maint_abort") # UX-13 P1: the page lives in app/static/ as three files - index.html, # app.css, app.js - plus the page words from app/ui_text/.json (English @@ -163,6 +165,14 @@ def build_app(controller, selfcfg=None) -> web.Application: return web.json_response({"ok": False, "error": result["error"]}, status=400) + elif what in MAINT_ACTIONS and body.get("unit") is not None: + # MAINT-11: one battery's cycle. No unit = the old bare call below. + result = await controller.handle_action(what, unit=body["unit"]) + if isinstance(result, dict) and "error" in result: + return web.json_response( + {"ok": False, "error": result["error"], + "code": result["code"], "unit": result["unit"]}, + status=409 if result["code"] != "unit" else 400) else: result = await controller.handle_action(what) if result is None: diff --git a/goodwe_controller/test_maint11.py b/goodwe_controller/test_maint11.py new file mode 100644 index 0000000..726ed70 --- /dev/null +++ b/goodwe_controller/test_maint11.py @@ -0,0 +1,386 @@ +"""MAINT-11: start or stop the maintenance cycle of ONE battery. +`python3 test_maint11.py` + +Graded here, at two and at three batteries: a start on a chosen battery with the +slot free (and it is that battery, not the one the bare button would pick); a +start with the slot held, in each moving phase, refused by name; an unknown +unit; a battery switched off; a battery that refuses to give power; a stop on a +battery that is not cycling; a stop on the cycling one; a request with no unit +doing what it always did; the web layer's codes; the words in both languages; +and a roster of one, whose requests, log and writes are pinned. +See DECISIONS/MAINT-11-cycle-per-battery.md. + +Mutating what this file grades? Read MUTATION.md first (stale bytecode). +""" + +import asyncio +import json +import os +import re +import socket +import sys +from datetime import datetime, timezone + +import aiohttp +from aiohttp import web as aioweb + +from app import messages, packmode, web +from app.main import Controller +from app.maintenance import CHARGE, DRAIN, HOLD, IDLE, POWER_MOVING +from app.store import DEFAULTS + +HERE = os.path.dirname(os.path.abspath(__file__)) +fails = [] + + +def check(name, cond): + print(f" {'ok ' if cond else 'FAIL'} {name}") + if not cond: + fails.append(name) + + +class MemStore: + def __init__(self): + self.data = dict(DEFAULTS) + + def set(self, k, v): + self.data[k] = v + + def get_time(self, k): + raw = self.data.get(k) + if not raw: + return None + if isinstance(raw, datetime): + return raw + parsed = datetime.fromisoformat(raw) + return parsed if parsed.tzinfo else parsed.replace(tzinfo=timezone.utc) + + def set_time(self, k, when): + self.data[k] = when.astimezone(timezone.utc).isoformat() if when else None + + def save(self): + pass + + +class Hass: + session = None + token = "" + + def __init__(self, n): + self.numbers = {"sensor.grid": 5000.0} + self.limits_by = {} + for k in range(n): + self.numbers[f"sensor.soc_{k}"] = 80.0 + self.numbers[f"sensor.batt_{k}"] = 0.0 + self.limits_by[f"number.sp_{k}"] = (-5000.0, 5000.0, 1.0) + self.writes = [] + + async def number(self, entity, invert=False): + v = self.numbers.get(entity) + return None if v is None else (-v if invert else v) + + async def limits(self, entity): + return self.limits_by.get(entity, (None, None, None)) + + async def set_number(self, entity, value): + self.writes.append((entity, value)) + return True + + async def call(self, domain, service, data): + return True + + +class Pub: + enabled = False + + def publish(self, *a, **k): + pass + + def close(self): + pass + + +NAMES = ["inverter-a", "inverter-b", "inverter-c"] +OPTS = dict(meter_source="off", meter_entity="sensor.grid", gain=0.6, + slew_w=6000, deadband_w=15, target_grid_w=0, step_w=10, + heartbeat_s=10, stale_input_s=15, maintenance_discharge_w=2000, + maintenance_charge_w=2000) + + +def controller(n, modes=False): + opts = dict(OPTS) + opts["inverters"] = [ + dict(name=NAMES[k], setpoint_entity=f"number.sp_{k}", + soc_entity=f"sensor.soc_{k}", batt_entity=f"sensor.batt_{k}", + max_w=5000.0) for k in range(n)] + if n == 1: + del opts["inverters"] + opts.update(setpoint_entity="number.sp_0", soc_entity="sensor.soc_0", + batt_entity="sensor.batt_0", max_w=5000.0) + if modes: + opts["battery_modes_enabled"] = True + hass = Hass(n) + c = Controller(opts, hass, MemStore(), Pub(), stop=asyncio.Event()) + for inv in c.inverters: + inv.dev_min, inv.dev_max = -inv.max_w, inv.max_w + c.auto = True + return c, hass + + +def act(c, what, unit=None): + if unit is None: + return asyncio.run(c.handle_action(what)) + return asyncio.run(c.handle_action(what, unit=unit)) + + +def phases(c): + return [m.phase for m in c.maints] + + +def snapshot(c, hass): + return (phases(c), list(c.events), list(hass.writes), + json.dumps(c.store.data, sort_keys=True, default=str)) + + +def settle(c): + """Events carry a clock stamp; compare them without it.""" + return [e.split(" ", 1)[1] for e in c.events] + + +for N in (2, 3): + print(f"\n{N} batteries") + names = NAMES[:N] + last = N - 1 + + # --- start with the slot free ------------------------------------------- + c, hass = controller(N) + bare_pick = c._maint_button(datetime.now(timezone.utc)) + check("with every battery never cycled the bare button picks the first " + "(this is the card's case)", bare_pick is c.maints[0]) + r = act(c, "maint_start", names[last]) + check(f"start on {names[last]}: its cycle starts, nobody else's", + r == {"unit": names[last], "phase": DRAIN} + and phases(c) == [IDLE] * last + [DRAIN]) + check("...and the log says a forced start", + any("forced start" in e for e in c.events)) + check("...and the machine holding the slot is the named one", + c.maint_index == last) + + c, hass = controller(N) + r = act(c, "maint_start", names[0]) + check("start on the first battery works the same way", + r["unit"] == names[0] and phases(c)[0] == DRAIN + and phases(c)[1:] == [IDLE] * last) + + # --- start with the slot held -------------------------------------------- + for phase in POWER_MOVING: + for holder in range(N): + c, hass = controller(N) + c.maints[holder].store.set("phase", phase) + before = snapshot(c, hass) + bad = [] + for k in range(N): + r = act(c, "maint_start", names[k]) + bad.append(not (r.get("code") == "busy" + and r["unit"] == names[holder] + and names[holder] in r["error"] + and "error" in r)) + check(f"slot held by {names[holder]} in {phase}: a start on any " + f"battery (itself too) is refused, naming {names[holder]}, " + f"and nothing changes", not any(bad) + and snapshot(c, hass) == before) + + # --- unknown unit -------------------------------------------------------- + c, hass = controller(N) + before = snapshot(c, hass) + rs = [act(c, w, u) for w in ("maint_start", "maint_abort") + for u in ("inverter-z", "", "Inverter-A", 7, ["inverter-a"])] + check("an unknown unit (wrong name, empty, wrong case, not a string) is " + "refused for start and for stop, and nothing changes", + all(r.get("code") == "unit" and r["error"] + == messages.maint_refused("unit") for r in rs) + and snapshot(c, hass) == before) + + # --- a battery switched off --------------------------------------------- + c, hass = controller(N, modes=True) + c.inverters[last].store_set(packmode.USE_KEY, False) + before = snapshot(c, hass) + r = act(c, "maint_start", names[last]) + check("a battery switched off (battery modes on) is refused by name and " + "nothing changes", r.get("code") == "off" and r["unit"] == names[last] + and names[last] in r["error"] and snapshot(c, hass) == before) + r = act(c, "maint_start", names[0]) + check("...while a battery that is on still starts", r.get("phase") == DRAIN + and phases(c)[0] == DRAIN) + c, hass = controller(N) + c.inverters[last].store_set(packmode.USE_KEY, False) + check("with battery modes off the stored switch is not consulted (it is " + "not shown, so it cannot refuse)", + act(c, "maint_start", names[last]).get("phase") == DRAIN) + + # --- a battery that refuses to give power --------------------------------- + c, hass = controller(N) + c.inverters[last].refused = 1 + r = act(c, "maint_start", names[last]) + check("a battery that refuses to give power starts like the bare button " + "would start it (its drain then waits, as before)", + r == {"unit": names[last], "phase": DRAIN}) + + # --- stop --------------------------------------------------------------- + c, hass = controller(N) + before = snapshot(c, hass) + r = act(c, "maint_abort", names[0]) + check("a stop on a battery that is not cycling is refused and nothing " + "changes (no 0 W written either)", r.get("code") == "idle" + and names[0] in r["error"] and snapshot(c, hass) == before) + c.maints[last].store.set("phase", CHARGE) + before = snapshot(c, hass) + r = act(c, "maint_abort", names[0]) + check(f"a stop on {names[0]} while {names[last]} cycles is refused, and " + f"{names[last]}'s cycle is not touched", r.get("code") == "idle" + and phases(c)[last] == CHARGE and snapshot(c, hass) == before) + + for phase in POWER_MOVING: + c, hass = controller(N) + c.maints[last].store.set("phase", phase) + r = act(c, "maint_abort", names[last]) + check(f"a stop on the cycling battery ({phase}) ends its cycle, " + f"logs the abort and sets every inverter to 0 W", + r == {"unit": names[last], "phase": IDLE} + and phases(c) == [IDLE] * N + and any("ABORT (operator)" in e for e in c.events) + and {w for w in hass.writes} + == {(f"number.sp_{k}", 0.0) for k in range(N)} + and all(i.target == 0.0 for i in c.inverters)) + + # --- the old request --------------------------------------------------- + c, hass = controller(N) + r = act(c, "maint_start") + check("a start with no unit is the old call: the bare button's pick, and " + "the old answer", r == {"phase": DRAIN} and phases(c)[0] == DRAIN + and any("forced start" in e for e in c.events)) + r = act(c, "maint_start") + check("...a second bare start re-enters the drain of the cycling battery, " + "as it always did", r == {"phase": DRAIN} and phases(c)[0] == DRAIN) + c.maints[last].store.set("phase", IDLE) + r = act(c, "maint_abort") + check("...and a bare stop stops the cycling battery and answers as before", + r == {"phase": IDLE} and phases(c) == [IDLE] * N + and any("ABORT (operator)" in e for e in c.events)) + c, hass = controller(N) + r = act(c, "maint_abort") + check("...a bare stop with nothing cycling is still accepted, as before", + r == {"phase": IDLE} and hass.writes + and {w[1] for w in hass.writes} == {0.0}) + +# --- the web layer ---------------------------------------------------------- +print("\nthe web layer") + + +async def serve(c, calls): + runner = aioweb.AppRunner(web.build_app(c, object())) + await runner.setup() + sock = socket.socket() + sock.bind(("127.0.0.1", 0)) + port = sock.getsockname()[1] + sock.close() + await aioweb.TCPSite(runner, "127.0.0.1", port).start() + out = [] + try: + async with aiohttp.ClientSession() as s: + for body in calls: + async with s.post(f"http://127.0.0.1:{port}/action", + json=body) as r: + out.append((r.status, await r.json(content_type=None))) + finally: + await runner.cleanup() + return out + + +c, hass = controller(2) +rs = asyncio.run(serve(c, [ + {"action": "maint_start", "unit": "inverter-b"}, + {"action": "maint_start", "unit": "inverter-a"}, + {"action": "maint_abort", "unit": "inverter-a"}, + {"action": "maint_start", "unit": "nope"}, + {"action": "maint_abort", "unit": "inverter-b"}, + {"action": "maint_start", "unit": None}, + {"action": "maint_abort"}, +])) +check("start with a unit: 200 and the unit's answer", + rs[0] == (200, {"ok": True, "result": {"unit": "inverter-b", + "phase": DRAIN}})) +check("start while the slot is held: 409, a code, the cycling battery, the " + "plain sentence", rs[1][0] == 409 and rs[1][1] == { + "ok": False, "code": "busy", "unit": "inverter-b", + "error": messages.maint_refused("busy", "inverter-b")}) +check("stop on a battery that is not cycling: 409 with the sentence", + rs[2][0] == 409 and rs[2][1]["code"] == "idle" + and rs[2][1]["error"] == messages.maint_refused("idle", "inverter-a")) +check("unknown unit: 400 with the sentence", rs[3][0] == 400 + and rs[3][1]["code"] == "unit" + and rs[3][1]["error"] == messages.maint_refused("unit")) +check("stop on the cycling battery: 200", rs[4] == ( + 200, {"ok": True, "result": {"unit": "inverter-b", "phase": IDLE}})) +check("a null unit is the bare call: 200 and the old answer", + rs[5] == (200, {"ok": True, "result": {"phase": DRAIN}})) +check("a request with no unit: 200 and the old answer", rs[6] == ( + 200, {"ok": True, "result": {"phase": IDLE}})) + +# --- the words -------------------------------------------------------------- +print("\nthe words") +words = {} +for lang in ("en", "nl"): + with open(os.path.join(HERE, "app", "ui_text", f"{lang}.json"), + encoding="utf-8") as f: + words[lang] = json.load(f)["home"]["batteries"]["cycle"] +for code, sentence in messages.MAINT_REFUSED.items(): + en = words["en"][code]["description"] + check(f"'{code}': the page's English is the API's sentence", + en == sentence.replace("{unit}", "{name}")) + nl = words["nl"][code]["description"] + check(f"'{code}': the Dutch has the same placeholders", + sorted(re.findall(r"\{\w+\}", nl)) + == sorted(re.findall(r"\{\w+\}", en)) and nl != en) +check("both languages carry the same keys", + json.dumps(words["en"], sort_keys=True).count('":') + == json.dumps(words["nl"], sort_keys=True).count('":') + and set(words["en"]) == set(words["nl"])) +check("every code the server can answer has a page sentence", + set(messages.MAINT_REFUSED) <= set(words["en"])) + +# --- the page ---------------------------------------------------------------- +print("\nthe page") +JS = open(os.path.join(HERE, "app", "static", "app.js"), + encoding="utf-8").read() +check("the buttons are drawn only with two or more batteries", + "S.batteries.length > 1 ? cycleButtons(b, i) : null" in JS) +check("both buttons post the battery's name and ask first", + 'JSON.stringify({ action: what, unit: b.name }) });' in JS and 'ask1("start_dialog", "maint_start")' in JS + and 'ask1("abort_dialog", "maint_abort")' in JS) +check("the old header buttons still post a bare request", + 'ask("maint_start", () => simple("maint_start"))' in JS + and "body: JSON.stringify({ action: what }) });" in JS) + +# --- a roster of one ----------------------------------------------------------- +print("\none battery: the requests, the log and the wire stay as they were") +c, hass = controller(1) +r1 = act(c, "maint_start") +r2 = act(c, "maint_start") +r3 = act(c, "maint_abort") +r4 = act(c, "maint_abort") +check("answers", [r1, r2, r3, r4] == [{"phase": DRAIN}, {"phase": DRAIN}, + {"phase": IDLE}, {"phase": IDLE}]) +check("log lines (stamp removed), newest first", settle(c) == [ + "maintenance: ABORT (operator)", "maintenance: ABORT (operator)", + "maintenance: forced start -> drain", + "maintenance: forced start -> drain"][:len(settle(c))] + and len(settle(c)) == 4) +check("wire: two stops, each 0 W", hass.writes == [("number.sp_0", 0.0)] * 2) +c, hass = controller(1) + +print() +if fails: + print(f"{len(fails)} FAILED") + sys.exit(1) +print("all passed") -- 2.54.0 From fd059126cbea62539488bb8546f676578104fa7b Mon Sep 17 00:00:00 2001 From: goodwe-review Date: Tue, 13 Oct 2026 23:06:47 +0200 Subject: [PATCH 2/4] MAINT-11 review: test an empty unit at the web layer and the page's own refusal words; drop a stale comment Two reviewer mutations survived the suite: the web layer routing only a truthy unit (an empty unit then started the bare pick), and the page showing the server's English instead of its own sentence by code. Both are now checked. The bare maint_start comment that said a per-battery unit would ship half a surface is replaced by a pointer to _maint_unit_action(). No behaviour change. Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/app/main.py | 6 ++---- goodwe_controller/test_maint11.py | 7 +++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 2b569aa..cb71db6 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -7564,10 +7564,8 @@ class Controller: # picked on its own. At N=1 both branches are `maints[0]` and this # is the shipped call, byte for byte. # - # A PER-PACK BUTTON IS A UI CARD, NOT THIS ONE. `web.py` posts a - # bare `maint_start` with no unit, and inventing a unit parameter - # here would ship half a surface: the page would still have one - # button and the API would have two meanings. + # This is the bare request (no unit). A request naming a battery + # was handled above by `_maint_unit_action()` (MAINT-11). _m = self._maint_button(now) for msg in _m.force_start(now): self.log_event(msg, hint=messages.hint("maintenance", diff --git a/goodwe_controller/test_maint11.py b/goodwe_controller/test_maint11.py index 726ed70..102c420 100644 --- a/goodwe_controller/test_maint11.py +++ b/goodwe_controller/test_maint11.py @@ -306,6 +306,7 @@ rs = asyncio.run(serve(c, [ {"action": "maint_abort", "unit": "inverter-b"}, {"action": "maint_start", "unit": None}, {"action": "maint_abort"}, + {"action": "maint_start", "unit": ""}, ])) check("start with a unit: 200 and the unit's answer", rs[0] == (200, {"ok": True, "result": {"unit": "inverter-b", @@ -326,6 +327,9 @@ check("a null unit is the bare call: 200 and the old answer", rs[5] == (200, {"ok": True, "result": {"phase": DRAIN}})) check("a request with no unit: 200 and the old answer", rs[6] == ( 200, {"ok": True, "result": {"phase": IDLE}})) +check("an empty unit is refused (400), not taken as the bare call", + rs[7][0] == 400 and rs[7][1]["code"] == "unit" + and phases(c) == [IDLE, IDLE]) # --- the words -------------------------------------------------------------- print("\nthe words") @@ -358,6 +362,9 @@ check("the buttons are drawn only with two or more batteries", check("both buttons post the battery's name and ask first", 'JSON.stringify({ action: what, unit: b.name }) });' in JS and 'ask1("start_dialog", "maint_start")' in JS and 'ask1("abort_dialog", "maint_abort")' in JS) +check("a refusal is shown in the page's language, by its code", + 'const own = j.code && t("batteries.cycle." + j.code);' in JS + and 'toast(own ? say(own.description' in JS) check("the old header buttons still post a bare request", 'ask("maint_start", () => simple("maint_start"))' in JS and "body: JSON.stringify({ action: what }) });" in JS) -- 2.54.0 From 5ee10613982d36fc1a6cbb3c0fe7d5f1300d8dd1 Mon Sep 17 00:00:00 2001 From: glenn schrooyen Date: Tue, 13 Oct 2026 23:18:21 +0200 Subject: [PATCH 3/4] MAINT-11 round 2: disable the cycle buttons that cannot work Stop cycle is disabled on a battery that is not cycling. Start cycle is disabled while any battery cycles and on a switched-off battery. Page only; the add-on still refuses each of these. DECISIONS notes the one-battery case. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01Bdqr7PiaHcBnzPituu2yTC --- .../DECISIONS/MAINT-11-cycle-per-battery.md | 3 +++ goodwe_controller/app/static/app.js | 11 +++++++++-- goodwe_controller/test_maint11.py | 9 +++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md b/goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md index 730c5b4..e10dfc5 100644 --- a/goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md +++ b/goodwe_controller/DECISIONS/MAINT-11-cycle-per-battery.md @@ -32,3 +32,6 @@ queues by waiting time, so nothing is lost. (SAFETY-73); its drain waits. The per-battery start does the same. - A start on the battery that is already cycling is refused as "busy", where the bare button restarts its drain. The bare button is unchanged. +- At one battery, a request that names the battery uses the new path, so a + second start is refused instead of restarting the drain. Nothing sends a name + at one battery, and the bare request is unchanged. diff --git a/goodwe_controller/app/static/app.js b/goodwe_controller/app/static/app.js index 868d9db..0897f15 100644 --- a/goodwe_controller/app/static/app.js +++ b/goodwe_controller/app/static/app.js @@ -352,15 +352,22 @@ finally { packBusy = false; } await refresh(true); } + // A button that cannot work is disabled (the add-on still refuses it): Stop + // needs this battery cycling; Start needs no battery cycling and this one on. + function cycleState(b) { + const cycling = (x) => x.maintenance !== "idle"; + return { start: !S.batteries.some(cycling) && b.use !== false, abort: cycling(b) }; + } function cycleButtons(b, i) { + const can = cycleState(b); const ask1 = (key, what) => () => { const w = t("batteries.cycle." + key); ask({ name: say(w.name, { name: b.name }), description: say(w.description, { name: b.name }), yes: w.yes }, () => cycleAct(what, b)); }; return h("div", { class: "cyc", style: "display:flex;gap:8px;flex-wrap:wrap;margin-top:10px" }, - h("button", { type: "button", class: "btn", id: "bcs-" + i, onclick: ask1("start_dialog", "maint_start") }, t("batteries.cycle.start")), - h("button", { type: "button", class: "btn danger", id: "bca-" + i, onclick: ask1("abort_dialog", "maint_abort") }, t("batteries.cycle.abort"))); + h("button", { type: "button", class: "btn", id: "bcs-" + i, disabled: !can.start, onclick: ask1("start_dialog", "maint_start") }, t("batteries.cycle.start")), + h("button", { type: "button", class: "btn danger", id: "bca-" + i, disabled: !can.abort, onclick: ask1("abort_dialog", "maint_abort") }, t("batteries.cycle.abort"))); } function onSwitch(b, i, where, lid) { const sw = h("button", { type: "button", class: "switch", role: "switch", id: "bsw-" + where + i, diff --git a/goodwe_controller/test_maint11.py b/goodwe_controller/test_maint11.py index 102c420..8451135 100644 --- a/goodwe_controller/test_maint11.py +++ b/goodwe_controller/test_maint11.py @@ -365,6 +365,15 @@ check("both buttons post the battery's name and ask first", check("a refusal is shown in the page's language, by its code", 'const own = j.code && t("batteries.cycle." + j.code);' in JS and 'toast(own ? say(own.description' in JS) +check("Stop is disabled on a battery that is not cycling", + 'abort: cycling(b) };' in JS and "disabled: !can.abort" in JS) +check("Start is disabled while any battery cycles", + "start: !S.batteries.some(cycling) && " in JS + and "disabled: !can.start" in JS) +check("Start is disabled on a switched-off battery (use false, modes on)", + "&& b.use !== false, abort" in JS) +check("a battery counts as cycling when its phase is not idle", + 'cycling = (x) => x.maintenance !== "idle"' in JS) check("the old header buttons still post a bare request", 'ask("maint_start", () => simple("maint_start"))' in JS and "body: JSON.stringify({ action: what }) });" in JS) -- 2.54.0 From 6aa1c32cbbe3e78d197349fcbfd1d41b05bea5d0 Mon Sep 17 00:00:00 2001 From: goodwe-review Date: Wed, 14 Oct 2026 00:39:37 +0200 Subject: [PATCH 4/4] MAINT-11 review round 2: pin each cycle button to its own disabled flag Reviewer mutations survived the round-2 page checks: Start forced always disabled (|| true), and the two flags swapped between Start and Stop, because the checks only looked for each flag somewhere in the file. The check now pins each flag to its own button. No behaviour change. Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/test_maint11.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/goodwe_controller/test_maint11.py b/goodwe_controller/test_maint11.py index 8451135..3a27fec 100644 --- a/goodwe_controller/test_maint11.py +++ b/goodwe_controller/test_maint11.py @@ -372,6 +372,9 @@ check("Start is disabled while any battery cycles", and "disabled: !can.start" in JS) check("Start is disabled on a switched-off battery (use false, modes on)", "&& b.use !== false, abort" in JS) +check("each button carries its own flag, and only that flag", + 'id: "bcs-" + i, disabled: !can.start, onclick: ask1("start_dialog"' in JS + and 'id: "bca-" + i, disabled: !can.abort, onclick: ask1("abort_dialog"' in JS) check("a battery counts as cycling when its phase is not idle", 'cycling = (x) => x.maintenance !== "idle"' in JS) check("the old header buttons still post a bare request", -- 2.54.0