diff --git a/src/handlers/vehicle_command.py b/src/handlers/vehicle_command.py index 10917566..b0730eb4 100644 --- a/src/handlers/vehicle_command.py +++ b/src/handlers/vehicle_command.py @@ -2,7 +2,7 @@ from dataclasses import dataclass import logging -from typing import TYPE_CHECKING, Final +from typing import TYPE_CHECKING, Any, Final from saic_ismart_client_ng.exceptions import SaicApiException, SaicLogoutException @@ -57,13 +57,51 @@ def __init__( def publisher(self) -> Publisher: return self.vehicle_state.publisher + def __report_command_failure( + self, + *, + command: str, + result_topic: str, + detail: str, + exc: Exception | None = None, + ) -> None: + if exc is not None: + LOG.exception("Command %s failed: %s", command, detail, exc_info=exc) + else: + LOG.error("Command %s failed: %s", command, detail) + try: + self.publisher.publish_str(result_topic, f"Failed: {detail}") + except Exception: + LOG.warning( + "Failed to publish failure result for command %s", + command, + exc_info=True, + ) + try: + error_topic = self.vehicle_state.get_topic(mqtt_topics.COMMAND_ERROR) + event_payload: dict[str, Any] = { + "event_type": "command_error", + "command": command, + "detail": detail, + } + self.publisher.publish_json(error_topic, event_payload) + except Exception: + LOG.warning( + "Failed to publish command error event for command %s", + command, + exc_info=True, + ) + async def handle_mqtt_command(self, *, topic: str, payload: str) -> None: analyzed_topic = self.__get_command_topics(topic) handler = self.__command_handlers.get(analyzed_topic.command_no_vin) if not handler: msg = f"No handler found for command topic {analyzed_topic.command_no_vin}" - self.publisher.publish_str(analyzed_topic.response_no_global, msg) - LOG.error(msg) + self.__report_command_failure( + command=analyzed_topic.command_no_vin, + result_topic=analyzed_topic.response_no_global, + detail=msg, + ) else: await self.__execute_mqtt_command_handler( handler=handler, payload=payload, analyzed_topic=analyzed_topic @@ -90,8 +128,9 @@ async def __execute_mqtt_command_handler( if execution_result.clear_command: self.publisher.clear_topic(topic_no_global) except MqttGatewayException as e: - self.publisher.publish_str(result_topic, f"Failed: {e.message}") - LOG.exception(e.message, exc_info=e) + self.__report_command_failure( + command=topic, result_topic=result_topic, detail=e.message, exc=e + ) except SaicLogoutException: LOG.warning( "API Client was logged out, attempting immediate relogin and retry" @@ -99,10 +138,12 @@ async def __execute_mqtt_command_handler( try: await self.relogin_handler.force_login() except Exception as login_err: - self.publisher.publish_str( - result_topic, f"Failed: relogin failed ({login_err})" + self.__report_command_failure( + command=topic, + result_topic=result_topic, + detail=f"relogin failed ({login_err})", + exc=login_err, ) - LOG.error("Immediate relogin failed", exc_info=login_err) return try: execution_result = await handler.handle(payload) @@ -115,19 +156,22 @@ async def __execute_mqtt_command_handler( if execution_result.clear_command: self.publisher.clear_topic(topic_no_global) except Exception as retry_err: - self.publisher.publish_str( - result_topic, f"Failed: {retry_err}" - ) - LOG.error( - "Command retry after relogin failed", exc_info=retry_err + self.__report_command_failure( + command=topic, + result_topic=result_topic, + detail=str(retry_err), + exc=retry_err, ) except SaicApiException as se: - self.publisher.publish_str(result_topic, f"Failed: {se.message}") - LOG.exception(se.message, exc_info=se) - except Exception as se: - self.publisher.publish_str(result_topic, "Failed unexpectedly") - LOG.exception( - "handle_mqtt_command failed with an unexpected exception", exc_info=se + self.__report_command_failure( + command=topic, result_topic=result_topic, detail=se.message, exc=se + ) + except Exception as e: + self.__report_command_failure( + command=topic, + result_topic=result_topic, + detail="unexpected error", + exc=e, ) def __get_command_topics(self, topic: str) -> _MqttCommandTopic: diff --git a/src/integrations/home_assistant/base.py b/src/integrations/home_assistant/base.py index 0c395104..848eef63 100644 --- a/src/integrations/home_assistant/base.py +++ b/src/integrations/home_assistant/base.py @@ -230,6 +230,34 @@ def _publish_lock( "lock", name, payload, custom_availability ) + def _publish_event( + self, + topic: str, + name: str, + event_types: list[str], + *, + enabled: bool = True, + entity_category: str | None = None, + device_class: str | None = None, + icon: str | None = None, + custom_availability: HaCustomAvailabilityConfig | None = None, + ) -> str: + payload: dict[str, Any] = { + "state_topic": self._get_state_topic(topic), + "event_types": event_types, + "enabled_by_default": enabled, + } + if entity_category is not None: + payload["entity_category"] = entity_category + if device_class is not None: + payload["device_class"] = device_class + if icon is not None: + payload["icon"] = icon + + return self._publish_ha_discovery_message( + "event", name, payload, custom_availability + ) + def _publish_sensor( self, topic: str, diff --git a/src/integrations/home_assistant/discovery.py b/src/integrations/home_assistant/discovery.py index 0e076850..ba1e9cd7 100644 --- a/src/integrations/home_assistant/discovery.py +++ b/src/integrations/home_assistant/discovery.py @@ -327,6 +327,15 @@ def __publish_ha_discovery_messages_real(self) -> None: ) self.__publish_lights_sensors() + # Command error event + self._publish_event( + mqtt_topics.COMMAND_ERROR, + "Command error", + ["command_error"], + entity_category="diagnostic", + icon="mdi:alert-circle", + ) + LOG.debug("Completed publishing Home Assistant discovery messages") def __publish_drivetrain_charging_sensors(self) -> None: diff --git a/src/mqtt_topics.py b/src/mqtt_topics.py index 18c2b1b9..691a9c2c 100644 --- a/src/mqtt_topics.py +++ b/src/mqtt_topics.py @@ -179,4 +179,7 @@ TYRES_REAR_LEFT_PRESSURE = TYRES + "/rearLeftPressure" TYRES_REAR_RIGHT_PRESSURE = TYRES + "/rearRightPressure" +COMMAND = "command" +COMMAND_ERROR = COMMAND + "/error" + VEHICLES = "vehicles" diff --git a/tests/handlers/test_vehicle_command.py b/tests/handlers/test_vehicle_command.py new file mode 100644 index 00000000..ecc568fb --- /dev/null +++ b/tests/handlers/test_vehicle_command.py @@ -0,0 +1,241 @@ +from __future__ import annotations + +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from saic_ismart_client_ng.exceptions import SaicApiException, SaicLogoutException + +from handlers.vehicle_command import VehicleCommandHandler +import mqtt_topics + +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_RESULT_TOPIC = ( + f"{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_CHARGING}/{mqtt_topics.RESULT_SUFFIX}" +) +COMMAND_ERROR_TOPIC = f"{VEHICLE_PREFIX}/{mqtt_topics.COMMAND_ERROR}" + + +def _build( + *, + saic_api: AsyncMock | None = None, + relogin_handler: AsyncMock | None = None, +) -> tuple[VehicleCommandHandler, MagicMock]: + """Build a VehicleCommandHandler with a MagicMock publisher. + + Returns (handler, mock_publisher) so callers can assert on mock_publisher + without going through the typed Publisher interface. + """ + mock_publisher = MagicMock() + vehicle_state = MagicMock() + vehicle_state.publisher = mock_publisher + vehicle_state.vin = VIN + vehicle_state.get_topic.side_effect = lambda t: f"{VEHICLE_PREFIX}/{t}" + return ( + VehicleCommandHandler( + vehicle_state=vehicle_state, + saic_api=saic_api or AsyncMock(), + relogin_handler=relogin_handler or AsyncMock(), + mqtt_topic=MQTT_TOPIC, + vehicle_prefix=VEHICLE_PREFIX, + ), + mock_publisher, + ) + + +class TestSuccessPath(unittest.IsolatedAsyncioTestCase): + async def test_successful_command_publishes_success(self) -> None: + handler, pub = _build() + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Success") + pub.publish_json.assert_not_called() + + +class TestNoHandlerFound(unittest.IsolatedAsyncioTestCase): + async def test_publishes_error_event(self) -> None: + handler, pub = _build() + bad_topic = f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/nonexistent/topic/set" + result_topic = f"{VEHICLE_PREFIX}/nonexistent/topic/{mqtt_topics.RESULT_SUFFIX}" + + await handler.handle_mqtt_command(topic=bad_topic, payload="test") + + pub.publish_str.assert_any_call( + result_topic, + "Failed: No handler found for command topic nonexistent/topic/set", + ) + pub.publish_json.assert_called_once() + event = pub.publish_json.call_args[0][1] + assert event["event_type"] == "command_error" + assert event["command"] == "nonexistent/topic/set" + + async def test_does_not_log_traceback(self) -> None: + handler, _ = _build() + bad_topic = f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/nonexistent/topic/set" + + with patch("handlers.vehicle_command.LOG") as mock_log: + await handler.handle_mqtt_command(topic=bad_topic, payload="test") + mock_log.error.assert_called_once() + mock_log.exception.assert_not_called() + + +class TestMqttGatewayException(unittest.IsolatedAsyncioTestCase): + async def test_publishes_error_event(self) -> None: + """An invalid payload triggers MqttGatewayException from payload conversion.""" + handler, pub = _build() + + await handler.handle_mqtt_command( + topic=CHARGING_SET_TOPIC, payload="not_a_boolean" + ) + + pub.publish_str.assert_any_call( + CHARGING_RESULT_TOPIC, + "Failed: Unsupported payload not_a_boolean for command " + "DrivetrainChargingCommand", + ) + pub.publish_json.assert_called_once() + event = pub.publish_json.call_args[0][1] + assert event["event_type"] == "command_error" + assert "Unsupported payload" in event["detail"] + + +class TestSaicApiException(unittest.IsolatedAsyncioTestCase): + async def test_publishes_error_event(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = SaicApiException( + "operation too frequent", return_code=8 + ) + handler, pub = _build(saic_api=saic_api) + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + pub.publish_str.assert_any_call( + CHARGING_RESULT_TOPIC, + "Failed: return code: 8, message: operation too frequent", + ) + pub.publish_json.assert_called_once() + event = pub.publish_json.call_args[0][1] + assert event["event_type"] == "command_error" + assert "operation too frequent" in event["detail"] + + +class TestUnexpectedException(unittest.IsolatedAsyncioTestCase): + async def test_uses_safe_detail(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = RuntimeError("secret internal detail") + handler, pub = _build(saic_api=saic_api) + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + pub.publish_str.assert_any_call( + CHARGING_RESULT_TOPIC, "Failed: unexpected error" + ) + event = pub.publish_json.call_args[0][1] + assert event["detail"] == "unexpected error" + assert "secret" not in event["detail"] + + +class TestSaicLogoutException(unittest.IsolatedAsyncioTestCase): + async def test_relogin_success_retries_command(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = [ + SaicLogoutException("logged out"), + None, + ] + handler, pub = _build(saic_api=saic_api) + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + relogin = handler.relogin_handler + 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_json.assert_not_called() + + async def test_relogin_failure_publishes_error_event(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = SaicLogoutException("logged out") + relogin = AsyncMock() + relogin.force_login.side_effect = Exception("login failed") + handler, pub = _build(saic_api=saic_api, relogin_handler=relogin) + + 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)" + ) + pub.publish_json.assert_called_once() + event = pub.publish_json.call_args[0][1] + assert "relogin failed" in event["detail"] + + async def test_retry_failure_publishes_error_event(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = [ + SaicLogoutException("logged out"), + RuntimeError("retry boom"), + ] + handler, pub = _build(saic_api=saic_api) + + 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_json.assert_called_once() + event = pub.publish_json.call_args[0][1] + assert event["detail"] == "retry boom" + + +class TestReportFailureResilience(unittest.IsolatedAsyncioTestCase): + async def test_publish_str_failure_does_not_prevent_error_event(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = SaicApiException("err", return_code=1) + handler, pub = _build(saic_api=saic_api) + pub.publish_str.side_effect = ConnectionError("broker down") + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + pub.publish_json.assert_called_once() + event = pub.publish_json.call_args[0][1] + assert event["event_type"] == "command_error" + + async def test_publish_json_failure_does_not_raise(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = SaicApiException("err", return_code=1) + handler, pub = _build(saic_api=saic_api) + pub.publish_json.side_effect = ConnectionError("broker down") + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + pub.publish_str.assert_called_once() + + +class TestErrorEventPayload(unittest.IsolatedAsyncioTestCase): + async def test_topic_uses_vehicle_prefix(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = SaicApiException("err", return_code=1) + handler, pub = _build(saic_api=saic_api) + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + error_topic = pub.publish_json.call_args[0][0] + assert error_topic == COMMAND_ERROR_TOPIC + + async def test_payload_structure(self) -> None: + saic_api = AsyncMock() + saic_api.control_charging.side_effect = SaicApiException( + "operation too frequent", return_code=8 + ) + handler, pub = _build(saic_api=saic_api) + + await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") + + event = pub.publish_json.call_args[0][1] + assert set(event.keys()) == {"event_type", "command", "detail"} + assert event["event_type"] == "command_error" + assert event["command"] == mqtt_topics.DRIVETRAIN_CHARGING_SET + assert "operation too frequent" in event["detail"]