diff --git a/goodwe_controller/DECISIONS/SAFETY-86-peak-over-hold.md b/goodwe_controller/DECISIONS/SAFETY-86-peak-over-hold.md index 2be80e8..612efb0 100644 --- a/goodwe_controller/DECISIONS/SAFETY-86-peak-over-hold.md +++ b/goodwe_controller/DECISIONS/SAFETY-86-peak-over-hold.md @@ -20,6 +20,10 @@ battery gave: 4500 W against a 3950 W aim (SAFETY-84 rig test, check 6). (owner order), and the owner rule for an Off battery is the same, "give way only for the shortfall". A battery Off by its mode (P_MODE, below maintenance) therefore still gives before the holding one. +- N>=2, while the holding battery gives: a site `hold` ceiling at the floor + (P_MAINTENANCE), so the loop does not ask for watts the hold keeps back. + Without it the moving pin re-logged the ALLOCATOR "could not be placed" + row each cycle and filled the action log (review, PR #161). - Below the aim nothing changes: the floor is 0 W, the hold is 0 W, the rows are identical to before (test_safety86.py part 2). - A site stop (a SET above `P_PEAK`) decides alone: nothing is raised at N>=2, @@ -29,14 +33,11 @@ battery gave: 4500 W against a 3950 W aim (SAFETY-84 rig test, check 6). - The hold resumes by itself: the rule is evaluated each cycle, so when the guard lets go the command is 0 W again. -**The level dips below the target: the hold finishes anyway.** The HOLD does -not read the level; it ends on its clock (`maintenance_hold_min`). Topping up -would mean going back to CHARGE, which waits for sun or a cheap window (up to -36 h, `CHARGE_TIMEOUT_MIN`) while the pack keeps the site's one maintenance -slot, so the other battery's cycle waits, and then a new full hold that the -guard can interrupt again. The guard only gives while the quarter is at risk, -and the next monthly cycle balances again. If the owner wants a top-up -instead, it is a change to `maintenance.py`'s HOLD branch, not to this rule. +**The level dips below the target: the hold finishes anyway.** Owner +decision 2026-10-01: the HOLD ends on its clock (`maintenance_hold_min`) even +when the peak guard pulls the level down; no top-up. A top-up would re-enter +CHARGE (up to 36 h, `CHARGE_TIMEOUT_MIN`) and keep the site's one maintenance +slot meanwhile. **Not changed.** - The peak guard still grades the emptiest battery (SAFETY-32): at N=2 with diff --git a/goodwe_controller/app/arbiter.py b/goodwe_controller/app/arbiter.py index 14dbd51..2ec7bee 100644 --- a/goodwe_controller/app/arbiter.py +++ b/goodwe_controller/app/arbiter.py @@ -202,6 +202,10 @@ CLAIM_SCOPE = { # sibling would have to give against it. Never filed at N=1. # DECISIONS/SAFETY-84-charge-at-two.md. "charge": SCOPE_SITE, + # main.py: `Claim.limit("hold", P_MAINTENANCE, hi=...)`, SAFETY-86. While a + # holding pack gives the peak shortfall at N>=2, the site target stays at + # the peak floor, what the pin and the others place. Never filed at N=1. + "hold": SCOPE_SITE, # --- UNIT: about one pack, one inverter, one device ------------------- # main.py: `Claim.limit("device", P_SAFETY, self.dev_min, self.dev_max, diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index e507731..3881917 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -2533,13 +2533,14 @@ class Controller: if c.source == peakguard.SOURCE and c.lo > rate: claims[i] = dataclasses.replace(c, priority=P_MAINTENANCE) - def _peak_over_hold(self, claims, maint_results) -> None: + def _peak_over_hold(self, claims, maint_results): """SAFETY-86: a peak floor outranks a maintenance HOLD (0 W). At N=1 the floor is refiled at P_MAINTENANCE, as for a drain. At N>=2 the holding pack gives only what the others cannot: its pin is raised - to the floor minus what they may place. A site stop still decides - alone. DECISIONS/SAFETY-86-peak-over-hold.md. + to the floor minus what they may place, and the floor is returned as + the site `hold` ceiling (else None). A site stop still decides alone. + DECISIONS/SAFETY-86-peak-over-hold.md. """ hold = {self.actuator_of("maintenance", inv) for inv, res in zip(self.inverters, maint_results) @@ -2569,13 +2570,15 @@ class Controller: if p is not None and p < 0.0: return # a pack pinned to charge: never split both ways others += allocator.hi_w(b) if p is None else p - give = max(claims[i].lo for i in peak) - others + floor = max(claims[i].lo for i in peak) + give = floor - others if give <= 0.0: return for i, c in enumerate(claims): if c.source == "maintenance" and c.kind == SET \ and c.actuator in hold: claims[i] = dataclasses.replace(c, value=give) + return floor def _charge_headroom(self, floor: float, claims) -> float: """SAFETY-84: at N>=2 a cheap-window charge pulls no more than the @@ -4352,7 +4355,13 @@ class Controller: reason="maintenance drain", actuator=self.actuator_of("drain"))) self._peak_over_drain(claims, maint_results) - self._peak_over_hold(claims, maint_results) + # --- SAFETY-86: while a HOLD gives, the site stays at the peak floor -- + # So the loop never asks for watts the hold keeps back. + hold_hi = self._peak_over_hold(claims, maint_results) + if hold_hi is not None: + claims.append(Claim.limit("hold", P_MAINTENANCE, hi=hold_hi, + reason="maintenance hold", + actuator=self.actuator_of("hold"))) # --- SAFETY-84: a cheap-window charge is a ceiling over the site ---- charge = self._charge_ceiling(claims) if charge is not None: diff --git a/goodwe_controller/app/messages.py b/goodwe_controller/app/messages.py index fa6a4b4..d353b16 100644 --- a/goodwe_controller/app/messages.py +++ b/goodwe_controller/app/messages.py @@ -836,6 +836,7 @@ WHY_LIMIT = { "reserve": "the battery reserve", "maintenance": "battery maintenance", "charge": "battery maintenance", + "hold": "battery maintenance", "refused": "a battery that refuses power", } diff --git a/goodwe_controller/test_arbiter.py b/goodwe_controller/test_arbiter.py index b367003..8fe90d4 100644 --- a/goodwe_controller/test_arbiter.py +++ b/goodwe_controller/test_arbiter.py @@ -149,7 +149,7 @@ check("...and `source` is still the first positional argument, which is what " Claim.limit("device", P_SAFETY, -1, 1).source == "device") print() -print("ARB-03 2. the split: fifteen sources, each classified where it is BUILT") +print("ARB-03 2. the split: sixteen sources, each classified where it is BUILT") # The classification is graded against what each claim is ABOUT, restated here # so that the table in arbiter.py and the expectation in a test are not the same # sentence read twice. test_peakguard.py part 10f pins this set against main.py's @@ -179,6 +179,9 @@ SITE_SOURCES = { "charge": "the site target may not go above a cheap-window maintenance " "charge, or a sibling would give against it. Filed at N>1 " "only", + # SAFETY-86. + "hold": "the site target stays at the peak floor while a holding pack " + "gives the shortfall. Filed at N>1 only", } UNIT_SOURCES = { "device": "one inverter's own reported min/max rating", @@ -210,9 +213,9 @@ for _src, _why in sorted(SITE_SOURCES.items()): print(f" site {_src:<11} {_why}") for _src, _why in sorted(UNIT_SOURCES.items()): print(f" unit {_src:<11} {_why}") -check("all FIFTEEN claim sources are classified, and only those fifteen", +check("all SIXTEEN claim sources are classified, and only those sixteen", set(CLAIM_SCOPE) == set(SITE_SOURCES) | set(UNIT_SOURCES)) -check("the eight that are about the GRID CONNECTION are site claims", +check("the nine that are about the GRID CONNECTION are site claims", {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_SITE} == set(SITE_SOURCES)) check("the seven that are about a PACK, a DEVICE or one write are per-inverter", {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_UNIT} == set(UNIT_SOURCES)) @@ -234,7 +237,7 @@ check("...and with none configured at all, likewise", all(actuator_for(s, "inverter-a", ()) == ACTUATOR_SITE for s in CLAIM_SCOPE)) check("with TWO the identity breaks and the split becomes real: the seven " "per-inverter sources route to the unit this controller drives, the " - "eight site sources stay on the grid connection", + "nine site sources stay on the grid connection", {s: actuator_for(s, "inverter-a", TWO) for s in CLAIM_SCOPE} == {**{s: "inverter-a" for s in UNIT_SOURCES}, **{s: ACTUATOR_SITE for s in SITE_SOURCES}}) @@ -515,7 +518,7 @@ check("...and at N=1, where every source legitimately routes to the site, the " Claim.limit("maintenance", P_MAINT_SHAPE, hi=0.0)]) == []) check("an unclassified ninth source is a violation wherever it is filed", scope_violations([Claim.limit("reserve", P_LOOP, hi=0.0)]) != []) -check("and the eight, routed by actuator_for() at a two-inverter site, are " +check("and the sixteen, routed by actuator_for() at a two-inverter site, are " "clean - which is what says the router and the checker agree", scope_violations( [Claim.limit(s, P_SAFETY, -1, 1, diff --git a/goodwe_controller/test_peakguard.py b/goodwe_controller/test_peakguard.py index d936a45..29bae44 100644 --- a/goodwe_controller/test_peakguard.py +++ b/goodwe_controller/test_peakguard.py @@ -2691,9 +2691,12 @@ check("...and so does the persistent notification, which outlives the page and " # site `hi` at P_MAINT_SHAPE, filed only at N>=2 while a pack's cheap-window # maintenance charge makes it charge, equal to that pack's own `hi`. Never at # N=1. DECISIONS/SAFETY-84-charge-at-two.md. +# SAFETY-86 added `hold`: a site `hi` at the peak floor while a holding pack +# gives at N>=2. Not a reserve either. DECISIONS/SAFETY-86-peak-over-hold.md. CLAIM_SOURCES = {"device", "supervised", "failsafe", "safety", "peak", "arbitrage", "maintenance", "loop", "circulation", - "sign_site", "pack", "refused", "mode", "drain", "charge"} + "sign_site", "pack", "refused", "mode", "drain", "charge", + "hold"} # The two appends that file a claim BUILT ELSEWHERE in the method. Named here so # that a THIRD such indirection reads as an unrecognised append rather than as a # silent gap in the scan. @@ -2735,7 +2738,7 @@ check("every `claims.append(...)` in main.py's cycle is one this scan can READ " "- a claim filed through a shape it cannot parse has to fail here rather " "than be counted as nothing, which is how a scan like this goes quietly " "vacuous", len(_appends) >= 10 and None not in _filed) -check("...and the sources that cycle can file are EXACTLY these FIFTEEN. THIS " +check("...and the sources that cycle can file are EXACTLY these SIXTEEN. THIS " "is what makes part 10 a tripwire: replay_loop() hand-builds the claim " "list, so it cannot see a claim main.py adds - and a loop-side SoC " "reserve is a new source. Filing one takes this line red on the line it " @@ -2875,7 +2878,7 @@ print(" site: " + ", ".join(sorted(s for s, v in CLAIM_SCOPE.items() + " | per-inverter: " + ", ".join(sorted(s for s, v in CLAIM_SCOPE.items() if v == SCOPE_UNIT))) check("...and the split is the one ARB-03 argued, with SAFETY-31's and " - "SAFETY-30's, OPT-09's, SAFETY-78's and SAFETY-84's additions: the eight about the GRID CONNECTION " + "SAFETY-30's, OPT-09's, SAFETY-78's, SAFETY-84's and SAFETY-86's additions: the nine about the GRID CONNECTION " "are site claims and the five about a pack, a device or one write are " "per-inverter. `circulation` is a site claim because its quantity is " "defined over the roster and is identically zero for any one unit; " @@ -2885,7 +2888,7 @@ check("...and the split is the one ARB-03 argued, with SAFETY-31's and " "against that unit's OWN measured power", {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_SITE} == {"loop", "peak", "arbitrage", "safety", "circulation", "sign_site", - "drain", "charge"} + "drain", "charge", "hold"} and {s for s, v in CLAIM_SCOPE.items() if v == SCOPE_UNIT} == {"device", "supervised", "failsafe", "maintenance", "pack", "refused", "mode"}) @@ -2990,7 +2993,8 @@ check("⚠️ AND THIS IS THE BREAKING HALF. Give the same Controller a second " "loop": ACTUATOR_SITE, "peak": ACTUATOR_SITE, "arbitrage": ACTUATOR_SITE, "safety": ACTUATOR_SITE, "circulation": ACTUATOR_SITE, "sign_site": ACTUATOR_SITE, - "drain": ACTUATOR_SITE, "charge": ACTUATOR_SITE}) + "drain": ACTUATOR_SITE, "charge": ACTUATOR_SITE, + "hold": ACTUATOR_SITE}) check("...and the roster it was restored to is the shipped one, so nothing " "below inherits a two-inverter Controller", c_src.units == (app_main.UNIT_A,) and len(c_src.inverters) == 1) @@ -3069,13 +3073,13 @@ _unit_sources_unrouted = [ if CLAIM_SCOPE.get(r[0]) == SCOPE_UNIT and "," not in r[1]] print(f" {len(_direct)} direct appends, each read at its own line: " + ", ".join(f"main.py:{ln} {s} -> {a}" for s, a, ln in _routing)) -check("all SIXTEEN of `Controller.cycle()`'s DIRECT appends route through " +check("all SEVENTEEN of `Controller.cycle()`'s DIRECT appends route through " "`self.actuator_of()`, so this scan can read the routing " "of EVERY one of them. An append that computed its actuator, or omitted " "the argument, or hand-wrote a string, fails HERE rather than being " "silently skipped - which is how a scan like this goes quietly vacuous, " "and 10f is in this file because that already happened once", - len(_direct) == 16 + len(_direct) == 17 and all(a is not None and a.startswith("self.actuator_of('") for _s, a, _ln in _routing)) check("⚠️ AND AT EVERY ONE OF THE ELEVEN THE LITERAL IT ROUTES THROUGH IS THE " @@ -3109,8 +3113,8 @@ check("...and the only appends this pin does NOT reach are the two that file a " "arbitrage.claim() by test_arbitrage.py", sorted(_indirect) == sorted(CLAIM_BUILDERS) and len(_direct) + len(_indirect) == len(_appends)) -check("...and between them the eighteen appends account for exactly the " - "fifteen " +check("...and between them the nineteen appends account for exactly the " + "sixteen " "sources 10f pins, so the routing pin and the source set are one scan " "and neither can drift from the other", {s for s, _a, _ln in _routing} | set(CLAIM_BUILDERS.values()) @@ -3171,7 +3175,7 @@ check("...and naming those three is not an accident of this fixture but the " {c.source for c in _cl2} == CLAIM_SOURCES - {"failsafe", "safety", "maintenance", "peak", "arbitrage", "circulation", "sign_site", - "refused", "mode", "drain", "charge"}) + "refused", "mode", "drain", "charge", "hold"}) # ⚠️ AND NOW THE HALF THAT WAS EVIDENCE-SHAPED AND OBSERVED NOTHING. The three # checks above grade WHICH ACTUATOR each claim lands on. They say nothing about # WHOSE NUMBERS the bound is built from - and those are two different defects diff --git a/goodwe_controller/test_safety86.py b/goodwe_controller/test_safety86.py index 5cb22fd..ce761d0 100644 --- a/goodwe_controller/test_safety86.py +++ b/goodwe_controller/test_safety86.py @@ -38,7 +38,8 @@ from app import packmode # noqa: E402 from app.arbiter import (Claim, P_MAINTENANCE, P_PEAK, P_SAFETY, # noqa resolve) from app.main import Controller # noqa: E402 -from app.maintenance import DRAIN, HOLD, IDLE, MaintResult # noqa: E402 +from app.maintenance import (CHARGE, DRAIN, HOLD, IDLE, # noqa: E402 + MaintResult) from app.store import unit_key # noqa: E402 fails = [] @@ -206,6 +207,47 @@ check("⭐ battery modes on, B Off: the Off battery still gives before the " and 540.0 <= cmd(_off_rows[-1])["a"] <= 560.0 and refused(_off_rows) == 0 and not alarmed(_off)) + + +def noisy(t): + """A 6500 W house that moves up to 150 W from one second to the next.""" + return 6500.0 + ((int(t) * 7919) % 301) - 150.0 + + +def no_ceiling(fn): + """Run with the pin raised but no `hold` site ceiling (review check).""" + real = Controller._peak_over_hold + + def pin_only(self, claims, results): + real(self, claims, results) + Controller._peak_over_hold = pin_only + try: + return fn() + finally: + Controller._peak_over_hold = real + + +def rows_logged(c, word): + return sum(1 for e in c.events if word in e) + + +_nz_rows, _nz = run(noisy, 2) +_nz_old = no_ceiling(lambda: run(noisy, 2))[1] +print(f" noisy house: ALLOCATOR rows {rows_logged(_nz, 'ALLOCATOR')}, " + f"clamp rows {rows_logged(_nz, 'OUTPUT_CLAMP')} (without the site " + f"ceiling {rows_logged(_nz_old, 'ALLOCATOR')} and " + f"{rows_logged(_nz_old, 'OUTPUT_CLAMP')})") +check("⭐ while A gives, the site target is the guard's floor (the `hold` " + "ceiling), so a noisy house does not fill the action log: no ALLOCATOR " + "row and at most one clamp row", + on_aim(_nz) and all(abs(r[1] - r[-1][1][1]) < 1e-6 for r in _nz_rows + if cmd(r)["a"] > 0.0) + and rows_logged(_nz, "ALLOCATOR") == 0 + and rows_logged(_nz, "OUTPUT_CLAMP") <= 1) +check("non-vacuous: without the ceiling the loop asks 5000 W and the " + "ALLOCATOR and clamp rows fill the 40-row log", + rows_logged(_nz_old, "ALLOCATOR") >= 10 and len(_nz_old.events) == 40) + # ============================================================================ # print("\n4. a month peak banked above the limit") BANK = {n: (run(6500.0, n, month_peak=4000.0), @@ -301,7 +343,8 @@ def pin_after(floor, *extra, live=(A, B), results=HOLDING, c=_c): Claim.set("maintenance", P_MAINTENANCE, 0.0, HOLD, actuator=A), Claim.limit("peak", P_PEAK, lo=floor, reason="quarter peak"), *extra] - Controller._peak_over_hold(fake, cl, results) + ceiling = Controller._peak_over_hold(fake, cl, results) + pin_after.ceiling = ceiling return next((x.value for x in cl if x.source == "maintenance" and x.kind == "set"), None), cl @@ -331,6 +374,15 @@ check("⭐ a site safety SET: A stays at 0 W and the site resolves to 0 W; " _p == 0.0 and resolve(_cl, units=(A, B), live={A, B}).target_w == 0.0 and pin_after(3000.0, Claim.set("failsafe", P_SAFETY, 0.0, actuator=B))[0] == 3000.0) +_p, _cl = pin_after(2600.0) +_hi = pin_after.ceiling +_cl += [Claim.limit("hold", P_MAINTENANCE, hi=_hi, actuator="site"), + Claim.set("loop", 10, 5000.0, "the loop asks for the whole house")] +pin_after(2500.0) +check("⭐ at N=2 the `hold` site ceiling is the floor itself, only when A " + "gives, and a 5000 W loop ask resolves to that floor", + _hi == 2600.0 and pin_after.ceiling is None + and resolve(_cl, units=(A, B), live={A, B}).target_w == 2600.0) check("only a HOLD: a DRAIN pin or an idle pack is not raised", pin_after(4000.0, results=[MaintResult(DRAIN, True, 0.0, False, []), HOLDING[1]])[0] == 0.0 @@ -355,8 +407,11 @@ _stop = refile(3000.0) + [Claim.set("safety", P_SAFETY, 0.0, "inputs missing")] check("⭐ at N=1 a refiled floor stays under a safety SET: 0 W", resolve(_stop).target_w == 0.0) -check("at N=1 outside a HOLD the claim is not touched by this rule", +check("at N=1 outside a HOLD (idle, or a CHARGE) the claim is not touched " + "by this rule", refile(3000.0, (MaintResult(IDLE, False, None, False, []),))[1] + .priority == P_PEAK + and refile(3000.0, (MaintResult(CHARGE, False, None, True, []),))[1] .priority == P_PEAK) print()