UX-13 P3 review: truthful peak lines, a clock-proof test, Simple stays default
checks / unit (pull_request) Successful in 11m10s
infra-pin / fresh (pull_request) Successful in 2s

- The peak limit's Why and plan block need a real power request (floor above
  0 W), not a blind or floor-limited guard. An Off battery gets the peak
  block only while it gives power. Why no longer says every battery gives.
- test_batteries_screen pins the fixture clock: unpinned it failed every
  morning. New checks cover the trade and cycle filters, the blind guard and
  an Off battery that A covers alone.
- Settings keeps only a picked view; a link to Installer is not remembered.
- Browser check: badges, unsaved change across views, locked value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
goodwe-review-agent
2026-09-28 23:20:33 +02:00
parent 2d2b6afb07
commit a9f7d78527
9 changed files with 148 additions and 26 deletions
Binary file not shown.

Before

Width:  |  Height:  |  Size: 91 KiB

After

Width:  |  Height:  |  Size: 91 KiB

@@ -93,8 +93,8 @@ What UX-02 made authoritative stays authoritative:
existing `/action` values, not options, so the table places no key for existing `/action` values, not options, so the table places no key for
them. The Batteries screen's Now, Today and Why lines are built in them. The Batteries screen's Now, Today and Why lines are built in
`homeview.py` from what the add-on knows this cycle; the plan per mode is `homeview.py` from what the add-on knows this cycle; the plan per mode is
OPT-11 P3/P4, so until then Today says "Not planned yet" OPT-11 P3/P4, so until then Today says "Not planned yet" when no block
(`test_batteries_screen.py`). is planned for the battery (`test_batteries_screen.py`).
## 4. What this costs ## 4. What this costs
+4 -3
View File
@@ -3263,9 +3263,10 @@ battery.
An Off battery that the peak limit still needs says so, with the power it 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." gives: "Off, but gives 0.5 kW for the peak limit."
**Settings** opens on the **Simple** view: the peak limit, the batteries' **Settings** opens on the **Simple** view, or on the view you last picked:
switches and modes, and shadow mode, each marked "Applies right away" or the peak limit, the batteries' switches and modes, and shadow mode, each
"Applies after a restart". **Installer** shows every setting. marked "Applies right away" or "Applies after a restart". **Installer** shows
every setting.
### Setup, for a new install ### Setup, for a new install
+11 -8
View File
@@ -380,9 +380,10 @@ def _cycle_hours(phase, soc, cap_kwh, cfg):
def peak_run(c, day0, day1, now, shadow, limit_w): def peak_run(c, day0, day1, now, shadow, limit_w):
"""While the batteries hold the grid under the peak limit, one block for """While the batteries hold the grid under the peak limit, one block for
the current 15-minute period on every battery.""" the current 15-minute period on every battery. An Off battery gets it
only while it gives power (OPT-11: Off covers only the shortfall)."""
pg = c.peak_guard pg = c.peak_guard
if pg is None or not pg.engaged: if not peak_engaged(c):
return [] return []
start = now - timedelta(minutes=now.minute % 15, seconds=now.second, start = now - timedelta(minutes=now.minute % 15, seconds=now.second,
microseconds=now.microsecond) microseconds=now.microsecond)
@@ -390,7 +391,8 @@ def peak_run(c, day0, day1, now, shadow, limit_w):
return [_run(i, "peak", start, end, day0, day1, return [_run(i, "peak", start, end, day0, day1,
run_peak_what(pg.ceiling_w or limit_w, shadow), run_peak_what(pg.ceiling_w or limit_w, shadow),
run_peak_why(), None) run_peak_why(), None)
for i in range(len(c.inverters))] for i, inv in enumerate(c.inverters)
if not _off(c, inv) or flow(inv.batt) == "giving"]
def house_runs(n, day0, day1, now, shadow, skip=()): def house_runs(n, day0, day1, now, shadow, skip=()):
@@ -526,9 +528,11 @@ def refusing(c, inv) -> int:
def peak_engaged(c) -> bool: def peak_engaged(c) -> bool:
"""The peak guard is asking the batteries for power this cycle.""" """The peak guard asks the batteries for power this cycle. Not when it is
blind or floor-limited: those are engaged with a 0 W floor (no charging
only), so no battery gives power for it."""
pg = getattr(c, "peak_guard", None) pg = getattr(c, "peak_guard", None)
return bool(pg is not None and pg.engaged) return bool(pg is not None and pg.engaged and (pg.floor_w or 0.0) > 0.0)
def battery_rows(c, shadow, running, held=False): def battery_rows(c, shadow, running, held=False):
@@ -559,7 +563,6 @@ def battery_rows(c, shadow, running, held=False):
reading=inv.soc is not None and inv.batt is not None, reading=inv.soc is not None and inv.batt is not None,
phase=phase, shadow=shadow, running=running, held=held, phase=phase, shadow=shadow, running=running, held=held,
refused=refusing(c, inv), peak=peak, refused=refusing(c, inv), peak=peak,
moving=flow(inv.batt) in ("giving", "charging"),
giving=flow(inv.batt) == "giving", other_refuses=other, giving=flow(inv.batt) == "giving", other_refuses=other,
sf_ahead=sf_ahead) sf_ahead=sf_ahead)
rows.append({ rows.append({
@@ -592,7 +595,7 @@ def battery_rows(c, shadow, running, held=False):
def battery_why(*, modes_on, mode, off, reading, phase, shadow, running, def battery_why(*, modes_on, mode, off, reading, phase, shadow, running,
held, refused, peak, moving, sf_ahead, giving=False, held, refused, peak, sf_ahead, giving=False,
other_refuses=None) -> str: other_refuses=None) -> str:
"""The key of one battery's "why" line (`home_words.why`), most urgent """The key of one battery's "why" line (`home_words.why`), most urgent
first. Pure, so every branch is tested without a controller.""" first. Pure, so every branch is tested without a controller."""
@@ -608,7 +611,7 @@ def battery_why(*, modes_on, mode, off, reading, phase, shadow, running,
return "cycle_mode" if modes_on else "cycle" return "cycle_mode" if modes_on else "cycle"
if off: if off:
# Off gives way to the peak limit only (OPT-11 design section 2). # Off gives way to the peak limit only (OPT-11 design section 2).
return "off_peak" if peak and moving else "off" return "off_peak" if peak and giving else "off"
if refused: if refused:
return "refuses_give" if refused > 0 else "refuses_charge" return "refuses_give" if refused > 0 else "refuses_charge"
if peak: if peak:
+6 -4
View File
@@ -473,9 +473,11 @@
window.UX13.batteries(batts); window.UX13.batteries(batts);
fillNotes(); fillNotes();
} }
function setMode(v, redraw) { // `remember`: only the person's own pick of a view is kept for next time;
// a jump to Installer (a link, a setting to fix) is not.
function setMode(v, redraw, remember) {
mode = v; mode = v;
try { localStorage.setItem("ux13.settings.view", v); } catch (e) { /* no storage */ } if (remember) try { localStorage.setItem("ux13.settings.view", v); } catch (e) { /* no storage */ }
document.querySelectorAll("#set-view [role=radio]").forEach((b) => { document.querySelectorAll("#set-view [role=radio]").forEach((b) => {
const on = b.dataset.v === v; const on = b.dataset.v === v;
b.setAttribute("aria-checked", String(on)); b.tabIndex = on ? 0 : -1; b.setAttribute("aria-checked", String(on)); b.tabIndex = on ? 0 : -1;
@@ -486,13 +488,13 @@
if (redraw && M && view === "settings") render(); if (redraw && M && view === "settings") render();
} }
document.querySelectorAll("#set-view [role=radio]").forEach((b, i, all) => { document.querySelectorAll("#set-view [role=radio]").forEach((b, i, all) => {
b.addEventListener("click", () => setMode(b.dataset.v, true)); b.addEventListener("click", () => setMode(b.dataset.v, true, true));
b.addEventListener("keydown", (e) => { b.addEventListener("keydown", (e) => {
const d = { ArrowRight: 1, ArrowDown: 1, ArrowLeft: -1, ArrowUp: -1 }[e.key]; const d = { ArrowRight: 1, ArrowDown: 1, ArrowLeft: -1, ArrowUp: -1 }[e.key];
if (!d) return; if (!d) return;
e.preventDefault(); e.preventDefault();
const j = all[(i + d + all.length) % all.length]; const j = all[(i + d + all.length) % all.length];
setMode(j.dataset.v, true); j.focus(); setMode(j.dataset.v, true, true); j.focus();
}); });
}); });
setMode(mode, false); setMode(mode, false);
+1 -1
View File
@@ -320,7 +320,7 @@
"off_peak": "The peak limit comes before Off. It rests again after.", "off_peak": "The peak limit comes before Off. It rests again after.",
"refuses_give": "The battery refuses to give power. The add-on tries again every 15 minutes.", "refuses_give": "The battery refuses to give power. The add-on tries again every 15 minutes.",
"refuses_charge": "The battery refuses to charge. The add-on tries again every 15 minutes.", "refuses_charge": "The battery refuses to charge. The add-on tries again every 15 minutes.",
"peak": "The peak limit needs power, so every battery gives.", "peak": "The peak limit needs power from the batteries.",
"modes_off": "Battery modes are off, so it follows the house.", "modes_off": "Battery modes are off, so it follows the house.",
"solar_first": "Solar first: it gets spare solar before the others.", "solar_first": "Solar first: it gets spare solar before the others.",
"leftover": "It gets the solar that Solar first batteries cannot take.", "leftover": "It gets the solar that Solar first batteries cannot take.",
+2 -2
View File
@@ -259,10 +259,10 @@
"cycle": "De maandelijkse cyclus houdt de meting van het laadniveau juist.", "cycle": "De maandelijkse cyclus houdt de meting van het laadniveau juist.",
"cycle_mode": "De maandelijkse cyclus gaat voor. Daarna volgt ze weer haar modus.", "cycle_mode": "De maandelijkse cyclus gaat voor. Daarna volgt ze weer haar modus.",
"off": "U hebt haar uitgezet.", "off": "U hebt haar uitgezet.",
"off_peak": "De piekgrens gaat voor Uit. Daarna rust ze weer.", "off_peak": "De piekgrens gaat voor op Uit. Daarna rust ze weer.",
"refuses_give": "De batterij weigert te leveren. De add-on probeert het elke 15 minuten opnieuw.", "refuses_give": "De batterij weigert te leveren. De add-on probeert het elke 15 minuten opnieuw.",
"refuses_charge": "De batterij weigert te laden. De add-on probeert het elke 15 minuten opnieuw.", "refuses_charge": "De batterij weigert te laden. De add-on probeert het elke 15 minuten opnieuw.",
"peak": "De piekgrens vraagt vermogen, dus elke batterij levert.", "peak": "De piekgrens vraagt vermogen van de batterijen.",
"modes_off": "Batterijmodi staan uit, dus ze volgt het huis.", "modes_off": "Batterijmodi staan uit, dus ze volgt het huis.",
"solar_first": "Eerst zonne-energie: ze krijgt het overschot vóór de andere.", "solar_first": "Eerst zonne-energie: ze krijgt het overschot vóór de andere.",
"leftover": "Ze krijgt de zonne-energie die de batterijen met Eerst zonne-energie niet opnemen.", "leftover": "Ze krijgt de zonne-energie die de batterijen met Eerst zonne-energie niet opnemen.",
+73 -6
View File
@@ -27,6 +27,7 @@ import os
import re import re
import socket import socket
import sys import sys
from datetime import datetime, timedelta, timezone
HERE = os.path.dirname(os.path.abspath(__file__)) HERE = os.path.dirname(os.path.abspath(__file__))
sys.path.insert(0, os.path.join(HERE, "tools")) sys.path.insert(0, os.path.join(HERE, "tools"))
@@ -54,13 +55,35 @@ def words(lang):
EN, NL = words("en"), words("nl") EN, NL = words("en"), words("nl")
B_NAME = D.OPTS["inverters"][1]["name"]
LANGS = {"en": EN, "nl": NL} LANGS = {"en": EN, "nl": NL}
def build(state, lang="en", c=None): # The fixture's prices are for the day it is built, so the plan depends on
# the hour. Pin the clock (review of #150: unpinned, this file failed every
# morning): the fixture is built at 00:15 on 2026-09-28, Brussels time. Part 1
# looks at 23:30, when the day's trades and cycle start are past, and part 3
# at 00:30, with all of them ahead.
_BUILT = datetime(2026, 9, 27, 22, 15, tzinfo=timezone.utc)
class _Clock(datetime):
@classmethod
def now(cls, tz=None):
return _BUILT.astimezone(tz) if tz else _BUILT.replace(tzinfo=None)
D.datetime = _Clock
_DAY0 = homeview.local_day(_BUILT)[0]
EVENING = _DAY0 + timedelta(hours=23, minutes=30)
MORNING = _DAY0 + timedelta(minutes=30)
def build(state, lang="en", c=None, now=None):
c = c or D.controller(state) c = c or D.controller(state)
s = c.status_payload() s = c.status_payload()
return homeview.build(c, level=s["level"], checks=s["checks"], lang=lang) return homeview.build(c, level=s["level"], checks=s["checks"], lang=lang,
now=now or EVENING)
# The six states and what each battery's Why line must be. # The six states and what each battery's Why line must be.
@@ -188,16 +211,60 @@ _kinds = {i: {r["kind"] for r in _p["plan"]["runs"] if r["battery"] == i}
print(f" {_kinds}") print(f" {_kinds}")
check("Off: no 'follows the house', no trade, no monthly cycle for B", check("Off: no 'follows the house', no trade, no monthly cycle for B",
not _kinds[1] and "house" in _kinds[0]) not _kinds[1] and "house" in _kinds[0])
_p = build("off", now=MORNING)
_m = build("modes", now=MORNING)
_kinds = {i: {r["kind"] for r in _p["plan"]["runs"] if r["battery"] == i}
for i in (0, 1)}
print(f" morning: {_kinds}, lines {_p['plan']['lines']}")
check("in the morning, with trades and a cycle ahead: A has them, Off B has "
"no block and waits for no cycle (with B on, it does)",
{"house", "grid", "give", "cycle"} <= _kinds[0] and not _kinds[1]
and not any(B_NAME in x for x in _p["plan"]["lines"])
and {"grid", "give"} <= {r["kind"] for r in _m["plan"]["runs"]
if r["battery"] == 1}
and any(B_NAME in x for x in _m["plan"]["lines"]))
_p = build("off_peak") _p = build("off_peak")
check("...but the peak block stays on it while the peak limit needs it", check("...but the peak block stays on it while the peak limit needs it",
{r["kind"] for r in _p["plan"]["runs"] if r["battery"] == 1} {r["kind"] for r in _p["plan"]["runs"] if r["battery"] == 1}
== {"peak"}) == {"peak"})
# Review: an Off battery that A covers alone rests, so it gets no peak block,
# and a card never says "every battery gives" beside it.
_c = D.controller("off_peak")
_c.inverters[1].batt = 0.0
_p = build("off_peak", c=_c)
_a, _b = _p["batteries"]
check("peak limit covered by A alone: Off B rests, has no peak block, and "
"A's Why does not claim every battery gives",
_b["why_key"] == "off" and _b["now"] == EN["home_words"]["now"]
["off_resting"] and not _b["today"]
and {r["kind"] for r in _p["plan"]["runs"] if r["battery"] == 0}
>= {"peak"} and _a["why_key"] == "peak"
and "every" not in _a["why"] and "elke" not in
build("off_peak", "nl", c=_c)["batteries"][0]["why"])
# Review: a blind or floor-limited guard is engaged with a 0 W floor. It
# forbids charging only, so no battery gives power for it.
_bad = []
for _kw in (dict(blind="meter gap"), dict(floor_limited=True)):
_c = D.controller("off_peak")
_c.peak_guard = homeview.peakguard.PeakDecision(
engaged=True, floor_w=0.0, ceiling_w=3500.0, ceiling_from="cap",
projected_w=None, **_kw)
_c.inverters[1].batt = 300.0 # B still going to 0 W
_p = build("off_peak", c=_c)
_a, _b = _p["batteries"]
if (_a["why_key"] == "peak" or _b["why_key"] == "off_peak"
or "peak limit" in _b["now"]
or any(r["kind"] == "peak" for r in _p["plan"]["runs"])):
_bad.append((_kw, _a["why_key"], _b["why_key"], _b["now"]))
print(f" {_bad}")
check("a blind or floor-limited peak guard (floor 0 W) draws no peak block "
"and no line says the peak limit takes power", not _bad)
# ============================================================================ # # ============================================================================ #
print("\n4. battery_why(): pure, most urgent first") print("\n4. battery_why(): pure, most urgent first")
BASE = dict(modes_on=True, mode=packmode.SOLAR_FIRST, off=False, reading=True, BASE = dict(modes_on=True, mode=packmode.SOLAR_FIRST, off=False, reading=True,
phase=IDLE, shadow=False, running=True, held=False, refused=0, phase=IDLE, shadow=False, running=True, held=False, refused=0,
peak=False, moving=False, sf_ahead=False) peak=False, giving=False, sf_ahead=False)
def why(**kw): def why(**kw):
@@ -211,9 +278,9 @@ _cases = [
(dict(shadow=True, phase=DRAIN), "shadow"), (dict(shadow=True, phase=DRAIN), "shadow"),
(dict(phase=DRAIN, off=True), "cycle_mode"), (dict(phase=DRAIN, off=True), "cycle_mode"),
(dict(phase=DRAIN, modes_on=False), "cycle"), (dict(phase=DRAIN, modes_on=False), "cycle"),
(dict(off=True, peak=True, moving=True), "off_peak"), (dict(off=True, peak=True, giving=True), "off_peak"),
(dict(off=True, peak=True, moving=False), "off"), (dict(off=True, peak=True, giving=False), "off"), # charging or resting
(dict(off=True, moving=True), "off"), (dict(off=True, giving=True), "off"),
(dict(refused=1, peak=True), "refuses_give"), (dict(refused=1, peak=True), "refuses_give"),
(dict(refused=-1), "refuses_charge"), (dict(refused=-1), "refuses_charge"),
(dict(peak=True, giving=True, other_refuses="B"), "peak"), (dict(peak=True, giving=True, other_refuses="B"), "peak"),
@@ -135,6 +135,54 @@ def actions(browser):
ctx.close() ctx.close()
def switching(browser):
# Review: an unsaved change survives the view switch both ways, the
# Installer's locked values still need "Change", and only the person's
# own pick of a view is remembered.
ctx, page, errors = open_page(browser, "modes", "#settings",
size="desktop", scheme="light", lang="en-GB")
chips = [c.locator(".chip").first.inner_text()
for c in page.locator("#simple > .card").all()]
check(f"Simple's badges: the peak limit and shadow mode need a restart, "
f"the batteries apply at once ({chips})",
chips == ["Applies after a restart", "Applies right away",
"Applies after a restart"])
num = page.locator("#simple .card input.num").first
num.fill("4.2")
num.dispatch_event("change")
page.wait_for_timeout(600)
n1 = page.locator("#sb-text").inner_text()
page.locator("#set-view [data-v=installer]").click()
page.wait_for_timeout(600)
val = page.locator("#groups [data-key=peak_cap_w] input.num").input_value()
n2 = page.locator("#sb-text").inner_text()
locked = (page.locator("#groups [data-key=peak_soc_floor] input").count(),
page.locator("#groups [data-key=peak_soc_floor] button.link")
.count())
page.locator("#set-view [data-v=simple]").click()
page.wait_for_timeout(600)
back = page.locator("#simple .card input.num").first.input_value()
n3 = page.locator("#sb-text").inner_text()
check("an unsaved peak limit survives Simple -> Installer -> Simple, and "
"the save bar counts it once", n1.startswith("1 ") and n2 == n1
and n3 == n1 and val == "4.2" and back == "4.2"
and page.locator("#savebar").is_visible())
check("...the Installer's locked value has no input until 'Change'",
locked == (0, 1))
ctx.close()
ctx, page, errors = open_page(browser, "running", "#batteries",
size="desktop", scheme="light", lang="en-GB")
page.locator("#batt-off a").click()
page.wait_for_timeout(800)
page.reload()
page.wait_for_load_state("networkidle")
page.wait_for_timeout(1200)
check("a jump to Installer from a link is not remembered: Settings "
"opens on Simple again", page.locator("#simple").is_visible()
and page.locator("#groups").is_hidden() and not errors)
ctx.close()
def modes_off(browser): def modes_off(browser):
ctx, page, errors = open_page(browser, "running", "#batteries", ctx, page, errors = open_page(browser, "running", "#batteries",
size="phone", scheme="light", lang="en-GB") size="phone", scheme="light", lang="en-GB")
@@ -172,6 +220,7 @@ def main():
sweep(b) sweep(b)
actions(b) actions(b)
month_peak(b) month_peak(b)
switching(b)
modes_off(b) modes_off(b)
b.close() b.close()
if FAILS: if FAILS: