862f0fa574b4072b7e35c4d4a70415dbfa2ccbe8
70
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  adminandClaude Opus 5
|
862f0fa574 |
Merge release/1.0 (TARIFF-01) into TARIFF-02
checks / unit (pull_request) Successful in 2m39s
TARIFF-01 (#17) landed on release/1.0 first, so this branch was two commits behind and Gitea reported the PR unmergeable. Integrating rather than rebasing: the two reviewed commits stay addressable, and the branch is protected against force-push anyway. ONE CONFLICT, AND IT IS THE BORING HALF OF THE ONE REVIEW PREDICTED. Review of #16 expected collisions in four places - the tail of `options:`, the tail of `schema:`, the rule list inside validate_startup_config(), and the tail of validate.py. None of them happened: TARIFF-01 touched neither config.yaml nor validate.py. It added app/tariff.py, test_tariff.py, two P1 scenario captures, and edits to main.py and p1.py, none of which this branch goes near. The only conflict was CHANGELOG.md, where both branches prepended a block under the same `## Unreleased` heading. Both blocks kept, TARIFF-02 above TARIFF-01 since it lands second. Nothing was dropped or reworded on either side, and the resolution adds no line of its own beyond the blank one between them. Checked rather than assumed after the merge: - all ELEVEN test files pass, including TARIFF-01's own test_tariff.py (which this branch had never run) and the 93 s test_loop.py; - the 24-mutation battery still has 0 survivors; - no semantic overlap in config: TARIFF-01's peak tracker reads the peak_cap_w / peak_forecast_entity options that already existed, so the price options added here neither shadow nor duplicate anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
adminandClaude Opus 5
|
e102cde841 |
TARIFF-02 review: close the three other doors to the markup, and stop overclaiming
checks / unit (pull_request) Successful in 2m34s
Review of PR #16 wrote a double-counting plugin four ways. PluginOptions blocked one. The one that decided it touched nothing but PRICE_OPTION_DEFAULTS - a public dict in the module every plugin must already import for register() - so "the number is not in scope" was simply false, and an absolute guarantee a reviewer trusts is worse than a stated limit, because they stop checking. register() now refuses, AT IMPORT, any plugin whose own code mentions a framework-owned name: FRAMEWORK_KEYS plus PRICE_OPTION_DEFAULTS, price_option and _o. code_leaks() parses the class and drops every docstring node before searching, because every plugin is REQUIRED to name its sourced components in prose - engie_dynamic says "markup" in its docstring and a raw text scan false-positives on the documentation the same file mandates. A check that fires on required documentation is a check somebody deletes. test_price.py asserts that a raw scan really would have false-positived, so the strip is proven load-bearing rather than asserted. It fails OPEN if inspect.getsource() cannot find the .py, which the add-on image always can. Refusing to start a battery controller because a static analysis could not run would trade a real outage for a hypothetical accounting error - so instead the checks assert source was found for all five shipped plugins, and a build that lost it is a red check rather than a quietly disarmed guard. All three reachable attacks are written out as plugins in the suite and each must be refused at registration. The cost is measured, not asserted: honest wholesale 0.188, every attack 0.219 - one markup high, every hour, invisible in every log this add-on writes. The guarantee is now stated at its true size everywhere it appears: the framework adds the complement exactly once, GIVEN A CORRECT sources DECLARATION. A plugin that declares {ENERGY} while its feed is all-in still doubles while touching no framework name, and no code can catch that - the declaration is an unverifiable claim about somebody else's tariff. Measured in the suite (0.276) so the limit is a number rather than a caveat. The docstring/declaration check verifies that a plugin's two claims AGREE, not that either is true; that now says so. The three surviving mutations: M3 - _NoData's reason taking a default. This is the identical survivor left open on RIG-01's _Unread.cause: the shape was borrowed from sim/riglock.py including its blind spot, which is what borrowing a shape does if nobody re-runs the mutations against the copy. Closed by SIGNATURE - no constructor or handler of an outcome type may carry a default - so the next one is red without anyone writing a new case. M5 - the ENERGY-required guard was untested. Its consequence is that adds becomes {ENERGY, TAXES} and the emitted price silently omits the energy it is pricing. M7 - deleting the TLS refusal went green AND the check named for it still passed, because the old parser stripped a fixed len("https://") and mangled http://api.energy-charts.info/p into the host pi.energy-charts.info, which then failed the ALLOW-LIST. An assertion that does not test what it names, inside a security check. Fixed on both sides: the scheme is parsed as a scheme, so an http URL now resolves to its real and declared host and only the TLS rule can refuse it; and the check asserts the REASON. Fixing it surfaced M7b - the no-scheme guard was survivable the same way - found by mutating my own fix, and closed the same way. Also carried forward: rule 7's refusal is correct forever but its stated reason ("the provider decides nothing") expires the moment TARIFF-03 or OPT-04 lands. test_price.py section 12 now greps app/ by AST for a load_prices consumer and goes red when one appears, telling whoever lands it to update the message in the same commit. The first draft of that tripwire used a substring and matched the comment in validate.py explaining the tripwire - the same false-positive class as the plugin docstrings, hit twice in one ticket, and fixed the same way. Documented rather than coded around: vars(read).get("points", []) and bool(read) both still get past the absence type, and neither is worth code - defining __bool__ would turn a line that reads like a guard into one that IS a guard by accident, which is how .get(k, default) got here in the first place. Stated plainly in DOCS/CHANGELOG/PR: every fixture is synthetic, so the five feed URLs and JSON shapes are verified against a belief about what those suppliers publish rather than against the feeds. A wrong shape degrades to no-price-data, not to a wrong price. Review's own DST verification recorded: 149,040 hourly instants, 2020-2036, zero mismatches against zoneinfo. The residual is political, not arithmetic, and the docstring now says so. 24 mutations driven through the suite, 0 survivors. Whole add-on suite green. No deleted lines outside this ticket's own files; main.py still untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
admin
|
0dbd485b5a |
TARIFF-01: measure the month's billed capacity peak, and make it survive a restart (#17)
The instrument SAFETY-07 will read: the highest clock-aligned 15-minute average offtake of the calendar month, persisted in /data/tariff.db across two retention tiers, with a monthly peak that recompute_month() proves has not diverged from the 15-minute tier. Files no claim, applies no clamp, commands nothing.
AC 1 was already built and unwired - p1.QuarterAverager on the base already does clock-aligned 900 s blocks, time-weighted over irregular spacing, offtake-only, per-phase, with the max_hold_s gap rule; verified on
|
||
|
glenn schrooyenandClaude Opus 5
|
f4914a174a |
TARIFF-01: measure the month's billed peak, and make it survive a restart
checks / unit (pull_request) Successful in 2m38s
The 15-minute block average was already built and unwired. p1.QuarterAverager computes clock-aligned 900 s blocks, time-weighted over irregular telegram spacing, offtake-only and per-phase; P1Ingest.blocks was a 96-entry list in memory that a restart erased. So this is the persistence and the monthly reduction, not a second accumulator - a second one would be a second source of truth for the exact quantity the money depends on. BLOCKS, NOT A SLIDING WINDOW. The card says "rolling"; 9.1 bills "the highest 15-minute average offtake" and states its own worked case per QUARTER - one 4998 W quarter moving a monthly peak from 4832 to 5013 W. A trailing window reports a peak the meter never bills, and reports it HIGH: a burst straddling :15 fills the window completely while contributing to two block averages at partial weight. DELIVERED, NOT INTENDED. tariff.py only ever sees P1Sample - the meter's own reading - and never a setpoint. SAFETY-20's open question does not arise for a capacity tariff: the utility integrates the current that flowed. AN UNREADABLE STORE IS NOT A ZERO PEAK. peak() answers OBSERVED / NO_RECORD / NOT_OBSERVED with peak_w None unless observed, which is riglock.holder()'s fix applied to something that spends money. A peak read as 0 W hands SAFETY-07 maximum headroom and disarms the guard at the moment its own storage breaks. Restart mid-block resumes the accumulator through a new snapshot()/restore() pair, checkpointed every 15 s and cleared in the same breath as a block close. The held sample keeps its ORIGINAL timestamp, so an outage is integrated under the same max_hold_s rule a running process would apply. quarter_start is the primary key and a re-bank may only ever RAISE a figure. Calendar months are LOCAL: a quarter starting 22:15 UTC on 31 August bills in September. No new option (the limit is the existing peak_cap_w), so config.yaml and validate.py are untouched and CFG-03 is not widened. No second peak entity: peak_forecast_entity stays the input to maintenance_charge_floor(), because repointing that consumer changes what moves energy and belongs to SAFETY-07. AC 3 is NOT met. It asks for 30 days of replay; two days of this house's real meter exist and are committed here. test_tariff.py part 9 replays them, reports the percentage next to the sample size in days, and asserts nothing about 95 %. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
adminandClaude Opus 5
|
852fc7ced6 |
TARIFF-02: one canonical price format, and a seam that keeps supplier names out of the optimiser
checks / unit (pull_request) Successful in 2m33s
Five provider plugins - generic_dayahead, engie_dynamic,
totalenergies_dynamic, eneco_eddy, manual_csv - all emitting exactly
{"prices": [{"start", "eur_per_kwh", "provisional"}]}, tz-aware at the
Brussels offset, EUR/kWh, all-in. Nothing consumes it yet; the seam
lands first, per the PM's ordering.
It is built in goodwe_controller/app/ rather than at specs.md 2.2's
ems/ingest/prices/, per the product owner's decision of 2026-08-30. The
behaviour 9.3 / 8.3 / 11.4 specifies is unchanged.
Three things shape the design, and each is the mechanism rather than a
comment saying to be careful:
Double-counting the markup is unreachable. A plugin does not return a
price - it returns the figure its source published and declares which of
{energy, markup, taxes} that figure already contains. assemble() adds
the complement, once, in the framework. A plugin is never handed
price_markup_eur_kwh: PluginOptions raises FrameworkOption on it, so the
line that adds the margin a second time cannot be written. Engie and
TotalEnergies publish a retail figure with their margin inside; adding
the configured 3 c/kWh on top would have made every arbitrage decision
wrong by that amount with nothing reporting it.
The product type is declared, not inferred from the plugin's name.
eneco_eddy is real-time/imbalance and gets 8.3's widened thresholds
through capabilities().product and volatility_regime(), never through a
name comparison - which is AC2. test_price.py asserts mechanically that
no plugin id appears anywhere in the framework's code, and drives a
sixth provider defined inside the test file to the right regime with
price.py untouched.
"No price data" is a type, not an empty list - the same rework
sim/riglock.py took under TEST-04. A fetch returns PriceRead.series(...)
or PriceRead.no_data(reason), and only the first carries fields. An
empty series is not constructible; no_data has no .points, no .at(), and
.get("points", []) raises rather than returning the default. A day of
free electricity and a day with no prices are different instructions -
arbitrage is pointless under one and forbidden under the other - and an
implementation returning [] cannot tell them apart. Every site-level
failure returns no_data; nothing raises into the control tick.
Also: injection stays a separate scalar (export_price_eur_kwh, AC4),
negative prices survive the boundary validator (17's regression case),
egress is allow-listed against each plugin's own declaration (16), and
no tzdata is used - the shipped Alpine image has none, so Brussels
offsets come from the EU rule and instants are carried as UTC.
validate.py grows rule 7: an unknown provider id, manual_csv with no
path, and - the HA overlap the PM told us to decide about -
price_provider set alongside price_now_entity/price_avg_entity, which is
two price sources where the HA pair silently wins. The HA sensors are
kept and still work alone; TARIFF-03 and OPT-04 are what replace them.
CFG-03 is not widened: the price defaults have ONE in-code mirror,
PRICE_OPTION_DEFAULTS, which validate.py imports rather than restating
and which test_price.py pins to config.yaml by reading the file.
test_price.py is runnable, assert-free in the house style, and every
verdict it prints is derived. It grades the markup arithmetic with
0.031/0.057 rather than the shipped 0.03/0.06, because 0.06 is exactly
twice 0.03 and a degenerate pair would make double-counting
indistinguishable from correct addition - and it asserts that
non-degeneracy as its first section. The DST checks run on the 23-hour
and 25-hour days, not on a normal day where a fixed 24-slot
implementation and a correct one agree. Thirteen mutations were driven
through it; all thirteen go red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
|
||
|
admin
|
9aadf469bc |
TEST-06: pin the cycle() ordering, the NOT COVERED string and the shipped meter_freeze_s (#14)
Closes the three mutations that survived SAFETY-18 suite green. Ordering pinned by consequence (N8 red above the assignment, N3 green below it); NOT COVERED asserted in both directions; shipped meter_freeze_s and its schema bounds pinned. notes_of()/all_notes_of() split so this guard and the whole notification surface are separately nameable, with the three surface-wide claims decided per scenario after SAFETY-03. app/ change is comment-only (AST-identical). |
||
|
glenn schrooyenandClaude Opus 5
|
4b521b2df0 |
ci: trigger a run on the rebased head
checks / unit (pull_request) Successful in 2m33s
No content change. The gate never created a run for |
||
|
glenn schrooyenandClaude Opus 5
|
0942228d65 |
Merge release/1.0 (SAFETY-03) into TEST-06
checks / unit (pull_request) Canceled after 0s
Two conflicts, both predicted, both mine as the later merger. app/main.py - #12 inserted `await self.check_output_clamp()` on the line immediately above `await self.check_meter_frozen()`, so its hunk abuts my comment correction. Resolved so EACH CALL CARRIES ITS OWN BOUNDARY, because the two guards do not share one: - check_output_clamp() reads `self.clamp_last`, which is None until write_setpoint() has run, so its boundary IS the write; - check_meter_frozen() grades `self.target`, assigned further up, so its boundary is the assignment. Letting my correction generalise across both would have been worse than the vague comment it replaces, because it would read as deliberate. The AST of app/main.py is identical to merged release/1.0 (+1251 bytes, all comment). test_freeze.py - #12 independently added `frozen_notes()` keyed on the same id. `notes_of()` subsumes it: same id, plus the title fallback that keeps the id's own check independently falsifiable, plus `all_notes_of()` for claims about the whole surface. THE THREE SURFACE-WIDE ASSERTIONS, DECIDED ONE AT A TIME. Measured on the merged tree: scenario surface ids freeze frozen meter, railed command 2 frozen + clamp 1 TEL-05 converged, -14.2 W 0 - 0 moving meter, a real 8 kW load 1 clamp 0 uncovered transport, no unchanged_s 1 clamp 0 - the wiring scenario STAYS SURFACE-WIDE and now asserts exactly TWO alarms with BOTH IDS NAMED. The answer to "is this still meant to raise one alarm" is no, it raises two, and a third guard has to come back to this line. That is the one assertion in the file that accounts for the controller's entire alarm output; - the CONVERGED case stays surface-wide and stays silent: at the operating point the whole add-on should have nothing to say, that is still true, and narrowing it would give away a real property; - the other two NARROW to notes_of(). The clamp alarm there is correct and is SAFETY-03's subject: a real 8 kW load does drive the loop past the rating, and an uncovered transport does not stop it. Asserting whole-surface silence would make this file fail whenever another guard correctly does its job. Each still owns the load-bearing half - THE FREEZE DETECTOR SAID NOTHING - and the surface is PRINTED so a reader sees what else fired without this file grading it. Re-verified on the merged tree: M1 RED 4, M2 RED 5, M3 RED 3 + RED 2, N6 RED 1, N3 green (correctly), N8 RED 4, R1 RED 2, R2 RED 1. All nine suites green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
admin
|
6aabb832fc |
SAFETY-03: alarm when a computed setpoint exceeds the rating, do not just clip it (#12)
The clamp already existed; the alarm did not, and specs 10.3 records a silently clipped integrator that reached 14,768 W while every dashboard read a legal 5000 W. OUTPUT_CLAMP now alarms on every engagement, with its own reason code, notification_id, status row and counter, distinct from ordinary saturation. Clamp lives in write_setpoint(), the one path every write takes; the alarm runs after it, out of the write path. Not a trip: the clamped value is written and the loop keeps running. Reviewed over two rounds. Round one held the PR on three findings: clamp_to_rating() returned early from the max_w stage so the device bound was never applied when the first stage engaged (the bigger the overshoot, the less the guard did), and two checks that could not fail - range(CLEAR_CYCLES - 1) issued zero clean cycles at CLEAR_CYCLES=1, and REASSERT_S + 9 passed for any value including an hour of silence. All three fixed in |
||
|
glenn schrooyenandClaude Opus 5
|
d31a12b7c6 |
TEST-06: the id-selection note said 'the only assertion'; there are two
checks / unit (pull_request) Successful in 2m32s
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyen
|
2983d7c3b0 |
Merge remote-tracking branch 'origin/release/1.0' into TEST-06
checks / unit (pull_request) Canceled after 0s
|
||
|
glenn schrooyenandClaude Opus 5
|
611d3dbcdf |
TEST-06 review follow-ups: two notification surfaces, the real ordering boundary, the schema bounds
1. notes_of() could not tell this guard's alarm from anybody else's. It selected by SERVICE, so against #12's app/ (which adds goodwe_output_clamp) eight assertions went red, AC1's core check among them. Blanket id-keying is not the repair and was measured, not guessed: selector vs #12's app/ id renamed by service (as committed) 8 red 1 red by id alone 3 red 6 red by id or title (shipped) 3 red 1 red all_notes_of() is the whole surface; notes_of() is this guard's, recognised by the id it should carry OR the title it writes - the `or` is what keeps the one assertion whose subject IS the id able to fail on its own. The three surface-wide claims stay surface-wide, each with a comment naming which surface it is about and why the next guard's author must decide rather than narrow them. They are the three that go red against #12, which is the correct signal. 2. The ordering comment named the wrong boundary. N3 - moving the call to immediately before write_setpoint() - is green and correctly so: self.target is assigned two lines earlier, write_setpoint() clamps only a local and can skip the write entirely. So the invariant is "after `self.target = resolution.target_w`", and the detector grades pre-clamp INTENT, not the wire. Corrected in test_freeze.py and in main.py's own note above the call (comment-only; I merge second and own the conflict). N8 - moving the call above the assignment - is RED 4. 3. The schema range's own bounds are now pinned. int(60,3600) -> int(1,86400) survived green: an operator could set meter_freeze_s: 86400 and the guard is dead with nothing red. Both bounds are re-derived rather than restated. 4. find_hwsim() crashed the whole suite on a checkout one level below the root (HERE.parents[1] -> IndexError, reproduced at C:/goodwe_controller). Guarded. And part 3 now names its transport in the summary: the rig hwsim and the stub are not equivalent - the rig path failed 1 run in 3 - so a green log has to say which one backed it. Also: the "revived transport re-rails" check is labelled a PRECONDITION like its sibling; freeze_rows() records that flipping p1_enabled exercises the row's content and never its gate; and the note about the deleted assertion is corrected - it was REDUNDANT, not undecidable. The reviewer falsified it in one line (move the hass.call above `if not fired: return`, RED 9). "I could not construct a falsifier, therefore it cannot fail" was exactly the inference this file exists to distrust. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
46383c5ae5 |
Merge release/1.0 (CI-01) into SAFETY-03 so the required check can run
checks / unit (pull_request) Successful in 2m32s
Branch protection on release/1.0 now requires the context `checks / unit (pull_request)`, and CI-01's workflow arrived after this branch was cut - so no run had ever been triggered for this PR and the required context would have sat pending forever. Clean merge, no conflicts: .gitattributes, .gitea/ci/Dockerfile and .gitea/workflows/checks.yml only. No add-on source or check is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
admin
|
5e966ca59f | CI-01: this repository's first automated check (#13) | ||
|
glenn schrooyenandClaude Opus 5
|
4e66583a52 |
SAFETY-03 review fixes: both clamp stages always run, and two checks that could not fail
1. BEHAVIOURAL, and a regression this PR introduced. clamp_to_rating() returned early from the max_w stage, so the device bound was applied only when the rating bound had NOT fired - the bigger the overshoot, the less the guard did. At max_w 5000 against a +-4000 device, +9999 emitted +5000 W, outside the device range, while +4500 was correctly held to +4000. It undid the pre-PR guarantee "never send a value the device will reject outright" and undercut this module's own case for max_w-not-min(), which rests on that stage catching a validation that did not run. Both stages now always run; the result names both bounds when both bite (max_w+device). Asserted over a 492-point sweep, because the defect was invisible to any single-value check sitting inside one stage. 2. The hysteresis check issued range(CLEAR_CYCLES - 1) clean cycles, which at CLEAR_CYCLES = 1 issues zero, passes, and prints "0 clean cycles do NOT clear it" - green for the chatter setting the constant exists to prevent. Now: ONE clean cycle must not clear the alarm, a literal that does not move with the constant, and the de-assert count is measured by looping until it clears. 3. The re-assert check stimulated at REASSERT_S + 9, green for any value including 3600 - the hour of silence the module promises not to produce. Now stated in wall-clock terms: a loop pinned across the 900 s capacity-tariff quarter must speak at least 5 times and on no more than 20% of cycles. Pinned in both directions. Red confirmed for each: the restored early return (3 checks), CLEAR_CYCLES = 1 (1), REASSERT_S = 3600 (1) and REASSERT_S = 0 (3). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
c34b5a2922 |
TEST-06: close the three gaps that survived SAFETY-18's suite green
None of the three is a bug in the shipped code. They are gaps in what the
suite could detect, and the next person to touch freeze.py had less
protection than the passing count implied.
1. WHERE check_meter_frozen() runs in cycle(). Moving it to the top left
the whole file green. Pinned by CONSEQUENCE, not by source order, so a
refactor that keeps the semantics stays green:
- the alarm must land on the SAME cycle the command reaches the rail
(at the top it grades the previous command against this cycle's
meter and arrives a cycle late);
- the dead-poll-task composition: poll task dies, stale_input_s is
crossed, the arbiter commands 0 W, and on THAT cycle the alarm is
already clear - because the detector ran after the 0 W write.
wired() now returns a per-cycle trace, which is what makes "on which
cycle" an observation rather than a hope.
2. The NOT COVERED status row. The old check only asserted that an
uncovered transport is SILENT, which a page claiming `armed` on a guard
that cannot run also satisfies. The string is now asserted in both
directions: the uncovered row must say NOT COVERED, name the
meter_source and homewizard_local, and warn; the covered row must still
say armed, or "does not say armed" would be satisfied by deleting the
armed branch.
3. meter_freeze_s. Pinned in test_validate.py by MAINT-04's regex
technique (no PyYAML in the image), and in test_freeze.py by its
consequences: the shipped value drives the replays, must stay inside
the 900 s billing quarter, and 3600/hold_s must stay inside the shipped
deadband. A Controller built with the key ABSENT must land on the same
number, which pins main.py's fallback without touching CFG-03.
All three demonstrated red before being accepted green, and every new
check was mutated to confirm it can fail; one candidate assertion ("no new
notification while parked at 0 W") was DELETED because no mutation could
break it, and replaced by the exact notification count across a revived
transport, which can.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
|
||
|
glenn schrooyenandClaude Opus 5
|
9c1a23328a |
CI-01: this repository's first automated check
checks / unit (pull_request) Successful in 2m39s
Before this commit `admin/goodwe-addon` had no `.gitea/workflows/` directory at all. Eight runnable test files - `test_loop.py` alone drives the real `run_control()` for 93 s of wall clock - and not one of them had ever been run by a machine. Every merge here was a human reading a diff, and the product owner's requirement that a green CI gate a merge could not be met on a repository with no CI to be green. One job, `unit`, with no `paths:` filter, so it reports on every PR and can therefore be a REQUIRED status check: Gitea leaves a required context it never receives pending forever, which is the failure mode on the other side of this ticket. Concurrency is scoped per PR from the start rather than to a bare group name - the bare name is what let the infra repo's run 521 be superseded while still pending, so its four jobs concluded `cancelled` with `started_at` at the Unix epoch and its PR merged on no evidence at all. The tests run in `.gitea/ci/Dockerfile`: `alpine:3` plus `py3-aiohttp` and `py3-paho-mqtt` from apk, matching goodwe_controller/Dockerfile exactly, because pip would install glibc wheels of a library the shipped image gets from Alpine. The runner itself cannot host them - bare python3, no aiohttp, no pip - and mutating a shared long-lived runner with `apk add` is invisible to anyone reading this repo. Two things measured while building it and recorded in the file rather than learned again: the checkout must sit two levels below the mount root or test_freeze.py's `find_hwsim()` dies on `IndexError: 1` before its first assertion, and test_freeze's hwsim section is silently absent here because admin/goodwe is private - so its PASS is a weaker claim than it looks, and the file says so. Verified green on release/1.0 content before this workflow was written: 8 of 8 files pass, ~4 minutes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
c040aac4b8 |
SAFETY-03: alarm when a computed setpoint exceeds the rating, do not just clip it
The clamp was never the missing half. specs.md 10.3 records an unclamped integrator climbing ~130 W per 4 s past 10 kW and reporting 14 768 W pinned at the 5 kW rail; clipped silently that is a flat, legal-looking 5000 W hold and nothing says the loop asked for three times it. write_setpoint() - the one path every write takes - now clamps to the VALIDATED max_w and reports whether the raw value differed; check_output_clamp() raises OUTPUT_CLAMP as a notification, an ERROR, an event row and a red status banner. Not a trip: the clamped value is written and the loop keeps running. The rating is max_w and nothing is min()ed at runtime - CFG-01 already refuses a max_w outside the device range at startup. The device range is applied second as defence in depth; it cannot bite after a successful validation, so if it ever does it alarms too and names the bound. No new config key. Decision grew i_clamped / clamped booleans, and that is the load-bearing part: `reason` is one string that slew-limiting and the saturation freeze overwrite, so an alarm keyed on it is silent on exactly the cycles a winding loop produces. Mutating main.py back onto the string drops the 10.3 replay from 2 engagements to 0. Distinct from saturation by construction - no shared reason code, notification id, status row or counter - and checked in the negative, after first asserting the saturation freeze really did engage. Two things found by running it rather than by reading it: a row per engagement flushed the 40-row event log and evicted SAFETY-18's own METER FROZEN? row (now coalesced per episode with an exact count), and a naive clamp turns a nan into a command at the full rating because every comparison against nan is False. test_clamp.py is new: assert-based, no framework. Six author mutations reproduced red, including "clamp kept, alarm deleted". Two of the check's own hypotheses about the freeze/clamp interaction were refused by its sweep and rewritten to what it measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
admin
|
0cdaa3bd3e |
SAFETY-18: alarm on a meter frozen at a wrong-but-fresh value (#11)
A meter answering 200 OK with a document that never changes reads healthy to every freshness mechanism in the stack while the loop rails. Detected by a conjunction - unchanged_s >= meter_freeze_s AND |command| within 5% of max_w - which fails in both directions for independent reasons. Alarm only: no claim, no latch, no 0 W. Reviewed: all six author mutations reproduced red (redder than reported), four more added, three survivors recorded for follow-up. Capture figures re-derived independently. Attack point 4 (dead poll task) measured, not just reasoned. Known follow-up: a device rating that clips max_w by >5% silently disarms the rail term while the status page says armed - does not affect the live install, where dev_max == max_w == 5000. |
||
|
glenn schrooyenandClaude Opus 5
|
7cde5d862a |
Merge release/1.0 into SAFETY-18 - CHANGELOG only
MAINT-04 (#10) landed on release/1.0 mid-review, so both branches had prepended a stanza. config.yaml and DOCS.md auto-merged clean; only the CHANGELOG needed a hand. Both stanzas are kept verbatim, MAINT-04's Unreleased above the new 0.5.2 - no text from either author edited. MAINT-04 is unreleased and 0.5.2 is being cut here, so a release manager may want to fold the two; that is an editorial call, not a reviewer's. Full suite green on the merged tree: arbiter, control, loop, maintenance, p1, validate, watchdog, freeze. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
admin
|
b0a17474a3 |
MAINT-04: refuse a maintenance charge the ceiling cannot deliver (#10)
Rule 6 and the default change verified; four claimed mutations plus six of my own all reproduce red. The cross-file default divergence is now pinned by a check that fails in both directions, including when someone closes it - verified against MaintConfig-only, one-call-site-refactored, both-refactored and one-literal-silently-changed. |
||
|
glenn schrooyenandClaude Opus 5
|
ffb8ec6373 |
MAINT-04 review: pin the default divergence with a check, not a comment
validate.py's comment said "if config.yaml's value moves, move this and main.py's together" and nothing enforced it - a comment where a check belonged, in a PR that elsewhere proves it knows the difference. test_validate.py now reads main.py and maintenance.py directly and asserts the divergence EXACTLY AS IT STANDS: config.yaml/validate.py at 2000, main.py's two fallbacks and MaintConfig's dataclass default at 2500. It records the mismatch rather than demanding it be closed, so it passes today and goes red if either side moves - including when somebody closes it in main.py, at which point the check and validate.py's block are deleted together. Nothing in a file another ticket owns is touched. The consequence is asserted through the real code as well as the literals: with the key absent the validator accepts, MaintConfig builds 2500 W against a 2000 W ceiling, and the arbiter resolves it to 0 W as a contradiction - the bug rule 6 exists to catch, passing rule 6. Production-unreachable (the Supervisor always supplies every key) but fully reachable from sim/inject.py and the tests, which is where anyone debugging maintenance works. Also: a CHANGELOG section for the upgrade path, covering both the case where the Supervisor re-derives the default and the case where it persisted the old one, since which applies was not verifiable without probing the live install. And maintenance.py's timeout comment said 13.5 kWh where everything else says 13.8 (300 Ah x 51.2 V, DoD 90); SAFETY-18 does not touch that file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
0ba3a200fe |
SAFETY-18: verify on the real rig, and record what it showed
AC3 says "verified on the rig". The previous commit drove hwsim in-process because `riglock status` reported UNKNOWN - which is the tool declining to observe, not an observation. riglock shells out to the local docker CLI; on a Windows host without DOCKER_HOST that resolves to a Docker Desktop npipe which is not running, and the result is indistinguishable from a genuinely absent rig. The rig was up the whole time. With DOCKER_HOST=ssh://homelab@192.168.2.147 it reads FREE. Re-run under sim/riglock.py against the real goodwe-sim compose project, with the shipped modules staged into goodwe-sim-plant so they poll that container's own hwsim while plantsim drives it. Both phases passed, lock released cleanly. The rig produced a result the in-process stub could not, and it strengthens the design rather than merely confirming it: in the NEGATIVE twin - +8000 W of house load, healthy meter, correctly railed command - `active_power_w` did not move at all, sitting at exactly 10130.0 W for 26 consecutive polls. The only thing keeping `unchanged_s` at 0.0 s was the kWh registers advancing. So the register terms in measurement_fingerprint() are not belt-and-braces for this detector, they ARE the negative case: fingerprinted on the power figure alone it would have raised a false alarm on a healthy meter under a real load, which is precisely how a safety guard gets switched off. Recorded in freeze.py so nobody simplifies the fingerprint without meeting that run first. No behaviour change - documentation and the harness note only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
glenn schrooyenandClaude Opus 5
|
7aa1bd052e |
SAFETY-18: a meter frozen at a wrong-but-fresh value now raises an alarm
A meter that answers 200 OK with a stale document forever is ARRIVING on every
poll, so every freshness mechanism in the stack reads it as healthy while the
loop integrates a constant error and rails: 360/135/75 s at the live install's
max_w: 5000 for a meter frozen at 350/1000/>=3000 W. Fifteen minutes of a 5 kW
unattended import sets the month's billed capaciteitstarief peak.
The detector is a conjunction, not the bare unchanged_s threshold TEL-05
measured and rejected:
unchanged_s >= meter_freeze_s AND |command| within 5 % of max_w
Both terms already existed and were already on the status page, and they fail
for independent reasons: at the converged point the command is nowhere near the
rail, and at the rail against a real load the kWh registers move in seconds. At
the shipped 300 s the relation is structural rather than a margin - the slowest
a healthy 1 Wh register can tick and still look unchanged is 12 W, which is
inside the 15 W deadband, so the command there is parked at 0 W.
Notification only, deliberately: an HA persistent notification with a fixed
notification_id, an ERROR log line, a UI event and a red status row. No claim,
no latch, no 0 W. A false trip parks the battery; a false notification costs an
alert. Only meter_source: homewizard_local is covered - the status page says
NOT COVERED elsewhere rather than pretending the guard is armed.
DOCS.md states the capacity-tariff exposure this does NOT close: peak_cap_w caps
maintenance charging and nothing caps the loop. That cap is SAFETY-07.
test_freeze.py grades both directions and was verified by breaking what it
watches (six mutations, all red): silent over the committed 40-minute converged
capture, silent over an hour of TEL-05's low-power case where the rejected bare
threshold is shown to fire, alarming on a genuinely frozen meter driven over
real HTTP through the real HomeWizardLocalSource (the rig's own hwsim freeze
fault when reachable), silent on the same railed command against a meter that is
still moving, and asserting on the service call that leaves Controller.cycle()
rather than on the watchdog's own flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC
|
||
|
glenn schrooyenandClaude Opus 5
|
878a6ce452 |
MAINT-04: refuse a maintenance charge the ceiling cannot deliver
config.yaml shipped max_w 2000 against maintenance_charge_w 2500. The cheap-window shaping files Claim.limit(hi = -maintenance_charge_w) against the supervised SAFETY limit's lo = -max_w, so lo > hi - the arbiter's clause 3, correctly resolved to 0 W and logged as a CONTRADICTION. Maintenance silently never charged in a cheap window on a default install, and the only evidence read like a bug in the arbiter rather than like the two numbers the operator set. validate_startup_config() now refuses maintenance_charge_w > max_w, the same shape as CFG-01's sign_disagree_w >= max_w. Equality is reachable (-max_w is a single valid point) and is not refused. Demonstrated by starting the add-on on the pre-fix pair: exit 78 and the refusal banner. maintenance_discharge_w was checked rather than assumed and is NOT the same case: the drain phase OWNS the actuator (Claim.set at P_MAINTENANCE), so the ceiling clamps it to +2000 W and the arbiter's explain() names the clamp every cycle. Loud, and it still moves energy - warned, not refused. The shipped maintenance rates drop to 2000 so the defaults pass the rule; max_w was deliberately not raised, because it is the supervised safety ceiling. test_validate.py now reads config.yaml itself, so a default the validator refuses cannot ship again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VXjA3LWccN1rJsk3okyPC |
||
|
admin
|
bb64f8fdd7 | SAFETY-02 follow-up: persist the latch, and make the gate assertion bite (#9) | ||
|
glenn schrooyenandClaude Opus 5
|
e40eb0768d |
SAFETY-02: the P_PEAK assertion was pinning the test's copy of the gate
Review finding 2, and it is the same duplication `disagrees()` just removed, so
it gets the same fix rather than a follow-up card.
`following_from()` in test_watchdog.py was a TRANSCRIPTION of the gate in
main.py, carrying "keep the two in step" as an instruction - which is exactly the
arrangement that does not keep two things in step. Measured: editing the shipped
gate alone to `winner in ("loop", "peak")` left every suite green while the
watchdog went blind for the duration of every peak event. The row whose whole job
is to force whoever lands peak shaving to come and change an expectation did not
do that.
The expression is now `watchdog.loop_is_following()`, called by main.py and by
the test. One definition, two callers, same reasoning as `disagrees()`: the copy
that drifts is always the one in the harness doing the grading.
⚠️ AND EXTRACTING IT WAS NOT SUFFICIENT - the drift proof still came out green
after the extraction, which is why it was worth running rather than assuming. The
realistic P_PEAK row has peak shaving asking for MORE than the loop, so the
magnitude term rejects it whatever `winner` says and masks the term under test. A
second row with peak asking for LESS makes `winner` the only deciding term. With
that row, widening the shipped gate fails two checks; without it, none.
Also enforcing the write ordering that was documented and not asserted: swapping
the two `store.set` calls passed everything. A store that fails the second write
now proves the property the ordering exists for - a crash mid-trip leaves a
reason with no latch, never a latch with no explanation. Swapping them now turns
that check red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
|
||
|
glenn schrooyenandClaude Opus 5
|
cc4e88490b |
SAFETY-02 review: a latch that a restart clears is not a latch
BLOCKER. `auto` was persisted and `sign_wd.tripped` was not, so reconstructing the Controller against the same store - which is what a container restart does - brought it back ARMED, into the same fault, unattended: tripped=False, banner back to NOT READY, and it resumed commanding -1000 then -2000 W before re-tripping 180 s later. "Restart the add-on" is the first thing an operator looking at a FAILSAFE banner and a 0 W inverter will try, and under a supervisor that restarts a crashed add-on it is a loop of three-minute wrong-direction bursts. DOCS.md's "it does not clear itself" was false across a restart. `sign_tripped` and `sign_reason` are Store keys now, beside the maintenance state and for the reason store.py's own docstring gives. Written the moment the trip happens - reason first, flag second, so a crash between the two can never persist a latch with no explanation - restored in Controller.__init__, cleared only by the operator's start. The regression is covered: reverting the restore fails seven checks in test_watchdog.py. Review follow-ups, no behaviour change: - The arming gate is tested against the REAL arbiter. It is the decision the ticket turns on and it was asserted in three comment blocks and verified by nothing - every case passed `following` in as a literal. Twelve claim configurations now derive it from resolve(), including the redirect case `winner == "loop"` alone would have got wrong and the `lo` limit that raises magnitude without flipping sign. - The MQTT `status: "failsafe"` value is tested. The check that claimed to cover it restated the line above it and never touched the payload. - The worked example described a config that does not ship: at the defaults maintenance_charge_w 2500 against max_w 2000 is an arbiter contradiction and 0 W, not a -2500 W charge. Rewritten at 1500 W. The contradiction is a real pre-existing bug - a default cheap-window maintenance charge silently never happens - filed separately and deliberately not fixed here. - Forward hazard recorded at the gate: `winner == "loop"` disarms the watchdog under any higher-priority SET, and a peak-shaving strategy at P_PEAK IS grid-following in sign, so it would blind this for every peak event. - disagrees() is public so the replay harness stops carrying its own copy of the predicate it grades. - held_s takes the clock instead of reading one. - The two limits are now each stated in both places rather than one each. - The no-false-positive argument is tied to control.py's freeze permitting the command to cross zero; under the shrink-only encoding control.py records as rejected, this would false-trip on a load step. - Measured figures corrected: 93.4 % / 4212 of 17280, per-cluster 0.3/0.9/10.5/0.0, and the near-circularity of those figures is now stated rather than leaned on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
admin
|
2513ba1f12 | SAFETY-02: the sign-disagreement watchdog (#8) | ||
|
glenn schrooyenandClaude Opus 5
|
9aed43990b |
SAFETY-02: a loop can be fresh, tracking and inside its rating and still push the wrong way
The stale-input watchdog asks whether the meter is still talking. The saturation freeze asks whether the inverter can follow. The output clamp asks whether the number is too big. A loop can pass all three while charging the battery out of the grid, because the only thing wrong is the sign. On 2026-08-20 a commercial controller did exactly that four times in one day, for up to 77 minutes at a stretch, reaching a commanded -14 768 W against a ±5000 W inverter. app/watchdog.py compares the emitted setpoint against the authoritative P1 figure and, when they oppose each other with both over 300 W for 180 s continuously, files a failsafe SET claim at P_SAFETY, commands 0 W and latches there until an operator starts control again. Validated against that day rather than against an argument: sim/replay_sign.py in the infra repo drives this module over the committed captures of both 2026-08-20 and the healthy 2026-08-23 and reaches 18 trips on the fault day - every one inside a window NOTES.md §4.7 accounts for - and 0 on the healthy day. The four episodes do share one signature: 0.3 / 0.9 / 10.5 / 0.0 % sign agreement inside them against 92.9 % healthy. Three decisions the ticket could not make: - It is DISARMED whenever the grid-following loop is not what is driving. Sign agreement is a property of a loop that follows the meter, and a maintenance grid charge opposes the meter by design - 2500 W of charge into a house drawing 500 W is +3000 W on the meter against a -2500 W command, for hours. The gate tests the emitted number, not the winning claim's name, because a limit claim can let the loop "win" and still replace its value. - The trip LATCHES. Commanding 0 W removes the command side of the comparison, so the disagreement always clears the moment the trip works. - sign_disagree_w at or above max_w is refused at startup. The arbiter never lets a command exceed max_w, so such a threshold leaves the watchdog incapable of firing while the status page still reports it armed. What it does not catch, said out loud: an inverter that ACTS opposite to its command moves the meter further the same way, leaving command and meter in agreement. That is the saturation freeze's job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
admin
|
35856a73ae | SAFETY-12: a loop that only thinks when the number changes cannot fail safe (#7) | ||
|
glenn schrooyenandClaude Opus 5
|
aae74d7571 |
SAFETY-12 review: name the safety claim that could not fire, and fix two figures
Review fixes. A. test_loop.py takes 73 s, not the ~35 s claimed in three places. That number is the whole defence against a future developer deleting a slow test, so it was understated by 2x in exactly the place it matters. Corrected - and it is now ~95 s (measured 93 s), because this commit adds the scenario below rather than trimming windows to make an inconvenient number smaller. Both call sites carry an instruction to re-measure if the windows change. B. "242 checks" was test_p1's own count quoted as the total for all five suites. The real figure is 395 across the five, plus 14 here. And the finding this PR should have led with. stale_input_s and the `inputs missing -> 0 W` claim both live INSIDE cycle(), so pre-fix, on a meter whose value happened to repeat, that claim could not fire at all. Constant meter, SoC entity dying at t=4 s, one binary: pre-fix final target 220.0 W, reason 'loop -> 220 W' post-fix final target 0.0 W, reason 'safety -> 0 W' The old controller held 220 W through the whole input outage with its status line still reading loop -> 220 W, and layer 1 does not cover that on a `meter_source: off` install - no P1 age sensor is published, so the firmware has nothing to veto on, while the heartbeat keeps its watchdog fed. The pre-fix behaviour was not "holds a stale command safely"; it was a failsafe a repeating meter switched off. Asserted in test_loop.py, both halves. Three points of reasoning tightened, none of them wrong before: - the re-tune is unreachable by construction, not merely by the reset: the combined cycle rate is max(arrival rate, 1/floor), never the sum, and 1/15 s is strictly slower than the ~1/5 s arrival rate. That holds only while the floor exceeds the meter's update interval - and the schema minimum of 5 s is exactly the reference meter's interval. Flagged for validate.py, not fixed here. - the ~11 s lower bracket is not only a dropped poll: a 5.0 s poller against a 4.97 s meter beats, so about every 14 minutes the poller steps over an update entirely. Reaching 15 s takes a beat-skip AND a failed poll. - the "freshness is decided upstream" guarantee is transport-conditional. On `meter_source: off` the grid figure comes straight from hass.number() with no age gate. Running the full law is still right; the claim is narrower than it was stated. The 15 s default is not independently justified - it rests on the firmware's 30 s ingest veto and moves if that ever does. Said beside the constant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
8cc9438aec |
SAFETY-12: record the six-minute observation, not just the unit check
test_loop.py proves the property in 35 s. This is the same three scenarios run for six and three real minutes each, on one binary, and it is what AC2 and AC3 were actually asked for: 1 cycle before vs 24 after on a constant meter with the command stuck at 220 W against a 350 W import, and byte-identical write sequences on both the converged and the normally-changing meter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
96a808a057 |
HANDOVER: run every suite before deploying, and say why one is slow
The pre-deploy command listed three of the five suites; test_validate and test_p1 were never added when they arrived. Adds those and test_loop, with a note that test_loop's ~35 s is the cost of proving a wall-clock property about a loop that sleeps 1 s a tick rather than a sign something is wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
88cd97cc7a |
SAFETY-12: the loop thinks on a clock, not only when the number moves
run_control() ran cycle() only when the meter value differed from the previous poll. A meter that repeats a value therefore ran no control cycles at all - while the heartbeat kept re-emitting the last command and the firmware kept putting it on the wire every 2 s. Layer 1 stayed fed and every indicator read healthy, so a command computed from an old reading went on being executed through whatever the load did next. TEL-05 measured it: 70 s at a literally constant 350.0 W, 14 arrivals, zero cycles. cycle_floor_s (default 15 s) is a deadline on the GAP between cycles, reset by every cycle whichever trigger fired - not a second scheduler. On a healthy meter it never elapses, so the cycle rate and the commanded sequence are unchanged; test_loop.py asserts that by driving the real loop twice on one binary (a huge cycle_floor_s reproduces the old behaviour) and diffing the writes. 15 s is bracketed: above the ~11 s worst gap on a merely-unlucky healthy meter (~5 s arrivals, 1 s detection, one dropped poll retried on the next tick), below meter_max_age_s and the firmware's 30 s ingest veto, and equal to stale_input_s. A floor cycle runs the full law. Freshness is already decided upstream - past meter_max_age_s the reading is None, which is a P_SAFETY 0 W claim - and the integrator advances per cycle, not per second, so bounding the gap bounds the accumulation. Not a freeze detector: nothing here decides whether a repeated reading is true. DOCS.md's "accepted deviation" passage is qualified rather than deleted - the window is now bounded at 3 x cycle_floor_s instead of open-ended, and its "nothing accumulates while it is stalled" argument is called out as having covered windup and not staleness. Same correction in control.py's comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
admin
|
e2e331ea54 | CFG-01 AC2: refuse to start on a config the lower layer will silently override (#6) | ||
|
glenn schrooyenandClaude Opus 5
|
547d796c5d |
CFG-01 review: widen the rule, refuse the firmware-veto case, fix the stop footgun
Review findings, all confirmed in code before changing anything. THE RULE DID NOT AUTHORISE ITS OWN BEST REFUSAL. The stated test was "makes a safety mechanism inoperative, or makes a stated limit untrue", and a blank setpoint_entity does neither - layer 1 pinning the inverter at 0 W is the failsafe WORKING, as validate.py's own text admitted. The real justification is a second, separate category: the add-on is inert and indistinguishable from healthy. Stated as (a) and (b), with each refusal tagged, and with both halves explicitly bounded - (a) is not "any two numbers that could disagree", (b) is not "anything degraded". Left implicit, the gap is where trivia-creep comes back in. meter_max_age_s ABOVE THE FIRMWARE'S VETO IS NOW REFUSED. The schema allows int(5,300); the firmware's threshold is a compile-time `max_age_s: "30"` and it trips layer 1 at `p1_age_s >= 30`. At 120 the add-on believes a 119 s reading is fresh and keeps commanding while the board has already vetoed and is holding at 0 W - healthy meter, running controller, nothing moving. Structurally identical to the max_w case, and unlike max_w it is settable on this install, so the headline behaviour finally has a live demonstration. That firmware file's own comment says a lower bound on this "belongs in CFG-01, not here". Only ABOVE is refused: below the veto the add-on gives up first, which is fail-safe and visible on its own status page. Scoped to P1 being enabled, because meter_max_age_s governs nothing otherwise. FIRMWARE_INGEST_VETO_S cannot be imported from a compile-time YAML substitution, so it carries a note on the single direction in which it can go stale - and a warning that TEL-06 is editing `ingest_stale`, so a matching integer may stop meaning a matching rule. stop= WAS A LIVE FOOTGUN. It defaulted to None, which made a Controller that cannot stop itself the DEFAULT one. On the late-refusal path that meant: log the banner, return, and leave the process alive forever with no control loop and no exit - the exact "inert but looks healthy" failure this module exists to refuse. Now required and keyword-only, so the broken shape does not construct; test_p1.py's five call sites pass an Event. That whole path had no test and no live demo. It has one now: it drives run_control() against a device that only reports a narrower range once the loop starts, and asserts it refuses, signals, returns, and writes NOTHING - plus the converse, that an agreeing device leads to real writes. This also covers the max_w composition that cannot be reached through the Supervisor on the live install. max_w <= 0 is refused. Schema-blocked in production, but validate_startup_config() is the simulation suite's entry point too, and a non-positive max_w inverts the arbiter's supervised bounds into a contradiction that reads as a safety trip rather than a typo. Verified live 2026-08-28 (backup goodwe_controller-cfg01r2-1787940076): meter_max_age_s 120 refused with a message naming 120, 30, and THE FIRMWARE WINS; no traceback; add-on restored byte-exact and running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
fc9bd89dca |
CFG-01: refuse to start on a configuration that cannot be honoured
main.py used to read the setpoint device's own +-5000 W rating, notice a
configured max_w above it, log one WARNING, and run anyway with
write_setpoint() quietly clamping. The operator's stated ceiling was never
the real ceiling and the only evidence had scrolled off the log. specs
§1.5 is explicit that such a config is refused at startup, not survived at
runtime.
validate_startup_config() runs in amain() before the broker, the web
server or any task exists, so a refusal cannot leave half a controller
running behind a UI. It refuses exactly three things, all sharing one
property - continuing makes a safety mechanism inoperative or makes a
stated limit untrue:
1. max_w outside the setpoint device's own rating.
2. No setpoint_entity: every write is a no-op, the device rating is
never read, and the ESP32 heartbeat that holds off the layer-1
failsafe never happens. It ran, looked healthy, and moved nothing.
3. A meter_source that can never produce a reading. These were already
caught, but they only DISABLED ingestion, which leaves grid None
forever: permanent inputs-missing, 0 W, and a status page
indistinguishable from a healthy meter nobody has fed yet.
Those last predicates move into p1.validate_meter_options(), which
build_source() also calls - one definition, two callers, not two
competing validators. ha_dsmr with half its entities and mqtt_p1 with a
blank topic gained the check the other two transports already had.
Deliberately not refused, because untidy is not unsafe: integrator_max_w
above max_w (the arbiter still bounds the wire to +-max_w, so the cost is
unwind latency - warned), a missing soc/batt entity (fail-safe and
already a red NOT READY banner), a setpoint entity that has not appeared
on the Core API yet (ESPHome is routinely late; re-checked in
run_control() once the real rating arrives), and a thin meter_poll_s
margin.
A refusal is legible as one: a banner saying in words that this is
configuration and nothing is broken, numbered problems, no traceback, and
exit 78 (EX_CONFIG).
Verified on the live add-on 2026-08-28, not by reading the code: a blank
setpoint_entity and a meter_poll_s equal to meter_max_age_s each stopped
the add-on with the banner and no stack trace.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
|
||
|
glenn schrooyenandClaude Opus 5
|
0422127cab |
HANDOVER said 0.2.0; the running add-on is 0.4.0
Two versions stale. 0.3.0 shipped SAFETY-04 and TEL-01, 0.4.0 shipped TEL-04 and TEL-05 and changed the meter transport to homewizard_local - so a reader taking this line at face value would look for a meter path that is no longer how this system reads the grid. Found in the 2026-08-27 backlog audit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
80ebb96e7d |
Two comments asserted an entity id that has never existed
mqtt.py said the P1 age entity "must be exactly sensor.p1_sample_age_s, because SAFETY-01's firmware watchdog subscribes to that literal id", and main.py:340 repeated it. Both are wrong, and wrong in the direction that disarms a safety layer. On HA 2026.8 object_id in a discovery payload does not determine entity_id. HA composes it from the device name plus the entity name, so this one is sensor.goodwe_p1_sample_age. That is what the firmware subscribes to, that is what exists on the live system (verified 2026-08-26), and the firmware's own comment records that sensor.p1_sample_age_s never existed on any HA. The hazard is not the stale name itself, it is that two shipped artefacts disagreed about a safety-critical id while sounding certain. A reader who trusted these comments would have "corrected" the firmware to a name that resolves to nothing - and the failure is silent, because has_state() simply stays false forever and the ingest guard never arms. The firmware calls that out as the shape that FAILS SILENTLY AND LOOKS CORRECT. The comment now says what actually determines the id, and states plainly that renaming the device or the entity's friendly name changes the composed id and disarms the guard. It also cross-references TEL-06, since the guard reads this sensor's value rather than its arrival and so cannot fire for a dead publisher. Found in a backlog audit, as the substance behind PKG-02's premise: the real coupling between firmware and add-on is names, not versions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
c70239cac2 |
0.4.0: the meter transports that fit this house
TEL-04 and TEL-05 merged without a version bump, so Home Assistant would not have offered the update - the same trap DEPLOY-01 caught at 0.2.1, recurring because merging a feature does not touch the version. ha_signed reads one signed entity. homewizard_local polls the meter's own API, where every HTTP response is a genuine arrival - the property the firmware watchdog needs, and the one HA cannot provide for a repeated reading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
glenn schrooyenandClaude Opus 5
|
08f17f65dd |
TEL-05: read the meter, not Home Assistant's opinion of the meter
sensor.p1_sample_age_s measured time since the value CHANGED, not since the meter REPORTED. Home Assistant fires no state_changed for a repeated reading and exposes no arrival signal at all - proven on the rig three ways and confirmed against the live house, where ten repeated meter values left last_reported frozen every time. Real captured data in sim/scenarios/ has inter-change gaps of 42.2 s and 97.0 s, and the firmware watchdog thresholds that age at 30 s, so it would have commanded 0 W on a perfectly healthy meter. meter_source: homewizard_local polls GET /api/v1/data. Every HTTP response is a genuine arrival. Same pipeline as ha_signed - only the arrival mechanism changed. The decisive evidence is the STEADY case, not the frozen one: hwsim served a literally constant 350.0 W for 70 s - the exact state where the HA path recorded samples: 0 across 45 s - and this transport took 14 arrivals with the age never exceeding the poll cadence. Reproduced independently by the reviewer against the real hwsim, and again by the tester at 14/14. 179 -> 242 checks. Twelve mutants from the reviewer and six from the developer, every one reddening a NAMED check rather than aborting the suite. WHAT THIS DOES NOT DO. The ticket asked for two things that cannot both be true: that the age reset on every HTTP response including an unchanged value, AND that a frozen meter show a climbing age. A frozen meter still answers 200 OK - it IS arriving - so no arrival detector can do both. That acceptance criterion was wrong and was written by the PM; the developer refused it rather than quietly building half of it. Freeze detection is delivered separately as unchanged_s on the energy registers, which the local API exposes and HA never did: 70.2 s frozen against 10.0 s steady. Deliberately unthresholded, because at the converged -10 W this controller aims for, a 1 Wh register needs about six minutes to move while the power figure legitimately repeats - any threshold would rebuild the false-trip cycle at the target operating point. It is surfaced on the status line so a human can read it, which is what makes leaving it unthresholded defensible rather than a dodge. So the flash covers meter SILENCE, not meter DISHONESTY. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
glenn schrooyenandClaude Opus 5
|
4d41b0a79e |
TEL-05 review follow-ups: show unchanged_s, name a timeout, keep the poll task alive
Four follow-ups on the reviewed and approved TEL-05 work. Additive; no shipped behaviour changes except the two failure paths below. 1. unchanged_s had no operator surface. DOCS.md told a reader "the transport tracks it as unchanged_s" and there was nowhere to look: main.py built the transport, scheduled run(), and never read the object again. The status page now shows it on the healthy P1 line. Still NOT thresholded and NOT folded into the age - that refusal was reviewed and upheld, because at the converged -10 W this controller aims for a 1 Wh register needs ~6 minutes to move, so any limit false-trips at the target operating point. The whole argument for leaving it to a human requires the human being able to see it. 2. The "equivalent mutant" note on the content_type guard was wrong, and the comment is downgraded to say so. web.Response(text=...) defaults to text/plain, so the fake meter CAN serve valid JSON under the wrong mimetype. Test added; shipped behaviour was already correct. 3. A timed-out poll logged an empty reason: str(asyncio.TimeoutError()) is "", so the status page read "last error:" and then nothing, on a hung meter, at the moment the battery had just gone to 0 W. Falls back to the class name. Note str(err), not `err or ...` - an exception object is always truthy. 4. submit() sat outside the try in poll_once() and run() had no except, so a raise would kill the poll task permanently and SILENTLY - safe (the age climbs, the controller commands 0 W) but indistinguishable from a dead meter. Both wrapped; poll_s is already the retry cadence, so no backoff. Also a comment at the parse_homewizard range(phases) slice: a 3-phase meter configured as 1-phase understates the capacity-tariff figure. Filed separately, not fixed here. 242 checks in test_p1.py (236 before, 6 new). test_control 55, test_arbiter 18, test_maintenance 21, all untouched and green. Each new check proved non-vacuous: six mutations, six named reds, no suite aborts, sources restored byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
98109a9b91 |
TEL-05: read the meter, not Home Assistant's opinion of the meter
A fourth meter_source, `homewizard_local`, polling a HomeWizard P1's own local API (GET /api/v1/data) instead of watching an HA entity. The point is the age sensor. sensor.p1_sample_age_s is FW-01's watchdog input, and on every transport we had it measured "time since the value CHANGED", not "time since the meter REPORTED". Home Assistant offers nothing better: a repeated reading emits no state_changed, advances last_reported on neither serialiser, and state_reported cannot be subscribed to at all. Measured twice - 70 s of a frozen meter on the ENV-01 rig, and ten repeated readings against the live house. Our own capture of this house's meter goes 42.2 s and 97.0 s between changes, both past the default meter_max_age_s of 30, so the age sensor would have commanded 0 W on a perfectly healthy meter. Here every HTTP response is an arrival. The meter answered, now, with its current reading; whether the number moved is not consulted. Five identical readings are five arrivals. Reuses TEL-01's pipeline rather than restructuring it: same split_signed sign convention as ha_signed, same make_sample, same ingest stamping, meter_max_age_s, clock-recomputed age, plausibility bounds and the §20 unsigned-decode rejection. A failed or timed-out poll submits nothing, so it is a missing reading - never 0 W - and does not reset the age. meter_poll_s (default 5 s, the meter's own rate) is checked against meter_max_age_s once at startup, like the ha_signed entity ids. ⚠️ An arrival stamp cannot see a FROZEN meter, and no arrival detector can - one answering 200 OK with a stale number is arriving. The local API does expose what HA never had (the total_power_*_kwh registers stop advancing) and the transport tracks it as `unchanged_s`, but it is deliberately not folded into the age and not thresholded: this controller regulates grid toward ~0 W, and at a converged -10 W the export register needs six minutes to move by its 1 Wh resolution while the power figure legitimately repeats. Thresholding that would rebuild the false-trip limit cycle at the exact operating point we aim for. test_p1.py 179 -> 236 checks. Still defaults to off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
ad9c5772a4 |
TEL-04: a meter source for the meter this house actually has
TEL-01 shipped ha_dsmr and mqtt_p1. Neither fits the installed meter - a HomeWizard P1 publishing one signed figure, where ha_dsmr requires two unsigned registers and refuses negatives. So sensor.p1_sample_age_s could not be produced here at all. ha_signed is a subclass of HaDsmrSource overriding only _wanted() and build(), so ingest timestamping, staleness, the clock-recomputed age, plausibility bounds and unavailable-is-never-zero are inherited by construction rather than copied. A reviewer traced every inherited member and confirmed nothing in the base assumes two entities. 122 -> 179 checks. Sign convention asserted against real captures in sim/scenarios/: -5710 W at 13:46 local under full sun, +775 W at midnight, verified to the timestamp by two people independently. WHAT THIS DOES NOT DO, documented in DOCS.md, the docstring and the CHANGELOG rather than discovered later: the age it publishes measures time since the VALUE CHANGED, not since the meter reported. Home Assistant exposes no arrival signal for a repeated reading - proven on the rig against a frozen meter (no state_changed, last_reported advancing on neither serialiser, state_reported rejected outright) and confirmed independently against the live house, where ten repeated values all left last_reported frozen. So this entity must NOT yet be thresholded by the firmware watchdog. ha_dsmr has the same blind spot and escapes only statistically, because a DSMR telegram moves several entities. The fix is to read the meter's own API, where every response is an arrival. That is TEL-05, and it is FW-01's real gate. Two equivalent mutants are known and recorded: the per-phase split, documented at the site, and the incomplete-phase-set guard, which is cosmetic - both paths return False, one via an extra log line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
glenn schrooyenandClaude Opus 5
|
1b343da8e3 |
DOCS: say plainly that ha_dsmr and mqtt_p1 have never seen real hardware
No meter in this installation uses either transport. Both were written to specs.md 5.2's assumption that a Belgian P1 exposes two unsigned registers, which the meter actually fitted here does not - it is the HomeWizard P1 that ha_signed reads. Their only coverage is test_p1.py and an end-to-end test against a fake Home Assistant. Deliberately not called "experimental". That word says the design is unfinished, which is not the defect and is vaguer than the truth; these are complete and reviewed, they have simply never had a real telegram through them. The failure this note is guarding against is a future session debugging a meter problem, treating those two paths as proven, and looking elsewhere. Placed where the mode is chosen rather than in a footnote, because the choice is the decision it should inform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
e663e10245 |
TEL-04 review: unshadow the helper, validate config at startup, and record
what the rig proved about the age sensor
Review findings 1, 3, 5 and 6. Finding 2 is deliberately untouched - it is
its own ticket.
3. `built` was rebound at test_p1.py:816 by `built = build_source(...)`,
silently disarming the build() wrapper for anything appended below it.
Renamed to `sel`. Reproduced the reviewer's failure before fixing:
appending a check that calls built() after that line gives
`TypeError: 'HaSignedSource' object is not callable` and aborts at 163 of
180; with the rename the same probe reaches 180 and passes.
5. DOCS.md now states the "length must equal meter_phases" constraint that
config.yaml already carried, plus what leaving the list empty actually
costs: on the surveyed reading the phases carry 2769 W of import while the
connection nets 187 W, so the tariff quantity is understated ~15x.
6. build_source now checks the ha_signed wiring once at startup instead of
once per telegram: a blank p1_net_entity, or a phase list whose length
disagrees with meter_phases, logs an error and disables ingestion. Both
otherwise fail in the single way indistinguishable from a healthy source
nobody has fed yet - no samples, a climbing age, the watchdog holding the
battery at 0 W, and nothing in the log.
1. THE AGE SENSOR. Measured on the ENV-01 rig against the real HomeWizard
integration, meter frozen via hwsim's `?fault=freeze` seam (cleared in a
finally:, rig verified restored):
- websocket state_changed for the meter over 70 s : 0
- last_reported advanced (REST serialiser) : no
- last_reported advanced (websocket serialiser) : no
- subscribe_events(state_reported) : rejected,
"Event filter is required for event state_reported"
So Home Assistant exposes NO arrival signal for a repeated reading, and
the proposed fix - stamp from last_reported via subscribe_entities - is
not available. subscribe_entities listens only to EVENT_STATE_CHANGED, and
as_compressed_state carries no last_reported at all.
The age is therefore "time since the value changed", which on ha_dsmr is
mostly harmless (a telegram moves several entities) and on ha_signed is
not: one entity means a healthy meter under a flat load is
indistinguishable from a dead one. Recorded loudly in DOCS.md, in the
HaSignedSource docstring and in the CHANGELOG, with the measured 42.2 s
and 97.0 s gaps from our own capture.
meter_max_age_s is deliberately NOT widened. The two conditions produce an
identical signal, so a larger number does not separate them - it only
chooses which of the two errors you get, and it would disarm the watchdog
for a genuinely dead meter as well. The honest fix is an arrival stamp the
meter itself provides.
test_p1.py: 174 -> 179 checks, all green. Other three suites unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
|
||
|
glenn schrooyenandClaude Opus 5
|
632be44f6c |
TEL-04: make a missing build() guard fail legibly instead of aborting
Review point from the non-vacuity run. Deleting build()'s "is the net entity cached at all" guard makes build() raise KeyError rather than return False. That still failed the suite, but by aborting it with a traceback at whichever check ran first - a red that costs the next person ten minutes deciding whether the suite is broken or the code is. New `built()` helper in test_p1.py wraps build() and turns an escaping exception into a returned value, so the comparison against True/False fails by name. Applied only to the ha_signed section; TEL-01's own checks are untouched. Mutation 4 before: 0 named checks red, aborted at check 123 of 174. Mutation 4 after: 2 named checks red - "nothing cached yet builds nothing" and "an unavailable entity does not build a sample" - all 174 reached. Still 174 checks, all green, and the other nine mutations are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa |
||
|
glenn schrooyenandClaude Opus 5
|
8b51a51e20 |
TEL-04: a third meter_source for a single signed entity
TEL-01 shipped ha_dsmr and mqtt_p1, and neither can read the meter that is
actually fitted here. The house has a HomeWizard P1 exposing ONE signed
entity, sensor.p1_meter_active_power (+ import, - export); ha_dsmr wants two
unsigned registers and refuses a negative one outright, which is every
exporting telegram. So sensor.p1_sample_age_s could not be produced at this
site, and FW-01's watchdog needs it - measured, not theoretical: the house P1
went 51.1 s and 36.2 s without a state change overnight, both past
meter_max_age_s 30, so without the age sensor the watchdog would false-trip
the battery to 0 W.
Adds meter_source: ha_signed, reading p1_net_entity (and optionally
p1_phase_net_entities in L1..L3 order for the capacity-tariff peak). The
derivation is split_signed(), sitting next to make_sample's subtraction for
the same reason it does - the moment a user is asked to write two template
sensors that split a signed value, the sign convention is back in unreviewed
YAML underneath a safety input, which is exactly what TEL-01 removed.
The transport is a subclass of HaDsmrSource overriding only _wanted() and
build(), so every rule TEL-01 established is inherited rather than
re-implemented: ingest timestamping, meter_max_age_s, the clock-recomputed
sensor.p1_sample_age_s republished ~1 Hz, the plausibility ceiling, the
"prime the cache from get_states but never build a sample out of it" rule,
"a reconnect emits nothing", and unavailable/unknown treated as a MISSING
reading and never as 0 W.
Defaults to off. An existing install is unaffected until it opts in.
test_p1.py: 122 -> 174 checks. Includes an end-to-end run of the new
transport against a fake Home Assistant websocket, and the sign convention
asserted against real captured readings from
sim/scenarios/ha-p1_meter_active_power-2026-08-{20,23}.json (-5710 W at
13:46 local under full sun is export; +775 W at midnight is import).
Non-vacuity: ten mutations of the new rules, each applied alone and reverted
byte-identical. Nine turn the suite red. The tenth - splitting the per-phase
signed values rather than passing them through - is an equivalent mutant,
because make_sample subtracts the two lists again and does not sign-check
per-phase figures. That is recorded in a ponytail: comment at the site rather
than left for the next reviewer to rediscover.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Du77usMj8XNKNFZGmUiWDa
|
||
|
glenn schrooyenandClaude Opus 5
|
c24bc0a011 |
Pin LF, because the deployment target is a Linux container
core.autocrlf=true gave the checkout CRLF .py and .yaml. run.sh happened to be LF, which is the only reason a plain copy would not have produced a "bad interpreter" failure on the add-on's entrypoint. 0.3.0 was deployed by extracting from git with autocrlf forced off and verified byte-identical to the blobs before copying. This makes that the default rather than something the deployer has to remember. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |