Merge release/1.0 (7337b79, OPT-11 P2) into safety-79-peak-margin

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

goodwe_controller/app/main.py

@@ -72,8 +72,9 @@ from .maintenance import (
     IDLE, POWER_MOVING, MaintConfig, Maintenance, MaintResult,
     arbitrate as maint_arbitrate,
 )
-from .mqtt import (MqttPublisher, announced_units, optional_keys, unit_slugs,
-                   unit_state_key)
+from .mqtt import (MODE_CONFIGS_KEY, MODE_LABELS, USE_OFF, USE_ON,
+                   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
@@ -905,9 +906,12 @@ class Controller:
         # turned them on behaves exactly as before (SAFETY-10 section 1). Each
         # battery's mode and on/off switch live in the store, not in options,
         # so a change applies at once (`pack_use`, `pack_mode` actions).
-        self.modes_on = opts.get("battery_modes_enabled") is True
+        self.modes_on = packmode.enabled(opts)
         # The units whose mode fault is logged already (edge-logged).
         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 = []
@@ -2062,6 +2066,65 @@ class Controller:
         pm = self.pack_mode(inv)
         return {"unit": inv.name, "use": pm.use, "mode": pm.mode}
 
+    def mode_values(self) -> dict:
+        """OPT-11 P2: each battery's mode and switch, for their HA state
+        topics. Read from the store, so they are right after a restart. Empty
+        while battery modes are off."""
+        if not self.modes_on:
+            return {}
+        out = {}
+        names = [inv.name for inv in self.inverters]
+        for inv, slug in zip(self.inverters, unit_slugs(names)):
+            pm = self.pack_mode(inv)
+            out[unit_state_key(slug, "mode")] = MODE_LABELS.get(pm.mode,
+                                                               pm.mode)
+            out[unit_state_key(slug, "use")] = USE_ON if pm.use else USE_OFF
+        return out
+
+    async def mqtt_command(self, topic, payload, retained) -> None:
+        """OPT-11 P2: one command from a battery's HA switch or mode list.
+
+        It goes through handle_action(), the page's /action path, so the
+        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, 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(
+                topic, payload, retained, packmode.MODES)
+            if why is None:
+                names = [inv.name for inv in self.inverters]
+                unit = dict(zip(unit_slugs(names), names)).get(slug)
+                result = await self.handle_action(action, unit=unit,
+                                                  value=value)
+                if not (isinstance(result, dict) and "error" in result):
+                    self.mqtt.publish(self.mode_values())
+                    return
+                why = result["error"]
+            else:
+                why = messages.pack_refused(why)
+            # 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,
+                         why)
+        except Exception:                               # noqa: BLE001
+            _LOG.exception("Home Assistant command could not be handled")
+
     def _maint_stop_off(self, machine, now) -> None:
         """OPT-11: stop this battery's maintenance cycle because it is off."""
         if machine.phase == IDLE:
@@ -5153,6 +5216,8 @@ class Controller:
             values[unit_state_key(slug, "soc")] = inv.soc
             values[unit_state_key(slug, "sign_latch")] = (
                 "tripped" if inv.sign_wd.tripped else "clear")
+        # OPT-11 P2: each battery's mode and switch; nothing while modes are off.
+        values.update(self.mode_values())
         self.mqtt.publish(values)
 
     async def shutdown(self) -> None:
@@ -6068,6 +6133,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()
@@ -6198,6 +6278,9 @@ async def amain() -> None:
         # took the control loop down with it.
         broker = None
         try:
+            # OPT-11 P2: each battery's switch and mode list, only while
+            # 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,
@@ -6217,6 +6300,8 @@ async def amain() -> None:
                 # is what `build_roster()` itself reads. At a roster of one this
                 # announces nothing new - see `announced_units()`.
                 units=roster_names(opts),
+                modes=_modes,
+                retract=_retract,
             )
         except Exception as err:  # noqa: BLE001
             _LOG.warning("MQTT unavailable (%s) - continuing without status entities", err)
@@ -6230,6 +6315,8 @@ async def amain() -> None:
 
         stop = asyncio.Event()
         controller = Controller(opts, hass, store, pub, stop=stop)
+        if callable(getattr(pub, "bind", None)):
+            pub.bind(asyncio.get_running_loop(), controller.mqtt_command)
         runner = await web.start(controller, port=INGRESS_PORT)
 
         loop = asyncio.get_running_loop()

goodwe_controller/app/messages.py

@@ -932,6 +932,11 @@ PACK_REFUSED = {
     "off": "Battery modes are off. Turn on “Battery modes” first.",
     "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": "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

@@ -7,8 +7,10 @@ That separation is deliberate; observability must never be able to take down
 control.
 """
 
+import asyncio
 import json
 import logging
+import re
 
 try:
     import paho.mqtt.client as mqtt
@@ -375,7 +377,26 @@ def _payload(name, object_id, unique_id, state_topic, device,
     return cfg
 
 
-def discovery_payloads(units=(), omit=(), include=()) -> list:
+def unit_unique_id(slug: str, key: str) -> str:
+    """One unit's entity `unique_id` (also its config topic's middle part)."""
+    return f"{BASE}_unit_{slug}_{key}"
+
+
+def _unit_places(names):
+    """`(slug, device, prefix)` for each roster name, in order."""
+    slugs = unit_slugs(names)
+    for i, (name, slug) in enumerate(zip(names, slugs)):
+        # ⚠️ `i == 0` IS THE ASYMMETRY, IN ONE LINE. The first roster entry
+        # keeps the device it already has and carries its name in the ENTITY
+        # name instead; every later one gets its own device and does not.
+        # Either way HA composes device-name + entity-name into the same
+        # string, which is why `object_id` can be written once for both.
+        device = DEVICE if i == 0 else unit_device(name, slug)
+        prefix = f"{_title(name)} " if i == 0 else ""
+        yield slug, device, prefix
+
+
+def discovery_payloads(units=(), omit=(), include=(), modes=()) -> list:
     """Every `(topic, payload)` this add-on announces, in announcement order.
 
     Split out of `_announce()` so that the acceptance criterion can be graded as
@@ -389,6 +410,8 @@ def discovery_payloads(units=(), omit=(), include=()) -> list:
     the output here (`_payload()`'s note, and test_mqtt.py part 1 asserts an
     ordered LIST precisely because a re-order is a zero-byte edit that
     re-announces every entity on a live install).
+
+    `modes` (OPT-11 P2) is the battery modes offered, empty when they are off.
     """
     omit = set(omit)
     include = [k for k in include if k not in omit]
@@ -400,19 +423,10 @@ def discovery_payloads(units=(), omit=(), include=()) -> list:
             f"homeassistant/sensor/{BASE}_{key}/config",
             _payload(name, object_id, f"{BASE}_{key}", f"{BASE}/{key}",
                      DEVICE, unit, dev_class, state_class, icon)))
-    names = announced_units(units)
-    slugs = unit_slugs(names)
     site_slug = _slug(DEVICE["name"])
-    for i, (name, slug) in enumerate(zip(names, slugs)):
-        # ⚠️ `i == 0` IS THE ASYMMETRY, IN ONE LINE. The first roster entry
-        # keeps the device it already has and carries its name in the ENTITY
-        # name instead; every later one gets its own device and does not.
-        # Either way HA composes device-name + entity-name into the same
-        # string, which is why `object_id` below can be written once for both.
-        device = DEVICE if i == 0 else unit_device(name, slug)
-        prefix = f"{_title(name)} " if i == 0 else ""
+    for slug, device, prefix in _unit_places(announced_units(units)):
         for key, oid, label, unit, dev_class, state_class, icon in UNIT_SENSORS:
-            unique_id = f"{BASE}_unit_{slug}_{key}"
+            unique_id = unit_unique_id(slug, key)
             out.append((
                 f"homeassistant/sensor/{unique_id}/config",
                 _payload(prefix + label,
@@ -427,12 +441,134 @@ def discovery_payloads(units=(), omit=(), include=()) -> list:
             f"homeassistant/sensor/{BASE}_{key}/config",
             _payload(name, object_id, f"{BASE}_{key}", f"{BASE}/{key}",
                      DEVICE, unit, dev_class, state_class, icon)))
+    # OPT-11 P2: last, and only when battery modes are on.
+    out.extend(mode_payloads(units, modes))
     return out
 
 
+# --- OPT-11 P2: each battery's switch and mode, as HA entities ---------------
+# Announced only when battery modes are on, after every other row, so no
+# existing row moves. A command goes through Controller.handle_action(), the
+# page's /action path. DECISIONS/OPT-11-battery-modes.md has the record.
+#
+# The select's options are these labels, in `packmode.MODES` order. A new mode
+# (P3) is a new entry here and a new option at the end; no id changes.
+MODE_LABELS = {"solar_first": "Solar first", "leftover_solar": "Leftover solar"}
+USE_ON, USE_OFF = "ON", "OFF"
+# A longer command payload is refused before it is decoded.
+MAX_COMMAND_BYTES = 64
+# The store key holding the mode config topics last announced, so that turning
+# modes off can remove exactly those (an empty retained config) and an install
+# that never turned them on publishes nothing new.
+MODE_CONFIGS_KEY = "mqtt_mode_configs"
+_MODE_CONFIG_RE = re.compile(
+    rf"homeassistant/(select|switch)/{BASE}_unit_[a-z0-9_]+_(mode|use)/config")
+_COMMAND_RE = re.compile(rf"{BASE}/unit/([a-z0-9_]+)/(mode|use)/set")
+# (entity key, HA component, entity name, icon)
+MODE_ENTITIES = [
+    ("mode", "select", "Mode", "mdi:battery-sync"),
+    ("use", "switch", "Use", "mdi:power"),
+]
+
+
+def mode_payloads(units, modes) -> list:
+    """The `(topic, payload)` for each battery's select and switch.
+
+    Every roster entry gets them, also at a roster of one. The device rule is
+    TEL-07's: the first battery's rows go on the site device with its name in
+    front, each later battery's on its own device. Empty when `modes` is empty.
+    """
+    if not modes:
+        return []
+    site_slug = _slug(DEVICE["name"])
+    options = [MODE_LABELS[m] for m in modes]
+    out = []
+    for slug, device, prefix in _unit_places([str(u) for u in units]):
+        for key, component, label, icon in MODE_ENTITIES:
+            unique_id = unit_unique_id(slug, key)
+            state = f"{BASE}/{unit_state_key(slug, key)}"
+            cfg = {
+                "name": prefix + label,
+                "object_id": f"{site_slug}_{slug}_{key}",
+                "unique_id": unique_id,
+                "state_topic": state,
+                "command_topic": f"{state}/set",
+            }
+            if component == "select":
+                cfg["options"] = options
+            else:
+                cfg["payload_on"] = USE_ON
+                cfg["payload_off"] = USE_OFF
+            # HA sends commands unretained; a retained one is ignored anyway.
+            cfg["retain"] = False
+            cfg["availability_topic"] = AVAILABILITY
+            cfg["device"] = device
+            cfg["icon"] = icon
+            out.append((f"homeassistant/{component}/{unique_id}/config", cfg))
+    return out
+
+
+def mode_retractions(stored, current) -> list:
+    """Config topics announced before and not now: each gets an empty
+    retained config, which removes the entity from HA. Anything in the store
+    that is not one of our mode config topics is ignored."""
+    if not isinstance(stored, list):
+        return []
+    keep = set(current)
+    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):
+    """`(slug, action, value, reject)` for one message on a command topic.
+
+    `action` is `pack_use` or `pack_mode`. `value` is what /action would be
+    sent: a bool for the switch, a mode key for the select, or None for a
+    payload that is neither (the store path then refuses it). `reject` is a
+    reason when the message is dropped here: replayed by the broker (retained),
+    too long, not UTF-8, or not a command topic.
+    """
+    m = _COMMAND_RE.fullmatch(topic) if isinstance(topic, str) else None
+    if m is None:
+        return None, None, None, "topic"
+    slug, key = m.group(1), m.group(2)
+    action = "pack_use" if key == "use" else "pack_mode"
+    if retained:
+        return slug, action, None, "retained"
+    if not isinstance(payload, (bytes, bytearray)) \
+            or len(payload) > MAX_COMMAND_BYTES:
+        return slug, action, None, "size"
+    try:
+        text = bytes(payload).decode("utf-8")
+    except UnicodeDecodeError:
+        return slug, action, None, "encoding"
+    if key == "use":
+        value = {USE_ON: True, USE_OFF: False}.get(text)
+    else:
+        value = next((k for k in modes if MODE_LABELS.get(k) == text), None)
+    return slug, action, value, None
+
+
 class MqttPublisher:
+    # OPT-11 P2. Class defaults, so a publisher built without them (and every
+    # test fixture that builds one with __new__) behaves as before.
+    modes = ()
+    retract = ()
+    _loop = None
+    _handler = None
+
     def __init__(self, host, port, username=None, password=None, omit=(),
-                 units=(), include=()):
+                 units=(), include=(), modes=(), retract=()):
         # `omit` drops sensor keys from discovery entirely. ⚠️ Announcing a
         # sensor that nothing will ever publish to is not harmless here:
         # p1_sample_age_s is a watchdog input, and an entity that exists but is
@@ -448,6 +584,10 @@ class MqttPublisher:
         # second reading of the option. Defaults to empty, so a caller that has
         # not been taught about it announces exactly what it announced before.
         self.include = [str(k) for k in include]
+        # OPT-11 P2: the modes offered (empty when battery modes are off), and
+        # the mode config topics to remove (`mode_retractions()`).
+        self.modes = tuple(modes)
+        self.retract = [str(t) for t in retract]
         self.enabled = mqtt is not None and bool(host)
         self.client = None
         if not self.enabled:
@@ -470,7 +610,8 @@ class MqttPublisher:
         # paho DROPS QoS-0 publishes when disconnected, silently. The result is
         # an add-on that logs "MQTT connected" and creates no entities at all.
         # As a bonus, this also re-announces after every reconnect.
-        self.client.on_connect = lambda *_args, **_kw: self._announce()
+        self.client.on_connect = lambda *_args, **_kw: self._on_connect()
+        self.client.on_message = self._on_message
         try:
             self.client.connect(host, int(port), keepalive=60)
             self.client.loop_start()
@@ -485,10 +626,35 @@ class MqttPublisher:
         # diffing what actually goes on the wire - and a payload built here
         # instead would be one this add-on publishes and no check has ever seen.
         for topic, cfg in discovery_payloads(self.units, self.omit,
-                                             self.include):
+                                             self.include, self.modes):
             self.client.publish(topic, json.dumps(cfg), retain=True)
+        for topic in self.retract:
+            self.client.publish(topic, "", retain=True)
         self.client.publish(AVAILABILITY, "online", retain=True)
 
+    def _on_connect(self) -> None:
+        self._announce()
+        if self.modes:
+            for key in ("mode", "use"):
+                self.client.subscribe(f"{BASE}/unit/+/{key}/set")
+
+    def bind(self, loop, handler) -> None:
+        """OPT-11 P2: hand each command to `handler(topic, payload, retained)`,
+        a coroutine function run on `loop` (the controller's)."""
+        self._loop, self._handler = loop, handler
+
+    def _on_message(self, _client, _userdata, msg) -> None:
+        # paho's network thread: nothing is done here but the handoff.
+        loop, handler = self._loop, self._handler
+        if loop is None or handler is None:
+            _LOG.debug("MQTT command before start-up ignored: %s", msg.topic)
+            return
+        try:
+            asyncio.run_coroutine_threadsafe(
+                handler(msg.topic, bytes(msg.payload), bool(msg.retain)), loop)
+        except RuntimeError as err:  # the loop has closed
+            _LOG.debug("MQTT command dropped: %s", err)
+
     def publish(self, values: dict) -> None:
         if not self.enabled or self.client is None:
             return

goodwe_controller/app/packmode.py

@@ -42,6 +42,12 @@ MODE_KEY = "pack_mode"
 SELL_KEY = "pack_sell"              # reserved for P3
 
 
+def enabled(opts) -> bool:
+    """Battery modes are on only when the option is exactly true. One answer,
+    read by the controller and by the MQTT announcement."""
+    return opts.get("battery_modes_enabled") is True
+
+
 @dataclass(frozen=True)
 class PackMode:
     """One battery's choice: on or off, its mode, and (P3) selling."""