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

OPT-11 P2 review: refusals stay counted, removals survive a start with no broker

Browse Source 
- A refusal past the log-once set or the 32 cap is counted in the log at
  1, 10, 100 and so on, so a repeated attack stays visible.
- The logged topic is printable only (no forged log lines).
- mode_announcement(): the start-up store logic, now tested with a real
  Store. A removed config topic stays stored, so a start with no broker
  cannot forget it (rename case tested).
- A test ties each HA mode option to the page's English name.
- packmode.enabled() is tested for "true", 1 and "false".
- Retained-command text, DOCS and DECISIONS updated to match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
admin
2026-09-28 23:58:34 +02:00
co-authored by Claude Opus 5.5
parent b310282729
commit c664e38ef4
7 changed files with 141 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
goodwe_controller/app/main.py
+31 -14
View File
@@ -73,9 +73,8 @@ from .maintenance import (
arbitrate as maint_arbitrate,
)
from .mqtt import (MODE_CONFIGS_KEY, MODE_LABELS, USE_OFF, USE_ON,
MqttPublisher, announced_units, mode_payloads,
mode_retractions, optional_keys, parse_command, unit_slugs,
unit_state_key)
MqttPublisher, announced_units, mode_configs,
optional_keys, parse_command, unit_slugs, unit_state_key)
from . import peakguard
from . import packshare
from . import pv as pv_read
@@ -908,6 +907,7 @@ class Controller:
self._mode_faults = set()
# OPT-11 P2: (topic, reason) of MQTT commands refused and logged once.
self._mqtt_refused = set()
self._mqtt_refused_more = 0
# [(pack name, why)] for packs that are due and waiting their turn.
# Empty at a roster of one, where nothing can defer anything.
self._maint_deferred = []
@@ -2079,7 +2079,8 @@ class Controller:
checks and the store write are the same. A retained command (one the
broker replays on subscribe, e.g. after a restart) is never applied:
the store already holds the last choice. A refusal is logged once per
topic and reason. Never raises: MQTT must not reach control.
topic and reason, at most 32, and further refusals are counted in the
log at 1, 10, 100 and so on. Never raises: MQTT must not reach control.
"""
try:
slug, action, value, why = parse_command(
@@ -2095,11 +2096,19 @@ class Controller:
why = result["error"]
else:
why = messages.pack_refused(why)
topic_s = str(topic)[:128]
# Printable only: a topic is sender-chosen, so no log injection.
topic_s = "".join(c if c.isprintable() else "?"
for c in str(topic)[:128])
_LOG.debug("Home Assistant command %s %s refused: %s", topic_s,
repr(payload)[:48], why)
if (topic_s, why) in self._mqtt_refused \
or len(self._mqtt_refused) >= 32:
# Not logged again, but counted: 1, 10, 100... stay visible.
self._mqtt_refused_more += 1
more = self._mqtt_refused_more
if str(more).rstrip("0") == "1":
_LOG.warning("Home Assistant commands still ignored: %d "
"more so far.", more)
return
self._mqtt_refused.add((topic_s, why))
_LOG.warning("Home Assistant command on %s ignored: %s", topic_s,
@@ -6115,6 +6124,21 @@ class ControlTaskDied(RuntimeError):
"""The control task ended while nothing had asked it to stop."""
def mode_announcement(opts, store) -> tuple:
"""OPT-11 P2: `(modes, retract)` for the MQTT publisher at start-up.
The config topics announced are kept in the store, so turning modes off
removes exactly those (`mqtt.mode_configs()`). An install that never
turned modes on gets nothing, and its store is not written.
"""
modes = packmode.MODES if packmode.enabled(opts) else ()
stored = store.data.get(MODE_CONFIGS_KEY)
keep, retract = mode_configs(roster_names(opts), modes, stored)
if keep and keep != stored:
store.set(MODE_CONFIGS_KEY, keep)
return modes, retract
def control_task_done(task: asyncio.Task, *, controller, stop: asyncio.Event,
died: list) -> None:
"""Done-callback on the control task: log its death now and wake amain()
@@ -6246,15 +6270,8 @@ async def amain() -> None:
broker = None
try:
# OPT-11 P2: each battery's switch and mode list, only while
# battery modes are on. The config topics are kept in the store,
# so turning modes off removes exactly the ones announced.
_modes = packmode.MODES if packmode.enabled(opts) else ()
_mode_cfgs = [t for t, _ in mode_payloads(roster_names(opts),
_modes)]
_retract = mode_retractions(store.data.get(MODE_CONFIGS_KEY),
_mode_cfgs)
if _modes:
store.set(MODE_CONFIGS_KEY, _mode_cfgs)
# battery modes are on (`mode_announcement()`).
_modes, _retract = mode_announcement(opts, store)
broker = await hass.mqtt_service()
pub = MqttPublisher(
broker.get("host") if broker else None,
goodwe_controller/app/messages.py
+1 -1
View File
@@ -933,7 +933,7 @@ PACK_REFUSED = {
"unit": "There is no battery with that name.",
"value": "That choice is not possible for this battery.",
# OPT-11 P2: a Home Assistant command dropped before it reaches a battery.
"retained": "A saved command is not used after a restart.",
"retained": "An old saved command is not used.",
"size": "That command is too long.",
"encoding": "That command is not plain text.",
"topic": "That is not a battery command.",
goodwe_controller/app/mqtt.py
+12 -2
View File
@@ -515,8 +515,18 @@ def mode_retractions(stored, current) -> list:
if not isinstance(stored, list):
return []
keep = set(current)
return [t for t in stored if isinstance(t, str)
and _MODE_CONFIG_RE.fullmatch(t) and t not in keep]
return list(dict.fromkeys(t for t in stored if isinstance(t, str)
and _MODE_CONFIG_RE.fullmatch(t)
and t not in keep))
def mode_configs(units, modes, stored) -> tuple:
"""`(keep, retract)` at start-up: the mode config topics to store, and the
ones to remove now. A removal stays in `keep`, so a start with no broker
does not forget it; resending an empty config is harmless."""
current = [t for t, _ in mode_payloads(units, modes)]
retract = mode_retractions(stored, current)
return current + retract, retract
def parse_command(topic, payload, retained, modes):