UX-11: safety log lines give one plain sentence and the code, the figures go to DEBUG #140

Merged
admin merged 3 commits from UX-11-short-safety-lines into release/1.0 2026-09-27 14:31:06 +02:00
Owner

Trello UX-11 (P3), from the UX-10 post-merge test on TEST-23 (check 8 and the minor findings).

What changed

  1. Safety log lines: one plain sentence and the code. Every WARNING/ERROR line that reported a safety reason now ends in the reason's code (messages.reason_code()). The reason itself, with its figures and limits, is logged on a DEBUG line right after it ("Evidence for inverter-a: ...") and stays in the action log events entry, so installers lose nothing. The five named reasons, plus circulation and the two clamp lines (same 25-word rule, same pattern). The clamp numbers stay in the notification (UX-08 2a item 3).
  2. state.json and backwards compatibility. reason is still the evidence text and its saved format is unchanged. Every saved reason has opened with its code since it was first written (git history checked for SIGN_DISAGREEMENT, UNIT_NOT_DELIVERING, CIRCULATING_POWER), and the code is all the ERROR line reads. So an old state file holding the long text loads and shows exactly like a new one: the code on the ERROR line, the saved text on the DEBUG line. A stop saved without a reason shows REASON_NOT_SAVED. test_messages part 7 restarts a Controller from a state file with the old long texts. test_watchdog's byte pin now pins the meaning: code first, then command, reading, time held and both limits, in that order.
  3. Scan (test_messages part 6). A WARNING+ line over 25 words as logged (built text filled in) fails. A new check requires every line that quotes reason_code(X) to be followed by a DEBUG call that logs X (with a mutation). The evidence texts keep the red-alarm shape check. Exceptions: 9 non-safety families still run long (tariff plan words, price feed reasons, state migration report x2, write refusals and claim scope errors that only a software fault raises, maintenance, trading gate, price history). They are listed in LONG_LINE_OK with a reason each, capped at 9; a listed family that gets short fails until removed; no safety reason may be listed. Shortening them is a follow-up.
  4. "The add-on log shows why". A meter that goes quiet past its limit logs one WARNING per episode (not per cycle): No meter reading for over 30 seconds, so control waits and then sets the battery to 0 W., and INFO Meter readings are back. This is a new log line, not a new alarm: no notification, no latch, nothing it decides.
  5. Why row past the 15 s hold: No usable meter reading for over 15 seconds: 0 W (or the battery power or level of an inverter) instead of A reading is missing: 0 W.
  6. Action log: 25 engagements (real plural), Shadow mode off: the add-on controls the battery again, starting from 0 W.

Unchanged: triggers, latches, thresholds, alarm log levels (WARNING/ERROR stay), notification ids, cadence, row colours, banner order. Choices are recorded in DECISIONS/UX-08-short-notifications.md (UX-11 section); STYLE.md's log-line rule and DOCS "For installers" are updated.

The five reasons, before and after (sample values, word counts as the scan counts them)

direction (per inverter)

  • before, ERROR (40 words): inverter-a is set to 0 W until control is started, because its power went the wrong way. SIGN_DISAGREEMENT: commanded -1000 W against a meter reading +1500 W for 181 s continuously (limits: both over 300 W, held over 180 s).
  • after, ERROR (18 words): inverter-a is set to 0 W until control is started, because its power went the wrong way: SIGN_DISAGREEMENT.
  • after, DEBUG right after it: Evidence for inverter-a: SIGN_DISAGREEMENT: commanded -1000 W against a meter reading +1500 W for 181 s continuously (limits: both over 300 W, held over 180 s).

delivery

  • before, ERROR (60 words): inverter-b is not delivering while control keeps running, and only stopping and starting control clears this. UNIT_NOT_DELIVERING: inverter-b is standing at +2000 W and delivering +0 W - 2000 W of its share (100 % of it) is not arriving - while inverter-a is not in that state. Nothing this add-on reads explains it - held for 121 s continuously (limit: over 120 s).
  • after, ERROR (17 words): inverter-b is not delivering while control keeps running, and only stopping and starting control clears this: UNIT_NOT_DELIVERING.
  • after, DEBUG right after it: Evidence for inverter-b: UNIT_NOT_DELIVERING: inverter-b is standing at +2000 W and delivering +0 W - 2000 W of its share (100 % of it) is not arriving - while inverter-a is not in that state. Nothing this add-on reads explains it - held for 121 s continuously (limit: over 120 s).

frozen meter

  • before, ERROR (52 words): The meter shows the same value while the battery is asked for full power, and control still follows it, so check the meter. METER_FROZEN: the meter has not changed for 612 s while the command is +5000 W, at the 5000 W limit. Limits: unchanged over 300 s, command over 4750 W.
  • after, ERROR (24 words): The meter shows the same value while the battery is asked for full power, and control still follows it, so check the meter: METER_FROZEN.
  • after, DEBUG right after it: Evidence: METER_FROZEN: the meter has not changed for 612 s while the command is +5000 W, at the 5000 W limit. Limits: unchanged over 300 s, command over 4750 W.

stale reading

  • before, ERROR (60 words): inverter-a: battery power reading stopped changing, set to 0 W until it changes, or 17 s after the command drops below 250 W. STALE_READING: +360 W unchanged for 35 s while the command moved 430 W to +430 W. The reading may have stopped, or the battery may be unable to follow the command, and this check cannot tell which.
  • after, ERROR (24 words): inverter-a: battery power reading stopped changing, set to 0 W until it changes, or 17 s after the command drops below 250 W: STALE_READING.
  • after, DEBUG right after it: Evidence for inverter-a: STALE_READING: +360 W unchanged for 35 s while the command moved 430 W to +430 W. The reading may have stopped, or the battery may be unable to follow the command, and this check cannot tell which.

peak guard

  • before, ERROR (63 words): Switch off large loads, the battery alone cannot keep this 15-minute period under the peak limit. PEAK_GUARD: this 15-minute period heads for 3870 W against a 3500 W limit (peak_cap_w), with 643 s left, so 518 W must come off the meter. The battery measured +1741 W and is asked for +2000 W, the most the inverters give, but +2259 W was needed.
  • after, ERROR (17 words): Switch off large loads, the battery alone cannot keep this 15-minute period under the peak limit: PEAK_GUARD.
  • after, DEBUG right after it: Evidence: PEAK_GUARD: this 15-minute period heads for 3870 W against a 3500 W limit (peak_cap_w), with 643 s left, so 518 W must come off the meter. The battery measured +1741 W and is asked for +2000 W, the most the inverters give, but +2259 W was needed.

All 36 test files pass locally in the CI image (with hwsim). TEST-23 verification follows as a comment.

🤖 Generated with Claude Code

Trello UX-11 (P3), from the UX-10 post-merge test on TEST-23 (check 8 and the minor findings). ## What changed 1. **Safety log lines: one plain sentence and the code.** Every WARNING/ERROR line that reported a safety reason now ends in the reason's code (`messages.reason_code()`). The reason itself, with its figures and limits, is logged on a DEBUG line right after it ("Evidence for inverter-a: ...") and stays in the action log `events` entry, so installers lose nothing. The five named reasons, plus circulation and the two clamp lines (same 25-word rule, same pattern). The clamp numbers stay in the notification (UX-08 2a item 3). 2. **`state.json` and backwards compatibility.** `reason` is still the evidence text and its saved format is unchanged. Every saved reason has opened with its code since it was first written (git history checked for `SIGN_DISAGREEMENT`, `UNIT_NOT_DELIVERING`, `CIRCULATING_POWER`), and the code is all the ERROR line reads. So an old state file holding the long text loads and shows exactly like a new one: the code on the ERROR line, the saved text on the DEBUG line. A stop saved without a reason shows `REASON_NOT_SAVED`. test_messages part 7 restarts a Controller from a state file with the old long texts. test_watchdog's byte pin now pins the meaning: code first, then command, reading, time held and both limits, in that order. 3. **Scan (test_messages part 6).** A WARNING+ line over 25 words **as logged** (built text filled in) fails. A new check requires every line that quotes `reason_code(X)` to be followed by a DEBUG call that logs X (with a mutation). The evidence texts keep the red-alarm shape check. Exceptions: 9 **non-safety** families still run long (tariff plan words, price feed reasons, state migration report x2, write refusals and claim scope errors that only a software fault raises, maintenance, trading gate, price history). They are listed in `LONG_LINE_OK` with a reason each, capped at 9; a listed family that gets short fails until removed; no safety reason may be listed. Shortening them is a follow-up. 4. **"The add-on log shows why".** A meter that goes quiet past its limit logs one WARNING per episode (not per cycle): `No meter reading for over 30 seconds, so control waits and then sets the battery to 0 W.`, and INFO `Meter readings are back.` This is a new log line, **not a new alarm**: no notification, no latch, nothing it decides. 5. **Why row past the 15 s hold**: `No usable meter reading for over 15 seconds: 0 W` (or the battery power or level of an inverter) instead of `A reading is missing: 0 W`. 6. **Action log**: `25 engagements` (real plural), `Shadow mode off: the add-on controls the battery again, starting from 0 W`. **Unchanged:** triggers, latches, thresholds, alarm log levels (WARNING/ERROR stay), notification ids, cadence, row colours, banner order. Choices are recorded in `DECISIONS/UX-08-short-notifications.md` (UX-11 section); STYLE.md's log-line rule and DOCS "For installers" are updated. ## The five reasons, before and after (sample values, word counts as the scan counts them) **direction (per inverter)** - before, ERROR (40 words): `inverter-a is set to 0 W until control is started, because its power went the wrong way. SIGN_DISAGREEMENT: commanded -1000 W against a meter reading +1500 W for 181 s continuously (limits: both over 300 W, held over 180 s).` - after, ERROR (18 words): `inverter-a is set to 0 W until control is started, because its power went the wrong way: SIGN_DISAGREEMENT.` - after, DEBUG right after it: `Evidence for inverter-a: SIGN_DISAGREEMENT: commanded -1000 W against a meter reading +1500 W for 181 s continuously (limits: both over 300 W, held over 180 s).` **delivery** - before, ERROR (60 words): `inverter-b is not delivering while control keeps running, and only stopping and starting control clears this. UNIT_NOT_DELIVERING: inverter-b is standing at +2000 W and delivering +0 W - 2000 W of its share (100 % of it) is not arriving - while inverter-a is not in that state. Nothing this add-on reads explains it - held for 121 s continuously (limit: over 120 s).` - after, ERROR (17 words): `inverter-b is not delivering while control keeps running, and only stopping and starting control clears this: UNIT_NOT_DELIVERING.` - after, DEBUG right after it: `Evidence for inverter-b: UNIT_NOT_DELIVERING: inverter-b is standing at +2000 W and delivering +0 W - 2000 W of its share (100 % of it) is not arriving - while inverter-a is not in that state. Nothing this add-on reads explains it - held for 121 s continuously (limit: over 120 s).` **frozen meter** - before, ERROR (52 words): `The meter shows the same value while the battery is asked for full power, and control still follows it, so check the meter. METER_FROZEN: the meter has not changed for 612 s while the command is +5000 W, at the 5000 W limit. Limits: unchanged over 300 s, command over 4750 W.` - after, ERROR (24 words): `The meter shows the same value while the battery is asked for full power, and control still follows it, so check the meter: METER_FROZEN.` - after, DEBUG right after it: `Evidence: METER_FROZEN: the meter has not changed for 612 s while the command is +5000 W, at the 5000 W limit. Limits: unchanged over 300 s, command over 4750 W.` **stale reading** - before, ERROR (60 words): `inverter-a: battery power reading stopped changing, set to 0 W until it changes, or 17 s after the command drops below 250 W. STALE_READING: +360 W unchanged for 35 s while the command moved 430 W to +430 W. The reading may have stopped, or the battery may be unable to follow the command, and this check cannot tell which.` - after, ERROR (24 words): `inverter-a: battery power reading stopped changing, set to 0 W until it changes, or 17 s after the command drops below 250 W: STALE_READING.` - after, DEBUG right after it: `Evidence for inverter-a: STALE_READING: +360 W unchanged for 35 s while the command moved 430 W to +430 W. The reading may have stopped, or the battery may be unable to follow the command, and this check cannot tell which.` **peak guard** - before, ERROR (63 words): `Switch off large loads, the battery alone cannot keep this 15-minute period under the peak limit. PEAK_GUARD: this 15-minute period heads for 3870 W against a 3500 W limit (peak_cap_w), with 643 s left, so 518 W must come off the meter. The battery measured +1741 W and is asked for +2000 W, the most the inverters give, but +2259 W was needed.` - after, ERROR (17 words): `Switch off large loads, the battery alone cannot keep this 15-minute period under the peak limit: PEAK_GUARD.` - after, DEBUG right after it: `Evidence: PEAK_GUARD: this 15-minute period heads for 3870 W against a 3500 W limit (peak_cap_w), with 643 s left, so 518 W must come off the meter. The battery measured +1741 W and is asked for +2000 W, the most the inverters give, but +2259 W was needed.` All 36 test files pass locally in the CI image (with hwsim). TEST-23 verification follows as a comment. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
admin added 1 commit 2026-09-27 13:35:11 +02:00
UX-11: safety log lines give one plain sentence and the code, the figures go to DEBUG
checks / unit (pull_request) Successful in 9m3s
infra-pin / fresh (pull_request) Successful in 2s
29837dfaa7
- Every WARNING/ERROR line that reported a safety reason (direction, delivery,
  stuck meter, stale reading, peak guard, circulation, over-rating) now ends in
  the reason's code (messages.reason_code()). The reason itself, figures and
  limits included, is on a DEBUG line right after it and in the action log.
- The reasons and state.json are unchanged, so an old state file loads and
  shows the same way: every saved reason opens with its code.
- test_messages part 6 checks every WARNING+ line at 25 words as logged; nine
  non-safety families are listed with a reason and capped. The byte pin in
  test_watchdog now pins the meaning, in order.
- The meter logs one WARNING per quiet episode, and INFO when back, so the red
  row's "the add-on log shows why" holds. The Why line past the hold names
  the missing reading. Action log: "25 engagements", "Shadow mode off: ...".

No trigger, latch, threshold, log level, notification id, cadence, row colour
or banner order changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
admin added 1 commit 2026-09-27 13:49:21 +02:00
UX-11: the meter WARNING names the limit only, as 'over 30 seconds'
checks / unit (pull_request) Successful in 9m4s
infra-pin / fresh (pull_request) Successful in 2s
c628099e69
TEST-23 showed 'No meter reading for 30 seconds, over the limit of 30
seconds': the age is 30.4 s, which rounds to the limit. The limit alone is
always true at the moment the line is logged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyR88rXETvwhTgx6yHE39p
Author
Owner

TEST-23 verification (13:36–13:53), images built from this branch with app/*.py hashes equal to the clone. Setup as in the UX-10 test: soak paused, both add-ons stopped, both /data volumes tarred with every file's sha256 and owner, options.json.pre-doc1 used (only log_level set to debug, to see the DEBUG lines), same volume, IP, port, hostname and env. A small script made the battery follow the power request and drove the meter.

PEAK_GUARD (load 6 000 W, 29837df):

13:45:02,546 ERROR   goodwe: Switch off large loads, the battery alone cannot keep this 15-minute period under the peak limit: PEAK_GUARD.
13:45:02,546 DEBUG   goodwe: Evidence: PEAK_GUARD: this 15-minute period heads for 3991 W against a 3500 W limit (peak_cap_w), with 897 s left, so 492 W must come off the meter. The battery measured +2003 W and is asked for +2000 W, the most the inverters give, but +2495 W was needed.

Action log: PEAK_GUARD_SATURATED - PEAK_GUARD: this 15-minute period heads for 3991 W … (unchanged). Clamp: WARNING The battery gives less than asked: OUTPUT_CLAMP. Held back 1 time so far. + DEBUG Evidence: OUTPUT_CLAMP: the correction the add-on builds up reached its +2000 W limit, command now +630 W. Event row: OUTPUT_CLAMP episode 1: 153 engagements - ….

STALE_READING (battery reading frozen, load 1 500 W):

13:48:20,361 ERROR   goodwe: inverter-a: battery power reading stopped changing, set to 0 W until it changes, or 17 s after the command drops below 250 W: STALE_READING.
13:48:20,361 DEBUG   goodwe: Evidence for inverter-a: STALE_READING: +411 W unchanged for 36 s while the command moved 1100 W to +1510 W. The reading may have stopped, or the battery may be unable to follow the command, and this check cannot tell which.
13:48:20,361 DEBUG   goodwe: inverter-a: the limits are unchanged over 35 s, a command over 250 W and a move by over 250 W.

Why after the hold: No usable battery power reading for over 15 seconds: 0 W.

Meter stopped (13:45:36–13:47:11): one WARNING in 95 s, the red row Meter: no reading for 75 seconds, limit 30 seconds. The add-on log shows why., Why No usable meter reading for over 15 seconds: 0 W, and INFO Meter readings are back. That run printed No meter reading for 30 seconds, over the limit of 30 seconds (30.4 s rounds to the limit), so c628099 names the limit only. Re-run on c628099 (13:50:16–13:51:36):

13:50:43,616 WARNING goodwe: No meter reading for over 30 seconds, so control waits and then sets the battery to 0 W.
13:51:40,592 INFO    goodwe: Meter readings are back.

Restored: test container removed; both volumes restored from tar, every file's sha256 and owner match, both tariff.db pass integrity_check (quarters 13:37–13:52 discarded). test23-addon is the original container renamed back, :16c796f 9896c53a…, hostname a5a20d236eb6, 10.77.34.40, options fa602196… and state aa4848a9… as before, 0 ERROR. arb stopped 13:37–13:52, options and state hashes unchanged, 0 ERROR. Soak paused 13:36:55–13:52:41, now unpaused. HA: configuration.yaml md5 c1048b26, 45 entities with the same per-domain counts, batt_raw 363.4, soc_raw 52.4, setpoint_raw 0, the two notifications the test raised dismissed (0 left). Test images removed. Nothing touched .158, .221 or .167.

TEST-23 verification (13:36–13:53), images built from this branch with app/*.py hashes equal to the clone. Setup as in the UX-10 test: soak paused, both add-ons stopped, both /data volumes tarred with every file's sha256 and owner, `options.json.pre-doc1` used (only `log_level` set to `debug`, to see the DEBUG lines), same volume, IP, port, hostname and env. A small script made the battery follow the power request and drove the meter. **PEAK_GUARD** (load 6 000 W, 29837df): ``` 13:45:02,546 ERROR goodwe: Switch off large loads, the battery alone cannot keep this 15-minute period under the peak limit: PEAK_GUARD. 13:45:02,546 DEBUG goodwe: Evidence: PEAK_GUARD: this 15-minute period heads for 3991 W against a 3500 W limit (peak_cap_w), with 897 s left, so 492 W must come off the meter. The battery measured +2003 W and is asked for +2000 W, the most the inverters give, but +2495 W was needed. ``` Action log: `PEAK_GUARD_SATURATED - PEAK_GUARD: this 15-minute period heads for 3991 W …` (unchanged). Clamp: `WARNING The battery gives less than asked: OUTPUT_CLAMP. Held back 1 time so far.` + `DEBUG Evidence: OUTPUT_CLAMP: the correction the add-on builds up reached its +2000 W limit, command now +630 W.` Event row: `OUTPUT_CLAMP episode 1: 153 engagements - …`. **STALE_READING** (battery reading frozen, load 1 500 W): ``` 13:48:20,361 ERROR goodwe: inverter-a: battery power reading stopped changing, set to 0 W until it changes, or 17 s after the command drops below 250 W: STALE_READING. 13:48:20,361 DEBUG goodwe: Evidence for inverter-a: STALE_READING: +411 W unchanged for 36 s while the command moved 1100 W to +1510 W. The reading may have stopped, or the battery may be unable to follow the command, and this check cannot tell which. 13:48:20,361 DEBUG goodwe: inverter-a: the limits are unchanged over 35 s, a command over 250 W and a move by over 250 W. ``` Why after the hold: `No usable battery power reading for over 15 seconds: 0 W`. **Meter stopped** (13:45:36–13:47:11): one WARNING in 95 s, the red row `Meter: no reading for 75 seconds, limit 30 seconds. The add-on log shows why.`, Why `No usable meter reading for over 15 seconds: 0 W`, and `INFO Meter readings are back.` That run printed `No meter reading for 30 seconds, over the limit of 30 seconds` (30.4 s rounds to the limit), so c628099 names the limit only. Re-run on c628099 (13:50:16–13:51:36): ``` 13:50:43,616 WARNING goodwe: No meter reading for over 30 seconds, so control waits and then sets the battery to 0 W. 13:51:40,592 INFO goodwe: Meter readings are back. ``` **Restored:** test container removed; both volumes restored from tar, every file's sha256 and owner match, both tariff.db pass integrity_check (quarters 13:37–13:52 discarded). `test23-addon` is the original container renamed back, `:16c796f` 9896c53a…, hostname a5a20d236eb6, 10.77.34.40, options `fa602196…` and state `aa4848a9…` as before, 0 ERROR. arb stopped 13:37–13:52, options and state hashes unchanged, 0 ERROR. Soak paused 13:36:55–13:52:41, now unpaused. HA: configuration.yaml md5 c1048b26, 45 entities with the same per-domain counts, batt_raw 363.4, soc_raw 52.4, setpoint_raw 0, the two notifications the test raised dismissed (0 left). Test images removed. Nothing touched .158, .221 or .167.
Author
Owner

Review of c628099 (reviewer agent). Base has not moved (merge-base = release/1.0 = c4032eb). test_messages, test_watchdog, test_clamp and test_freshness pass in goodwe-addon-checks:1.

Safety diff: I blanked the strings and diffed the logic. Every hunk in main.py only changes what a _LOG call or log_event says, adds a _LOG.debug, or is display-only (why_words, missing_reading()). No trigger, latch, threshold, log level, notification id, cadence, colour or banner changed. The meter block in read_inputs() only logs and sets a flag. watchdog.py changes a comment only.

Findings. I will push the fixes for R1–R4 myself:

  • R1 (must fix). After a restart, the evidence is lost at the default log level. log_level defaults to info (config.yaml:899), so installers never see the DEBUG "Evidence …" line. events lives only in memory (main.py:373) and is empty after a restart. None of the four restart-latch paths (sign per inverter, delivery, site sign, circulation; main.py ~611/641/675/729) writes an event, and /status has no reason field. Before this PR the saved figures were on the ERROR line. Now, after a restart, they are only in /data/state.json, which an installer can't easily read. DOCS says "on the DEBUG line … and in events", which is false on this path. Fix: each restart path also writes the saved reason to the action log (no hint, so the page line is unchanged). Part 7 will pin it. DOCS will say that DEBUG needs log_level: debug.
  • R2. The byte pin in test_watchdog was loosened without cause. The reason string did not change (the watchdog.py diff is a comment only), and it is still what state.json holds. The meaning checks are fine, but the literal should stay beside them. DECISIONS item 2 will say so.
  • R3. The "no safety reason may be listed" check matches only on key names ("reason_code" in k). I put the direction line back to quoting inv.sign_wd.reason, classified it in TEXT and listed it in LONG_LINE_OK. That mutation was caught only by the separate 30-word-sentence rule, not by this guard. Fix: also refuse any listed key whose TEXT maker is one of SAFETY_REASONS.
  • R4. The quiet-meter WARNING can repeat on a flapping meter. It logs once per episode, and an episode ends at the next sample. On ha_signed, a healthy meter under a flat load goes 42–97 s between changes (p1.py ~827, above the 30 s default). That gives one WARNING + INFO pair per gap, possibly hundreds a day. Fix: at most one WARNING (and its INFO) per 10 minutes. Episodes in between log at DEBUG. There is still no control change.

Checked and fine:

  • Scan. A 26-word peak line fails ("over 25 words (26)"). Dropping the peak DEBUG evidence fails ("missing: main.py:3989"). Adding main.py:messages.reason_code(g.reason) to LONG_LINE_OK fails. I checked the reasons of the 9 exceptions against the code. refusal (SAFETY-28 gate) and scope_errors are software faults only. The others are tariff, price and maintenance texts, none of them safety.
  • Backwards compatibility. Part 7 builds a real Controller from a store holding the old long texts, so it runs the real load path. REASON_NOT_SAVED is a greppable code-like token on a log line, which is fine.
  • Readability. Direction (18 words), delivery (17), frozen (24) and peak (17) read plainly. STALE keeps "17 s after the command drops below 250 W". That is justified: it is the one fact the owner can act on (the stop lets go by itself), it is SAFETY-57's release clause, and test_freshness pins it. No change.
  • Scope creep. Circulation keeps its reason in events ("CIRCULATING POWER - …"). The clamp ERROR's asked, limit and sent figures are in messages.output_clamp(). The loop-bound WARNING's figures are in the coalesced events row. Nothing is lost.
Review of c628099 (reviewer agent). Base has not moved (merge-base = release/1.0 = c4032eb). test_messages, test_watchdog, test_clamp and test_freshness pass in `goodwe-addon-checks:1`. **Safety diff:** I blanked the strings and diffed the logic. Every hunk in `main.py` only changes what a `_LOG` call or `log_event` says, adds a `_LOG.debug`, or is display-only (`why_words`, `missing_reading()`). No trigger, latch, threshold, log level, notification id, cadence, colour or banner changed. The meter block in `read_inputs()` only logs and sets a flag. `watchdog.py` changes a comment only. **Findings. I will push the fixes for R1–R4 myself:** - **R1 (must fix). After a restart, the evidence is lost at the default log level.** `log_level` defaults to `info` (config.yaml:899), so installers never see the DEBUG "Evidence …" line. `events` lives only in memory (main.py:373) and is empty after a restart. None of the four restart-latch paths (sign per inverter, delivery, site sign, circulation; main.py ~611/641/675/729) writes an event, and `/status` has no reason field. Before this PR the saved figures were on the ERROR line. Now, after a restart, they are only in `/data/state.json`, which an installer can't easily read. DOCS says "on the DEBUG line … and in `events`", which is false on this path. Fix: each restart path also writes the saved reason to the action log (no hint, so the page line is unchanged). Part 7 will pin it. DOCS will say that DEBUG needs `log_level: debug`. - **R2. The byte pin in test_watchdog was loosened without cause.** The reason string did not change (the watchdog.py diff is a comment only), and it is still what `state.json` holds. The meaning checks are fine, but the literal should stay beside them. DECISIONS item 2 will say so. - **R3. The "no safety reason may be listed" check matches only on key names** (`"reason_code" in k`). I put the direction line back to quoting `inv.sign_wd.reason`, classified it in TEXT and listed it in `LONG_LINE_OK`. That mutation was caught only by the separate 30-word-sentence rule, not by this guard. Fix: also refuse any listed key whose TEXT maker is one of `SAFETY_REASONS`. - **R4. The quiet-meter WARNING can repeat on a flapping meter.** It logs once per episode, and an episode ends at the next sample. On `ha_signed`, a healthy meter under a flat load goes 42–97 s between changes (p1.py ~827, above the 30 s default). That gives one WARNING + INFO pair per gap, possibly hundreds a day. Fix: at most one WARNING (and its INFO) per 10 minutes. Episodes in between log at DEBUG. There is still no control change. **Checked and fine:** - **Scan.** A 26-word peak line fails ("over 25 words (26)"). Dropping the peak DEBUG evidence fails ("missing: main.py:3989"). Adding `main.py:messages.reason_code(g.reason)` to `LONG_LINE_OK` fails. I checked the reasons of the 9 exceptions against the code. `refusal` (SAFETY-28 gate) and `scope_errors` are software faults only. The others are tariff, price and maintenance texts, none of them safety. - **Backwards compatibility.** Part 7 builds a real `Controller` from a store holding the old long texts, so it runs the real load path. `REASON_NOT_SAVED` is a greppable code-like token on a log line, which is fine. - **Readability.** Direction (18 words), delivery (17), frozen (24) and peak (17) read plainly. STALE keeps "17 s after the command drops below 250 W". That is justified: it is the one fact the owner can act on (the stop lets go by itself), it is SAFETY-57's release clause, and test_freshness pins it. No change. - **Scope creep.** Circulation keeps its reason in `events` ("CIRCULATING POWER - …"). The clamp ERROR's asked, limit and sent figures are in `messages.output_clamp()`. The loop-bound WARNING's figures are in the coalesced events row. Nothing is lost.
admin added 1 commit 2026-09-27 14:21:07 +02:00
UX-11 review: restored stops log their figures to events, the quiet-meter WARNING is rate-limited
checks / unit (pull_request) Successful in 9m7s
infra-pin / fresh (pull_request) Successful in 1s
3c298af3d8
- After a restart, each restored stop (direction per inverter and site,
  delivery, circulation) also writes its saved reason to the action log.
  log_level defaults to info and events starts empty, so the DEBUG
  evidence alone left the figures only in state.json.
- A flapping meter (ha_signed under a flat load, 42-97 s between changes)
  logs at most one WARNING/INFO pair per 10 minutes, with the episodes in
  between at DEBUG.
- LONG_LINE_OK refuses any key whose text is a safety reason.
- test_watchdog keeps the byte pin beside the meaning pin, since the text
  did not change.
- DOCS says the DEBUG line needs log_level debug.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Owner

Fixes pushed as 3c298af (ls-remote verified). CI is green on it: checks / unit and infra-pin / fresh. This resolves every point in #1847:

  • R1. Fixed. The four restart-latch paths now also call log_event() with the saved reason, e.g. FAILSAFE inverter-a, saved before the restart: SIGN_DISAGREEMENT: …, or …: REASON_NOT_SAVED when none was saved. Each carries the same messages.hint() the live stop uses (stop_restored, not_delivering, site_direction, circulating), because part 6 requires a hint on every event. Part 7 now checks that all three old long texts and REASON_NOT_SAVED are in the restarted Controller's events. DOCS "For installers: notifications" now says the figures are in events, including after a restart, and that the DEBUG line only shows with "Log level" (log_level) set to debug, not at the default info.
  • R2. Fixed. test_watchdog keeps the dev's meaning check and puts the literal byte pin back beside it. DECISIONS item 2 is updated.
  • R3. Fixed. The guard also refuses any LONG_LINE_OK key whose TEXT maker is in SAFETY_REASONS. I re-ran my direction-line mutation, and it now fails on this guard directly.
  • R4. Fixed. METER_QUIET_REPEAT_S = 600: at most one WARNING/INFO pair per 10 minutes, and episodes in between log at DEBUG. It still logs only and decides nothing. The part 7 test runs 3 episodes and expects 2 WARNINGs, 2 INFOs and 1 DEBUG pair. DOCS and DECISIONS (new item 7) are updated.

Merging with head_commit_id pinned.

Fixes pushed as **3c298af** (ls-remote verified). CI is green on it: `checks / unit` and `infra-pin / fresh`. This resolves every point in #1847: - **R1. Fixed.** The four restart-latch paths now also call `log_event()` with the saved reason, e.g. `FAILSAFE inverter-a, saved before the restart: SIGN_DISAGREEMENT: …`, or `…: REASON_NOT_SAVED` when none was saved. Each carries the same `messages.hint()` the live stop uses (`stop_restored`, `not_delivering`, `site_direction`, `circulating`), because part 6 requires a hint on every event. Part 7 now checks that all three old long texts and `REASON_NOT_SAVED` are in the restarted Controller's `events`. DOCS "For installers: notifications" now says the figures are in `events`, including after a restart, and that the DEBUG line only shows with "Log level" (log_level) set to `debug`, not at the default `info`. - **R2. Fixed.** test_watchdog keeps the dev's meaning check and puts the literal byte pin back beside it. DECISIONS item 2 is updated. - **R3. Fixed.** The guard also refuses any `LONG_LINE_OK` key whose TEXT maker is in `SAFETY_REASONS`. I re-ran my direction-line mutation, and it now fails on this guard directly. - **R4. Fixed.** `METER_QUIET_REPEAT_S = 600`: at most one WARNING/INFO pair per 10 minutes, and episodes in between log at DEBUG. It still logs only and decides nothing. The part 7 test runs 3 episodes and expects 2 WARNINGs, 2 INFOs and 1 DEBUG pair. DOCS and DECISIONS (new item 7) are updated. Merging with `head_commit_id` pinned.
admin merged commit d1f7e3336e into release/1.0 2026-09-27 14:31:06 +02:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/goodwe-addon#140