admin/goodwe-addon
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

SAFETY-24: an energy floor under the peak guard's forced discharge
checks / unit (pull_request) Successful in 3m29s
Details

Browse Source 
SAFETY-07 can command a discharge to hold a billed quarter down, and
nothing below it consulted the state of charge. On the 2026-08-20 replay
it asks for 878 Wh in a day. Relying on the BMS is not absurd - it is a
real floor and it works - but it is the pack's protection rather than the
operator's, and a month of aggressive quarters leaves the battery
repeatedly at that cut-out with no EMS-side record of why. The first
symptom is a pack never available when wanted, which is indistinguishable
from the SoC drift MAINT-01 exists to correct.

decide() now takes soc and soc_floor, both required and neither
defaulted, and discharge_permitted() is the one place the question is
answered. Above the floor nothing changes. At or below it the guard files
a 0 W floor instead of a sized one - one-directional, so charging stays
blocked and no discharge is capped - and reports floor_limited with the
reason it bound. It is not a trip: every stop in 11.2's table still
outranks the claim, and the floor is never negative, so 9.1's asymmetry
survives as a property of the object (0 negative floors over 1296 swept
combinations, SoC included).

The SoC is graded only after `wanted > 0` is established, so it is never
evaluated where it governs nothing. An unreadable SoC is floor-limited,
not a full battery - a full battery is exactly the value that would
disarm the floor, and this project shipped that defaulting once already.

Failing to shave is not silent: one notification per episode
(goodwe_peak_soc_floor), distinct from the saturation alarm because the
two ask for opposite operator actions, plus its own status row and an
action-log line.

Verified by replay. The 2026-08-20 capture is driven against a modelled
pack sized from the day's own measured appetite, so it genuinely
exhausts: without the floor it reaches the BMS cut-out and holds there
for 1227 guard cycles; with it the pack stops at 20.0 % and never touches
the BMS. The energy withheld equals the energy between the two floors to
within one cycle at the rail. The honest half is reported too - the worst
quarter goes 4338 W -> 5030 W, because a floor-limited guard shaves less
on purpose.

Two new options, mirrored once in peakguard.PEAK_OPTION_DEFAULTS
(TARIFF-02's pattern), imported by validate.py and main.py rather than
typed as literals and pinned to config.yaml by a check that reads the
file - so CFG-03 still tracks three places. validate.py rule 8 refuses
peak_soc_floor <= bms_soc_floor at startup, in CFG-01/MAINT-04's manner.
maintenance_soc_floor is deliberately not reused and not renamed: it is a
drain target, not a spend limit, and the rename is recorded as a decision
to revisit if a third floor appears.

Eleven mutations of the shipped source were run against the checks; all
eleven go red and there are no survivors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
This commit is contained in:
glenn schrooyen
2026-08-31 00:29:36 +02:00
co-authored by Claude Opus 5
parent e7c95bfd8a
commit e709554857
7 changed files with 1177 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/peakguard.py
+222 -14
View File
@@ -246,19 +246,100 @@ validate.py are untouched by this ticket, exactly as TARIFF-01 left them, and
mismatch trap 14's own warning box describes.
------------------------------------------------------------------------------
⚠️ WHAT THIS GUARD DOES NOT HAVE: AN ENERGY BACKSTOP. SEE SAFETY-24.
SAFETY-24: THE ENERGY BACKSTOP THIS GUARD SHIPPED WITHOUT
The graded branch can FORCE DISCHARGE, and nothing in this file or in the loop
below it consults the state of charge - `soc_floor` belongs to the maintenance
cycle, and freeze.py already records that for the grid-following loop "the BMS
is the sole energy backstop". On the 2026-08-20 replay the guard asks for 878 Wh
in a day. Against a pack sitting near 9 % that is not nothing, and the guard will
keep asking as long as the meter says the quarter is over the ceiling.
SAFETY-07 left this file saying, in its own words, that the graded branch can
FORCE DISCHARGE and that nothing below it consults the state of charge - "the
BMS is the sole energy backstop" (freeze.py). On the 2026-08-20 replay the guard
asks for 878 Wh in a day, and it keeps asking for as long as the meter says the
quarter is over the ceiling. That is the behaviour working as designed, and it
is unbounded from below by anything in the EMS.
That is deliberate for THIS card and it is filed as SAFETY-24. It is also the
reason this must not go to a client install before SAFETY-24 lands: on the
reference site the BMS is a known quantity and an operator is present, and on a
client site neither is true.
⚠️ STATE THE RISK FAIRLY, because overstating it is how a real one gets ignored.
Relying on the BMS is not absurd: it is a genuine floor and it works. But it is
THE PACK's protection, not the OPERATOR's. A month of aggressive quarters can
leave the battery repeatedly parked at that floor with no EMS-side record of
why, and the first symptom - a pack that is never available when it is wanted -
is indistinguishable from the SoC drift MAINT-01 exists to correct.
So `decide()` now takes `soc` and `soc_floor`, and `discharge_permitted()` is
the one place the question is answered. Four things about the shape of it:
1. IT IS NOT A TRIP, AND IT MUST NOT BECOME ONE. SAFETY-07's claim is a LIMIT
and every stop in 11.2's table still outranks it. This floor is about
ENERGY - how far down the guard may spend the pack for money - not about
last-resort safety. The action when it binds is therefore to stop ASKING
for more discharge, not to command anything.
2. THE ACTION IS `floor_w = 0.0`, WHICH IS THE BLIND BRANCH'S ACTION FOR THE
BLIND BRANCH'S REASON. Zero is one-directional: it forbids CHARGING, which
can only ever lower delivered offtake, and it forbids nothing else. It
cannot pull the loop off a discharge it is already doing, because a `lo`
limit structurally cannot cap discharge - the property this module's whole
shape rests on, and one a NEGATIVE floor would destroy. Nothing here can
produce a negative floor: the only two values `floor_w` ever takes are 0.0
and `min(wanted, achievable)` with `wanted > 0` already established.
⚠️ AND IT DOES NOT BOUND THE LOOP. Self-consumption may still drain the
pack past this floor, exactly as SAFETY-18 recorded. That is not fixed
here and must not be claimed: this card bounds THE PEAK GUARD's OWN forced
discharge, which is the thing SAFETY-07 introduced. A loop-side SoC guard
is a different lever with a different right value.
3. IT IS GRADED ONLY WHERE IT IS NOT DEGENERATE. The SoC question is asked
AFTER `wanted > 0` is established - i.e. only where a forced discharge is
actually being asked for. Asked earlier it would grade a quantity that
governs nothing, and would report "floor-limited" on a quiet afternoon
where the guard was never going to discharge anyway.
4. AN UNREADABLE SoC IS NOT A FULL BATTERY. `soc=None` is floor-limited, not
permitted. This is `PeakReading`'s lesson in the one place it bites
hardest: a full battery is exactly the value that would disarm this floor,
so a failed read defaulting to one would silently restore the unbounded
discharge this card exists to remove. Both new arguments are REQUIRED
keyword-only and neither has a default, which is TEST-14 read as a rule
rather than as a story: a required field that gains a default is how three
separate files in this repo shipped a quiet fallback.
AND FAILING TO SHAVE MUST NOT BE SILENT. The whole point of the guard is the
euro figure, so a floor-limited cycle carries `floor_limited=True` and main.py
raises ONE notification on the edge, saying the quarter will be exceeded - the
same shape, and the same argument, as `saturated`. The two are mutually
exclusive by construction: a floor-limited decision asks for nothing, so it
cannot have run out of inverter.
------------------------------------------------------------------------------
CFG-03: TWO NEW OPTIONS, MIRRORED ONCE, AND WHY BOTH ARE NEEDED
SAFETY-07 added none and said so. This card cannot: an EMS-side floor is a
number only the site knows, and refusing one set below the pack's own cut-out
means knowing that cut-out too.
* `peak_soc_floor` - the EMS floor. ⚠️ NOT `maintenance_soc_floor`. That is
the maintenance cycle's DRAIN TARGET: the depth a monthly recalibration
deliberately goes to, once, under supervision. This is the depth a
money-saving discharge may never reach, on any day of the month. Sharing
one key would couple two features whose right values differ and would widen
CFG-03 rather than narrow it.
* `bms_soc_floor` - the pack's own cut-out (register 0x0560 on the reference
unit, 10 %). It is here so validate.py rule 8 can REFUSE a `peak_soc_floor`
at or below it. That configuration is CFG-01 rule (a): the operator states
a floor, the BMS enforces a different one, and the status page reports an
energy backstop that can never bind.
Both defaults are mirrored ONCE, in `PEAK_OPTION_DEFAULTS` below, which is
TARIFF-02's `PRICE_OPTION_DEFAULTS` pattern - imported by validate.py and by
main.py, pinned to config.yaml by test_peakguard.py rather than transcribed.
This module still reads no option itself: `decide()` takes both as ARGUMENTS,
makes no `.get()` call, and the mirror is a default table, not a read.
⚠️ A DECISION RECORDED, NOT A RENAME DONE. Now that a second SoC floor exists,
`maintenance_soc_floor` is the ambiguous name of the two - it reads like "the
SoC floor" and is not. Renaming it is an option migration on a live install for
a clarity gain, so the decision recorded here is: KEEP IT, and carry the
distinction in DOCS.md and in this comment. Revisit if a THIRD floor appears -
a loop-side guard would be the third - at which point the migration buys enough
to be worth its cost.
------------------------------------------------------------------------------
AND IT READS NO PER-PHASE QUANTITY, ON PURPOSE
@@ -287,8 +368,44 @@ BLIND_FROZEN = "SAFETY-18: the meter is answering without changing"
CEILING_CAP = "peak_cap_w"
CEILING_MONTH_PEAK = "the month's banked peak"
# SAFETY-24. Why the forced discharge may not be sized. Reasons, not booleans,
# for BLIND_*'s reason: the status page, the action log and the operator
# notification all have to say WHICH, and "floor-limited" alone is unactionable.
FLOOR_SOC_UNREADABLE = ("the battery state of charge was not measured this "
"cycle, and an unreadable SoC is not a full battery")
FLOOR_AT_SOC_FLOOR = "state of charge is at or below the peak guard's SoC floor"
SOURCE = "peak"
# ⚠️ CFG-03. THE ONE IN-CODE MIRROR OF config.yaml FOR THIS CARD'S OPTIONS, and
# it is TARIFF-02's `PRICE_OPTION_DEFAULTS` pattern for TARIFF-02's reason: the
# add-on already carries option defaults in three places (config.yaml's
# `options:`, config.yaml's `schema:`, and main.py's literal `.get()`
# fallbacks), and a fourth is the open ticket. validate.py and main.py both
# import THIS dict rather than typing a literal, and test_peakguard.py pins it
# to config.yaml by READING the file - so a drift goes red instead of shipping.
#
# ⚠️ NOT A READ. `decide()` takes both values as arguments and this module makes
# no `.get()` call at all; the dict is a default table for its two callers.
PEAK_OPTION_DEFAULTS = {
# The EMS-side floor on SAFETY-07's forced discharge, in percent.
#
# 20 is DERIVED from the two numbers this repository already has, not
# chosen. The reference pack is 300 Ah at 51.2 V with DoD 90 - 13.8 kWh
# usable, the figure validate.py's rule 6 note already quotes - and the
# 2026-08-20 replay models 878 Wh of guard discharge in a day, which is
# 6.4 % of that. A floor 10 points above the pack's own 10 % cut-out
# therefore leaves the guard roughly 1.6 of its worst observed days of room
# before it stops, which is the smallest margin that lets the guard do its
# job on a bad day and still never reach the BMS.
"peak_soc_floor": 20,
# The pack's OWN cut-out, in percent: what the BMS enforces regardless of
# anything here. On the reference unit this is register 0x0560, read as 10,
# and it is consistent with DoD 90 on a full pack. It exists so validate.py
# rule 8 can refuse a `peak_soc_floor` at or below it - see that rule.
"bms_soc_floor": 10,
}
@dataclass(frozen=True)
class PeakDecision:
@@ -309,9 +426,19 @@ class PeakDecision:
excess_w: float = 0.0
wanted_w: float | None = None # the floor before `achievable_w` clamped it
saturated: bool = False
# SAFETY-24. True when a forced discharge WAS wanted and the EMS-side SoC
# floor refused to size it. Mutually exclusive with `saturated` by
# construction: this branch asks for nothing, so it cannot have run out of
# inverter. `soc_floor_reason` says WHICH of the two ways it bound.
floor_limited: bool = False
soc: float | None = None
soc_floor: float | None = None
soc_floor_reason: str | None = None
reason: str = ""
def explain(self) -> str:
if self.floor_limited:
return self.reason
if not self.engaged:
if self.projected_w is None:
return f"peak guard idle - {self.reason}"
@@ -383,7 +510,44 @@ def blindness(projection, max_hold_s, frozen: bool) -> str | None:
return None
def decide(projection, *, batt_w, cap_w, reading, achievable_w,
def discharge_permitted(soc, soc_floor) -> str | None:
"""SAFETY-24. May the guard size a forced discharge, or why may it not?
Returns None when it may, and otherwise the REASON it may not - which the
decision, the status page, the action log and the operator notification all
carry, because "floor-limited" on its own tells nobody what to do.
THE ONE PLACE THIS QUESTION IS ANSWERED, and it is public for the reason
`freeze.looks_frozen()`, `watchdog.loop_is_following()` and
`shaping_suspended()` are: a safety predicate transcribed into its own check
is one copy plus a future disagreement about what the check means, and the
copy that drifts is always the grading one.
⚠️ `soc is None` IS NOT PERMISSION. A failed read is not an observation, and
a FULL BATTERY is exactly the value that would disarm this floor - so a
`float(soc or 100)` here would restore the unbounded discharge this card
removes, silently, on the one install whose SoC sensor is misnamed. There is
deliberately no default argument on either parameter.
⚠️ AND `soc_floor is None` IS NOT PERMISSION EITHER, for the same reason one
step further out: a caller that has no floor configured has not thereby been
granted an unbounded one.
Boundary: `<=`, not `<`. A pack sitting exactly ON the floor has already
spent everything this guard was allowed to spend; permitting one more cycle
there would make the floor mean "one cycle below the floor".
"""
if soc is None:
return FLOOR_SOC_UNREADABLE
if soc_floor is None:
return ("no peak guard SoC floor is configured, so there is nothing to "
"bound a forced discharge with")
if float(soc) <= float(soc_floor):
return FLOOR_AT_SOC_FLOOR
return None
def decide(projection, *, batt_w, cap_w, reading, achievable_w, soc, soc_floor,
max_hold_s=None, frozen: bool = False) -> PeakDecision:
"""The whole guard. Pure: no clock, no I/O, no state carried between calls.
@@ -395,6 +559,13 @@ def decide(projection, *, batt_w, cap_w, reading, achievable_w,
claim can NEVER produce `lo > hi` in the resolver - a
contradiction there resolves to 0 W, which would park the
battery in the exact minute it should be discharging hardest.
`soc` SAFETY-24. MEASURED state of charge, percent. None when the
sensor for it did not answer - which is floor-limited, not
permission.
`soc_floor` SAFETY-24. The percent below which this guard may not spend
the pack for money. REQUIRED, and deliberately without a
default: a default here would be CFG-03's fourth defaults
site and TEST-14's quiet fallback in one line.
"""
ceiling, from_where = quarter_ceiling_w(cap_w, reading)
ceil_top = abs(float(achievable_w))
@@ -454,12 +625,41 @@ def decide(projection, *, batt_w, cap_w, reading, achievable_w,
wanted_w=wanted,
reason="the load is under the level this quarter can afford")
# SAFETY-24 - AND ONLY HERE. `wanted > 0` is now established, so a forced
# discharge really is being asked for and the SoC is the quantity that
# decides whether it may be sized. Asked one line earlier it would grade a
# number that governs nothing on every quiet cycle of the day, and would
# report "floor-limited" over a guard that was never going to discharge.
denied = discharge_permitted(soc, soc_floor)
if denied is not None:
# 0.0, not `wanted`, and not a negative number. Zero forbids CHARGING -
# one-directional, cannot cost pack energy - and forbids nothing else;
# a `lo` limit structurally cannot cap the discharge the loop is already
# doing. See the module note: this bounds THE GUARD's forced discharge,
# not the grid-following loop, which still has no SoC guard.
return PeakDecision(
engaged=True, floor_w=0.0, ceiling_w=ceiling,
ceiling_from=from_where, projected_w=projected, excess_w=ex,
wanted_w=wanted, floor_limited=True, soc=soc, soc_floor=soc_floor,
soc_floor_reason=denied,
reason=(
f"PEAK GUARD FLOOR-LIMITED: the quarter projects "
f"{projected:.0f} W against a {ceiling:.0f} W ceiling "
f"({from_where}) and {wanted:+.0f} W of discharge would hold "
f"it, but {denied}"
+ ("" if soc is None else f" ({float(soc):.0f} %")
+ ("" if soc is None or soc_floor is None
else f" vs {float(soc_floor):.0f} %")
+ ("" if soc is None else ")")
+ " - commanding no forced discharge, so THIS QUARTER WILL GO "
"OVER THE CEILING. Charging stays blocked (floor 0 W)"))
floor = min(wanted, ceil_top)
saturated = floor < wanted
return PeakDecision(
engaged=True, floor_w=floor, ceiling_w=ceiling, ceiling_from=from_where,
blind=None, projected_w=projected, excess_w=ex, wanted_w=wanted,
saturated=saturated,
saturated=saturated, soc=soc, soc_floor=soc_floor,
reason=(
f"PEAK GUARD: quarter projects {projected:.0f} W against a "
f"{ceiling:.0f} W ceiling ({from_where}) with "
@@ -513,5 +713,13 @@ def claim(decision: PeakDecision) -> Claim | None:
"""
if not decision.engaged or decision.floor_w is None:
return None
# Three reasons, not two. A `lo = 0 W` claim reaches the arbiter from THREE
# different states now - blind, floor-limited, and a genuine floor that
# happens to be zero - and the arbiter's explain() is what an operator reads
# when the battery stops charging. Spelling them the same way would make the
# SAFETY-24 case indistinguishable from the SAFETY-18 one on the one line
# that gets read.
return Claim.limit(SOURCE, P_PEAK, lo=float(decision.floor_w),
reason=("blind" if decision.blind else "quarter peak"))
reason=("blind" if decision.blind
else "SoC floor" if decision.floor_limited
else "quarter peak"))