diff --git a/app/fanout/mqtt_ha.py b/app/fanout/mqtt_ha.py index ebd3578..ef11b0d 100644 --- a/app/fanout/mqtt_ha.py +++ b/app/fanout/mqtt_ha.py @@ -169,6 +169,30 @@ def _extract_gps_reading(lpp_sensors: list[dict]) -> dict | None: return None +def _legacy_geo_sensor_topics(nid: str, lpp_sensors: list[dict]) -> list[str]: + """Discovery topics for GPS/location readings that older versions published + as numeric sensors before GPS was routed to the device_tracker. + + These configs were published ``retain=True``, so after an upgrade they linger + in the broker and HA keeps recreating a dead ``sensor.*_gps_ch*`` entity whose + ``{{ value_json.lpp_gps_ch* }}`` template no longer resolves. We recompute the + topics from current telemetry — replicating the pre-filter ``_assign_lpp_keys`` + numbering for geo sensors only — so they can be cleared even across a restart, + when the in-memory ``_discovery_topics`` history no longer remembers them. + """ + counts: dict[str, int] = {} + topics: list[str] = [] + for sensor in lpp_sensors or []: + if not _is_geo_sensor(sensor): + continue + base = _lpp_sensor_key(sensor.get("type_name", "unknown"), sensor.get("channel", 0)) + n = counts.get(base, 0) + 1 + counts[base] = n + key = base if n == 1 else f"{base}_{n}" + topics.append(f"homeassistant/sensor/meshcore_{nid}/{key}/config") + return topics + + def _assign_lpp_keys(lpp_sensors: list[dict]) -> list[tuple[dict, str, int]]: """Pair each LPP sensor dict with a disambiguated flat key and occurrence. @@ -609,6 +633,8 @@ class MqttHaModule(FanoutModule): configs: list[tuple[str, dict]] = [] cached_repeater_states: list[tuple[str, dict[str, Any]]] = [] + # Retained GPS-sensor configs published by older versions, to be cleared. + legacy_geo_topics: list[str] = [] radio_name = self._radio_name or "MeshCore Radio" configs.extend(_radio_discovery_configs(self._prefix, self._radio_key, radio_name)) @@ -630,6 +656,7 @@ class MqttHaModule(FanoutModule): configs.extend( _lpp_discovery_configs(self._prefix, pub_key, device, lpp_sensors, state_topic) ) + legacy_geo_topics.extend(_legacy_geo_sensor_topics(nid, lpp_sensors)) if latest_data: cached_repeater_states.append( ( @@ -657,6 +684,7 @@ class MqttHaModule(FanoutModule): self._prefix, pub_key, ct_device, ct_lpp_sensors, ct_state_topic ) ) + legacy_geo_topics.extend(_legacy_geo_sensor_topics(ct_nid, ct_lpp_sensors)) if latest_ct_data: ct_payload = _contact_telemetry_payload(latest_ct_data) cached_repeater_states.append( @@ -670,8 +698,16 @@ class MqttHaModule(FanoutModule): # longer generate (e.g. the legacy broken GPS numeric sensor, now routed # to the device_tracker, or sensors from an untracked node). Without this # the broker's retained config would keep recreating the dead entity. + # + # `_discovery_topics` only covers configs this process published, so it + # cannot reach a stale GPS sensor config left retained by an older + # version before the upgrade+restart. `legacy_geo_topics` recomputes + # those deterministically from current telemetry to close that gap. new_topics = [topic for topic, _ in configs] - stale = [t for t in self._discovery_topics if t not in new_topics] + new_topic_set = set(new_topics) + stale_set = {t for t in self._discovery_topics if t not in new_topic_set} + stale_set.update(t for t in legacy_geo_topics if t not in new_topic_set) + stale = sorted(stale_set) if stale: await self._clear_retained_topics(stale) self._discovery_topics = new_topics diff --git a/tests/test_mqtt_ha.py b/tests/test_mqtt_ha.py index b8e739a..a422cff 100644 --- a/tests/test_mqtt_ha.py +++ b/tests/test_mqtt_ha.py @@ -13,6 +13,7 @@ from app.fanout.mqtt_ha import ( _device_payload, _extract_gps_reading, _is_geo_sensor, + _legacy_geo_sensor_topics, _lpp_discovery_configs, _lpp_sensor_key, _message_event_discovery_config, @@ -398,6 +399,66 @@ class TestStaleDiscoveryCleanup: assert stale_topic in cleared assert stale_topic not in mod._discovery_topics + def test_legacy_geo_sensor_topics_for_gps(self): + nid = "ccdd11223344" + topics = _legacy_geo_sensor_topics( + nid, + [ + {"channel": 1, "type_name": "gps", "value": {"latitude": 1.0, "longitude": 2.0}}, + {"channel": 2, "type_name": "temperature", "value": 23.5}, + ], + ) + assert topics == [f"homeassistant/sensor/meshcore_{nid}/lpp_gps_ch1/config"] + + def test_legacy_geo_sensor_topics_disambiguates_duplicates(self): + nid = "ccdd11223344" + gps = {"channel": 1, "type_name": "gps", "value": {"latitude": 1.0, "longitude": 2.0}} + topics = _legacy_geo_sensor_topics(nid, [gps, gps]) + assert topics == [ + f"homeassistant/sensor/meshcore_{nid}/lpp_gps_ch1/config", + f"homeassistant/sensor/meshcore_{nid}/lpp_gps_ch1_2/config", + ] + + @pytest.mark.asyncio + async def test_publish_discovery_clears_legacy_gps_sensor_after_restart(self): + """A retained GPS sensor config from an older version must be cleared even + when this process never published it (in-memory history is empty).""" + contact_key = "ccdd11223344" + mod = MqttHaModule("test", _base_config(tracked_contacts=[contact_key])) + mod._radio_key = "aabbccddeeff" + mod._radio_name = "MyRadio" + mod._publisher = MagicMock() + mod._publisher.connected = True + mod._publisher.publish = AsyncMock() + mod._clear_retained_topics = AsyncMock() + # Fresh process after upgrade: nothing remembered from a prior run. + mod._discovery_topics = [] + mod._resolve_contact_name = AsyncMock(return_value="MCRadio2") + mod._resolve_latest_contact_telemetry = AsyncMock( + return_value={ + "timestamp": 1234, + "data": { + "lpp_sensors": [ + { + "channel": 1, + "type_name": "gps", + "value": {"latitude": 21.0, "longitude": -21.0}, + }, + ], + }, + } + ) + + await mod._publish_discovery() + + nid = _node_id(contact_key) + legacy_topic = f"homeassistant/sensor/meshcore_{nid}/lpp_gps_ch1/config" + cleared = mod._clear_retained_topics.call_args[0][0] + assert legacy_topic in cleared + # The dead GPS sensor config must not be re-published. + published = [c.args[0] for c in mod._publisher.publish.call_args_list] + assert legacy_topic not in published + class TestMqttHaHealth: @pytest.mark.asyncio