From 410b15df9205b660f7ee1b6800393f78cca1b473 Mon Sep 17 00:00:00 2001 From: Richard Kroegel <42204099+rikroe@users.noreply.github.com> Date: Mon, 26 Jun 2023 19:05:50 +0200 Subject: [PATCH] Improve exception handling for BMW remote services (#92199) Co-authored-by: rikroe --- .../components/bmw_connected_drive/button.py | 12 +++++- .../components/bmw_connected_drive/lock.py | 16 ++++++- .../components/bmw_connected_drive/notify.py | 10 ++++- .../components/bmw_connected_drive/select.py | 7 ++- .../bmw_connected_drive/test_select.py | 43 +++++++++++++++++++ 5 files changed, 81 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/bmw_connected_drive/button.py b/homeassistant/components/bmw_connected_drive/button.py index d221c011445..2fd0ea91d08 100644 --- a/homeassistant/components/bmw_connected_drive/button.py +++ b/homeassistant/components/bmw_connected_drive/button.py @@ -6,12 +6,14 @@ from dataclasses import dataclass import logging from typing import TYPE_CHECKING, Any +from bimmer_connected.models import MyBMWAPIError from bimmer_connected.vehicle import MyBMWVehicle from bimmer_connected.vehicle.remote_services import RemoteServiceStatus from homeassistant.components.button import ButtonEntity, ButtonEntityDescription from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import BMWBaseEntity @@ -111,7 +113,10 @@ class BMWButton(BMWBaseEntity, ButtonEntity): async def async_press(self) -> None: """Press the button.""" if self.entity_description.remote_function: - await self.entity_description.remote_function(self.vehicle) + try: + await self.entity_description.remote_function(self.vehicle) + except MyBMWAPIError as ex: + raise HomeAssistantError(ex) from ex elif self.entity_description.account_function: _LOGGER.warning( "The 'Refresh from cloud' button is deprecated. Use the" @@ -120,6 +125,9 @@ class BMWButton(BMWBaseEntity, ButtonEntity): " https://www.home-assistant.io/integrations/bmw_connected_drive/#update-the-state--refresh-from-api" " for details" ) - await self.entity_description.account_function(self.coordinator) + try: + await self.entity_description.account_function(self.coordinator) + except MyBMWAPIError as ex: + raise HomeAssistantError(ex) from ex self.coordinator.async_update_listeners() diff --git a/homeassistant/components/bmw_connected_drive/lock.py b/homeassistant/components/bmw_connected_drive/lock.py index c7495e3145a..6608206a0ee 100644 --- a/homeassistant/components/bmw_connected_drive/lock.py +++ b/homeassistant/components/bmw_connected_drive/lock.py @@ -4,12 +4,14 @@ from __future__ import annotations import logging from typing import Any +from bimmer_connected.models import MyBMWAPIError from bimmer_connected.vehicle import MyBMWVehicle from bimmer_connected.vehicle.doors_windows import LockState from homeassistant.components.lock import LockEntity from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import BMWBaseEntity @@ -66,7 +68,12 @@ class BMWLock(BMWBaseEntity, LockEntity): # update callback response self._attr_is_locked = True self.async_write_ha_state() - await self.vehicle.remote_services.trigger_remote_door_lock() + try: + await self.vehicle.remote_services.trigger_remote_door_lock() + except MyBMWAPIError as ex: + self._attr_is_locked = False + self.async_write_ha_state() + raise HomeAssistantError(ex) from ex self.coordinator.async_update_listeners() @@ -79,7 +86,12 @@ class BMWLock(BMWBaseEntity, LockEntity): # update callback response self._attr_is_locked = False self.async_write_ha_state() - await self.vehicle.remote_services.trigger_remote_door_unlock() + try: + await self.vehicle.remote_services.trigger_remote_door_unlock() + except MyBMWAPIError as ex: + self._attr_is_locked = True + self.async_write_ha_state() + raise HomeAssistantError(ex) from ex self.coordinator.async_update_listeners() diff --git a/homeassistant/components/bmw_connected_drive/notify.py b/homeassistant/components/bmw_connected_drive/notify.py index 036d5147c4f..4a9f7679dc4 100644 --- a/homeassistant/components/bmw_connected_drive/notify.py +++ b/homeassistant/components/bmw_connected_drive/notify.py @@ -4,6 +4,7 @@ from __future__ import annotations import logging from typing import Any, cast +from bimmer_connected.models import MyBMWAPIError from bimmer_connected.vehicle import MyBMWVehicle from homeassistant.components.notify import ( @@ -19,6 +20,7 @@ from homeassistant.const import ( CONF_ENTITY_ID, ) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from .const import DOMAIN @@ -87,7 +89,11 @@ class BMWNotificationService(BaseNotificationService): if k in ATTR_LOCATION_ATTRIBUTES } ) - - await vehicle.remote_services.trigger_send_poi(location_dict) + try: + await vehicle.remote_services.trigger_send_poi(location_dict) + except TypeError as ex: + raise ValueError(str(ex)) from ex + except MyBMWAPIError as ex: + raise HomeAssistantError(ex) from ex else: raise ValueError(f"'data.{ATTR_LOCATION}' is required.") diff --git a/homeassistant/components/bmw_connected_drive/select.py b/homeassistant/components/bmw_connected_drive/select.py index 7c2ef4fed32..3467322a4af 100644 --- a/homeassistant/components/bmw_connected_drive/select.py +++ b/homeassistant/components/bmw_connected_drive/select.py @@ -4,6 +4,7 @@ from dataclasses import dataclass import logging from typing import Any +from bimmer_connected.models import MyBMWAPIError from bimmer_connected.vehicle import MyBMWVehicle from bimmer_connected.vehicle.charging_profile import ChargingMode @@ -11,6 +12,7 @@ from homeassistant.components.select import SelectEntity, SelectEntityDescriptio from homeassistant.config_entries import ConfigEntry from homeassistant.const import UnitOfElectricCurrent from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import BMWBaseEntity @@ -123,6 +125,9 @@ class BMWSelect(BMWBaseEntity, SelectEntity): self.vehicle.vin, option, ) - await self.entity_description.remote_service(self.vehicle, option) + try: + await self.entity_description.remote_service(self.vehicle, option) + except MyBMWAPIError as ex: + raise HomeAssistantError(ex) from ex self.coordinator.async_update_listeners() diff --git a/tests/components/bmw_connected_drive/test_select.py b/tests/components/bmw_connected_drive/test_select.py index b5a13a13b63..97da6f81d6e 100644 --- a/tests/components/bmw_connected_drive/test_select.py +++ b/tests/components/bmw_connected_drive/test_select.py @@ -1,4 +1,7 @@ """Test BMW selects.""" +from unittest.mock import AsyncMock + +from bimmer_connected.models import MyBMWAPIError, MyBMWRemoteServiceError from bimmer_connected.vehicle.remote_services import RemoteServices import pytest import respx @@ -8,6 +11,7 @@ from homeassistant.components.bmw_connected_drive.coordinator import ( BMWDataUpdateCoordinator, ) from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from . import setup_mocked_integration @@ -87,3 +91,42 @@ async def test_update_triggers_fail( ) assert RemoteServices.trigger_remote_service.call_count == 0 assert BMWDataUpdateCoordinator.async_update_listeners.call_count == 0 + + +@pytest.mark.parametrize( + ("raised", "expected"), + [ + (MyBMWRemoteServiceError, HomeAssistantError), + (MyBMWAPIError, HomeAssistantError), + (ValueError, ValueError), + ], +) +async def test_remote_service_exceptions( + hass: HomeAssistant, + raised: Exception, + expected: Exception, + bmw_fixture: respx.Router, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test exception handling for remote services.""" + + # Setup component + assert await setup_mocked_integration(hass) + + # Setup exception + monkeypatch.setattr( + RemoteServices, + "trigger_remote_service", + AsyncMock(side_effect=raised), + ) + + # Test + with pytest.raises(expected): + await hass.services.async_call( + "select", + "select_option", + service_data={"option": "16"}, + blocking=True, + target={"entity_id": "select.i4_edrive40_ac_charging_limit"}, + ) + assert RemoteServices.trigger_remote_service.call_count == 1