From c38dd1e9693b7cc26e38357785d9d48577364b49 Mon Sep 17 00:00:00 2001 From: Claude review Date: Tue, 29 Sep 2026 22:31:52 +0200 Subject: [PATCH] OPT-15 review: bound unmeasured charge stops, test the hold's rules and M15 - A second unmeasured-headroom stop in a quarter latches the charge: a headroom missing every other cycle flipped it 1799 times in 30 min. - Unit test of the run-edge hold's end conditions and of a small step. - Outside its probe a refuser does not make the others rest (M15 red). - DECISIONS: one review line. Co-Authored-By: Claude Opus 5.5 --- .../DECISIONS/OPT-11-battery-modes.md | 6 ++ goodwe_controller/app/main.py | 19 +++-- goodwe_controller/test_price_run_fixes.py | 76 +++++++++++++++++++ 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/goodwe_controller/DECISIONS/OPT-11-battery-modes.md b/goodwe_controller/DECISIONS/OPT-11-battery-modes.md index c1e9494..6318b35 100644 --- a/goodwe_controller/DECISIONS/OPT-11-battery-modes.md +++ b/goodwe_controller/DECISIONS/OPT-11-battery-modes.md @@ -105,3 +105,9 @@ P1 is its section 6 P1, and the pull request lists what was measured. banks up to 1.4 W over the aim (48.6 W under the cap), and a battery reading older than the meter can make a charge start look like a house over the aim, a real guard stop. + - Review: an unmeasured stop that never latched let a headroom missing + every other cycle flip the charge 1799 times in 30 min, so the second one + in a quarter latches (at most 4 changes a quarter). House steps of +3 and + +5 kW during the hold, N=1 and 2: the peak guard's floor ends the hold at + once, and no banked quarter differs from the loop without the hold by + more than +33 W (1342 W, far under the aim). diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 8002524..9349285 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -944,6 +944,7 @@ class Controller: self._run_done = {} self._mode_trade_was = False self._charge_blocked_q = None + self._charge_gap_q = None # OPT-15: an unmeasured stop's quarter self._ref_cheap = None # OPT-15: a run edge the loop holds at E: (E, step sign, until, the # last battery reading). @@ -2174,13 +2175,17 @@ class Controller: blocked=self._charge_blocked_q == quarter, done_charge=done_c, done_sell=done_s) # A charge the guard or the headroom stopped stays off this period. - # OPT-15: only a MEASURED stop latches; an unmeasured headroom (None) - # stops this tick only, so a gap cannot cost the whole quarter. - if was and self.mode_tick.action != modetrade.CHARGE and ( - self.mode_tick.why == "peak" - or (self.mode_tick.why == "headroom" - and self.arb_headroom_w is not None)): - self._charge_blocked_q = quarter + # OPT-15: only a MEASURED stop latches; the first unmeasured headroom + # (None) in a quarter stops this tick only, so a gap cannot cost the + # whole quarter. A second one latches, so gaps cannot flip the charge. + if was and self.mode_tick.action != modetrade.CHARGE: + why = self.mode_tick.why + gap = why == "headroom" and self.arb_headroom_w is None + if why == "peak" or (why == "headroom" and not gap) or ( + gap and self._charge_gap_q == quarter): + self._charge_blocked_q = quarter + elif gap: + self._charge_gap_q = quarter return self.mode_tick def _run_mark(self, kind, run, packs, out) -> set: diff --git a/goodwe_controller/test_price_run_fixes.py b/goodwe_controller/test_price_run_fixes.py index 4f5d22a..386c0e5 100644 --- a/goodwe_controller/test_price_run_fixes.py +++ b/goodwe_controller/test_price_run_fixes.py @@ -265,6 +265,34 @@ if _c1d.mode_tick is not None and _c1d.mode_tick.action == modetrade.CHARGE: check("an unmeasured headroom (None) stops the charge that tick only; a " "measured 0 W stops it for the rest of the quarter", _ok1d) + +# 1e. Review of #157: gaps cannot flip the charge. A headroom forced to None +# every other cycle flipped it 1799 times in 30 min before this; the second +# unmeasured stop in a quarter now latches it. +def _gaps(c): + real, k = c._mode_roles, [0] + + def mr(now, maint): + k[0] += 1 + if k[0] % 2 == 0: + c.arb_headroom_w = None + return real(now, maint) + c._mode_roles = mr + + +_rows1e = run(2.0, 0.5, lag=LAG, patch=_gaps)[0] +_flips = {} +_on1e = next((i for i, r in enumerate(_rows1e) + if r["action"] == arbitrage.CHARGE), len(_rows1e)) +for _a, _b in zip(_rows1e[_on1e:], _rows1e[_on1e + 1:]): + if (_a["action"] == arbitrage.CHARGE) != (_b["action"] == arbitrage.CHARGE): + _flips[q(_b["now"])] = _flips.get(q(_b["now"]), 0) + 1 +print(f" None every other cycle: charge on/off changes per quarter " + f"{sorted(_flips.values())}") +check("a headroom unmeasured every other cycle changes the charge at most " + "4 times a quarter (restart, stop, restart, stop and latch)", + _flips and max(_flips.values()) <= 4) + # ============================================================================ # print("\n2. F2: the export overshoot at a charge end") # A fills to its 95 % ceiling at about 04:03 under an 800 W house, and @@ -367,6 +395,41 @@ check("...and when the peak floor takes the command during the hold, the " "hold ends: 10 s after the edge B gives its rating (grid 4500 - 2500 W), " "not the aim the floor holds", present(_g10) and _g10 <= 2100) +# Review of #157: the hold's own rules, on `_reseed_run_edge()` and +# `_edge_grid()` directly (target -10 W, deadband 15 W, EDGE_HOLD_S 18 s). +_T0 = UTC0 + + +def _held(grid, batt, s, last_b=-2500.0, end=False): + """(what the loop reads, still held) for a charge end E=800 W at +s.""" + f = SimpleNamespace( + _edge=(800.0, 1.0, _T0 + timedelta(seconds=main.EDGE_HOLD_S), last_b), + tuning=ctl.Tuning(target_grid_w=-10.0), grid=grid, + site_batt_w=batt, i_w=0.0) + g = main.Controller._edge_grid(f, end, _T0 + timedelta(seconds=s)) + return g, f._edge is not None + + +_hs = main.EDGE_HOLD_S +check("the hold: the loop reads the target while the meter still shows the " + "step; it ends on a new trade or claim, when the meter comes within the " + "deadband of the target, and at EDGE_HOLD_S (later only while the " + "battery still moves to E, and never past twice that)", + _held(3300.0, -2500.0, 1) == (-10.0, True) + and _held(3300.0, -2500.0, 1, end=True) == (3300.0, False) + and _held(5.0, -2500.0, 1) == (-10.0, True) + and _held(4.0, -2500.0, 1) == (4.0, False) + and _held(3300.0, -2500.0, _hs + 1) == (3300.0, False) + and _held(3300.0, -2000.0, _hs + 1) == (-10.0, True) + and _held(3300.0, -2000.0, 2 * _hs + 1) == (3300.0, False)) +_f = SimpleNamespace(site_batt_w=500.0, grid=90.0, _edge=None, + tuning=ctl.Tuning(target_grid_w=-10.0), site_max_w=5000.0, + loop_bound=lambda claims: None, target=None, i_w=None) +main.Controller._reseed_run_edge(_f, [], _T0) +check("...and a small step is held too (E 600 W from 500 W)", + _f.target == 600.0 and _f._edge is not None and _f._edge[:2] == ( + 600.0, 1.0)) + # ============================================================================ # print("\n3. F3: a charge refusal inside a cheap run is probed") # A refuses to charge from its first charge; the plant accepts again at @@ -401,6 +464,19 @@ check("...and the probe after the plant accepts again ends the refusal and " _end3 is not None and _end3["now"] < at(3) and any(r["action"] == arbitrage.CHARGE and r["shares"].get(A, 0) < -1000 for r in ROWS3 if r["now"] > _end3["now"])) +# Review of #157 (M15): outside its probe a refuser is no charger, so the +# others are not made to rest. With solar to spare, Solar first B takes it. +_rows3s = run(1.9, 0.5, soc_a=30.0, refuse=("a",), steps=[(2, -1500)])[0] +_pt = [r["now"] for r in _rows3s if r["probing"]] +_sun = [r for r in _rows3s if r["refused"] and not r["probing"] + and r["now"] >= at(2 + 5 / 60) and not any( + timedelta(0) < r["now"] - t <= timedelta(seconds=30) for t in _pt)] +_bsun = sorted(r["shares"].get(B, 0) for r in _sun) +print(f" solar 1500 W over the house, A refusing: B's share outside probes " + f"{_bsun[0] if _bsun else None} .. {_bsun[-1] if _bsun else None} W") +check("...outside a probe the others are not held at rest: Solar first B " + "takes the spare solar (from 30 s after a probe)", + _sun and all(r["shares"].get(B, 0) < -1000 for r in _sun)) _pk = modetrade.Pack(A, True, packmode.CHEAP_HOURS, False, 2500.0, 30.0, 10.0, refused=-1, probing=True) _runs = SimpleNamespace(on=True, cheap=SimpleNamespace(holds=lambda n: True),