Merge pull request #402 from nanomad/fix/ha-unknown-after-restart

fix: republish command entity states after broker/HA restart

Author: nanomad

src/handlers/vehicle.py

@@ -264,7 +264,13 @@ def publish_ha_discovery_messages(self, *, force: bool = False) -> None:
             LOG.info(
                 f"Sending HA discovery messages for {self.vin_info.vin} (Force: {force})"
             )
-            self.__ha_discovery.publish_ha_discovery_messages(force=force)
+            published = self.__ha_discovery.publish_ha_discovery_messages(force=force)
+            if published:
+                self.vehicle_state.republish_command_states()
+
+    def reset_ha_discovery(self) -> None:
+        if self.__ha_discovery is not None:
+            self.__ha_discovery.published = False
 
     async def update_vehicle_status(
         self,

src/integrations/home_assistant/discovery.py

@@ -53,21 +53,22 @@ def __init__(
         )
         self.published = False
 
-    def publish_ha_discovery_messages(self, *, force: bool = False) -> None:
+    def publish_ha_discovery_messages(self, *, force: bool = False) -> bool:
         if not self.__vehicle_state.is_complete():
             LOG.warning(
                 "Skipping Home Assistant discovery messages as vehicle state is not yet complete"
             )
-            return
+            return False
 
         if self.published and not force:
             LOG.debug(
                 "Skipping Home Assistant discovery messages as it was already published"
             )
-            return
+            return False
 
         self.__publish_ha_discovery_messages_real()
         self.published = True
+        return True
 
     def __publish_ha_discovery_messages_real(self) -> None:
         LOG.debug("Publishing Home Assistant discovery messages")

src/mqtt_gateway.py

@@ -207,6 +207,13 @@ async def on_charging_detected(self, vin: str) -> None:
         else:
             LOG.debug(f"Charging detected for unknown vin {vin}")
 
+    @override
+    def on_mqtt_reconnected(self) -> None:
+        LOG.info("MQTT reconnected, resetting HA discovery for all vehicles")
+        for vin, vh in self.vehicle_handlers.items():
+            LOG.debug(f"Resetting HA discovery for vehicle {vin}")
+            vh.reset_ha_discovery()
+
     @override
     async def on_mqtt_global_command_received(
         self, *, topic: str, payload: str

src/publisher/core.py

@@ -30,6 +30,14 @@ async def on_mqtt_global_command_received(
     ) -> None:
         raise NotImplementedError("Should have implemented this")
 
+    def on_mqtt_reconnected(self) -> None:  # noqa: B027
+        """Reset state when the MQTT client reconnects after a connection loss.
+
+        This is intentionally synchronous because it is called from gmqtt's
+        synchronous on_connect callback. It is also intentionally not abstract
+        so that implementations can opt in without being forced to override.
+        """
+
 
 class Publisher(ABC):
     def __init__(self, config: Configuration) -> None:

src/publisher/mqtt_publisher.py

@@ -79,6 +79,8 @@ def __on_connect(
             LOG.info("Connected to MQTT broker")
             if not self.first_connection:
                 self.enable_commands()
+                if self.command_listener is not None:
+                    self.command_listener.on_mqtt_reconnected()
             self.first_connection = False
             self.keepalive()
         else:

src/vehicle.py

@@ -451,6 +451,58 @@ def configure_missing(self) -> None:
                 f"initial gateway startup from an invalid state {self.refresh_mode}",
             )
 
+    def republish_command_states(self) -> None:
+        """Unconditionally publish all command entity values to MQTT.
+
+        This bypasses change detection so that retained messages are restored
+        after a broker or Home Assistant restart.
+        """
+        if self.refresh_period_active != -1:
+            self.publisher.publish_int(
+                self.get_topic(mqtt_topics.REFRESH_PERIOD_ACTIVE),
+                self.refresh_period_active,
+            )
+        if self.refresh_period_inactive != -1:
+            self.publisher.publish_int(
+                self.get_topic(mqtt_topics.REFRESH_PERIOD_INACTIVE),
+                self.refresh_period_inactive,
+            )
+        if self.refresh_period_after_shutdown != -1:
+            self.publisher.publish_int(
+                self.get_topic(mqtt_topics.REFRESH_PERIOD_AFTER_SHUTDOWN),
+                self.refresh_period_after_shutdown,
+            )
+        if self.refresh_period_inactive_grace != -1:
+            self.publisher.publish_int(
+                self.get_topic(mqtt_topics.REFRESH_PERIOD_INACTIVE_GRACE),
+                self.refresh_period_inactive_grace,
+            )
+        if self.refresh_period_charging > 0:
+            self.publisher.publish_int(
+                self.get_topic(mqtt_topics.REFRESH_PERIOD_CHARGING),
+                self.refresh_period_charging,
+            )
+        if self.refresh_mode is not None:
+            self.publisher.publish_str(
+                self.get_topic(mqtt_topics.REFRESH_MODE),
+                self.refresh_mode.value,
+            )
+        if self.__remote_ac_temp is not None:
+            self.publisher.publish_int(
+                self.get_topic(mqtt_topics.CLIMATE_REMOTE_TEMPERATURE),
+                self.__remote_ac_temp,
+            )
+        if self.target_soc is not None:
+            self.publisher.publish_int(
+                self.get_topic(mqtt_topics.DRIVETRAIN_SOC_TARGET),
+                self.target_soc.percentage,
+            )
+        if self.charge_current_limit is not None:
+            self.publisher.publish_str(
+                self.get_topic(mqtt_topics.DRIVETRAIN_CHARGECURRENT_LIMIT),
+                self.charge_current_limit.limit,
+            )
+
     def handle_charge_status(
         self, charge_info_resp: ChrgMgmtDataResp
     ) -> ChrgMgmtDataRespProcessingResult:

tests/test_vehicle_state.py

@@ -6,10 +6,14 @@
 from apscheduler.schedulers.blocking import BlockingScheduler
 import pytest
 from saic_ismart_client_ng.api.vehicle.schema import VinInfo
+from saic_ismart_client_ng.api.vehicle_charging import (
+    ChargeCurrentLimitCode,
+    TargetBatteryCode,
+)
 
 from configuration import Configuration
 import mqtt_topics
-from vehicle import VehicleState
+from vehicle import RefreshMode, VehicleState
 from vehicle_info import VehicleInfo
 
 from .common_mocks import (
@@ -124,6 +128,64 @@ def test_handle_charge_status_with_phev_ignore_values(self) -> None:
         assert result.scheduled_charging is None
         assert self.get_topic(mqtt_topics.DRIVETRAIN_SOC_TARGET) not in self.publisher.map
 
+    def test_republish_command_states_after_configure_missing(self) -> None:
+        self.vehicle_state.configure_missing()
+        self.publisher.map.clear()
+
+        self.vehicle_state.republish_command_states()
+
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.REFRESH_PERIOD_ACTIVE), 30
+        )
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.REFRESH_PERIOD_INACTIVE), 86400
+        )
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.REFRESH_PERIOD_AFTER_SHUTDOWN), 120
+        )
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.REFRESH_PERIOD_INACTIVE_GRACE), 600
+        )
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.CLIMATE_REMOTE_TEMPERATURE), 22
+        )
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.REFRESH_MODE), RefreshMode.PERIODIC.value
+        )
+
+    def test_republish_command_states_skips_unset_values(self) -> None:
+        self.vehicle_state.republish_command_states()
+
+        # Refresh periods are -1 and optional values are None, so they should not be published
+        assert self.get_topic(mqtt_topics.REFRESH_PERIOD_ACTIVE) not in self.publisher.map
+        assert self.get_topic(mqtt_topics.REFRESH_PERIOD_INACTIVE) not in self.publisher.map
+        assert self.get_topic(mqtt_topics.REFRESH_PERIOD_AFTER_SHUTDOWN) not in self.publisher.map
+        assert self.get_topic(mqtt_topics.REFRESH_PERIOD_INACTIVE_GRACE) not in self.publisher.map
+        assert self.get_topic(mqtt_topics.DRIVETRAIN_SOC_TARGET) not in self.publisher.map
+        assert self.get_topic(mqtt_topics.DRIVETRAIN_CHARGECURRENT_LIMIT) not in self.publisher.map
+        assert self.get_topic(mqtt_topics.CLIMATE_REMOTE_TEMPERATURE) not in self.publisher.map
+        # refresh_mode defaults to RefreshMode.OFF (never None), so it IS always published
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.REFRESH_MODE), RefreshMode.OFF.value
+        )
+
+    def test_republish_command_states_includes_api_values(self) -> None:
+        self.vehicle_state.configure_missing()
+        self.vehicle_state.update_target_soc(TargetBatteryCode.P_80)
+        self.vehicle_state.update_charge_current_limit(ChargeCurrentLimitCode.C_MAX)
+        self.publisher.map.clear()
+
+        self.vehicle_state.republish_command_states()
+
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.DRIVETRAIN_SOC_TARGET),
+            TargetBatteryCode.P_80.percentage,
+        )
+        self.assert_mqtt_topic(
+            self.get_topic(mqtt_topics.DRIVETRAIN_CHARGECURRENT_LIMIT),
+            ChargeCurrentLimitCode.C_MAX.limit,
+        )
+
     @staticmethod
     def get_topic(sub_topic: str) -> str:
         return f"/vehicles/{VIN}/{sub_topic}"