Optional cleanup of discovery configs for removed packs (2.3.0)

jkbms2mqtt/DOCS.md

@@ -63,6 +63,7 @@ Two supported transports between the bus and this add-on:
 | `recording_enabled` | `false` | When on, every Modbus transaction is logged to the add-on log at DEBUG (no separate file). |
 | `install_dashboard` | `true` | Auto-write a ready-made Lovelace dashboard + bank-aggregates package into `<config>/jkbms2mqtt/` on startup. See [Dashboard](#dashboard) below. |
 | `dashboard_cells` | `16` | Cells per pack for that generated dashboard (homogeneous bank). Leave at your largest pack; unused cell rows just show *Unavailable*. |
+| `clean_orphaned_discovery` | `false` | On startup, clear retained discovery configs of packs no longer in `bms_ids`. See [Removing a pack](#removing-a-pack). |
 
 ## Verifying it works
 
@@ -130,6 +131,33 @@ as editable controls for a tier that is on, as read-only rows otherwise.
 
 To opt out entirely, set `install_dashboard: false`; nothing is written.
 
+## Removing a pack
+
+Home Assistant learns about entities from *retained* MQTT discovery messages.
+Those survive on the broker until someone clears them, so a pack you remove
+from `bms_ids` — decommissioned, or its DIP-switch address changed — leaves
+about a hundred dead entities behind. They show as unavailable and come back
+after every Home Assistant restart. The same happens to every pack if you
+change `bms_name_prefix`.
+
+Set `clean_orphaned_discovery: true` and restart the add-on. On startup it
+reads the retained discovery configs on the broker and clears the ones that
+belong to this bridge (its `discovery_prefix` and `bms_name_prefix`) for slave
+addresses that are no longer in `bms_ids`. Every cleared topic is logged. The
+packs you still poll are untouched.
+
+> **Why it is off by default.** Two bridges can share one broker. A pack this
+> instance does not poll may legitimately belong to the other one, and clearing
+> its configs would delete that instance's entities from Home Assistant. Turn
+> the option on deliberately, check the log, and turn it off again if you like —
+> the cleanup only needs to run once.
+
+Prefer to do it by hand? Publish an empty retained message to each stale topic:
+
+```sh
+mosquitto_pub -r -n -t 'homeassistant/sensor/BMS_7_device_total_voltage/config'
+```
+
 ## Writes
 
 Every BMS setting is **always visible** in Home Assistant as a status sensor

jkbms2mqtt/config.yaml

@@ -1,6 +1,6 @@
 ---
 name: jkbms2mqtt
-version: "2.2.2"
+version: "2.3.0"
 slug: jkbms2mqtt
 description: JK-BMS to MQTT bridge using the standard RS485 Modbus V1.0 / V1.1 protocol
 url: https://github.com/basmeerman/jkbms2mqtt
@@ -48,6 +48,7 @@ options:
   recording_enabled: false
   install_dashboard: true
   dashboard_cells: 16
+  clean_orphaned_discovery: false
 
 schema:
   transport: list(tcp_gateway|usb_serial)
@@ -72,3 +73,4 @@ schema:
   recording_enabled: bool
   install_dashboard: bool
   dashboard_cells: int(1,32)
+  clean_orphaned_discovery: bool

jkbms2mqtt/src/jkbms2mqtt/app.py

@@ -19,7 +19,7 @@
 from jkbms2mqtt.bms_runner import BmsRunner
 from jkbms2mqtt.config import Settings, load_settings
 from jkbms2mqtt.entities import writable_by_command_topic_suffix
-from jkbms2mqtt.mqtt import BRIDGE_AVAILABILITY_TOPIC
+from jkbms2mqtt.mqtt import BRIDGE_AVAILABILITY_TOPIC, orphan_removals, render
 from jkbms2mqtt.transport import build_client, connect_with_backoff
 from jkbms2mqtt.write_executor import WriteExecutor, WriteRequest
 
@@ -51,6 +51,52 @@ def configure_logging(settings: Settings) -> None:
         logging.getLogger("pymodbus").setLevel(logging.DEBUG)
 
 
+# How long to keep reading retained discovery configs. MQTT has no
+# end-of-retained marker, so this is a settle window: collection stops once
+# nothing new has arrived for this long.
+DISCOVERY_SETTLE_S = 2.0
+
+
+async def _clean_orphaned_discovery(  # pragma: no cover - MQTT glue
+    mqtt: MqttClient, settings: Settings
+) -> None:
+    """Clear retained discovery configs of packs no longer in ``bms_ids``.
+
+    Called before the command-topic subscriptions on purpose: ``mqtt.messages``
+    is one shared queue, so draining it here while ``/set`` topics were already
+    subscribed could swallow a user's write. Until we subscribe to them, no
+    command can arrive.
+
+    Which topics get cleared is decided by ``mqtt.orphan_removals`` — pure and
+    unit-tested. This function only does the broker conversation.
+    """
+    wildcard = f"{settings.discovery_prefix}/+/+/config"
+    await mqtt.subscribe(wildcard, qos=0)
+    seen: list[str] = []
+    messages = aiter(mqtt.messages)
+    try:
+        while True:
+            message = await asyncio.wait_for(anext(messages), DISCOVERY_SETTLE_S)
+            # An already-cleared config replays as an empty payload; skip it.
+            if message.retain and message.payload:
+                seen.append(str(message.topic))
+    except TimeoutError:
+        pass
+    finally:
+        await mqtt.unsubscribe(wildcard)
+
+    removals = orphan_removals(seen, settings=settings)
+    for removal in removals:
+        topic, payload = render(removal)
+        await mqtt.publish(topic, payload=payload, qos=1, retain=True)
+        logger.info("cleared orphaned discovery config: %s", topic)
+    logger.info(
+        "orphaned-discovery cleanup: %d retained config(s) on the broker, %d cleared",
+        len(seen),
+        len(removals),
+    )
+
+
 async def run(settings: Settings) -> None:  # pragma: no cover - top-level glue
     """Run the bridge until SIGTERM / SIGINT."""
     configure_logging(settings)
@@ -93,6 +139,12 @@ async def publish_write_output(topic: str, payload: str) -> None:
             client=client, settings=settings, publish=publish_write_output
         )
 
+        # Opt-in: clear retained discovery configs of packs that are no longer
+        # configured. Must happen before the /set subscriptions below — see the
+        # helper's docstring.
+        if settings.clean_orphaned_discovery:
+            await _clean_orphaned_discovery(mqtt, settings)
+
         # Always subscribe to every /set topic. The write executor enforces tier
         # gating and publishes a structured error to <bms>/error if a user posts
         # to a parameter whose tier is disabled by config — so the user gets

jkbms2mqtt/src/jkbms2mqtt/config.py

@@ -84,6 +84,13 @@ class Settings(BaseModel):
     # banks: leave at the max; unused cell rows simply show Unavailable.
     dashboard_cells: Annotated[int, Field(ge=1, le=32)] = 16
 
+    # On startup, clear retained discovery configs of packs that are no longer
+    # in bms_ids (a decommissioned pack, or a changed bms_name_prefix), which
+    # HA would otherwise keep forever as dead entities. Off by default: two
+    # bridges can share one broker, and a pack this instance does not poll may
+    # legitimately belong to the other one. See DOCS.md.
+    clean_orphaned_discovery: bool = False
+
     @field_validator("bms_ids", mode="before")
     @classmethod
     def _parse_bms_ids(cls, v: object) -> object:

jkbms2mqtt/src/jkbms2mqtt/mqtt.py

@@ -16,6 +16,8 @@
 
 import json
 import logging
+import re
+from collections.abc import Iterable
 from dataclasses import dataclass
 from datetime import datetime
 from typing import Any
@@ -112,6 +114,41 @@ def discovery_removal(
     )
 
 
+def orphan_removals(topics: Iterable[str], *, settings: Settings) -> list[DiscoveryMessage]:
+    """Removals for retained discovery configs of packs this bridge no longer polls.
+
+    ``topics`` is what the broker replayed under
+    ``<discovery_prefix>/+/+/config``. A topic is cleared only when it is
+    unmistakably ours — our discovery prefix, a known component, and a device
+    id shaped ``<bms_name_prefix>_<slave id>_device_<object_id>`` — and its
+    slave id is **not** in ``bms_ids``. Everything else is left alone: other
+    integrations, another jkbms2mqtt instance with a different prefix, and the
+    packs we still poll (whose own removals ``build_discovery_messages``
+    already handles).
+
+    Pure on purpose. What gets deleted is decided here, under test; the
+    subscribe / collect side is glue in ``app.run``.
+    """
+    pattern = re.compile(
+        rf"^{re.escape(settings.discovery_prefix)}/(?P<component>[^/]+)/"
+        rf"{re.escape(settings.bms_name_prefix)}_(?P<slave>\d+)_device_[^/]+/config$"
+    )
+    components = {c.value for c in Component}
+    configured = set(settings.bms_ids)
+
+    out: list[DiscoveryMessage] = []
+    for topic in topics:
+        match = pattern.match(topic)
+        if match is None or match["component"] not in components:
+            continue
+        if int(match["slave"]) in configured:
+            continue
+        # Clear the topic exactly as the broker reported it, rather than
+        # rebuilding it from an object_id we may no longer know.
+        out.append(DiscoveryMessage(topic=topic, payload=None))
+    return out
+
+
 def _read_only_category(entity_category: str | None) -> str | None:
     """Category for a setting published read-only because its tier is off.
 

jkbms2mqtt/tests/unit/test_mqtt.py

@@ -22,6 +22,7 @@
     discovery_for_packed_bit,
     discovery_for_read_only,
     discovery_for_writable,
+    orphan_removals,
     render,
     state_message_last_seen,
     state_messages_from_live,
@@ -489,6 +490,80 @@ def test_render_returns_compact_json(self) -> None:
         assert json.loads(payload_bytes) == msg.payload
 
 
+class TestOrphanRemovals:
+    """Clearing retained configs of packs the bridge no longer polls (issue #25)."""
+
+    def _topic(self, component: str, bms: str, object_id: str, prefix: str = "homeassistant") -> str:
+        return f"{prefix}/{component}/{bms}_device_{object_id}/config"
+
+    def test_clears_a_pack_that_is_no_longer_configured(self) -> None:
+        s = _settings(bms_ids=[1, 2])
+        topics = [
+            self._topic("sensor", "BMS_1", "total_voltage"),
+            self._topic("sensor", "BMS_7", "total_voltage"),
+            self._topic("number", "BMS_7", "max_charge_current"),
+        ]
+        removals = orphan_removals(topics, settings=s)
+        assert [m.topic for m in removals] == topics[1:]
+        assert all(m.payload is None for m in removals)
+
+    def test_leaves_configured_packs_alone(self) -> None:
+        s = _settings(bms_ids=[1, 2, 3])
+        topics = [self._topic("sensor", f"BMS_{n}", "total_voltage") for n in (1, 2, 3)]
+        assert orphan_removals(topics, settings=s) == []
+
+    def test_ignores_another_integration(self) -> None:
+        s = _settings(bms_ids=[1])
+        topics = [
+            "homeassistant/sensor/zigbee_thermostat/config",
+            "homeassistant/light/kitchen_lamp/config",
+            "zigbee2mqtt/bridge/config",
+        ]
+        assert orphan_removals(topics, settings=s) == []
+
+    def test_ignores_another_bridge_under_a_different_prefix(self) -> None:
+        """A second jkbms2mqtt with its own discovery prefix owns its topics."""
+        s = _settings(bms_ids=[1])
+        topics = [self._topic("sensor", "BMS_7", "total_voltage", prefix="ha-other")]
+        assert orphan_removals(topics, settings=s) == []
+
+    def test_respects_the_bms_name_prefix(self) -> None:
+        s = _settings(bms_ids=[1], bms_name_prefix="PACK")
+        topics = [
+            self._topic("sensor", "PACK_9", "total_voltage"),
+            self._topic("sensor", "BMS_9", "total_voltage"),  # another instance
+        ]
+        assert [m.topic for m in orphan_removals(topics, settings=s)] == [topics[0]]
+
+    def test_ignores_an_unknown_component(self) -> None:
+        s = _settings(bms_ids=[1])
+        topics = [self._topic("climate", "BMS_7", "total_voltage")]
+        assert orphan_removals(topics, settings=s) == []
+
+    def test_ignores_a_malformed_device_id(self) -> None:
+        s = _settings(bms_ids=[1])
+        topics = [
+            "homeassistant/sensor/BMS_x_device_total_voltage/config",  # non-numeric id
+            "homeassistant/sensor/BMS_7_total_voltage/config",  # no _device_
+            "homeassistant/sensor/BMS_7_device_/config",  # empty object_id
+            "homeassistant/sensor/BMS_7_device_total_voltage/state",  # not a config topic
+        ]
+        assert orphan_removals(topics, settings=s) == []
+
+    def test_empty_broker(self) -> None:
+        assert orphan_removals([], settings=_settings(bms_ids=[1])) == []
+
+    def test_removals_render_as_empty_payloads(self) -> None:
+        s = _settings(bms_ids=[1])
+        (removal,) = orphan_removals(
+            [self._topic("binary_sensor", "BMS_4", "charging_switch")], settings=s
+        )
+        assert render(removal) == (
+            "homeassistant/binary_sensor/BMS_4_device_charging_switch/config",
+            b"",
+        )
+
+
 # -- State messages -------------------------------------------------------------------