diff --git a/goodwe_controller/DECISIONS/SAFETY-73-refusal-per-inverter.md b/goodwe_controller/DECISIONS/SAFETY-73-refusal-per-inverter.md index c30a334..bddc1a9 100644 --- a/goodwe_controller/DECISIONS/SAFETY-73-refusal-per-inverter.md +++ b/goodwe_controller/DECISIONS/SAFETY-73-refusal-per-inverter.md @@ -26,14 +26,15 @@ standing import, new -16 W. Both cut out: old keeps asking two empty batteries 1. **A refusing unit is closed in that direction** by its own P_SAFETY unit claim, source `refused`. It engages when the unit gives under - `REFUSED_BELOW_W` (100 W) for `SETTLE_CYCLES` (12) cycles in one direction - while asked more than `PROBE_W` (200 W), or when its stale-reading gate + `REFUSED_BELOW_W` (100 W) for `SETTLE_CYCLES` (12) cycles, spanning at least + `REFUSED_SPAN_S` (22 s), in one direction while asked more than `PROBE_W` + (200 W), or when its stale-reading gate trips on a held reading under 100 W. The allocator then hands its share to the others within their own limits (owner decision 1). It is a hard unit limit that always intersects, so OPT-11's P_MODE layer fits inside it. 2. **It binds every strategy's ask on that unit** (decision 2): peak guard, arbitrage and maintenance are placed around it like the loop. -3. **A refusal above the configured cut-out** logs INFO plus one WARNING ending +3. **A refusal to give power above the configured cut-out** logs INFO plus one WARNING ending in `REFUSES_ABOVE_CUT_OUT` (decision 3). SAFETY-42/43 no longer report such a unit: its share is 0 W, and `account()` treats it as parked. 4. **All units refusing the same way** pass that sign to `compute()`, so the @@ -79,6 +80,20 @@ SAFETY-73 method made to raise. target: worst grid +0 W with the reseed. Phase A measured -1059 W without the reseed and the site hold. +## Review changes + +- **The evidence must span 22 s, not only 12 cycles.** At a 1 s meter the 12 + cycles were 11 s, shorter than a healthy reading's lag (dead time plus a + 10-17 s publish). The reviewer's harness (real `Controller`, both units with + 6 s dead time and a 13-17 s publish, load steps 300/3000 W every 2 min) then + refused both healthy units 10-16 times an hour, and each time the site held + 0 W: +50-85 Wh import an hour and a +4.6 kW grid peak against +3.7 kW without + the refusal. At a 2 s tick there were none. `REFUSED_SPAN_S` is + `(SETTLE_CYCLES - 1) * LOOP_TICK_S`, so 2 s and slower ticks are unchanged. +- **The cut-out WARNING is for a refusal to give power only.** A full battery + that stops taking charge is always above its cut-out level, and the WARNING + told the installer to check a setting that has nothing to do with it. + ## Not covered - A unit that gives a steady part of its ask (100 W or more) is a limit, as at diff --git a/goodwe_controller/DOCS.md b/goodwe_controller/DOCS.md index 28a58f7..4ba090b 100644 --- a/goodwe_controller/DOCS.md +++ b/goodwe_controller/DOCS.md @@ -551,15 +551,15 @@ needs, so a test never trips it. A battery that gives some power, even 100 W, is not refusing and keeps its request. **With two or more inverters** the same rule works per inverter. An inverter -whose battery gives less than 100 W for 12 cycles while asked for more than -200 W is asked 0 W, and the other inverters take over its share, each up to +whose battery gives less than 100 W for 12 cycles, and at least 22 s, while +asked for more than 200 W is asked 0 W, and the other inverters take over its share, each up to its own limits. The status page names it: *inverter-a: the battery refuses to give power, the others take over*. It is tested at 200 W every 15 minutes, one inverter at a time, and only while the site asks at least 200 W that way. -It still accepts charge, and a minute of charging ends the refusal. When every +It still accepts charge, and 12 cycles of charging end the refusal. When every inverter refuses the same way, the whole site asks 0 W as with one inverter. -If the battery refuses while its level reads above its “Battery cut-out level”, -the log adds one warning ending in `REFUSES_ABOVE_CUT_OUT`: check that setting. +If the battery refuses to give power while its level reads above its “Battery +cut-out level”, the log adds one warning ending in `REFUSES_ABOVE_CUT_OUT`: check that setting. A battery that refuses is not reported as *not delivering*. *For installers:* SAFETY-52 and SAFETY-73. The event log reads diff --git a/goodwe_controller/app/inverter.py b/goodwe_controller/app/inverter.py index c5370ef..328c9fa 100644 --- a/goodwe_controller/app/inverter.py +++ b/goodwe_controller/app/inverter.py @@ -319,12 +319,14 @@ class Inverter: # SAFETY-73. The direction this unit's battery refuses (+1 give, -1 # take, 0 none), when that refusal or its last probe started, the - # current probe's start, and the cycles of evidence so far. Memory - # only, like SAFETY-52's: stop and start end it. Used at N>1 only. + # current probe's start, and the cycles of evidence so far and when + # they began. Memory only, like SAFETY-52's: stop and start end it. + # Used at N>1 only. self.refused = 0 self.refused_at = None self.probe_from = None self.ref_count = 0 + self.ref_since = None self.ref_sign = 0 # -- store --------------------------------------------------------------- diff --git a/goodwe_controller/app/main.py b/goodwe_controller/app/main.py index 9921d2e..30df409 100644 --- a/goodwe_controller/app/main.py +++ b/goodwe_controller/app/main.py @@ -236,6 +236,15 @@ def pack_energy_of(inv): inv.capacity_kwh, inv.max_w) +# SAFETY-73 review: the evidence must also span this long, not only +# SETTLE_CYCLES cycles. SETTLE_CYCLES was derived at the 2 s meter tick (22 s +# from its first to its twelfth cycle). A 1 s meter makes 12 cycles only 11 s, +# shorter than a healthy battery reading's lag (BATT_OBLIGED_S, 17 s, plus the +# plant's dead time), and both inverters of a site were then refused after +# ordinary load steps. At 2 s and slower this changes nothing. +REFUSED_SPAN_S = (_ctl.SETTLE_CYCLES - 1) * _ctl.LOOP_TICK_S + + def _refused_interval(sign: int) -> tuple: """SAFETY-73: the (lo, hi) a refusing unit may be asked: 0 W that way.""" return (-math.inf, 0.0) if sign > 0 else (0.0, math.inf) @@ -1171,7 +1180,9 @@ class Controller: _LOG.info("%s: the battery refuses power, so it is asked 0 W and the " "other inverters take over: BATTERY_REFUSES.", inv.name) _LOG.debug("Evidence for %s: BATTERY_REFUSES: %s.", inv.name, evidence) - if (inv.soc is not None and inv.bms_soc_floor is not None + # Only a refusal to GIVE: a full battery that stops taking charge is + # above its cut-out level by definition and says nothing about it. + if (sign > 0 and inv.soc is not None and inv.bms_soc_floor is not None and float(inv.soc) > float(inv.bms_soc_floor)): # Owner decision 3: a refusal above the configured cut-out level # may mean that level is set wrong. One line, no safety stop. @@ -1243,12 +1254,15 @@ class Controller: # (a flapping load) reads ~0 through the plant's own lag, and that # is a battery following, not refusing (TEST-19 flapping family). if abs(standing) > _ctl.PROBE_W and abs(b) < _ctl.REFUSED_BELOW_W: - inv.ref_count = (min(inv.ref_count + 1, _ctl.SETTLE_CYCLES) - if sign == inv.ref_sign else 1) + if sign != inv.ref_sign or not inv.ref_count: + inv.ref_count, inv.ref_since = 0, self._now_mono + inv.ref_count = min(inv.ref_count + 1, _ctl.SETTLE_CYCLES) else: inv.ref_count = 0 inv.ref_sign = sign - if inv.ref_count >= _ctl.SETTLE_CYCLES: + if inv.ref_count >= _ctl.SETTLE_CYCLES and ( + inv.ref_since is None or self._now_mono is None + or self._now_mono - inv.ref_since >= REFUSED_SPAN_S): self._unit_refusal_start( inv, sign, f"asked {standing:+.0f} W, the battery gave {b:+.0f} W " diff --git a/goodwe_controller/test_delivery.py b/goodwe_controller/test_delivery.py index 9d07d5a..2688c84 100644 --- a/goodwe_controller/test_delivery.py +++ b/goodwe_controller/test_delivery.py @@ -592,21 +592,16 @@ check("...and the SHARES now differ, which is OPT-09 and not a weakening of the "cards meet here from opposite sides: OPT-09 stops the ASK, this file " "grades the ALARM, and neither can do the other's half", _pw != _pf - and _sf[-1]["shares"][1] == 0.0 - and max(r["shares"][1] for r in _sw) == 2000.0) -# SAFETY-73. A unit that gives < 100 W for 12 cycles while asked > 500 W is a -# battery that refuses (owner, 2026-09-27): it is closed, its share moves, and -# SAFETY-42 is left with units that deliver something but not enough. This -# fixture compresses `delivery_hold_s` to 1 s at 0.2 s cycles, so the report -# still comes first here. On a site 185 s against 60 s puts the refusal first. -_ref_at = next((i for i, r in enumerate(_sw) if r["shares"][1] == 0.0), None) -check("SAFETY-73: the wedged arm's B, giving 0 W against 2000 W, is refused " - "after 12 cycles of that evidence and asked 0 W from then, the report " - "having come first only because this fixture's hold is 1 s", - _ref_at is not None and _ref_at >= control.SETTLE_CYCLES - and all(r["shares"][1] > 0 for r in _sw[:_ref_at]) - and all(r["shares"][1] == 0.0 for r in _sw[_ref_at:]) - and _cw.inverters[1].refused == 1) + and _sf[-1]["shares"][1] == 0.0 and _sw[-1]["shares"][1] == 2000.0) +# SAFETY-73. A unit that gives < 100 W while asked > 200 W is refused after +# SETTLE_CYCLES cycles that also span REFUSED_SPAN_S (22 s). This fixture's 14 +# cycles are 0.2 s apart, so the wedged B meets the count and not the span, and +# SAFETY-42's report stands alone here. test_refusal_units.py grades the +# refusal. On a site the 185 s hold puts the refusal first. +check("SAFETY-73: the wedged arm's B is not refused in this fixture: 12 cycles " + "of evidence in under 3 s are not the 22 s a refusal needs", + _cw.inverters[1].refused == 0 + and _cw.inverters[1].ref_count >= control.SETTLE_CYCLES) check("...and the state of charge is still what tells them apart", [r["soc"] for r in _sw] != [r["soc"] for r in _sf]) print(f" both arms, last cycle: grid {_sw[-1]['grid']:.0f} W, shares " @@ -654,10 +649,10 @@ check("...and `account()` STILL turns this pack away if the ask does arrive - " account(live=True, parked=False, soc=_cold_b.soc, bms_floor=_cold_b.bms_soc_floor) is not None) check("...and the WEDGED arm's B differs from it in exactly that one place - " - "which is why it was asked for its share until the refusal, and named", + "which is why it is still asked for its share, and still named", account(live=True, parked=False, soc=_cw.inverters[1].soc, bms_floor=_cw.inverters[1].bms_soc_floor) is None - and _sw[0]["shares"][1] > 0 and _sw[-1]["named"] == ("inverter-b",)) + and float(_cw.inverters[1].standing_w()) > 0) # ⚠️ WHICH FLOOR, AND THIS IS A WIRING CHECK THAT THE PURE ONES ABOVE CANNOT # MAKE. `peakguard.pack_floor_pct()` conjoins a pack's own cut-out with the @@ -930,13 +925,9 @@ check("...and EVERY WATT IS THE SAME. Grid, site target, both shares and both " "reported or not - which is what 'it does not act' means, stated as a " "measurement instead of as an intention", _power_domain(_sw) == _power_domain(_sn)) -_trip_at = next((i for i, r in enumerate(_sw) if r["named"]), None) -check("...the wedged unit is still commanded its share after the trip, not " - "parked by this guard: this is NOT SAFETY-02's remedy. What later takes " - "the share is SAFETY-73's refusal, which runs the same in the arm where " - "this guard never trips (the line above)", - _trip_at is not None and _ref_at is not None and _trip_at < _ref_at - and all(r["shares"][1] > 0 for r in _sw[_trip_at:_ref_at])) +check("...the wedged unit is still being commanded its share after the trip, " + "not parked: this is NOT SAFETY-02's remedy and must not become it", + abs(float(_cw.inverters[1].target)) > 0) check("...and nothing named `delivery` reaches the arbiter - the resolution's " "own sentence is unchanged by the trip", "deliver" not in _cw.reason.lower() and _cw.reason == _cn.reason) diff --git a/goodwe_controller/test_refusal_units.py b/goodwe_controller/test_refusal_units.py index 525a4b9..4366a7a 100644 --- a/goodwe_controller/test_refusal_units.py +++ b/goodwe_controller/test_refusal_units.py @@ -1,7 +1,7 @@ """Runnable check for SAFETY-73. `python3 test_refusal_units.py` SAFETY-52 per inverter. With two or more inverters, a battery that refuses -(gives < 100 W for 12 cycles while asked > 500 W, or trips the stale-reading +(gives < 100 W for 12 cycles and 22 s while asked > 200 W, or trips the stale-reading gate on a ~0 W reading) is closed in that direction by its own P_SAFETY limit, so the allocator hands its share to the others. It is probed at 200 W every 15 minutes, one inverter at a time. When every inverter refuses the same way @@ -33,6 +33,8 @@ over this file / test_delivery / test_refusal. Every one is red here: U11 main.py: the above-cut-out WARNING condition inverted 1/0/0 U12 main.py: SAFETY-42's account no longer sees a small ask 1/0/0 U13 main.py: a unit engages above `saturation_w`, not PROBE_W 1/0/0 + U14 main.py: the evidence need not span REFUSED_SPAN_S (review) 1/0/0 + U15 main.py: the WARNING also for a refusal to take (review) 1/0/0 On 652097a itself this file stops at the first sample (`Inverter` has no `refused`); its OLD arms reproduce that tree's N>1 behaviour instead, and @@ -61,6 +63,7 @@ def check(name, cond): TICK = 5.0 CUT_W = -4.0 +FULL_W = 40.0 H = 3600.0 BASE = datetime(2026, 9, 6, 19, 30, tzinfo=timezone.utc) UNITS = ("a", "b") @@ -89,23 +92,39 @@ class _Pub: class _Hass: - """A house, one meter, and two batteries that follow unless cut out.""" + """A house, one meter, and two batteries that follow unless cut out. - def __init__(self, load, cut, soc, jitter): + `lag` (review): (dead s, publish s). Each battery reading then shows the + power of `dead` s before its last publish; the power itself and the meter + follow at once. `full[u](t)`: that battery takes at most FULL_W of charge. + """ + + def __init__(self, load, cut, soc, jitter, lag=None, full=None): self.load, self.cut, self.soc, self.jitter = load, cut, soc, jitter + self.lag, self.full = lag, full or {} self.cmd = {u: 0.0 for u in UNITS} + self.hist = {u: [(-1e18, 0.0)] for u in UNITS} self.t = 0.0 - def batt(self, u): - c = self.cmd[u] - if self.cut[u](self.t) and c >= CUT_W: + def power(self, u, t=None): + t = self.t if t is None else t + c = next((v for tt, v in reversed(self.hist[u]) if tt <= t), 0.0) + if self.cut[u](t) and c >= CUT_W: if self.jitter: - return CUT_W + ((int(self.t / TICK) % 3) - 1) * 3.0 + return CUT_W + ((int(t / TICK) % 3) - 1) * 3.0 return CUT_W + if u in self.full and self.full[u](t) and c < -FULL_W: + return -FULL_W return float(round(c)) + def batt(self, u): + if self.lag is None: + return self.power(u) + dead, pub = self.lag + return self.power(u, (self.t // pub) * pub - dead) + def grid(self): - return self.load(self.t) - sum(self.batt(u) for u in UNITS) + return self.load(self.t) - sum(self.power(u) for u in UNITS) async def limits(self, e): return (-2000.0, 2000.0, 1.0) @@ -126,6 +145,7 @@ class _Hass: async def set_number(self, e, v): self.cmd[e[-1]] = float(v) + self.hist[e[-1]].append((self.t, float(v))) return True @@ -136,10 +156,12 @@ def _off(c): def run(load, cut, dur, *, soc=None, jitter=False, old=False, events=None, - extra=None): + extra=None, tick=None, lag=None, full=None): """Real cycles every TICK of simulated time. Rows are dicts per cycle.""" async def go(): - hass = _Hass(load, cut, soc or {"a": 50.0, "b": 50.0}, jitter) + hass = _Hass(load, cut, soc or {"a": 50.0, "b": 50.0}, jitter, lag, + full) + dt = tick or TICK inv = [{"name": f"inverter-{u}", "setpoint_entity": f"number.sp_{u}", "soc_entity": f"sensor.soc_{u}", "batt_entity": f"sensor.batt_{u}", @@ -187,7 +209,7 @@ def run(load, cut, dur, *, soc=None, jitter=False, old=False, events=None, ra=c.inverters[0].refused, rb=c.inverters[1].refused, da=c.inverters[0].delivery_wd.tripped, db=c.inverters[1].delivery_wd.tripped)) - t += TICK + t += dt finally: m.datetime = real_dt return rows, trips, c @@ -448,6 +470,41 @@ check("shadow mode: no inverter is refused and both stay at 0 W", c.inverters[0].refused == 0 and c.inverters[1].refused == 0 and all(r["a"] == 0.0 and r["b"] == 0.0 for r in rows)) +# Review. A 1 s meter makes 12 cycles 11 s, shorter than a healthy battery +# reading's lag (6 s dead time plus a 13 s publish). Ordinary load steps then +# refused both inverters and dropped the site to 0 W (16 times an hour). The +# evidence must also span REFUSED_SPAN_S. +_steps = (lambda t: 300.0 if int(t // 120) % 2 == 0 else 3000.0) +rows, _, c = run(_steps, {"a": never, "b": never}, H, tick=1.0, + lag=(6.0, 13.0)) +check("a 1 s meter and slow battery readings: load steps for an hour refuse " + "neither healthy inverter", + not any(r["ra"] or r["rb"] for r in rows)) +rows, _, c = run(lambda t: 3000.0, {"a": cut_at(120), "b": never}, 600, + tick=1.0, lag=(6.0, 13.0)) +_eng = next((r["t"] for r in rows if r["ra"]), None) +check("...and the same slow readings still refuse a battery that is cut " + "out, after the span and well inside the 185 s report", + _eng is not None and 120 + m.REFUSED_SPAN_S <= _eng < 185 + and not any(r["da"] for r in rows)) + +# Review. A full battery that stops taking charge is refused (charge), and +# it is always above its cut-out level: that says nothing about the setting, +# so it gets the INFO line and no REFUSES_ABOVE_CUT_OUT warning. +_g2 = _Grab() +_lg.setLevel(logging.INFO) +_lg.addHandler(_g2) +rows, _, c = run(lambda t: -3000.0, {"a": never, "b": never}, 600, + soc={"a": 99.0, "b": 60.0}, full={"a": lambda t: t >= 120}) +_lg.removeHandler(_g2) +_lg.setLevel(_lv) +check("a full battery that stops taking charge is refused that way only, " + "with the INFO line and no warning about its cut-out level", + c.inverters[0].refused == -1 + and any(lv == logging.INFO and "BATTERY_REFUSES" in x + for lv, x in _g2.lines) + and not any("REFUSES_ABOVE_CUT_OUT" in x for _lv2, x in _g2.lines)) + # ============================================================================== print("6. one inverter is untouched") # ==============================================================================