From 5acfbd43f337033a3fe423a30a38aa5c142552f4 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Wed, 24 Jun 2026 23:08:04 +0200 Subject: [PATCH] fix(command): publish command results with retain=False MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Command result topics ("Success" / "Failed: ...") were published with the default retain=True, so a stale result from a previous run would persist on the broker and re-appear after a gateway restart. Results are transient responses — they should not survive a reconnect. --- src/handlers/vehicle_command.py | 6 +++--- tests/handlers/test_vehicle_command.py | 23 +++++++++++++++-------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/handlers/vehicle_command.py b/src/handlers/vehicle_command.py index e7f994c..e1840d1 100644 --- a/src/handlers/vehicle_command.py +++ b/src/handlers/vehicle_command.py @@ -70,7 +70,7 @@ def __report_command_failure( else: LOG.error("Command %s failed: %s", command, detail) try: - self.publisher.publish_str(result_topic, f"Failed: {detail}") + self.publisher.publish_str(result_topic, f"Failed: {detail}", retain=False) except Exception: LOG.warning( "Failed to publish failure result for command %s", @@ -138,7 +138,7 @@ async def __execute_mqtt_command_handler( try: execution_result = await handler.handle(payload, retained=retained) - self.publisher.publish_str(result_topic, "Success") + self.publisher.publish_str(result_topic, "Success", retain=False) if execution_result.force_refresh: self.vehicle_state.set_refresh_mode( RefreshMode.FORCE, f"after command execution on topic {topic}" @@ -165,7 +165,7 @@ async def __execute_mqtt_command_handler( return try: execution_result = await handler.handle(payload, retained=retained) - self.publisher.publish_str(result_topic, "Success") + self.publisher.publish_str(result_topic, "Success", retain=False) if execution_result.force_refresh: self.vehicle_state.set_refresh_mode( RefreshMode.FORCE, diff --git a/tests/handlers/test_vehicle_command.py b/tests/handlers/test_vehicle_command.py index 4815b5e..57cfaaf 100644 --- a/tests/handlers/test_vehicle_command.py +++ b/tests/handlers/test_vehicle_command.py @@ -55,7 +55,7 @@ async def test_successful_command_publishes_success(self) -> None: await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") - pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Success") + pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Success", retain=False) pub.publish_json.assert_not_called() @@ -70,6 +70,7 @@ async def test_publishes_error_event(self) -> None: pub.publish_str.assert_any_call( result_topic, "Failed: No handler found for command topic nonexistent/topic/set", + retain=False, ) pub.publish_json.assert_called_once() event = pub.publish_json.call_args[0][1] @@ -99,6 +100,7 @@ async def test_publishes_error_event(self) -> None: CHARGING_RESULT_TOPIC, "Failed: Unsupported payload not_a_boolean for command " "DrivetrainChargingCommand", + retain=False, ) pub.publish_json.assert_called_once() event = pub.publish_json.call_args[0][1] @@ -119,6 +121,7 @@ async def test_publishes_error_event(self) -> None: pub.publish_str.assert_any_call( CHARGING_RESULT_TOPIC, "Failed: return code: 8, message: operation too frequent", + retain=False, ) pub.publish_json.assert_called_once() event = pub.publish_json.call_args[0][1] @@ -135,7 +138,7 @@ async def test_uses_safe_detail(self) -> None: await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") pub.publish_str.assert_any_call( - CHARGING_RESULT_TOPIC, "Failed: unexpected error" + CHARGING_RESULT_TOPIC, "Failed: unexpected error", retain=False ) event = pub.publish_json.call_args[0][1] assert event["detail"] == "unexpected error" @@ -157,7 +160,7 @@ async def test_relogin_success_retries_command(self) -> None: assert isinstance(relogin, AsyncMock) relogin.force_login.assert_awaited_once() assert saic_api.control_charging.await_count == 2 - pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Success") + pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Success", retain=False) pub.publish_json.assert_not_called() async def test_relogin_failure_publishes_error_event(self) -> None: @@ -170,7 +173,7 @@ async def test_relogin_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: relogin failed (login failed)" + CHARGING_RESULT_TOPIC, "Failed: relogin failed (login failed)", retain=False ) pub.publish_json.assert_called_once() event = pub.publish_json.call_args[0][1] @@ -186,7 +189,9 @@ 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", retain=False + ) pub.publish_json.assert_called_once() event = pub.publish_json.call_args[0][1] assert event["detail"] == "retry boom" @@ -277,7 +282,7 @@ async def test_retained_force_refresh_mode_dropped(self) -> None: ) vehicle_state.set_refresh_mode.assert_not_called() - pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success") + pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success", retain=False) async def test_retained_charging_detection_refresh_mode_dropped(self) -> None: handler, pub = _build() @@ -290,7 +295,7 @@ async def test_retained_charging_detection_refresh_mode_dropped(self) -> None: ) vehicle_state.set_refresh_mode.assert_not_called() - pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success") + pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success", retain=False) async def test_retained_periodic_refresh_mode_applied(self) -> None: handler, _pub = _build() @@ -339,7 +344,9 @@ 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_float.assert_any_call(TOTAL_BATTERY_CAPACITY_STATE_TOPIC, 50.0) - pub.publish_str.assert_any_call(TOTAL_BATTERY_CAPACITY_RESULT_TOPIC, "Success") + pub.publish_str.assert_any_call( + TOTAL_BATTERY_CAPACITY_RESULT_TOPIC, "Success", retain=False + ) async def test_battery_capacity_zero_payload_publishes_model_default(self) -> None: """Payload `0` clears the override; the per-model default is republished."""