From f44cb9b03eec70a21f890a077136c67b12fd4b0b Mon Sep 17 00:00:00 2001 From: Simone Chemelli Date: Mon, 19 May 2025 18:30:34 +0300 Subject: [PATCH] Add action exceptions to Comelit integration (#143581) * Add action exceptions to Comelit integration * missing decorator * update quality scale --- homeassistant/components/comelit/climate.py | 3 + homeassistant/components/comelit/cover.py | 2 + .../components/comelit/humidifier.py | 5 + homeassistant/components/comelit/light.py | 2 + .../components/comelit/manifest.json | 2 +- .../components/comelit/quality_scale.yaml | 4 +- homeassistant/components/comelit/strings.json | 3 + homeassistant/components/comelit/switch.py | 2 + homeassistant/components/comelit/utils.py | 40 ++++++++ tests/components/comelit/test_utils.py | 93 +++++++++++++++++++ 10 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 tests/components/comelit/test_utils.py diff --git a/homeassistant/components/comelit/climate.py b/homeassistant/components/comelit/climate.py index e7890cddff8..69d95da01bf 100644 --- a/homeassistant/components/comelit/climate.py +++ b/homeassistant/components/comelit/climate.py @@ -23,6 +23,7 @@ from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from .const import DOMAIN from .coordinator import ComelitConfigEntry, ComelitSerialBridge from .entity import ComelitBridgeBaseEntity +from .utils import bridge_api_call # Coordinator is used to centralize the data updates PARALLEL_UPDATES = 0 @@ -155,6 +156,7 @@ class ComelitClimateEntity(ComelitBridgeBaseEntity, ClimateEntity): self._update_attributes() super()._handle_coordinator_update() + @bridge_api_call async def async_set_temperature(self, **kwargs: Any) -> None: """Set new target temperature.""" if ( @@ -171,6 +173,7 @@ class ComelitClimateEntity(ComelitBridgeBaseEntity, ClimateEntity): self._attr_target_temperature = target_temp self.async_write_ha_state() + @bridge_api_call async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None: """Set hvac mode.""" diff --git a/homeassistant/components/comelit/cover.py b/homeassistant/components/comelit/cover.py index d4eaa9223df..691ebaec638 100644 --- a/homeassistant/components/comelit/cover.py +++ b/homeassistant/components/comelit/cover.py @@ -14,6 +14,7 @@ from homeassistant.helpers.restore_state import RestoreEntity from .coordinator import ComelitConfigEntry, ComelitSerialBridge from .entity import ComelitBridgeBaseEntity +from .utils import bridge_api_call # Coordinator is used to centralize the data updates PARALLEL_UPDATES = 0 @@ -83,6 +84,7 @@ class ComelitCoverEntity(ComelitBridgeBaseEntity, RestoreEntity, CoverEntity): """Return if the cover is opening.""" return self._current_action("opening") + @bridge_api_call async def _cover_set_state(self, action: int, state: int) -> None: """Set desired cover state.""" self._last_state = self.state diff --git a/homeassistant/components/comelit/humidifier.py b/homeassistant/components/comelit/humidifier.py index 816d5c6bb38..0c43744aadd 100644 --- a/homeassistant/components/comelit/humidifier.py +++ b/homeassistant/components/comelit/humidifier.py @@ -23,6 +23,7 @@ from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from .const import DOMAIN from .coordinator import ComelitConfigEntry, ComelitSerialBridge from .entity import ComelitBridgeBaseEntity +from .utils import bridge_api_call # Coordinator is used to centralize the data updates PARALLEL_UPDATES = 0 @@ -154,6 +155,7 @@ class ComelitHumidifierEntity(ComelitBridgeBaseEntity, HumidifierEntity): self._update_attributes() super()._handle_coordinator_update() + @bridge_api_call async def async_set_humidity(self, humidity: int) -> None: """Set new target humidity.""" if not self._attr_is_on: @@ -171,6 +173,7 @@ class ComelitHumidifierEntity(ComelitBridgeBaseEntity, HumidifierEntity): self._attr_target_humidity = humidity self.async_write_ha_state() + @bridge_api_call async def async_set_mode(self, mode: str) -> None: """Set humidifier mode.""" await self.coordinator.api.set_humidity_status( @@ -179,6 +182,7 @@ class ComelitHumidifierEntity(ComelitBridgeBaseEntity, HumidifierEntity): self._attr_mode = mode self.async_write_ha_state() + @bridge_api_call async def async_turn_on(self, **kwargs: Any) -> None: """Turn on.""" await self.coordinator.api.set_humidity_status( @@ -187,6 +191,7 @@ class ComelitHumidifierEntity(ComelitBridgeBaseEntity, HumidifierEntity): self._attr_is_on = True self.async_write_ha_state() + @bridge_api_call async def async_turn_off(self, **kwargs: Any) -> None: """Turn off.""" await self.coordinator.api.set_humidity_status( diff --git a/homeassistant/components/comelit/light.py b/homeassistant/components/comelit/light.py index 27d9a8d57dd..c04b88c7819 100644 --- a/homeassistant/components/comelit/light.py +++ b/homeassistant/components/comelit/light.py @@ -12,6 +12,7 @@ from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from .coordinator import ComelitConfigEntry, ComelitSerialBridge from .entity import ComelitBridgeBaseEntity +from .utils import bridge_api_call # Coordinator is used to centralize the data updates PARALLEL_UPDATES = 0 @@ -39,6 +40,7 @@ class ComelitLightEntity(ComelitBridgeBaseEntity, LightEntity): _attr_name = None _attr_supported_color_modes = {ColorMode.ONOFF} + @bridge_api_call async def _light_set_state(self, state: int) -> None: """Set desired light state.""" await self.coordinator.api.set_device_status(LIGHT, self._device.index, state) diff --git a/homeassistant/components/comelit/manifest.json b/homeassistant/components/comelit/manifest.json index bea84c6b805..44101f0fd06 100644 --- a/homeassistant/components/comelit/manifest.json +++ b/homeassistant/components/comelit/manifest.json @@ -7,6 +7,6 @@ "integration_type": "hub", "iot_class": "local_polling", "loggers": ["aiocomelit"], - "quality_scale": "bronze", + "quality_scale": "silver", "requirements": ["aiocomelit==0.12.3"] } diff --git a/homeassistant/components/comelit/quality_scale.yaml b/homeassistant/components/comelit/quality_scale.yaml index 7465193ffa9..4fbbd79d60d 100644 --- a/homeassistant/components/comelit/quality_scale.yaml +++ b/homeassistant/components/comelit/quality_scale.yaml @@ -26,9 +26,7 @@ rules: unique-config-entry: done # Silver - action-exceptions: - status: todo - comment: wrap api calls in try block + action-exceptions: done config-entry-unloading: done docs-configuration-parameters: status: exempt diff --git a/homeassistant/components/comelit/strings.json b/homeassistant/components/comelit/strings.json index 973fcad1999..7a04b5d2d04 100644 --- a/homeassistant/components/comelit/strings.json +++ b/homeassistant/components/comelit/strings.json @@ -89,6 +89,9 @@ "cannot_authenticate": { "message": "Error authenticating" }, + "cannot_retrieve_data": { + "message": "Error retrieving data: {error}" + }, "update_failed": { "message": "Failed to update data: {error}" } diff --git a/homeassistant/components/comelit/switch.py b/homeassistant/components/comelit/switch.py index 658f37f70af..1896071596f 100644 --- a/homeassistant/components/comelit/switch.py +++ b/homeassistant/components/comelit/switch.py @@ -13,6 +13,7 @@ from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from .coordinator import ComelitConfigEntry, ComelitSerialBridge from .entity import ComelitBridgeBaseEntity +from .utils import bridge_api_call # Coordinator is used to centralize the data updates PARALLEL_UPDATES = 0 @@ -56,6 +57,7 @@ class ComelitSwitchEntity(ComelitBridgeBaseEntity, SwitchEntity): if device.type == OTHER: self._attr_device_class = SwitchDeviceClass.OUTLET + @bridge_api_call async def _switch_set_state(self, state: int) -> None: """Set desired switch state.""" await self.coordinator.api.set_device_status( diff --git a/homeassistant/components/comelit/utils.py b/homeassistant/components/comelit/utils.py index fe05e2412b0..5d16f6232df 100644 --- a/homeassistant/components/comelit/utils.py +++ b/homeassistant/components/comelit/utils.py @@ -1,13 +1,53 @@ """Utils for Comelit.""" +from collections.abc import Awaitable, Callable, Coroutine +from functools import wraps +from typing import Any, Concatenate + +from aiocomelit.exceptions import CannotAuthenticate, CannotConnect, CannotRetrieveData from aiohttp import ClientSession, CookieJar from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import aiohttp_client +from .const import DOMAIN +from .entity import ComelitBridgeBaseEntity + async def async_client_session(hass: HomeAssistant) -> ClientSession: """Return a new aiohttp session.""" return aiohttp_client.async_create_clientsession( hass, verify_ssl=False, cookie_jar=CookieJar(unsafe=True) ) + + +def bridge_api_call[_T: ComelitBridgeBaseEntity, **_P]( + func: Callable[Concatenate[_T, _P], Awaitable[None]], +) -> Callable[Concatenate[_T, _P], Coroutine[Any, Any, None]]: + """Catch Bridge API call exceptions.""" + + @wraps(func) + async def cmd_wrapper(self: _T, *args: _P.args, **kwargs: _P.kwargs) -> None: + """Wrap all command methods.""" + try: + await func(self, *args, **kwargs) + except CannotConnect as err: + self.coordinator.last_update_success = False + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="cannot_connect", + translation_placeholders={"error": repr(err)}, + ) from err + except CannotRetrieveData as err: + self.coordinator.last_update_success = False + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="cannot_retrieve_data", + translation_placeholders={"error": repr(err)}, + ) from err + except CannotAuthenticate: + self.coordinator.last_update_success = False + self.coordinator.config_entry.async_start_reauth(self.hass) + + return cmd_wrapper diff --git a/tests/components/comelit/test_utils.py b/tests/components/comelit/test_utils.py new file mode 100644 index 00000000000..413d0d0e561 --- /dev/null +++ b/tests/components/comelit/test_utils.py @@ -0,0 +1,93 @@ +"""Tests for Comelit SimpleHome switch platform.""" + +from unittest.mock import AsyncMock + +from aiocomelit.exceptions import CannotAuthenticate, CannotConnect, CannotRetrieveData +import pytest + +from homeassistant.components.comelit.const import DOMAIN +from homeassistant.components.switch import DOMAIN as SWITCH_DOMAIN, SERVICE_TURN_ON +from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState +from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError + +from . import setup_integration + +from tests.common import MockConfigEntry + +ENTITY_ID = "switch.switch0" + + +@pytest.mark.parametrize( + ("side_effect", "key", "error"), + [ + (CannotConnect, "cannot_connect", "CannotConnect()"), + (CannotRetrieveData, "cannot_retrieve_data", "CannotRetrieveData()"), + ], +) +async def test_bridge_api_call_exceptions( + hass: HomeAssistant, + mock_serial_bridge: AsyncMock, + mock_serial_bridge_config_entry: MockConfigEntry, + side_effect: Exception, + key: str, + error: str, +) -> None: + """Test bridge_api_call decorator for exceptions.""" + + await setup_integration(hass, mock_serial_bridge_config_entry) + + assert (state := hass.states.get(ENTITY_ID)) + assert state.state == STATE_OFF + + mock_serial_bridge.set_device_status.side_effect = side_effect + + # Call API + with pytest.raises(HomeAssistantError) as exc_info: + await hass.services.async_call( + SWITCH_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: ENTITY_ID}, + blocking=True, + ) + + assert exc_info.value.translation_domain == DOMAIN + assert exc_info.value.translation_key == key + assert exc_info.value.translation_placeholders == {"error": error} + + +async def test_bridge_api_call_reauth( + hass: HomeAssistant, + mock_serial_bridge: AsyncMock, + mock_serial_bridge_config_entry: MockConfigEntry, +) -> None: + """Test bridge_api_call decorator for reauth.""" + + await setup_integration(hass, mock_serial_bridge_config_entry) + + assert (state := hass.states.get(ENTITY_ID)) + assert state.state == STATE_OFF + + mock_serial_bridge.set_device_status.side_effect = CannotAuthenticate + + # Call API + await hass.services.async_call( + SWITCH_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: ENTITY_ID}, + blocking=True, + ) + + assert mock_serial_bridge_config_entry.state is ConfigEntryState.LOADED + + flows = hass.config_entries.flow.async_progress() + assert len(flows) == 1 + + flow = flows[0] + assert flow.get("step_id") == "reauth_confirm" + assert flow.get("handler") == DOMAIN + + assert "context" in flow + assert flow["context"].get("source") == SOURCE_REAUTH + assert flow["context"].get("entry_id") == mock_serial_bridge_config_entry.entry_id