From d7f0731d1c373fa0b4b39a3e677d6b446b1611de Mon Sep 17 00:00:00 2001 From: Tianyu Liu Date: Wed, 24 Jun 2026 16:21:15 +0200 Subject: [PATCH] ha_discovery: publish state/availability under home_automation prefix Follow the standard MQTT Discovery split: discovery config stays under the HA discovery prefix (homeassistant/.../config, required), but the actual state and availability are published under our own prefix (default home_automation/). The config payload's state_topic/availability fields point at the new prefix so HA reads state from there. - new config ha_state_topic_prefix (default home_automation). - build_discovery_payload + all publish_* split discovery_prefix (config topic) from state_prefix (state/availability). energy-cost still omits availability; modbus availability structure unchanged. Tests updated. --- .env.example | 3 + app/config.py | 3 + app/services/config_page.py | 7 ++ app/services/ha_discovery.py | 58 ++++++---- tests/test_energy_expose.py | 4 +- tests/test_ha_discovery.py | 212 +++++++++++++++++++++++++++++++++-- 6 files changed, 253 insertions(+), 34 deletions(-) diff --git a/.env.example b/.env.example index 5d0b377..fbce5ec 100644 --- a/.env.example +++ b/.env.example @@ -43,8 +43,11 @@ MQTT_TLS_ENABLED=false # Optional: Home Assistant MQTT Discovery. # Requires MQTT_ENABLED=true and a running MQTT broker. +# HA_DISCOVERY_PREFIX is the config topic prefix (must be "homeassistant" for HA auto-discovery). +# HA_STATE_TOPIC_PREFIX is the prefix for state/availability topics (separate from discovery). HA_DISCOVERY_ENABLED=false HA_DISCOVERY_PREFIX=homeassistant +HA_STATE_TOPIC_PREFIX=home_automation # Optional: DSMR smart-meter ingest via MQTT (M6; default off — opt-in). # Set DSMR_INGEST_ENABLED=true to subscribe to the DSMR MQTT topic and diff --git a/app/config.py b/app/config.py index 1aedb9d..ece2eac 100644 --- a/app/config.py +++ b/app/config.py @@ -57,6 +57,9 @@ class Settings(BaseSettings): # Home Assistant MQTT Discovery (T08 wires into CONFIG_FIELDS/UI; T11 does publishing). ha_discovery_enabled: bool = False ha_discovery_prefix: str = "homeassistant" + # State/availability topics use a separate prefix so they live outside the HA discovery + # namespace. HA still receives state via the state_topic declared in the discovery config. + ha_state_topic_prefix: str = "home_automation" # DSMR smart-meter ingest via MQTT (M6; default off — opt-in). # Subscribe to dsmr_mqtt_topic when dsmr_ingest_enabled=True. diff --git a/app/services/config_page.py b/app/services/config_page.py index 5573cc6..d05128b 100644 --- a/app/services/config_page.py +++ b/app/services/config_page.py @@ -122,6 +122,12 @@ CONFIG_FIELDS: tuple[ConfigField, ...] = ( "ha_discovery_prefix", "HA Discovery Prefix", ), + ConfigField( + "Home Assistant Discovery", + "HA_STATE_TOPIC_PREFIX", + "ha_state_topic_prefix", + "HA State Topic Prefix", + ), ConfigField("Modbus", "MODBUS_POLLING_ENABLED", "modbus_polling_enabled", "Modbus Polling Enabled", input_type="checkbox"), ConfigField( "DSMR", @@ -351,6 +357,7 @@ def _settings_payload(settings: Settings) -> dict[str, Any]: "mqtt_tls_enabled": settings.mqtt_tls_enabled, "ha_discovery_enabled": settings.ha_discovery_enabled, "ha_discovery_prefix": settings.ha_discovery_prefix, + "ha_state_topic_prefix": settings.ha_state_topic_prefix, "dsmr_ingest_enabled": settings.dsmr_ingest_enabled, "dsmr_mqtt_topic": settings.dsmr_mqtt_topic, "dsmr_sample_interval_s": settings.dsmr_sample_interval_s, diff --git a/app/services/ha_discovery.py b/app/services/ha_discovery.py index a99f834..3b8a3f4 100644 --- a/app/services/ha_discovery.py +++ b/app/services/ha_discovery.py @@ -15,12 +15,17 @@ Design doing any work. Callers never need to guard these conditions. - **Stable unique_id**: derived from device ``uuid`` + metric ``key``, never from mutable fields like ``friendly_name`` or auto-increment DB ids. -- **Discovery topic format**: ``////config`` +- **Discovery topic format**: ``////config`` where ``node_id`` is the device uuid (slugified to be safe) and - ``object_id`` is a uuid-prefixed stable string. -- **State topic format**: ``////state`` -- **Availability topic**: ``/modbus//availability`` (shared - across all entities of the same device; publishes "online"/"offline"). + ``object_id`` is a uuid-prefixed stable string. Uses ``ha_discovery_prefix`` + (default ``"homeassistant"``), which must stay under the HA discovery namespace. +- **State topic format**: ``////state`` + Uses ``ha_state_topic_prefix`` (default ``"home_automation"``), separate from + the discovery namespace so HA discovery and state/availability topics live under + different prefixes. +- **Availability topic**: ``/modbus//availability`` (shared + across all entities of the same device; publishes "online"/"offline"). Also uses + the state prefix, not the discovery prefix. - **best-effort**: functions catch all exceptions internally so that callers (e.g. the Modbus poll loop) never crash due to MQTT publish errors. """ @@ -107,7 +112,8 @@ def _unique_id(entity: ExposableEntity) -> str: def build_discovery_payload( entity: ExposableEntity, - prefix: str, + discovery_prefix: str, + state_prefix: str | None = None, ) -> tuple[str, dict[str, Any]]: """Build the HA MQTT Discovery config topic and payload dict for *entity*. @@ -115,8 +121,14 @@ def build_discovery_payload( ---------- entity: An ``ExposableEntity`` (from ``build_catalog``). - prefix: - The discovery prefix (e.g. ``"homeassistant"``). + discovery_prefix: + The HA Discovery config topic prefix (e.g. ``"homeassistant"``). + This determines where HA looks for the discovery config topic. + state_prefix: + The prefix used for state and availability topics (e.g. + ``"home_automation"``). When omitted or ``None``, falls back to + ``discovery_prefix`` for backward compatibility (e.g. in tests that + pass only one prefix). Returns ------- @@ -124,10 +136,13 @@ def build_discovery_payload( ``(topic, config_dict)`` — the config topic and the HA Discovery payload (not yet JSON-serialised). """ + if state_prefix is None: + state_prefix = discovery_prefix + device_uuid = entity.device.identifiers[1] - avail_topic = _availability_topic(device_uuid, prefix) - state_t = _state_topic(entity, prefix) - topic = _discovery_topic(entity, prefix) + avail_topic = _availability_topic(device_uuid, state_prefix) + state_t = _state_topic(entity, state_prefix) + topic = _discovery_topic(entity, discovery_prefix) config: dict[str, Any] = { "unique_id": _unique_id(entity), @@ -188,7 +203,8 @@ def publish_discovery(session: Session) -> None: logger.debug("publish_discovery: skipped (MQTT/Discovery not enabled or not connected)") return - prefix = settings.ha_discovery_prefix + discovery_prefix = settings.ha_discovery_prefix + state_prefix = settings.ha_state_topic_prefix try: catalog = build_catalog(session) @@ -199,7 +215,7 @@ def publish_discovery(session: Session) -> None: for entry in catalog: entity = entry.entity try: - topic, config = build_discovery_payload(entity, prefix) + topic, config = build_discovery_payload(entity, discovery_prefix, state_prefix) if entry.enabled: payload = json.dumps(config) mqtt_manager.publish(topic, payload, retain=True) @@ -241,7 +257,7 @@ def publish_states(session: Session) -> None: logger.debug("publish_states: skipped (MQTT/Discovery not enabled or not connected)") return - prefix = settings.ha_discovery_prefix + state_prefix = settings.ha_state_topic_prefix try: catalog = build_catalog(session) @@ -255,7 +271,7 @@ def publish_states(session: Session) -> None: continue entity = entry.entity try: - _publish_entity_state(entity, prefix, session) + _publish_entity_state(entity, state_prefix, session) except Exception: logger.exception( "publish_states: error publishing state for %r; continuing", entity.key @@ -329,13 +345,13 @@ def publish_device_state(session: Session, device: Any) -> None: if not _should_publish(settings): return - prefix = settings.ha_discovery_prefix + state_prefix = settings.ha_state_topic_prefix device_uuid = device.uuid online = bool(device.last_poll_ok) # Publish availability topic first. try: - avail_topic = _availability_topic(device_uuid, prefix) + avail_topic = _availability_topic(device_uuid, state_prefix) avail_payload = "online" if online else "offline" mqtt_manager.publish(avail_topic, avail_payload, retain=False) except Exception: @@ -367,7 +383,7 @@ def publish_device_state(session: Session, device: Any) -> None: if entity.component == "binary_sensor" and "online" in entity.key: # Publish the binary_sensor state too. try: - state_t = _state_topic(entity, prefix) + state_t = _state_topic(entity, state_prefix) mqtt_manager.publish(state_t, "ON", retain=False) except Exception: logger.exception( @@ -383,7 +399,7 @@ def publish_device_state(session: Session, device: Any) -> None: value = entity.value_getter(session) if value is None: continue - state_t = _state_topic(entity, prefix) + state_t = _state_topic(entity, state_prefix) mqtt_manager.publish(state_t, str(value), retain=False) except Exception: logger.exception( @@ -406,8 +422,8 @@ def publish_device_offline(session: Session, device_uuid: str) -> None: return try: - prefix = settings.ha_discovery_prefix - avail_topic = _availability_topic(device_uuid, prefix) + state_prefix = settings.ha_state_topic_prefix + avail_topic = _availability_topic(device_uuid, state_prefix) mqtt_manager.publish(avail_topic, "offline", retain=False) except Exception: logger.exception( diff --git a/tests/test_energy_expose.py b/tests/test_energy_expose.py index 0e52a8b..d957d4d 100644 --- a/tests/test_energy_expose.py +++ b/tests/test_energy_expose.py @@ -90,11 +90,13 @@ def _make_settings( mqtt_enabled: bool = True, ha_discovery_enabled: bool = True, ha_discovery_prefix: str = "homeassistant", + ha_state_topic_prefix: str = "home_automation", ) -> MagicMock: s = MagicMock() s.mqtt_enabled = mqtt_enabled s.ha_discovery_enabled = ha_discovery_enabled s.ha_discovery_prefix = ha_discovery_prefix + s.ha_state_topic_prefix = ha_state_topic_prefix return s @@ -691,7 +693,7 @@ def test_energy_entity_discovery_topics_contain_correct_node_id() -> None: state_class="total", ) - topic, config = build_discovery_payload(entity, prefix="homeassistant") + topic, config = build_discovery_payload(entity, discovery_prefix="homeassistant") # node_id: hyphens replaced with underscores → "energy_cost" expected_node = "energy_cost" diff --git a/tests/test_ha_discovery.py b/tests/test_ha_discovery.py index 27bdbbd..f8c9784 100644 --- a/tests/test_ha_discovery.py +++ b/tests/test_ha_discovery.py @@ -90,11 +90,13 @@ def _make_settings( mqtt_enabled: bool = True, ha_discovery_enabled: bool = True, ha_discovery_prefix: str = "homeassistant", + ha_state_topic_prefix: str = "home_automation", ) -> MagicMock: s = MagicMock() s.mqtt_enabled = mqtt_enabled s.ha_discovery_enabled = ha_discovery_enabled s.ha_discovery_prefix = ha_discovery_prefix + s.ha_state_topic_prefix = ha_state_topic_prefix return s @@ -142,7 +144,7 @@ def test_build_discovery_payload_topic_format() -> None: name="My Meter Voltage", ) - topic, config = build_discovery_payload(entity, prefix="homeassistant") + topic, config = build_discovery_payload(entity, discovery_prefix="homeassistant") # node_id replaces hyphens with underscores node_id = uuid_val.replace("-", "_") @@ -168,7 +170,7 @@ def test_build_discovery_payload_unique_id_from_uuid() -> None: name="Meter A Current", ) - _, config = build_discovery_payload(entity, prefix="homeassistant") + _, config = build_discovery_payload(entity, discovery_prefix="homeassistant") # unique_id must contain the device uuid (normalised) uid = config["unique_id"].replace("-", "").replace("_", "") @@ -210,8 +212,8 @@ def test_build_discovery_payload_unique_id_is_stable_across_renames() -> None: name="New Name Voltage", ) - _, config_a = build_discovery_payload(entity_a, prefix="homeassistant") - _, config_b = build_discovery_payload(entity_b, prefix="homeassistant") + _, config_a = build_discovery_payload(entity_a, discovery_prefix="homeassistant") + _, config_b = build_discovery_payload(entity_b, discovery_prefix="homeassistant") assert config_a["unique_id"] == config_b["unique_id"], ( "unique_id must not change when friendly_name changes" @@ -234,7 +236,7 @@ def test_build_discovery_payload_device_block() -> None: name="Friendly Name Voltage", ) - _, config = build_discovery_payload(entity, prefix="homeassistant") + _, config = build_discovery_payload(entity, discovery_prefix="homeassistant") assert "device" in config assert "identifiers" in config["device"] @@ -259,7 +261,7 @@ def test_build_discovery_payload_sensor_fields() -> None: state_class="total_increasing", ) - _, config = build_discovery_payload(entity, prefix="homeassistant") + _, config = build_discovery_payload(entity, discovery_prefix="homeassistant") assert config["device_class"] == "energy" assert config["unit_of_measurement"] == "kWh" @@ -284,7 +286,7 @@ def test_build_discovery_payload_binary_sensor() -> None: name="Meter Online Online", ) - _, config = build_discovery_payload(entity, prefix="homeassistant") + _, config = build_discovery_payload(entity, discovery_prefix="homeassistant") assert config.get("payload_on") == "ON" assert config.get("payload_off") == "OFF" @@ -307,7 +309,7 @@ def test_build_discovery_payload_state_topic_matches_node_and_object() -> None: name="Meter Voltage", ) - _, config = build_discovery_payload(entity, prefix="homeassistant") + _, config = build_discovery_payload(entity, discovery_prefix="homeassistant") state_topic = config["state_topic"] assert "homeassistant/sensor/" in state_topic @@ -991,7 +993,7 @@ def test_build_discovery_payload_includes_availability_topic() -> None: name="Avail Meter Voltage", ) - _, config = build_discovery_payload(entity, prefix="homeassistant") + _, config = build_discovery_payload(entity, discovery_prefix="homeassistant") expected_avail_topic = _availability_topic(uuid_val, "homeassistant") assert "availability" in config @@ -1142,7 +1144,9 @@ def test_real_provider_value_getter_returns_latest_reading_value(disco_db) -> No topics_to_payloads: dict[str, Any] = {t: p for t, p in published} # Availability topic must be "online". - avail_topic = f"homeassistant/modbus/{uuid_val.replace('-', '_')}/availability" + # State/availability topics now use ha_state_topic_prefix ("home_automation"), not + # ha_discovery_prefix ("homeassistant"). + avail_topic = f"home_automation/modbus/{uuid_val.replace('-', '_')}/availability" assert avail_topic in topics_to_payloads, ( f"Availability topic {avail_topic!r} not published. All topics: {list(topics_to_payloads)}" ) @@ -1153,7 +1157,8 @@ def test_real_provider_value_getter_returns_latest_reading_value(disco_db) -> No # Voltage state topic must carry the real value from the reading (231.5). node_id = uuid_val.replace("-", "_") voltage_obj_id = f"modbus_{uuid_val.replace('-', '_')}_voltage" - voltage_state_topic = f"homeassistant/sensor/{node_id}/{voltage_obj_id}/state" + # State topics use ha_state_topic_prefix ("home_automation"). + voltage_state_topic = f"home_automation/sensor/{node_id}/{voltage_obj_id}/state" assert voltage_state_topic in topics_to_payloads, ( f"Voltage state topic {voltage_state_topic!r} not published. " f"Sensor state topics: {[t for t in topics_to_payloads if 'sensor' in t and 'state' in t]}" @@ -1165,7 +1170,8 @@ def test_real_provider_value_getter_returns_latest_reading_value(disco_db) -> No # Online binary_sensor state topic must be "ON" (last_poll_ok=True). online_obj_id = f"modbus_{uuid_val.replace('-', '_')}_online" - online_state_topic = f"homeassistant/binary_sensor/{node_id}/{online_obj_id}/state" + # State topics use ha_state_topic_prefix ("home_automation"). + online_state_topic = f"home_automation/binary_sensor/{node_id}/{online_obj_id}/state" assert online_state_topic in topics_to_payloads, ( f"Online state topic {online_state_topic!r} not published. " f"Topics: {list(topics_to_payloads)}" @@ -1173,3 +1179,185 @@ def test_real_provider_value_getter_returns_latest_reading_value(disco_db) -> No assert topics_to_payloads[online_state_topic] == "ON", ( f"Expected 'ON' for online sensor, got {topics_to_payloads[online_state_topic]!r}" ) + + +# --------------------------------------------------------------------------- +# 10. Topic prefix separation: config vs state/availability +# --------------------------------------------------------------------------- + + +def test_build_discovery_payload_state_topic_uses_state_prefix() -> None: + """When discovery_prefix and state_prefix differ, state_topic must use state_prefix.""" + from app.integrations.expose import DeviceInfo, ExposableEntity + from app.services.ha_discovery import build_discovery_payload + + uuid_val = "aaaaaaaa-1111-2222-3333-444444444444" + device = DeviceInfo(identifiers=("modbus", uuid_val), name="Split Prefix Meter") + entity = ExposableEntity( + key=f"modbus.{uuid_val}.voltage", + component="sensor", + device=device, + device_class="voltage", + unit="V", + name="Split Prefix Meter Voltage", + ) + + topic, config = build_discovery_payload( + entity, + discovery_prefix="homeassistant", + state_prefix="home_automation", + ) + + # Config topic must use the discovery prefix. + assert topic.startswith("homeassistant/"), ( + f"Config topic must start with 'homeassistant/', got {topic!r}" + ) + assert topic.endswith("/config"), f"Config topic must end with /config, got {topic!r}" + + # State topic in payload must use the state prefix. + state_topic = config["state_topic"] + assert state_topic.startswith("home_automation/"), ( + f"state_topic must start with 'home_automation/', got {state_topic!r}" + ) + assert state_topic.endswith("/state"), f"state_topic must end with /state, got {state_topic!r}" + + +def test_build_discovery_payload_availability_topic_uses_state_prefix() -> None: + """When state_prefix differs from discovery_prefix, availability must use state_prefix.""" + from app.integrations.expose import DeviceInfo, ExposableEntity + from app.services.ha_discovery import build_discovery_payload + + uuid_val = "bbbbbbbb-1111-2222-3333-444444444444" + device = DeviceInfo(identifiers=("modbus", uuid_val), name="Avail Split Meter") + entity = ExposableEntity( + key=f"modbus.{uuid_val}.voltage", + component="sensor", + device=device, + device_class="voltage", + unit="V", + name="Avail Split Meter Voltage", + ) + + _, config = build_discovery_payload( + entity, + discovery_prefix="homeassistant", + state_prefix="home_automation", + ) + + assert "availability" in config, "Entity with provides_availability=True must have availability" + avail_topics = [a["topic"] for a in config["availability"]] + assert len(avail_topics) == 1 + avail_topic = avail_topics[0] + assert avail_topic.startswith("home_automation/"), ( + f"Availability topic must start with 'home_automation/', got {avail_topic!r}" + ) + # The /modbus/ path segment must still be present (hardcoded structure preserved). + assert "/modbus/" in avail_topic, ( + f"Availability topic must contain /modbus/, got {avail_topic!r}" + ) + assert avail_topic.endswith("/availability"), ( + f"Availability topic must end with /availability, got {avail_topic!r}" + ) + + +def test_build_discovery_payload_custom_state_prefix_changes_state_not_config() -> None: + """Custom ha_state_topic_prefix changes state/availability topics but NOT config topic.""" + from app.integrations.expose import DeviceInfo, ExposableEntity + from app.services.ha_discovery import build_discovery_payload + + uuid_val = "cccccccc-5555-6666-7777-888888888888" + device = DeviceInfo(identifiers=("modbus", uuid_val), name="Custom Prefix Meter") + entity = ExposableEntity( + key=f"modbus.{uuid_val}.current", + component="sensor", + device=device, + device_class="current", + unit="A", + name="Custom Prefix Meter Current", + ) + + topic_default, config_default = build_discovery_payload( + entity, + discovery_prefix="homeassistant", + state_prefix="home_automation", + ) + topic_custom, config_custom = build_discovery_payload( + entity, + discovery_prefix="homeassistant", + state_prefix="custom_prefix", + ) + + # Config topic must be identical (discovery_prefix unchanged). + assert topic_default == topic_custom, ( + f"Config topic must not change with different state_prefix. " + f"default={topic_default!r}, custom={topic_custom!r}" + ) + + # State topic must differ. + assert config_default["state_topic"] != config_custom["state_topic"], ( + "state_topic must change when state_prefix changes" + ) + assert config_custom["state_topic"].startswith("custom_prefix/"), ( + f"Custom state_topic must start with 'custom_prefix/', got {config_custom['state_topic']!r}" + ) + + # Availability topic must differ. + avail_default = config_default["availability"][0]["topic"] + avail_custom = config_custom["availability"][0]["topic"] + assert avail_default != avail_custom, ( + "Availability topic must change when state_prefix changes" + ) + assert avail_custom.startswith("custom_prefix/"), ( + f"Custom avail topic must start with 'custom_prefix/', got {avail_custom!r}" + ) + + +def test_config_ha_state_topic_prefix_default_and_configurable() -> None: + """ha_state_topic_prefix must default to 'home_automation' and be configurable.""" + from app.config import Settings + + # Default value. + s = Settings(_env_file=None) + assert s.ha_state_topic_prefix == "home_automation", ( + f"Default ha_state_topic_prefix must be 'home_automation', got {s.ha_state_topic_prefix!r}" + ) + + # Configurable via constructor. + s2 = Settings(_env_file=None, ha_state_topic_prefix="custom_state") + assert s2.ha_state_topic_prefix == "custom_state", ( + f"ha_state_topic_prefix must be overridable, got {s2.ha_state_topic_prefix!r}" + ) + + +def test_config_page_ha_state_topic_prefix_in_discovery_section() -> None: + """HA_STATE_TOPIC_PREFIX must appear in the 'Home Assistant Discovery' config section.""" + from app.services.config_page import CONFIG_FIELDS + + ha_state_field = next( + (f for f in CONFIG_FIELDS if f.env_name == "HA_STATE_TOPIC_PREFIX"), None + ) + assert ha_state_field is not None, ( + "HA_STATE_TOPIC_PREFIX must be registered in CONFIG_FIELDS" + ) + assert ha_state_field.section == "Home Assistant Discovery", ( + f"HA_STATE_TOPIC_PREFIX must be in 'Home Assistant Discovery' section, " + f"got {ha_state_field.section!r}" + ) + assert ha_state_field.setting_attr == "ha_state_topic_prefix", ( + f"setting_attr must be 'ha_state_topic_prefix', got {ha_state_field.setting_attr!r}" + ) + + +def test_settings_payload_includes_ha_state_topic_prefix() -> None: + """_settings_payload must include ha_state_topic_prefix so it can be round-tripped.""" + from app.config import Settings + from app.services.config_page import _settings_payload + + s = Settings(_env_file=None, ha_state_topic_prefix="my_prefix") + payload = _settings_payload(s) + assert "ha_state_topic_prefix" in payload, ( + "_settings_payload must include 'ha_state_topic_prefix'" + ) + assert payload["ha_state_topic_prefix"] == "my_prefix", ( + f"Expected 'my_prefix', got {payload['ha_state_topic_prefix']!r}" + )