From 810ca89641a2936903a453ef7c60bc51750d3c5d Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sat, 9 May 2026 17:33:08 +0200 Subject: [PATCH 1/4] fix: persist HA-driven gateway settings via retained /set commands Adds retain=true to HA discovery for the four refresh_period_* numbers, refresh_mode, and totalBatteryCapacity, so the broker keeps the user's last `/set` value across gateway restarts. Plumbs the gmqtt retain flag through the command-dispatch path and drops a retained one-shot refresh mode (force, charging_detection) so a single-shot poll does not loop on every restart. Also suppresses clear_command on retained replays so the broker does not erase the user's intent. Unblocks PR #440 (fix/ha-number-optimistic-state). --- CHANGELOG.md | 16 +++ src/handlers/command/base.py | 25 +++- src/handlers/command/gateway/refresh_mode.py | 27 +++- src/handlers/vehicle.py | 16 ++- src/handlers/vehicle_command.py | 18 ++- src/integrations/home_assistant/base.py | 4 +- src/integrations/home_assistant/discovery.py | 6 + src/mqtt_gateway.py | 15 ++- src/publisher/core.py | 6 +- src/publisher/mqtt_publisher.py | 11 +- tests/handlers/test_vehicle_command.py | 120 ++++++++++++++++- tests/integrations/home_assistant/__init__.py | 0 .../home_assistant/test_discovery_retain.py | 122 ++++++++++++++++++ tests/test_mqtt_publisher.py | 4 +- tests/test_vehicle_state.py | 38 +++++- 15 files changed, 390 insertions(+), 38 deletions(-) create mode 100644 tests/integrations/home_assistant/__init__.py create mode 100644 tests/integrations/home_assistant/test_discovery_retain.py diff --git a/CHANGELOG.md b/CHANGELOG.md index db1eb6e7..dcddda11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Change Log +## Unreleased + +### Fixed + +* Persist user-set HA gateway entities across gateway restarts by retaining + their `/set` commands on the MQTT broker (refresh mode, all four refresh + periods, and total battery capacity). On reconnect the existing command- + dispatch path replays the retained value before `configure_missing()` would + apply config defaults. A retained one-shot refresh mode (`force`, + `charging_detection`) is dropped on replay so a single-shot poll does not + fire on every restart. + + Note: on first upgrade only entities you change *after* the upgrade become + persistent. Existing retained STATE values on the broker are not converted + into retained `/set` commands. + ## 0.11.0 ### Added diff --git a/src/handlers/command/base.py b/src/handlers/command/base.py index 8b2bae47..cf7ce47a 100644 --- a/src/handlers/command/base.py +++ b/src/handlers/command/base.py @@ -50,7 +50,9 @@ def topic(cls) -> str: raise NotImplementedError @abstractmethod - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, payload: str, *, retained: bool = False + ) -> CommandProcessingResult: raise NotImplementedError @property @@ -84,7 +86,12 @@ def supports_empty_payload(self) -> bool: return False @override - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, + payload: str, + *, + retained: bool = False, + ) -> CommandProcessingResult: normalized_payload = payload.strip().lower() if len(normalized_payload) == 0 and not self.supports_empty_payload: @@ -113,7 +120,12 @@ async def _get_action_result(self, _action_result: T) -> CommandProcessingResult pass @override - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, + payload: str, + *, + retained: bool = False, + ) -> CommandProcessingResult: normalized_payload = payload.strip().lower() if len(normalized_payload) == 0: @@ -145,7 +157,12 @@ def supports_empty_payload(self) -> bool: return False @override - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, + payload: str, + *, + retained: bool = False, + ) -> CommandProcessingResult: if len(payload.strip()) == 0 and not self.supports_empty_payload: return RESULT_DO_NOTHING diff --git a/src/handlers/command/gateway/refresh_mode.py b/src/handlers/command/gateway/refresh_mode.py index 870cdfcb..3ce07078 100644 --- a/src/handlers/command/gateway/refresh_mode.py +++ b/src/handlers/command/gateway/refresh_mode.py @@ -1,14 +1,18 @@ from __future__ import annotations +import logging from typing import override +from exceptions import MqttGatewayException from handlers.command.base import ( RESULT_DO_NOTHING, CommandProcessingResult, PayloadConvertingCommandHandler, ) import mqtt_topics -from vehicle import RefreshMode +from vehicle import ONE_SHOT_REFRESH_MODES, RefreshMode + +LOG = logging.getLogger(__name__) class RefreshModeCommand(PayloadConvertingCommandHandler[RefreshMode]): @@ -23,6 +27,27 @@ def convert_payload(payload: str) -> RefreshMode: normalized_payload = payload.strip().lower() return RefreshMode.get(normalized_payload) + @override + async def handle( + self, payload: str, *, retained: bool = False + ) -> CommandProcessingResult: + if len(payload.strip()) == 0: + return RESULT_DO_NOTHING + try: + refresh_mode = self.convert_payload(payload) + except Exception as e: + msg = f"Error converting payload {payload} for command {self.name()}" + raise MqttGatewayException(msg) from e + if retained and refresh_mode in ONE_SHOT_REFRESH_MODES: + # Retained one-shot modes would re-fire on every gateway restart. + LOG.info( + "Dropping retained one-shot refresh mode %s for VIN %s", + refresh_mode.value, + self.vin, + ) + return RESULT_DO_NOTHING + return await self.handle_typed_payload(refresh_mode) + @override async def handle_typed_payload( self, refresh_mode: RefreshMode diff --git a/src/handlers/vehicle.py b/src/handlers/vehicle.py index 2607f7a3..7f189080 100644 --- a/src/handlers/vehicle.py +++ b/src/handlers/vehicle.py @@ -241,10 +241,9 @@ def __should_poll(self) -> bool: ) def __should_complete_configuration(self, start_time: datetime.datetime) -> bool: - return ( - not self.vehicle_state.is_complete() - and datetime.datetime.now(tz=datetime.UTC) > start_time + datetime.timedelta(seconds=10) - ) + return not self.vehicle_state.is_complete() and datetime.datetime.now( + tz=datetime.UTC + ) > start_time + datetime.timedelta(seconds=10) def __refresh_openwb( self, @@ -334,8 +333,12 @@ async def update_scheduled_battery_heating_status( ) return scheduled_battery_heating_status - async def handle_mqtt_command(self, *, topic: str, payload: str) -> None: - await self.__command_handler.handle_mqtt_command(topic=topic, payload=payload) + async def handle_mqtt_command( + self, *, topic: str, payload: str, retained: bool = False + ) -> None: + await self.__command_handler.handle_mqtt_command( + topic=topic, payload=payload, retained=retained + ) def __setup_ha_discovery( self, vehicle_state: VehicleState, vin_info: VehicleInfo, config: Configuration @@ -344,7 +347,6 @@ def __setup_ha_discovery( return HomeAssistantDiscovery(vehicle_state, vin_info, config) return None - def handle_charging_station_energy_imported( self, imported_energy_wh: float ) -> None: diff --git a/src/handlers/vehicle_command.py b/src/handlers/vehicle_command.py index f06596e8..4ec3ffbe 100644 --- a/src/handlers/vehicle_command.py +++ b/src/handlers/vehicle_command.py @@ -91,7 +91,9 @@ def __report_command_failure( command, ) - async def handle_mqtt_command(self, *, topic: str, payload: str) -> None: + async def handle_mqtt_command( + self, *, topic: str, payload: str, retained: bool = False + ) -> None: analyzed_topic = self.__get_command_topics(topic) handler = self.__command_handlers.get(analyzed_topic.command_no_vin) if not handler: @@ -103,7 +105,10 @@ async def handle_mqtt_command(self, *, topic: str, payload: str) -> None: ) else: await self.__execute_mqtt_command_handler( - handler=handler, payload=payload, analyzed_topic=analyzed_topic + handler=handler, + payload=payload, + analyzed_topic=analyzed_topic, + retained=retained, ) async def __execute_mqtt_command_handler( @@ -112,19 +117,20 @@ async def __execute_mqtt_command_handler( handler: CommandHandlerBase, payload: str, analyzed_topic: _MqttCommandTopic, + retained: bool, ) -> None: topic = analyzed_topic.command_no_vin topic_no_global = analyzed_topic.command_no_global result_topic = analyzed_topic.response_no_global try: - execution_result = await handler.handle(payload) + execution_result = await handler.handle(payload, retained=retained) self.publisher.publish_str(result_topic, "Success") if execution_result.force_refresh: self.vehicle_state.set_refresh_mode( RefreshMode.FORCE, f"after command execution on topic {topic}" ) - if execution_result.clear_command: + if execution_result.clear_command and not retained: self.publisher.clear_topic(topic_no_global) except MqttGatewayException as e: self.__report_command_failure( @@ -145,14 +151,14 @@ async def __execute_mqtt_command_handler( ) return try: - execution_result = await handler.handle(payload) + execution_result = await handler.handle(payload, retained=retained) self.publisher.publish_str(result_topic, "Success") if execution_result.force_refresh: self.vehicle_state.set_refresh_mode( RefreshMode.FORCE, f"after command execution on topic {topic}", ) - if execution_result.clear_command: + if execution_result.clear_command and not retained: self.publisher.clear_topic(topic_no_global) except Exception as retry_err: self.__report_command_failure( diff --git a/src/integrations/home_assistant/base.py b/src/integrations/home_assistant/base.py index 848eef63..a7603ddc 100644 --- a/src/integrations/home_assistant/base.py +++ b/src/integrations/home_assistant/base.py @@ -46,14 +46,16 @@ def _publish_select( enabled: bool = True, value_template: str = "{{ value }}", command_template: str = "{{ value }}", + retain: bool = False, icon: str | None = None, custom_availability: HaCustomAvailabilityConfig | None = None, ) -> str: - payload = { + payload: dict[str, Any] = { "state_topic": self._get_state_topic(topic), "command_topic": self._get_command_topic(topic), "value_template": value_template, "command_template": command_template, + "retain": str(retain).lower(), "options": options, "enabled_by_default": enabled, } diff --git a/src/integrations/home_assistant/discovery.py b/src/integrations/home_assistant/discovery.py index d7c73207..1a8cafe7 100644 --- a/src/integrations/home_assistant/discovery.py +++ b/src/integrations/home_assistant/discovery.py @@ -167,6 +167,7 @@ def __publish_ha_discovery_messages_real(self) -> None: mode="box", min_value=0.0, step=0.001, + retain=True, ) self._publish_sensor( @@ -640,6 +641,7 @@ def __publish_gateway_sensors(self) -> None: value_template="{{ value }}", command_template="{{ value }}", icon="mdi:refresh", + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -651,6 +653,7 @@ def __publish_gateway_sensors(self) -> None: min_value=30, max_value=60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -662,6 +665,7 @@ def __publish_gateway_sensors(self) -> None: min_value=1 * 60 * 60, max_value=5 * 24 * 60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -673,6 +677,7 @@ def __publish_gateway_sensors(self) -> None: min_value=30, max_value=12 * 60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -684,6 +689,7 @@ def __publish_gateway_sensors(self) -> None: min_value=30, max_value=12 * 60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_sensor( diff --git a/src/mqtt_gateway.py b/src/mqtt_gateway.py index e5bf676b..d97af18a 100644 --- a/src/mqtt_gateway.py +++ b/src/mqtt_gateway.py @@ -255,7 +255,9 @@ async def __register_alarm_switches( def __create_vehicle_handler(self, vin_info: VinInfo) -> VehicleHandler: vin = vin_info.vin - total_battery_capacity = self.configuration.battery_capacity_map.get(vin, None) if vin else None + total_battery_capacity = ( + self.configuration.battery_capacity_map.get(vin, None) if vin else None + ) info = VehicleInfo(vin_info, total_battery_capacity) account_prefix = f"{self.configuration.saic_user}/{mqtt_topics.VEHICLES}/{vin}" vehicle_state = VehicleState( @@ -356,11 +358,18 @@ def vehicle_handlers(self) -> dict[str, VehicleHandler]: @override async def on_mqtt_command_received( - self, *, vin: str, topic: str, payload: str + self, *, vin: str, topic: str, payload: str, retained: bool = False ) -> None: vehicle_handler = self.get_vehicle_handler(vin) if vehicle_handler: - await vehicle_handler.handle_mqtt_command(topic=topic, payload=payload) + await vehicle_handler.handle_mqtt_command( + topic=topic, payload=payload, retained=retained + ) + elif retained: + LOG.info( + f"Retained command for unknown vin {vin} received on {topic};" + f" handler not yet registered, dropping replay" + ) else: LOG.debug(f"Command for unknown vin {vin} received") diff --git a/src/publisher/core.py b/src/publisher/core.py index b6628d83..50fdf6b5 100644 --- a/src/publisher/core.py +++ b/src/publisher/core.py @@ -16,7 +16,7 @@ class MqttCommandListener(ABC): @abstractmethod async def on_mqtt_command_received( - self, *, vin: str, topic: str, payload: str + self, *, vin: str, topic: str, payload: str, retained: bool = False ) -> None: raise NotImplementedError("Should have implemented this") @@ -187,7 +187,9 @@ def anonymize_str(value: str) -> str: def anonymize_device_id(self, device_id: str) -> str: elements = device_id.split("###", maxsplit=1) if len(elements) == 2: - return f"{self.anonymize_str(elements[0])}###{self.anonymize_str(elements[1])}" + return ( + f"{self.anonymize_str(elements[0])}###{self.anonymize_str(elements[1])}" + ) return self.anonymize_str(device_id) @staticmethod diff --git a/src/publisher/mqtt_publisher.py b/src/publisher/mqtt_publisher.py index 4f079fc4..c177acea 100644 --- a/src/publisher/mqtt_publisher.py +++ b/src/publisher/mqtt_publisher.py @@ -151,11 +151,16 @@ async def __on_message( payload = payload.decode("utf-8") else: payload = str(payload) - await self.__on_message_real(topic=topic, payload=payload) + retained = bool(_properties.get("retain", 0)) if _properties else False + await self.__on_message_real( + topic=topic, payload=payload, retained=retained + ) except Exception as e: LOG.exception(f"Error while processing MQTT message: {e}") - async def __on_message_real(self, *, topic: str, payload: str) -> None: + async def __on_message_real( + self, *, topic: str, payload: str, retained: bool + ) -> None: if topic in self.vin_by_charge_state_topic: LOG.debug(f"Received message over topic {topic} with payload {payload}") vin = self.vin_by_charge_state_topic[topic] @@ -194,7 +199,7 @@ async def __on_message_real(self, *, topic: str, payload: str) -> None: vin = self.get_vin_from_topic(topic) if self.command_listener is not None: await self.command_listener.on_mqtt_command_received( - vin=vin, topic=topic, payload=payload + vin=vin, topic=topic, payload=payload, retained=retained ) async def __handle_imported_energy(self, topic: str, payload: str) -> None: diff --git a/tests/handlers/test_vehicle_command.py b/tests/handlers/test_vehicle_command.py index ecc568fb..3362944a 100644 --- a/tests/handlers/test_vehicle_command.py +++ b/tests/handlers/test_vehicle_command.py @@ -1,5 +1,6 @@ from __future__ import annotations +from typing import cast import unittest from unittest.mock import AsyncMock, MagicMock, patch @@ -7,11 +8,14 @@ from handlers.vehicle_command import VehicleCommandHandler import mqtt_topics +from vehicle import RefreshMode MQTT_TOPIC = "saic" VIN = "vin_test_000000000" VEHICLE_PREFIX = f"vehicles/{VIN}" -CHARGING_SET_TOPIC = f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_CHARGING_SET}" +CHARGING_SET_TOPIC = ( + f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_CHARGING_SET}" +) CHARGING_RESULT_TOPIC = ( f"{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_CHARGING}/{mqtt_topics.RESULT_SUFFIX}" ) @@ -182,9 +186,7 @@ async def test_retry_failure_publishes_error_event(self) -> None: await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") - pub.publish_str.assert_any_call( - CHARGING_RESULT_TOPIC, "Failed: retry boom" - ) + pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Failed: retry boom") pub.publish_json.assert_called_once() event = pub.publish_json.call_args[0][1] assert event["detail"] == "retry boom" @@ -239,3 +241,113 @@ async def test_payload_structure(self) -> None: assert event["event_type"] == "command_error" assert event["command"] == mqtt_topics.DRIVETRAIN_CHARGING_SET assert "operation too frequent" in event["detail"] + + +REFRESH_MODE_SET_TOPIC = f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.REFRESH_MODE_SET}" +REFRESH_MODE_RESULT_TOPIC = ( + f"{VEHICLE_PREFIX}/{mqtt_topics.REFRESH_MODE}/{mqtt_topics.RESULT_SUFFIX}" +) +TOTAL_BATTERY_CAPACITY_SET_TOPIC = ( + f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY_SET}" +) +TOTAL_BATTERY_CAPACITY_RESULT_TOPIC = ( + f"{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY}" + f"/{mqtt_topics.RESULT_SUFFIX}" +) + + +class TestRetainedReplay(unittest.IsolatedAsyncioTestCase): + """Behavior for retained `/set` commands replayed on broker reconnect. + + Idempotent values (refresh periods, OFF/PERIODIC mode, battery capacity) + must seed in-memory state. One-shot refresh modes (FORCE / + CHARGING_DETECTION) must be dropped to avoid looping a poll on every + gateway restart. + """ + + async def test_retained_force_refresh_mode_dropped(self) -> None: + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="force", retained=True + ) + + vehicle_state.set_refresh_mode.assert_not_called() + pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success") + + async def test_retained_charging_detection_refresh_mode_dropped(self) -> None: + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, + payload="charging_detection", + retained=True, + ) + + vehicle_state.set_refresh_mode.assert_not_called() + pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success") + + async def test_retained_periodic_refresh_mode_applied(self) -> None: + handler, _pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="periodic", retained=True + ) + + vehicle_state.set_refresh_mode.assert_called_once() + mode_arg = vehicle_state.set_refresh_mode.call_args[0][0] + assert mode_arg is RefreshMode.PERIODIC + + async def test_retained_off_refresh_mode_applied(self) -> None: + handler, _pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="off", retained=True + ) + + vehicle_state.set_refresh_mode.assert_called_once() + mode_arg = vehicle_state.set_refresh_mode.call_args[0][0] + assert mode_arg is RefreshMode.OFF + + async def test_non_retained_force_still_applied(self) -> None: + handler, _pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="force", retained=False + ) + + vehicle_state.set_refresh_mode.assert_called_once() + mode_arg = vehicle_state.set_refresh_mode.call_args[0][0] + assert mode_arg is RefreshMode.FORCE + + async def test_retained_battery_capacity_replays_to_vehicle_info(self) -> None: + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=TOTAL_BATTERY_CAPACITY_SET_TOPIC, payload="50.0", retained=True + ) + + vehicle_state.update_battery_capacity.assert_called_once_with(50.0) + pub.publish_str.assert_any_call(TOTAL_BATTERY_CAPACITY_RESULT_TOPIC, "Success") + + async def test_retained_clear_command_skipped(self) -> None: + """A retained replay must not delete its own retained command from the broker. + + DrivetrainChargingCommand returns RESULT_REFRESH_AND_CLEAR. When called + with retained=True the dispatcher must skip the clear_topic call so the + retained intent remains on the broker for the next restart. + """ + handler, pub = _build() + + await handler.handle_mqtt_command( + topic=CHARGING_SET_TOPIC, payload="true", retained=True + ) + + pub.clear_topic.assert_not_called() + pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Success") diff --git a/tests/integrations/home_assistant/__init__.py b/tests/integrations/home_assistant/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/integrations/home_assistant/test_discovery_retain.py b/tests/integrations/home_assistant/test_discovery_retain.py new file mode 100644 index 00000000..45fa7898 --- /dev/null +++ b/tests/integrations/home_assistant/test_discovery_retain.py @@ -0,0 +1,122 @@ +from __future__ import annotations + +import json +import unittest + +from apscheduler.schedulers.blocking import BlockingScheduler +from saic_ismart_client_ng.api.vehicle.schema import ( + VehicleModelConfiguration, + VinInfo, +) + +from configuration import Configuration +from integrations.home_assistant.discovery import HomeAssistantDiscovery +import mqtt_topics +from tests.common_mocks import VIN +from tests.mocks import MessageCapturingConsolePublisher +from vehicle import RefreshMode, VehicleState +from vehicle_info import VehicleInfo + +# Six entities whose `/set` commands HA must retain so the user's last value +# survives a gateway restart. Stored as the path suffix of the entity's state +# topic so we can match against published HA discovery payloads. +RETAINED_ENTITY_TOPICS = { + mqtt_topics.REFRESH_MODE, + mqtt_topics.REFRESH_PERIOD_ACTIVE, + mqtt_topics.REFRESH_PERIOD_INACTIVE, + mqtt_topics.REFRESH_PERIOD_AFTER_SHUTDOWN, + mqtt_topics.REFRESH_PERIOD_INACTIVE_GRACE, + mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY, +} + +# Sample of writable entities that must NOT be retained. SOC target / charge +# current are API-backed; charging is action-bearing. +NON_RETAINED_ENTITY_TOPICS = { + mqtt_topics.DRIVETRAIN_SOC_TARGET, + mqtt_topics.DRIVETRAIN_CHARGECURRENT_LIMIT, +} + + +def _make_discovery() -> tuple[ + HomeAssistantDiscovery, MessageCapturingConsolePublisher +]: + config = Configuration() + config.anonymized_publishing = False + config.ha_discovery_prefix = "homeassistant" + publisher = MessageCapturingConsolePublisher(config) + vin_info = VinInfo() + vin_info.vin = VIN + vin_info.series = "EH32 S" + vin_info.modelName = "MG4 Electric" + vin_info.modelYear = "2022" + vin_info.vehicleModelConfiguration = [ + VehicleModelConfiguration("BATTERY", "BATTERY", "1"), + VehicleModelConfiguration("BType", "Battery", "1"), + ] + vehicle_info = VehicleInfo(vin_info, None) + account_prefix = f"/vehicles/{VIN}" + scheduler = BlockingScheduler() + vehicle_state = VehicleState(publisher, scheduler, account_prefix, vehicle_info) + vehicle_state.refresh_period_active = 30 + vehicle_state.refresh_period_inactive = 120 + vehicle_state.refresh_period_after_shutdown = 60 + vehicle_state.refresh_period_inactive_grace = 600 + vehicle_state.refresh_mode = RefreshMode.PERIODIC + discovery = HomeAssistantDiscovery(vehicle_state, vehicle_info, config) + return discovery, publisher + + +def _writable_payloads( + publisher: MessageCapturingConsolePublisher, +) -> list[dict[str, object]]: + """Return every published discovery payload that has a `command_topic`.""" + payloads: list[dict[str, object]] = [] + for raw in publisher.map.values(): + try: + payload = json.loads(raw) + except (TypeError, json.JSONDecodeError): + continue + if isinstance(payload, dict) and "command_topic" in payload: + payloads.append(payload) + return payloads + + +def _payload_for_state_topic_suffix( + payloads: list[dict[str, object]], suffix: str +) -> dict[str, object] | None: + for payload in payloads: + state_topic = payload.get("state_topic") + if isinstance(state_topic, str) and state_topic.endswith(f"/{suffix}"): + return payload + return None + + +class TestDiscoveryRetainFlag(unittest.TestCase): + """The six idempotent persistence-relevant entities must be retained.""" + + def test_required_entities_have_retain_true(self) -> None: + discovery, publisher = _make_discovery() + discovery.publish_ha_discovery_messages() + payloads = _writable_payloads(publisher) + + for topic in RETAINED_ENTITY_TOPICS: + payload = _payload_for_state_topic_suffix(payloads, topic) + assert payload is not None, ( + f"No writable HA discovery payload found for topic {topic}" + ) + assert payload.get("retain") == "true", ( + f"Expected retain=true for {topic}, got {payload.get('retain')!r}" + ) + + def test_non_retained_entities_keep_retain_false(self) -> None: + discovery, publisher = _make_discovery() + discovery.publish_ha_discovery_messages() + payloads = _writable_payloads(publisher) + + for topic in NON_RETAINED_ENTITY_TOPICS: + payload = _payload_for_state_topic_suffix(payloads, topic) + if payload is None: + continue # entity not published for this vehicle config + assert payload.get("retain") in ("false", None), ( + f"Expected retain!=true for {topic}, got {payload.get('retain')!r}" + ) diff --git a/tests/test_mqtt_publisher.py b/tests/test_mqtt_publisher.py index fa65b290..2416dc9f 100644 --- a/tests/test_mqtt_publisher.py +++ b/tests/test_mqtt_publisher.py @@ -24,10 +24,11 @@ async def on_mqtt_global_command_received( @override async def on_mqtt_command_received( - self, *, vin: str, topic: str, payload: str + self, *, vin: str, topic: str, payload: str, retained: bool = False ) -> None: self.received_vin = vin self.received_payload = payload.strip().lower() + self.received_retained = retained @override def setUp(self) -> None: @@ -39,6 +40,7 @@ def setUp(self) -> None: self.mqtt_client.command_listener = self self.received_vin = "" self.received_payload = "" + self.received_retained = False self.vehicle_base_topic = ( f"{self.mqtt_client.configuration.mqtt_topic}/{USER}/vehicles/{VIN}" ) diff --git a/tests/test_vehicle_state.py b/tests/test_vehicle_state.py index e0a47cdd..03b15583 100644 --- a/tests/test_vehicle_state.py +++ b/tests/test_vehicle_state.py @@ -197,6 +197,28 @@ def test_republish_command_states_skips_unset_values(self) -> None: self.get_topic(mqtt_topics.REFRESH_MODE), RefreshMode.OFF.value ) + def test_configure_missing_skips_when_retained_value_present(self) -> None: + """Sentinel guards in configure_missing preserve retained-replay values. + + Retained `/set` replay seeds in-memory state before configure_missing + runs; configure_missing must then leave those values alone. + """ + self.vehicle_state.set_refresh_period_active(45) + self.vehicle_state.update_battery_capacity(50.0) + + self.vehicle_state.configure_missing() + + assert self.vehicle_state.refresh_period_active == 45 + assert self.vehicle_state.vehicle.custom_battery_capacity == 50.0 + + def test_configure_missing_applies_defaults_when_no_retained(self) -> None: + assert self.vehicle_state.refresh_period_active == -1 + self.vehicle_state.configure_missing() + assert self.vehicle_state.refresh_period_active == 30 + assert self.vehicle_state.refresh_period_inactive == 86400 + assert self.vehicle_state.refresh_period_after_shutdown == 120 + assert self.vehicle_state.refresh_period_inactive_grace == 600 + def test_republish_command_states_includes_api_values(self) -> None: self.vehicle_state.configure_missing() self.vehicle_state.update_target_soc(TargetBatteryCode.P_80) @@ -280,7 +302,9 @@ def test_handle_vehicle_status_rejects_max_int32_timestamp(self) -> None: def test_handle_vehicle_status_rejects_drifted_timestamp(self) -> None: resp = get_mock_vehicle_status_resp() resp.statusTime = 1000000000 # 2001-09-09, well outside 15 min window - with pytest.raises(VehicleStatusDriftException, match="drifted more than 15 minutes"): + with pytest.raises( + VehicleStatusDriftException, match="drifted more than 15 minutes" + ): self.vehicle_state.handle_vehicle_status(resp) def test_mileage_of_day_published_when_valid(self) -> None: @@ -303,7 +327,9 @@ def test_mileage_of_day_skipped_when_exceeds_total(self) -> None: chrg_mgmt_data_resp = get_mock_charge_management_data_resp() # Set mileageOfDay raw value higher than total mileage raw value assert chrg_mgmt_data_resp.rvsChargeStatus is not None - chrg_mgmt_data_resp.rvsChargeStatus.mileageOfDay = (DRIVETRAIN_MILEAGE + 100) * 10 + chrg_mgmt_data_resp.rvsChargeStatus.mileageOfDay = ( + DRIVETRAIN_MILEAGE + 100 + ) * 10 self.vehicle_state.handle_charge_status(chrg_mgmt_data_resp) assert ( @@ -318,7 +344,9 @@ def test_mileage_since_last_charge_skipped_when_exceeds_total(self) -> None: chrg_mgmt_data_resp = get_mock_charge_management_data_resp() assert chrg_mgmt_data_resp.rvsChargeStatus is not None - chrg_mgmt_data_resp.rvsChargeStatus.mileageSinceLastCharge = (DRIVETRAIN_MILEAGE + 100) * 10 + chrg_mgmt_data_resp.rvsChargeStatus.mileageSinceLastCharge = ( + DRIVETRAIN_MILEAGE + 100 + ) * 10 self.vehicle_state.handle_charge_status(chrg_mgmt_data_resp) assert ( @@ -442,9 +470,7 @@ def test_periodic_after_shutdown_publishes_after_shutdown_phase(self) -> None: self.vehicle_state.configure_missing() # Car just shut down, grace period active self.vehicle_state.hv_battery_active = False - self.vehicle_state.last_car_shutdown = datetime.datetime.now( - tz=datetime.UTC - ) + self.vehicle_state.last_car_shutdown = datetime.datetime.now(tz=datetime.UTC) self.vehicle_state.last_car_activity = datetime.datetime.min.replace( tzinfo=datetime.UTC ) From f53c56d703d86a0bc844bef1708fae1e98cca557 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sat, 9 May 2026 17:39:55 +0200 Subject: [PATCH 2/4] fix: log dropped retained replays at WARN, not INFO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A retained /set arriving for an unknown VIN means we lost the user's intent — surface it at WARN so it shows up in default log views. --- src/mqtt_gateway.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mqtt_gateway.py b/src/mqtt_gateway.py index d97af18a..cfac6343 100644 --- a/src/mqtt_gateway.py +++ b/src/mqtt_gateway.py @@ -366,7 +366,7 @@ async def on_mqtt_command_received( topic=topic, payload=payload, retained=retained ) elif retained: - LOG.info( + LOG.warning( f"Retained command for unknown vin {vin} received on {topic};" f" handler not yet registered, dropping replay" ) From 8c172d05b8aaf8cb730f1aa81a823def5efe3b54 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sat, 9 May 2026 17:52:35 +0200 Subject: [PATCH 3/4] fix: preserve retained OFF refresh mode across restarts Drops RefreshMode.OFF from INVALID_STARTUP_REFRESH_MODES and changes the constructor default for refresh_mode from OFF to PERIODIC. Before, OFF was in the INVALID set because OFF was the constructor default and a gateway booting in OFF would never poll. Now that retained `/set off` is replayed on reconnect (via the parent commit), OFF is a legitimate persistent user choice and must be preserved by configure_missing. FORCE / CHARGING_DETECTION remain in INVALID_STARTUP_REFRESH_MODES as a belt-and-braces guard alongside the primary drop in RefreshModeCommand. Also addresses two review nits: - refresh_mode.handle empty-payload guard now mirrors the parent's `not self.supports_empty_payload` clause for contract parity - drops a redundant `if _properties` defensive check; gmqtt always populates the properties dict with the retain flag --- src/handlers/command/gateway/refresh_mode.py | 2 +- src/publisher/mqtt_publisher.py | 2 +- src/vehicle.py | 62 +++++++++++--------- tests/test_vehicle_state.py | 9 ++- 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/handlers/command/gateway/refresh_mode.py b/src/handlers/command/gateway/refresh_mode.py index 3ce07078..cd9c1ae0 100644 --- a/src/handlers/command/gateway/refresh_mode.py +++ b/src/handlers/command/gateway/refresh_mode.py @@ -31,7 +31,7 @@ def convert_payload(payload: str) -> RefreshMode: async def handle( self, payload: str, *, retained: bool = False ) -> CommandProcessingResult: - if len(payload.strip()) == 0: + if len(payload.strip()) == 0 and not self.supports_empty_payload: return RESULT_DO_NOTHING try: refresh_mode = self.convert_payload(payload) diff --git a/src/publisher/mqtt_publisher.py b/src/publisher/mqtt_publisher.py index c177acea..b441fef7 100644 --- a/src/publisher/mqtt_publisher.py +++ b/src/publisher/mqtt_publisher.py @@ -151,7 +151,7 @@ async def __on_message( payload = payload.decode("utf-8") else: payload = str(payload) - retained = bool(_properties.get("retain", 0)) if _properties else False + retained = bool(_properties.get("retain", 0)) await self.__on_message_real( topic=topic, payload=payload, retained=retained ) diff --git a/src/vehicle.py b/src/vehicle.py index 669e1a54..d8f7b7a7 100644 --- a/src/vehicle.py +++ b/src/vehicle.py @@ -71,8 +71,10 @@ def get(mode: str) -> RefreshMode: ) #: Refresh modes that are not valid at startup and must be replaced with PERIODIC. +#: Only one-shot modes are coerced; OFF is a legitimate persistent user choice +#: (restored from retained `/set` on reconnect) and must be preserved. INVALID_STARTUP_REFRESH_MODES: Final[frozenset[RefreshMode]] = frozenset( - {RefreshMode.OFF, RefreshMode.FORCE, RefreshMode.CHARGING_DETECTION} + {RefreshMode.FORCE, RefreshMode.CHARGING_DETECTION} ) @@ -108,13 +110,21 @@ def __init__( ) self.vehicle: Final[VehicleInfo] = vin_info self.mqtt_vin_prefix = account_prefix - self.last_car_activity: datetime.datetime = datetime.datetime.min.replace(tzinfo=datetime.UTC) - self.last_successful_refresh: datetime.datetime = datetime.datetime.min.replace(tzinfo=datetime.UTC) + self.last_car_activity: datetime.datetime = datetime.datetime.min.replace( + tzinfo=datetime.UTC + ) + self.last_successful_refresh: datetime.datetime = datetime.datetime.min.replace( + tzinfo=datetime.UTC + ) self.__last_failed_refresh: datetime.datetime | None = None self.__failed_refresh_counter = 0 self.__refresh_period_error = 30 - self.last_car_shutdown: datetime.datetime = datetime.datetime.now(tz=datetime.UTC) - self.last_car_vehicle_message: datetime.datetime = datetime.datetime.min.replace(tzinfo=datetime.UTC) + self.last_car_shutdown: datetime.datetime = datetime.datetime.now( + tz=datetime.UTC + ) + self.last_car_vehicle_message: datetime.datetime = ( + datetime.datetime.min.replace(tzinfo=datetime.UTC) + ) # treat high voltage battery as active, if we don't have any other information self.__hv_battery_active = True self.__hv_battery_active_from_car = True @@ -128,8 +138,8 @@ def __init__( self.charge_current_limit: ChargeCurrentLimitCode | None = None self.refresh_period_charging = 0 self.charge_polling_min_percent = charge_polling_min_percent - self.refresh_mode = RefreshMode.OFF - self.previous_refresh_mode = RefreshMode.OFF + self.refresh_mode = RefreshMode.PERIODIC + self.previous_refresh_mode = RefreshMode.PERIODIC self.__polling_phase: PollingPhase | None = None self.__remote_ac_temp: int | None = None self.__remote_ac_running: bool = False @@ -243,7 +253,11 @@ def update_scheduled_charging( tz = self.__user_timezone if self.refresh_period_inactive_grace > 0: # Add a grace period to the start time, so that the car is not woken up too early - now = datetime.datetime.now(tz=tz) if tz else datetime.datetime.now().astimezone() + now = ( + datetime.datetime.now(tz=tz) + if tz + else datetime.datetime.now().astimezone() + ) dt = now.replace( hour=start_time.hour, minute=start_time.minute, @@ -426,20 +440,16 @@ def __should_do_periodic_refresh(self) -> bool: self.__publish_polling_phase(PollingPhase.ERROR_RECOVERY) return result if self.is_charging and self.refresh_period_charging > 0: - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_charging)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_charging)) LOG.debug(f"HV battery is charging. Should refresh: {result}") self.__publish_polling_phase(PollingPhase.CHARGING) return result if self.hv_battery_active: - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_active)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_active)) LOG.debug(f"HV battery is active. Should refresh: {result}") self.__publish_polling_phase(PollingPhase.ACTIVE) return result @@ -447,21 +457,17 @@ def __should_do_periodic_refresh(self) -> bool: seconds=float(self.refresh_period_inactive_grace) ) if last_shutdown_plus_refresh > datetime.datetime.now(tz=datetime.UTC): - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_after_shutdown)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_after_shutdown)) LOG.debug( f"Refresh grace period after shutdown has not passed. Should refresh: {result}" ) self.__publish_polling_phase(PollingPhase.AFTER_SHUTDOWN) return result - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_inactive)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_inactive)) LOG.debug( f"HV battery is inactive and refresh period after shutdown is over. Should refresh: {result}" ) diff --git a/tests/test_vehicle_state.py b/tests/test_vehicle_state.py index 03b15583..faff700b 100644 --- a/tests/test_vehicle_state.py +++ b/tests/test_vehicle_state.py @@ -192,9 +192,9 @@ def test_republish_command_states_skips_unset_values(self) -> None: 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 + # refresh_mode defaults to RefreshMode.PERIODIC (never None), so it IS always published self.assert_mqtt_topic( - self.get_topic(mqtt_topics.REFRESH_MODE), RefreshMode.OFF.value + self.get_topic(mqtt_topics.REFRESH_MODE), RefreshMode.PERIODIC.value ) def test_configure_missing_skips_when_retained_value_present(self) -> None: @@ -219,6 +219,11 @@ def test_configure_missing_applies_defaults_when_no_retained(self) -> None: assert self.vehicle_state.refresh_period_after_shutdown == 120 assert self.vehicle_state.refresh_period_inactive_grace == 600 + def test_configure_missing_preserves_retained_off_refresh_mode(self) -> None: + self.vehicle_state.set_refresh_mode(RefreshMode.OFF, "retained replay") + self.vehicle_state.configure_missing() + assert self.vehicle_state.refresh_mode == RefreshMode.OFF + def test_republish_command_states_includes_api_values(self) -> None: self.vehicle_state.configure_missing() self.vehicle_state.update_target_soc(TargetBatteryCode.P_80) From b0afe1feba03f74d6c77473b81a00fd6145e3987 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sat, 9 May 2026 18:01:09 +0200 Subject: [PATCH 4/4] fix: drop retained replays for non-replayable commands at dispatcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in CommandHandlerBase.is_replayable_when_retained() classmethod (default False) and gates the dispatcher: any retained `/set` for a handler that hasn't opted in is dropped with a WARN log before reaching the handler. Defense-in-depth against non-HA producers (node-RED, custom scripts, mosquitto_pub) that may mistakenly publish action-bearing commands with retain=true. Without this guard such a stale retained command (e.g. a retained `charging/set true`) would re-fire the SAIC API call on every gateway restart. Opted in: RefreshMode, the four RefreshPeriod_*, and TotalBatteryCapacity — exactly the six entities whose HA discovery payload also declares retain=true. Single source of truth lives next to the handler logic. --- src/handlers/command/base.py | 13 ++++++++++++ .../drivetrain_total_battery_capacity.py | 5 +++++ src/handlers/command/gateway/refresh_mode.py | 6 ++++++ .../command/gateway/refresh_period.py | 20 ++++++++++++++++++ src/handlers/vehicle_command.py | 13 ++++++++++++ tests/handlers/test_vehicle_command.py | 21 ++++++++++++------- 6 files changed, 70 insertions(+), 8 deletions(-) diff --git a/src/handlers/command/base.py b/src/handlers/command/base.py index cf7ce47a..888be4fd 100644 --- a/src/handlers/command/base.py +++ b/src/handlers/command/base.py @@ -44,6 +44,19 @@ def __init__(self, saic_api: SaicApi, vehicle_state: VehicleState) -> None: def name(cls) -> str: return cls.__name__ + @classmethod + def is_replayable_when_retained(cls) -> bool: + """Whether the dispatcher may invoke this handler with retained=True. + + Default False: action-bearing commands (charging start/stop, locks, + climate, FORCE refresh, etc.) would re-fire on every gateway restart + if their `/set` topic was retained on the broker, so the dispatcher + drops the replay before the handler runs. Override to True only on + idempotent value-bearing handlers whose HA discovery payload also + declares `retain: true`. + """ + return False + @classmethod @abstractmethod def topic(cls) -> str: diff --git a/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py b/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py index e4ac5855..849f16a3 100644 --- a/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py +++ b/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py @@ -14,6 +14,11 @@ class DrivetrainTotalBatteryCapacitySetCommand(FloatCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: diff --git a/src/handlers/command/gateway/refresh_mode.py b/src/handlers/command/gateway/refresh_mode.py index cd9c1ae0..57851ead 100644 --- a/src/handlers/command/gateway/refresh_mode.py +++ b/src/handlers/command/gateway/refresh_mode.py @@ -16,6 +16,12 @@ class RefreshModeCommand(PayloadConvertingCommandHandler[RefreshMode]): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + # OFF / PERIODIC are persistent user choices; one-shots dropped in handle(). + return True + @classmethod @override def topic(cls) -> str: diff --git a/src/handlers/command/gateway/refresh_period.py b/src/handlers/command/gateway/refresh_period.py index 5723b465..91cae45e 100644 --- a/src/handlers/command/gateway/refresh_period.py +++ b/src/handlers/command/gateway/refresh_period.py @@ -11,6 +11,11 @@ class RefreshPeriodActiveCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: @@ -23,6 +28,11 @@ async def handle_typed_payload(self, payload: int) -> CommandProcessingResult: class RefreshPeriodInactiveCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: @@ -35,6 +45,11 @@ async def handle_typed_payload(self, payload: int) -> CommandProcessingResult: class RefreshPeriodInactiveGraceCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: @@ -47,6 +62,11 @@ async def handle_typed_payload(self, payload: int) -> CommandProcessingResult: class RefreshPeriodAfterShutdownCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: diff --git a/src/handlers/vehicle_command.py b/src/handlers/vehicle_command.py index 4ec3ffbe..e7f994ca 100644 --- a/src/handlers/vehicle_command.py +++ b/src/handlers/vehicle_command.py @@ -123,6 +123,19 @@ async def __execute_mqtt_command_handler( topic_no_global = analyzed_topic.command_no_global result_topic = analyzed_topic.response_no_global + if retained and not handler.is_replayable_when_retained(): + # A retained `/set` for an action-bearing command would re-fire the + # action on every gateway restart. Drop it before invoking the + # handler. Only handlers that explicitly opt in via + # ``replayable_when_retained = True`` see retained replays. + LOG.warning( + "Dropping retained replay for non-replayable command %s on %s; " + "this command should not have been published with retain=true", + handler.name(), + topic, + ) + return + try: execution_result = await handler.handle(payload, retained=retained) self.publisher.publish_str(result_topic, "Success") diff --git a/tests/handlers/test_vehicle_command.py b/tests/handlers/test_vehicle_command.py index 3362944a..d9009831 100644 --- a/tests/handlers/test_vehicle_command.py +++ b/tests/handlers/test_vehicle_command.py @@ -336,18 +336,23 @@ async def test_retained_battery_capacity_replays_to_vehicle_info(self) -> None: vehicle_state.update_battery_capacity.assert_called_once_with(50.0) pub.publish_str.assert_any_call(TOTAL_BATTERY_CAPACITY_RESULT_TOPIC, "Success") - async def test_retained_clear_command_skipped(self) -> None: - """A retained replay must not delete its own retained command from the broker. - - DrivetrainChargingCommand returns RESULT_REFRESH_AND_CLEAR. When called - with retained=True the dispatcher must skip the clear_topic call so the - retained intent remains on the broker for the next restart. + async def test_retained_action_command_dropped_at_dispatcher(self) -> None: + """Retained `/set` for an action-bearing command is dropped at the dispatcher. + + DrivetrainChargingCommand has not opted in via + is_replayable_when_retained(). A retained replay of `charging/set` (e.g. + from a non-HA client that mistakenly retained the topic) must NOT + invoke the handler — otherwise the SAIC charging API call would re-fire + on every gateway restart. """ - handler, pub = _build() + saic_api = AsyncMock() + handler, pub = _build(saic_api=saic_api) await handler.handle_mqtt_command( topic=CHARGING_SET_TOPIC, payload="true", retained=True ) + # Handler never ran: no API call, no Success/result publish, no clear_topic + saic_api.control_charging.assert_not_called() + pub.publish_str.assert_not_called() pub.clear_topic.assert_not_called() - pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Success")