From fad73d4012a76c86d1c60a39461702805eb1f4c2 Mon Sep 17 00:00:00 2001 From: goodwe-review Date: Mon, 28 Sep 2026 01:18:33 +0200 Subject: [PATCH] UX-13 P1 review: Home draws only what the cycle would do - Trade blocks: none on a battery with no room, no sale at a negative price or while the emptiest battery is at arbitrage's floor, nothing when a level is unknown. A give after a charge may use what the charge put in. - The refusal line names the real reason: prices not published vs gap. - The plan's peak line uses the guard's ceiling (this month's peak once above the cap), as the peak card does. - House below zero: 0 W inside the idle band, no figure beyond it. - Price bars are "Low price" / "High price": the add-on trades in one hour. - Control toggle: one request in flight at a time. - test_home: the new cases; the house-sign mutation now goes red. Co-Authored-By: Claude Opus 5.5 --- goodwe_controller/app/homeview.py | 85 ++++++++++++++++++++++----- goodwe_controller/app/static/app.js | 6 ++ goodwe_controller/app/ui_text/en.json | 8 +-- goodwe_controller/test_home.py | 75 +++++++++++++++++++++-- 4 files changed, 149 insertions(+), 25 deletions(-) diff --git a/goodwe_controller/app/homeview.py b/goodwe_controller/app/homeview.py index c4e56b7..4e58505 100644 --- a/goodwe_controller/app/homeview.py +++ b/goodwe_controller/app/homeview.py @@ -85,9 +85,16 @@ def flow(w, *, shadow=False): def now_block(*, grid_w, pv_w, batt_w, has_solar, shadow): """The four tiles. `house_w` needs all three readings, so without a solar reading it is None rather than a number that leaves the sun out.""" - house = None + house, disagree = None, False if has_solar and None not in (grid_w, pv_w, batt_w): house = float(grid_w) + float(pv_w) + float(batt_w) + # The three are read at slightly different moments, so the sum can dip + # under zero. Inside the idle band that is 0 W; beyond it the readings + # disagree and no house figure is shown. + if house < -IDLE_W: + house, disagree = None, True + elif house < 0.0: + house = 0.0 return { "solar_w": None if pv_w is None else float(pv_w), "house_w": house, @@ -96,7 +103,7 @@ def now_block(*, grid_w, pv_w, batt_w, has_solar, shadow): "has_solar": bool(has_solar), "lines": { "solar": tile_solar(pv_w, has_solar), - "house": tile_house(house, has_solar), + "house": tile_house(house, has_solar, disagree), "grid": tile_grid(grid_w), "batt": tile_batt(batt_w, shadow), }, @@ -156,7 +163,9 @@ class DayPrices: def hour_bars(points, day0, day1): """One bar per local hour: the mean of its prices, or None when unknown. - `kind` is cheap / dear / mid by thirds of today's range (display only).""" + `kind` is cheap / dear / mid by thirds of today's range (display only). + The page calls these "low price" and "high price", not cheap hours: the + add-on trades in the one cheapest and one dearest hour, not in a third.""" n = int(round(_hours(day1, day0))) bars = [] for k in range(n): @@ -221,7 +230,19 @@ def trade_verdict(c, plan): uncertainty=frozenset({Uncertainty.NO_WEATHER})) return verdict.match( allowed=lambda v: (v, "allowed"), - refused=lambda _v: (None, "peak" if guard else "not_worth")) + refused=lambda _v: (None, refusal_key(plan, guard))) + + +def refusal_key(plan, guard): + """Which of `TariffPlan.arbitrage()`'s refusals fired, in its own order: + the peak guard, no prices, under two published points, then the gap.""" + if guard: + return "peak" + if not plan.has_prices: + return "no_prices" + if sum(1 for p in _points(plan) if not p.provisional) < 2: + return "not_firm" + return "not_worth" def _point_end(plan, point): @@ -231,26 +252,47 @@ def _point_end(plan, point): return point.start + timedelta(hours=1) +# Under this many kWh a block is not drawn: the cycle would not move it. +MIN_KWH = 0.05 + + def trade_runs(c, plan, verdict, day0, day1, now, shadow): """A charge block and a give block on each battery, where the verdict put them. The site trades as one; each battery's share is capped by its own - rating and by the room its level leaves (arbitrage's own floor/ceiling).""" + rating and by the room its level leaves (arbitrage's own floor/ceiling). + Only what `arbitrage.decide()` would still do is drawn: no block on a + battery with no room, no sale at a negative price, and no sale while the + emptiest battery is at the floor (the cycle's `soc` is the emptiest). + With a level unknown the cycle does not trade, so nothing is drawn.""" floor, ceiling = arbitrage.soc_bounds(c.o) + if any(inv.soc is None for inv in c.inverters): + return [], [], "no_level" + level = [float(inv.soc) for inv in c.inverters] + legs = sorted((("grid", verdict.cheapest), ("give", verdict.dearest)), + key=lambda kp: kp[1].start) runs, later = [], [] - for kind, pt in (("grid", verdict.cheapest), ("give", verdict.dearest)): + for kind, pt in legs: end = _point_end(plan, pt) if end <= now: continue if not (day0 <= pt.start < day1): later.append((kind, pt)) continue + if kind == "give" and (pt.eur_per_kwh < 0 or min(level) <= floor): + continue hrs = (end - pt.start).total_seconds() / 3600.0 for i, inv in enumerate(c.inverters): kwh = inv.max_w / 1000.0 * hrs - if inv.capacity_kwh and inv.soc is not None: - room = (ceiling - inv.soc if kind == "grid" - else inv.soc - floor) - kwh = min(kwh, max(0.0, room) / 100.0 * inv.capacity_kwh) + cap = inv.capacity_kwh + if cap: + room = (ceiling - level[i] if kind == "grid" + else level[i] - floor) + kwh = min(kwh, max(0.0, room) / 100.0 * cap) + # A later give may use what this charge put in: an upper + # bound, like every amount here ("up to"). + level[i] += (kwh if kind == "grid" else -kwh) / cap * 100.0 + if kwh < MIN_KWH: + continue eur = kwh * pt.eur_per_kwh if kind == "grid": what, why, amount = (run_grid_what(kwh, shadow), @@ -262,7 +304,7 @@ def trade_runs(c, plan, verdict, day0, day1, now, shadow): run_give_amount(eur)) runs.append(_run(i, kind, pt.start, end, day0, day1, what, why, amount, kwh=kwh, eur=eur, price=pt.eur_per_kwh)) - return runs, later + return runs, later, None def maintenance_runs(c, day0, day1, now, shadow): @@ -387,9 +429,11 @@ def build_plan(c, now, shadow, running, held=False): if running: runs += house_runs(len(c.inverters), day0, day1, now, shadow) if verdict is not None: - trade, later = trade_runs(c, fresh, verdict, day0, day1, now, - shadow) + trade, later, no_trade = trade_runs(c, fresh, verdict, day0, day1, + now, shadow) runs += trade + if no_trade: + lines.append(line_no_trade(no_trade)) for kind, pt in later: lines.append(line_trade_tomorrow(kind, hhmm(pt.start), pt.eur_per_kwh, shadow)) @@ -400,7 +444,10 @@ def build_plan(c, now, shadow, running, held=False): lines += waits runs += peak_run(c, day0, day1, now, shadow, limit_w) if c.p1_enabled: - lines.append(line_peak_limit(limit_w, shadow)) + # The guard's ceiling: the cap, or this month's peak once above it. + pg = c.peak_guard + lines.append(line_peak_limit( + (pg.ceiling_w if pg is not None else None) or limit_w, shadow)) lines = [x for x in lines if x] kinds = {r["kind"] for r in runs} @@ -663,9 +710,11 @@ def tile_solar(pv_w, has_solar) -> str: return "Producing" if pv_w > IDLE_W else "Not producing" -def tile_house(house_w, has_solar) -> str: +def tile_house(house_w, has_solar, disagree=False) -> str: if not has_solar: return "Needs a solar reading" + if disagree: + return "Readings do not add up" return "No reading" if house_w is None else "Using" @@ -805,7 +854,9 @@ def line_no_trade(key) -> str: "peak.", "no_prices": "No trading on price yet: no prices have arrived.", "peak": "No trading on price now: the peak limit comes first.", + "not_firm": "No trading on price yet: the prices are not published.", "not_worth": "No trading on price today. The price gap is too small.", + "no_level": "No trading on price now: a battery level is unknown.", }[key] @@ -884,6 +935,7 @@ def text_samples() -> list: ((None, False), (None, True), (3900, True), (0, True))] out += [("title", tile_house(v, s)) for v, s in ((None, False), (None, True), (1100, True))] + out.append(("title", tile_house(None, True, True))) out += [("title", tile_grid(v)) for v in (None, 1200, -800, 5)] out += [("title", tile_batt(v, sh)) for v in (None, 2500, -2500, 0)] for phase in (M_IDLE, M_DRAIN, M_CHARGE, M_HOLD): @@ -928,5 +980,6 @@ def text_samples() -> list: ("plan", line_region("own", None)), ("plan", line_peak_not_covered())] out += [("plan", line_no_trade(k)) for k in - ("not_covered", "no_prices", "peak", "not_worth")] + ("not_covered", "no_prices", "peak", "not_firm", "not_worth", + "no_level")] return out diff --git a/goodwe_controller/app/static/app.js b/goodwe_controller/app/static/app.js index 01f9d0a..fb9810a 100644 --- a/goodwe_controller/app/static/app.js +++ b/goodwe_controller/app/static/app.js @@ -309,12 +309,18 @@ /* ---------------- control buttons ---------------- */ // The action is a toggle, so the page reads the live state first and sends // nothing when control is already where the person asked it to be. + // One request at a time: a second confirm while the first is in flight + // would read the same state and toggle it back. + let toggling = false; async function setControl(want) { + if (toggling) return; + toggling = true; try { const now = await getStatus(); if (!now.control || now.control.running !== want) await act("auto_toggle"); toast(want ? t("toast.started") : t("toast.stopped")); } catch (e) { toast(t("toast.failed")); } + finally { toggling = false; } refresh(); } $("#btn-ctl").addEventListener("click", () => { diff --git a/goodwe_controller/app/ui_text/en.json b/goodwe_controller/app/ui_text/en.json index 75d3b0c..6170326 100644 --- a/goodwe_controller/app/ui_text/en.json +++ b/goodwe_controller/app/ui_text/en.json @@ -42,8 +42,8 @@ "plan": { "name": "Today's plan", "description": "Point at a bar or a block for details, or tap it on a phone.", - "cheap": "Cheap hour", - "dear": "Expensive hour", + "cheap": "Low price", + "dear": "High price", "estimate": "Estimate", "grid": "Charges from the grid", "give": "Gives power", @@ -53,8 +53,8 @@ "now": "now", "time_to": "to", "per_kwh": "per kWh", - "cheap_tip": "a cheap hour", - "dear_tip": "an expensive hour", + "cheap_tip": "low for today", + "dear_tip": "high for today", "estimate_tip": "estimate from yesterday", "none_tip": "No price known", "no_prices": {"description": "No prices for today, so the chart shows only the plan."}, diff --git a/goodwe_controller/test_home.py b/goodwe_controller/test_home.py index 395e316..40e21c2 100644 --- a/goodwe_controller/test_home.py +++ b/goodwe_controller/test_home.py @@ -184,6 +184,19 @@ check("`now` carries the four tiles in W and a plain line for each", <= set(s1["now"]) and set(s1["now"]["lines"]) == {"solar", "house", "grid", "batt"} and s1["now"]["grid_w"] == 800.0 and s1["now"]["batt_w"] == 960.0) +_nh = homeview.now_block(grid_w=-3000.0, pv_w=1000.0, batt_w=1500.0, + has_solar=True, shadow=False) +_nz = homeview.now_block(grid_w=-1030.0, pv_w=1000.0, batt_w=0.0, + has_solar=True, shadow=False) +_np = homeview.now_block(grid_w=800.0, pv_w=1000.0, batt_w=-300.0, + has_solar=True, shadow=False) +check("House = grid (+ import) + solar + battery (+ giving): 800 + 1000 - 300 " + "= 1500 W", + _np["house_w"] == 1500.0 and _np["lines"]["house"] == "Using") +check("...a sum below zero is not shown as a positive figure: 0 W inside " + "the idle band, and no figure beyond it", + _nz["house_w"] == 0.0 and _nh["house_w"] is None + and _nh["lines"]["house"] == homeview.tile_house(None, True, True)) check("...and with no solar reading the house figure is NOT invented: it is " "None and the tile says why", s1["now"]["house_w"] is None and s1["now"]["has_solar"] is False @@ -254,12 +267,53 @@ check("each block carries kWh and €: charging is capped by the battery's own " and [r["eur_kwh"] for r in _grid] == [0.14, 0.14] and "2.5 kWh" in _grid[0]["what"] and "€0.14" in _grid[0]["why"] and "€0.35" in _grid[0]["amount"]) -# Giving: A holds 62 - 20 = 42 % of 9.6 = 4.03 -> 2.5. B holds 35 - 20 = 15 % -# of 5.4 = 0.81 kWh, which is what it can give above arbitrage's floor. +# Giving at 19:00 comes after the 13:00 charge, so each battery may give what +# it will hold then: B is at 35 % + 2.5 kWh of 5.4 = 81 %, and 81 - 20 = 61 % +# of 5.4 = 3.3 kWh, so 2.5 (an upper bound, like every amount here). check("...and giving is capped by what each battery holds above arbitrage's " - "floor (Battery B: 15 % of 5.4 kWh)", - [r["kwh"] for r in _give] == [2.5, 0.81] - and [r["eur"] for r in _give] == [0.9, 0.29]) + "floor after the charge before it", + [r["kwh"] for r in _give] == [2.5, 2.5] + and [r["eur"] for r in _give] == [0.9, 0.9]) + + +def trade(soc, prices=MOCK, provisional_from=None, now=NOW, opts=ARB): + c = make(opts, soc=soc) + c.tariff_plan = provider_plan(prices, provisional_from=provisional_from) + c.tariff_at, c.peak_guard = now, quiet_guard() + s = status(c, now=now) + return ([(r["kind"], r["battery"], r["kwh"]) for r in s["plan"]["runs"] + if r["kind"] in ("grid", "give")], s["plan"]["lines"]) + + +# Review of #144: only what arbitrage.decide() would still do is drawn. +_t, _l = trade((96.0, 35.0)) +check("a battery at arbitrage's ceiling gets no charge block, not " + "'Charges up to 0.0 kWh'", ("grid", 0, 0.0) not in _t + and [k for k in _t if k[0] == "grid"] == [("grid", 1, 2.5)]) +# 20:00 local: the dearest (19:00) is over; tomorrow's charge is a line. Put +# the dearest later today instead, with the emptiest pack at the floor. +_late = MOCK[:22] + [.40, .24] +_t, _l = trade((62.0, 20.0), prices=_late, now=NOW + timedelta(hours=8)) +_t2, _l2 = trade((62.0, 35.0), prices=_late, now=NOW + timedelta(hours=8)) +check("no give block while the emptiest battery is at arbitrage's floor: " + "the cycle's sale is graded on the emptiest pack (at 35 % both give)", + not [k for k in _t if k[0] == "give"] + and [k[:2] for k in _t2] == [("give", 0), ("give", 1)]) +_t, _l = trade((62.0, 35.0), prices=[x - 0.5 for x in MOCK]) +check("no give block at a negative price (the cycle refuses that sale)", + not [k for k in _t if k[0] == "give"] + and [k for k in _t if k[0] == "grid"]) +# Running, a missing level is a red row and the site is held. In shadow mode +# the plan is still drawn, so this is where the level gate shows. +_t, _l = trade((None, 35.0), opts=dict(ARB, shadow_mode="on")) +check("with a battery level unknown the cycle does not trade, so nothing is " + "drawn and a line says why (shadow mode, which still plans)", + _t == [] and homeview.line_no_trade("no_level") in _l) +_t, _l = trade((62.0, 35.0), provisional_from=DAY0) +check("a plan with no published prices yet says so, not 'the price gap is " + "too small'", + _t == [] and homeview.line_no_trade("not_firm") in _l + and homeview.line_no_trade("not_worth") not in _l) check("the price bars are today's 24 hours, from the real series, with the " "cheap and expensive thirds marked", [b["eur_kwh"] for b in s_arb["plan"]["prices"]] == MOCK @@ -295,6 +349,17 @@ check("while the peak limit holds, the verdict refuses as the cycle does: no " and [(r["from"], r["to"]) for r in s_peak["plan"]["runs"] if r["kind"] == "peak"] == [("11:30", "11:45"), ("11:30", "11:45")]) +c_pk = make(ARB) +c_pk.p1_enabled = True # the peak line is shown where P1 runs +c_pk.tariff_plan, c_pk.tariff_at = provider_plan(), NOW +c_pk.peak_guard = PeakDecision(engaged=False, floor_w=None, ceiling_w=4200.0, + ceiling_from="month_peak", projected_w=1240.0) +s_pk = status(c_pk) +check("the plan's peak line names the guard's ceiling, which is this month's " + "peak once it is above the limit", + homeview.line_peak_limit(4200.0, False) in s_pk["plan"]["lines"] + and s_pk["month_peak"]["line"].endswith("above 4.2 kW.")) + c_blind = make(ARB) c_blind.tariff_plan, c_blind.tariff_at = provider_plan(), NOW s_blind = status(c_blind) # meter_source off: the guard never runs