UX-08 phase 2a: short, plain notifications, and a ratchet for the rest
checks / unit (pull_request) Successful in 8m54s
infra-pin / fresh (pull_request) Successful in 1s

Every HA notification the add-on raises now has a title of at most 5
everyday words and a one-sentence body; a red alarm may use three so no
action is lost. The texts live in app/messages.py, the rule in STYLE.md,
and the detail that moved out in DOCS.md "Notifications", which each body
links to. No notification_id, trigger or cadence changed.

test_messages.py enforces the rule on every notification, fails any
notification worded outside app/messages.py, and ratchets the kinds not yet
rewritten (rows, banners, refusals, logs, options) through a shrinking
allowlist of 200. Six tests re-pinned from prose to meaning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
This commit is contained in:
glenn schrooyen
2026-09-27 01:00:54 +02:00
co-authored by Claude Opus 5.5
parent 2503de124e
commit 50ccea3dd7
13 changed files with 1170 additions and 185 deletions
+44 -133
View File
@@ -53,6 +53,7 @@ from .clamp import (
clamp_to_rating, is_commandable, loop_clamp_note,
)
from . import freshness
from . import messages
from .freeze import (
FreezeWatchdog, FreezeWatchdogConfig, looks_frozen, rail_unreachable, rail_w,
)
@@ -997,6 +998,8 @@ class Controller:
# the value a clamp bit is a value written to ONE device.
# (engaged, note, over_rating) - SAFETY-40 splits the last one out.
self.loop_clamped: tuple[bool, str, bool] = (False, "", False)
# UX-08: the numbers of a loop-side over-rating, for the notification.
self.loop_clamp_facts: tuple = ()
self.stopping = False
# SAFETY-61. time.monotonic() at the end of the last run_control() pass.
self.loop_pass_mono = None
@@ -3280,6 +3283,9 @@ class Controller:
loop_clamp_note(decision),
decision.clamped,
)
self.loop_clamp_facts = (
(messages.Clamp(None, decision.requested_w, decision.clamped_w,
"rating"),) if decision.clamped else ())
# SITE. compute() regulates `grid_w` - the P1 measurement - to
# zero. Its subject is the meter, and §6.2's architecture is ONE
# integrator on P1 whose output is then SPLIT. This is the claim the
@@ -3303,6 +3309,7 @@ class Controller:
# rather than letting the last value stand is what makes the alarm
# de-assert when control is stopped.
self.loop_clamped = (False, "", False)
self.loop_clamp_facts = ()
# SAFETY-28. A new resolution is a new tick, and the previous record is
# stale from this line onwards. Incremented BEFORE resolve() so that the
@@ -3726,10 +3733,17 @@ class Controller:
if engaged_write:
reason = (", ".join(f"{inv.name}: {inv.clamp_last.describe()}"
for inv in clamped_units) + ". " + CLAMP_BUG_SIGNAL)
_named = len(self.inverters) > 1
facts = [messages.Clamp(inv.name if _named else None,
inv.clamp_last.requested_w,
inv.clamp_last.applied_w,
inv.clamp_last.bound)
for inv in clamped_units]
else:
reason = loop_note if engaged else ""
facts = self.loop_clamp_facts if engaged else ()
fired = self.clamp_wd.note(engaged, reason, datetime.now(timezone.utc),
bug=bug)
bug=bug, facts=facts)
# --- THE ACTION LOG, AND IT IS COALESCED PER EPISODE ------------------
# Every engagement is recorded, but consecutive ones are recorded by
@@ -3766,14 +3780,8 @@ class Controller:
# it can never replace a BUG SIGNAL notice still on screen.
_LOG.warning("SAFETY-03 - %s Engagements so far: %d.",
self.clamp_wd.reason, self.clamp_wd.engagements)
await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_integrator_bound",
"title": "GoodWe: the control loop is at its integrator bound",
"message": (f"{self.clamp_wd.reason}\n\nNo command exceeded "
f"the rating and the controller is STILL RUNNING. "
f"See the add-on documentation, 'The winding-loop "
f"alarm'."),
})
await self.hass.call("persistent_notification", "create",
messages.loop_bound().data())
return
_LOG.error(
"SAFETY-03 - %s Silently clipping this is how a winding integrator "
@@ -3785,16 +3793,10 @@ class Controller:
# A fixed notification_id, for the reason check_meter_frozen() gives -
# and a DIFFERENT one from every other alarm this add-on raises, because
# a clamp is not a frozen meter and neither is a saturation freeze.
await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_output_clamp",
"title": "GoodWe: a computed setpoint exceeded the rating",
"message": (
f"{self.clamp_wd.reason}\n\nThe command has been clamped and "
f"the controller is STILL RUNNING. This is NOT the inverter "
f"failing to follow a legal command (that is the saturation "
f"freeze, which is normal and does not alarm) - the value asked "
f"for was itself out of range."),
})
# UX-08: the words are messages.output_clamp()'s; the numbers are the
# ones `reason` was written from (ClampWatchdog.facts), asked first.
await self.hass.call("persistent_notification", "create",
messages.output_clamp(self.clamp_wd.facts).data())
async def check_peak_saturated(self) -> None:
"""SAFETY-07. Alarm ONLY when the guard has run out of inverter.
@@ -3836,18 +3838,8 @@ class Controller:
# A fixed notification_id, and a DIFFERENT one from every other alarm
# here - check_meter_frozen() gives the reasoning. A saturated peak guard
# is not a frozen meter and is not an out-of-range setpoint.
await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_peak_saturated",
"title": "GoodWe: the capacity-peak guard has run out of inverter",
"message": (
f"{g.reason}.\n\nThe battery is being commanded to everything "
f"it can give and the quarter is STILL projected over the "
f"{g.ceiling_w:.0f} W ceiling, so this quarter will set or raise "
f"the month's billed capaciteitstarief peak. This is not a "
f"controller fault - the load is simply larger than the "
f"inverter. Shedding load in the next few minutes is the only "
f"thing that changes the bill."),
})
await self.hass.call("persistent_notification", "create",
messages.peak_saturated(g.ceiling_w).data())
async def check_peak_floor_limited(self) -> None:
"""SAFETY-24. Alarm when the EMS SoC floor stops the guard shaving.
@@ -3912,45 +3904,23 @@ class Controller:
# think the pack is AT it. `decide()` guards its own reason string the
# same way; a f-string that formats a None here would turn the one
# notification that matters into a traceback.
_at_floor = ("at the floor" if g.soc_floor is None
else f"at {float(g.soc_floor):.0f} %")
# ⚠️ SAFETY-32. WHICH PACK STOPPED THE SITE. `g.soc_floor` is the floor
# the decision was actually taken with, so this re-grades nothing and
# cannot disagree with the alarm it is printed inside. At a roster of one
# the sentence is omitted entirely and this notification is byte-for-byte
# what SAFETY-25 shipped - there is exactly one pack and naming it would
# be noise on every existing install.
# no pack is named - there is exactly one.
_blocked = [name for name, why
in peakguard.pack_permissions(self.packs, g.soc_floor)
if why is not None]
_which = ("" if len(self.inverters) < 2 else
f"\n\nSAFETY-32 - WHICH PACK: this is graded per pack against "
f"that pack's own BMS cut-out (bms_soc_floor), and the "
f"pack(s) at or below their own floor right now are "
f"{', '.join(_blocked) if _blocked else '(none - see the reason above)'}"
f". The other packs on the roster are not the reason the guard "
f"stopped and raising their cut-out will not change this.")
await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_peak_soc_floor",
"title": "GoodWe: the capacity-peak guard has run out of battery",
"message": (
f"{g.reason}.\n\nTHIS QUARTER WILL GO OVER THE "
f"{g.ceiling_w:.0f} W ceiling and will set or raise the "
f"month's billed capaciteitstarief peak, because the peak guard "
f"is not allowed to discharge any further: {g.soc_floor_reason}."
f" This is not a controller fault - it is the EMS-side energy "
f"floor (peak_soc_floor) doing what it is set to do, and it sits "
f"deliberately above the pack's own BMS cut-out. Charging stays "
f"blocked. Shedding load is the only thing that changes the bill "
f"now; if this pack can afford a deeper discharge, lower "
f"peak_soc_floor.\n\nSAFETY-25 - WHAT THIS FLOOR IS NOT: "
f"peak_soc_floor bounds THE PEAK GUARD ONLY. It is not a "
f"pack-wide reserve, and the battery is NOT being held "
f"{_at_floor} - ordinary self-consumption has no SoC "
f"floor at all and will go on discharging past it, down to the "
f"pack's own BMS cut-out. What has stopped is the guard buying "
f"you a lower bill, not the pack being spent." + _which),
})
# ⚠️ SAFETY-25's sentence (the reserve binds the peak guard only) is
# part of messages.peak_reserve() on every branch, the unreadable-SoC
# one included.
await self.hass.call("persistent_notification", "create",
messages.peak_reserve(
g.ceiling_w, soc_unread=g.soc is None,
reserve_unset=(g.soc is not None
and g.soc_floor is None),
packs=(_blocked if len(self.inverters) > 1
else ())).data())
async def check_circulating(self) -> None:
"""SAFETY-31 + SAFETY-34. The alarm, and it must survive a restart.
@@ -3989,36 +3959,13 @@ class Controller:
names = self.units
pos, neg = opposing([inv.batt for inv in self.inverters],
min_w=self.circ_wd.cfg.min_w / 2.0)
who = ""
if pos and neg:
who = ("\n\nRight now " + ", ".join(names[i] for i in pos)
+ " is discharging while " + ", ".join(names[i] for i in neg)
+ " charges.")
# A fixed notification_id, and a DIFFERENT one from every other alarm
# this add-on raises - check_meter_frozen() gives the reasoning. A
# circulating loop is not a frozen meter, is not an out-of-range
# setpoint and is not a saturated peak guard.
await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_circulating_power",
"title": "GoodWe: the inverters are pushing against each other",
"message": (
f"{self.circ_wd.reason}{who}\n\nTHE METER CANNOT SEE THIS. "
f"Circulating power nets to zero at P1, so the site reads idle "
f"while the packs are cycled through two conversion stages "
f"continuously. Every other guard in this add-on is a consumer "
f"of that net figure, which is why none of them fired.\n\n"
f"CONTROL HAS BEEN STOPPED - every inverter is commanded 0 W "
f"and held there. That removes this controller as one of the "
f"two participants, and it makes anything still happening "
f"VISIBLE at the meter instead of cancelled at it.\n\n"
f"⚠️ IF THE GRID READING DOES NOT MOVE AFTER THIS, one inverter "
f"is no longer listening to this add-on and no command will "
f"reach it - that has happened twice on this hardware, once for "
f"6 h 25 m. The remaining actuator is a human at the isolator.\n"
f"\nThis alarm survives a Home Assistant restart. Restarting "
f"will not clear it; start control from the add-on UI once you "
f"know why two inverters were opposing."),
})
# this add-on raises - check_meter_frozen() gives the reasoning.
await self.hass.call("persistent_notification", "create",
messages.circulating(
[names[i] for i in pos] if pos and neg else (),
[names[i] for i in neg] if pos and neg else (),
).data())
def _reannounce_due(self, notification_id: str) -> bool:
"""SAFETY-34: has this latched alarm gone REANNOUNCE_S without being re-asserted?"""
@@ -4075,38 +4022,8 @@ class Controller:
# surface is best-effort by design.
self._delivery_announced.add(inv.name)
self._announced_mono[nid] = _mono()
peers = ", ".join(n for n in names if n != inv.name) or "no sibling"
await self.hass.call("persistent_notification", "create", {
"notification_id": nid,
"title": f"GoodWe: {inv.name} is not delivering its share",
"message": (
f"{inv.delivery_wd.reason}\n\nWHY NOTHING ELSE SAID SO. A "
f"unit sitting at 0 W against a non-zero share is not "
f"pushing the wrong way, it is ABSENT: the per-unit sign "
f"check needs opposition on both sides of "
f"{self.unit_sign_cfg.min_w:.0f} W and never opens, the "
f"site sum still agrees with the meter in "
f"direction, and circulating power is exactly zero when one "
f"unit does nothing. The only other trace is a saturation "
f"freeze, which this add-on's own code calls an alarm "
f"operators learn to ignore.\n\nNOTHING HAS BEEN COMMANDED "
f"DIFFERENTLY. This guard reports and does not act. "
f"Commanding an inverter that has stopped listening "
f"achieves nothing - that has happened on this hardware, "
f"once for 6 h 25 m - and the only re-allocation this "
f"add-on can express is all-or-nothing, which would throw "
f"away the output of a unit that is delivering PART of its "
f"share. So {peers} is still regulating and the site is "
f"short by up to {inv.rating_w():.0f} W until somebody "
f"looks at {inv.name}.\n\nWHAT TO CHECK, in order: is "
f"{inv.name} answering on RS485 at all; is its pack colder "
f"or emptier than its configured bms_soc_floor says (if it "
f"is, that setting does not describe this battery and this "
f"alarm is telling you so); and is anything else writing to "
f"it.\n\nThis alarm survives a Home Assistant restart. "
f"Restarting will not clear it; start control from the "
f"add-on UI once you know why."),
})
await self.hass.call("persistent_notification", "create",
messages.not_delivering(inv.name, slug).data())
async def check_meter_frozen(self) -> None:
"""SAFETY-18. Alarm on a meter that answers forever without changing.
@@ -4144,14 +4061,8 @@ class Controller:
# ⚠️ A fixed notification_id, so a second episode REPLACES the first
# rather than stacking. An operator who has to dismiss eleven identical
# notifications learns to dismiss the twelfth without reading it.
await self.hass.call("persistent_notification", "create", {
"notification_id": "goodwe_meter_frozen",
"title": "GoodWe: the P1 meter may be frozen",
"message": (
f"{self.freeze_wd.reason}.\n\nThe controller is STILL RUNNING "
f"and still following this reading. Check the meter; if it is "
f"stuck, stop control from the add-on UI."),
})
await self.hass.call("persistent_notification", "create",
messages.meter_frozen(unchanged).data())
# -- tasks ---------------------------------------------------------------
# -- OPT-04: the background tariff refresh -------------------------------