From aaecb47125099bac51cd18f081a9988ba9f4aa4b Mon Sep 17 00:00:00 2001 From: Simone Chemelli Date: Mon, 3 Mar 2025 17:57:42 +0100 Subject: [PATCH] Add strict typing to Comelit (#139455) * Add quality scale and strict typing to Comelit * mypy * fix strings * remove quality scale * revert quality scale changes * improve typing * letfover * update typing based on new lib * align to platform * cleanup * apply review comments (part 1) * apply review comment ( part 2) * apply review comments * align * align test data * TypedDict * better casting --- .strict-typing | 1 + .../components/comelit/alarm_control_panel.py | 6 +- .../components/comelit/binary_sensor.py | 9 +- homeassistant/components/comelit/climate.py | 120 +++++++----------- .../components/comelit/coordinator.py | 44 +++++-- .../components/comelit/humidifier.py | 73 ++++------- homeassistant/components/comelit/sensor.py | 19 +-- homeassistant/components/comelit/switch.py | 5 +- mypy.ini | 10 ++ tests/components/comelit/const.py | 6 +- .../comelit/snapshots/test_diagnostics.ambr | 4 +- 11 files changed, 140 insertions(+), 157 deletions(-) diff --git a/.strict-typing b/.strict-typing index 8d0d71e85fe..56d3e299281 100644 --- a/.strict-typing +++ b/.strict-typing @@ -136,6 +136,7 @@ homeassistant.components.clicksend.* homeassistant.components.climate.* homeassistant.components.cloud.* homeassistant.components.co2signal.* +homeassistant.components.comelit.* homeassistant.components.command_line.* homeassistant.components.config.* homeassistant.components.configurator.* diff --git a/homeassistant/components/comelit/alarm_control_panel.py b/homeassistant/components/comelit/alarm_control_panel.py index 6ea4e97f12e..0a01dd957a6 100644 --- a/homeassistant/components/comelit/alarm_control_panel.py +++ b/homeassistant/components/comelit/alarm_control_panel.py @@ -6,7 +6,7 @@ import logging from typing import cast from aiocomelit.api import ComelitVedoAreaObject -from aiocomelit.const import ALARM_AREAS, AlarmAreaState +from aiocomelit.const import AlarmAreaState from homeassistant.components.alarm_control_panel import ( AlarmControlPanelEntity, @@ -56,7 +56,7 @@ async def async_setup_entry( async_add_entities( ComelitAlarmEntity(coordinator, device, config_entry.entry_id) - for device in coordinator.data[ALARM_AREAS].values() + for device in coordinator.data["alarm_areas"].values() ) @@ -92,7 +92,7 @@ class ComelitAlarmEntity(CoordinatorEntity[ComelitVedoSystem], AlarmControlPanel @property def _area(self) -> ComelitVedoAreaObject: """Return area object.""" - return self.coordinator.data[ALARM_AREAS][self._area_index] + return self.coordinator.data["alarm_areas"][self._area_index] @property def available(self) -> bool: diff --git a/homeassistant/components/comelit/binary_sensor.py b/homeassistant/components/comelit/binary_sensor.py index a895f8dc511..c17057d19d1 100644 --- a/homeassistant/components/comelit/binary_sensor.py +++ b/homeassistant/components/comelit/binary_sensor.py @@ -5,7 +5,6 @@ from __future__ import annotations from typing import cast from aiocomelit import ComelitVedoZoneObject -from aiocomelit.const import ALARM_ZONES from homeassistant.components.binary_sensor import ( BinarySensorDeviceClass, @@ -29,7 +28,7 @@ async def async_setup_entry( async_add_entities( ComelitVedoBinarySensorEntity(coordinator, device, config_entry.entry_id) - for device in coordinator.data[ALARM_ZONES].values() + for device in coordinator.data["alarm_zones"].values() ) @@ -49,7 +48,7 @@ class ComelitVedoBinarySensorEntity( ) -> None: """Init sensor entity.""" self._api = coordinator.api - self._zone = zone + self._zone_index = zone.index super().__init__(coordinator) # Use config_entry.entry_id as base for unique_id # because no serial number or mac is available @@ -59,4 +58,6 @@ class ComelitVedoBinarySensorEntity( @property def is_on(self) -> bool: """Presence detected.""" - return self.coordinator.data[ALARM_ZONES][self._zone.index].status_api == "0001" + return ( + self.coordinator.data["alarm_zones"][self._zone_index].status_api == "0001" + ) diff --git a/homeassistant/components/comelit/climate.py b/homeassistant/components/comelit/climate.py index 6906c9bf735..3433d1bdf04 100644 --- a/homeassistant/components/comelit/climate.py +++ b/homeassistant/components/comelit/climate.py @@ -3,7 +3,7 @@ from __future__ import annotations from enum import StrEnum -from typing import Any, cast +from typing import Any, TypedDict, cast from aiocomelit import ComelitSerialBridgeObject from aiocomelit.const import CLIMATE @@ -16,7 +16,8 @@ from homeassistant.components.climate import ( UnitOfTemperature, ) from homeassistant.const import ATTR_TEMPERATURE, PRECISION_TENTHS -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from homeassistant.helpers.update_coordinator import CoordinatorEntity @@ -42,22 +43,23 @@ class ClimaComelitCommand(StrEnum): AUTO = "auto" -API_STATUS: dict[str, dict[str, Any]] = { - ClimaComelitMode.OFF: { - "action": "off", - "hvac_mode": HVACMode.OFF, - "hvac_action": HVACAction.OFF, - }, - ClimaComelitMode.LOWER: { - "action": "lower", - "hvac_mode": HVACMode.COOL, - "hvac_action": HVACAction.COOLING, - }, - ClimaComelitMode.UPPER: { - "action": "upper", - "hvac_mode": HVACMode.HEAT, - "hvac_action": HVACAction.HEATING, - }, +class ClimaComelitApiStatus(TypedDict): + """Comelit Clima API status.""" + + hvac_mode: HVACMode + hvac_action: HVACAction + + +API_STATUS: dict[str, ClimaComelitApiStatus] = { + ClimaComelitMode.OFF: ClimaComelitApiStatus( + hvac_mode=HVACMode.OFF, hvac_action=HVACAction.OFF + ), + ClimaComelitMode.LOWER: ClimaComelitApiStatus( + hvac_mode=HVACMode.COOL, hvac_action=HVACAction.COOLING + ), + ClimaComelitMode.UPPER: ClimaComelitApiStatus( + hvac_mode=HVACMode.HEAT, hvac_action=HVACAction.HEATING + ), } MODE_TO_ACTION: dict[HVACMode, ClimaComelitCommand] = { @@ -114,69 +116,41 @@ class ComelitClimateEntity(CoordinatorEntity[ComelitSerialBridge], ClimateEntity self._attr_unique_id = f"{config_entry_entry_id}-{device.index}" self._attr_device_info = coordinator.platform_device_info(device, device.type) - @property - def _clima(self) -> list[Any]: - """Return clima device data.""" + @callback + def _handle_coordinator_update(self) -> None: + """Handle updated data from the coordinator.""" + device = self.coordinator.data[CLIMATE][self._device.index] + if not isinstance(device.val, list): + raise HomeAssistantError("Invalid clima data") + # CLIMATE has a 2 item tuple: # - first for Clima # - second for Humidifier - return self.coordinator.data[CLIMATE][self._device.index].val[0] + values = device.val[0] - @property - def _api_mode(self) -> str: - """Return device mode.""" - # Values from API: "O", "L", "U" - return self._clima[2] + _active = values[1] + _mode = values[2] # Values from API: "O", "L", "U" + _automatic = values[3] == ClimaComelitMode.AUTO - @property - def _api_active(self) -> bool: - "Return device active/idle." - return self._clima[1] + self._attr_current_temperature = values[0] / 10 - @property - def _api_automatic(self) -> bool: - """Return device in automatic/manual mode.""" - return self._clima[3] == ClimaComelitMode.AUTO + self._attr_hvac_action = None + if _mode == ClimaComelitMode.OFF: + self._attr_hvac_action = HVACAction.OFF + if not _active: + self._attr_hvac_action = HVACAction.IDLE + if _mode in API_STATUS: + self._attr_hvac_action = API_STATUS[_mode]["hvac_action"] - @property - def target_temperature(self) -> float: - """Set target temperature.""" - return self._clima[4] / 10 + self._attr_hvac_mode = None + if _mode == ClimaComelitMode.OFF: + self._attr_hvac_mode = HVACMode.OFF + if _automatic: + self._attr_hvac_mode = HVACMode.AUTO + if _mode in API_STATUS: + self._attr_hvac_mode = API_STATUS[_mode]["hvac_mode"] - @property - def current_temperature(self) -> float: - """Return current temperature.""" - return self._clima[0] / 10 - - @property - def hvac_mode(self) -> HVACMode | None: - """HVAC current mode.""" - - if self._api_mode == ClimaComelitMode.OFF: - return HVACMode.OFF - - if self._api_automatic: - return HVACMode.AUTO - - if self._api_mode in API_STATUS: - return API_STATUS[self._api_mode]["hvac_mode"] - - return None - - @property - def hvac_action(self) -> HVACAction | None: - """HVAC current action.""" - - if self._api_mode == ClimaComelitMode.OFF: - return HVACAction.OFF - - if not self._api_active: - return HVACAction.IDLE - - if self._api_mode in API_STATUS: - return API_STATUS[self._api_mode]["hvac_action"] - - return None + self._attr_target_temperature = values[4] / 10 async def async_set_temperature(self, **kwargs: Any) -> None: """Set new target temperature.""" diff --git a/homeassistant/components/comelit/coordinator.py b/homeassistant/components/comelit/coordinator.py index fcb149b21d6..a569a397c85 100644 --- a/homeassistant/components/comelit/coordinator.py +++ b/homeassistant/components/comelit/coordinator.py @@ -2,7 +2,7 @@ from abc import abstractmethod from datetime import timedelta -from typing import Any +from typing import TypedDict, TypeVar, cast from aiocomelit import ( ComeliteSerialBridgeApi, @@ -13,7 +13,7 @@ from aiocomelit import ( exceptions, ) from aiocomelit.api import ComelitCommonApi -from aiocomelit.const import BRIDGE, VEDO +from aiocomelit.const import ALARM_AREAS, ALARM_ZONES, BRIDGE, VEDO from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant @@ -26,7 +26,20 @@ from .const import _LOGGER, DOMAIN type ComelitConfigEntry = ConfigEntry[ComelitBaseCoordinator] -class ComelitBaseCoordinator(DataUpdateCoordinator[dict[str, Any]]): +class AlarmDataObject(TypedDict): + """TypedDict for Alarm data objects.""" + + alarm_areas: dict[int, ComelitVedoAreaObject] + alarm_zones: dict[int, ComelitVedoZoneObject] + + +T = TypeVar( + "T", + bound=dict[str, dict[int, ComelitSerialBridgeObject]] | AlarmDataObject, +) + + +class ComelitBaseCoordinator(DataUpdateCoordinator[T]): """Base coordinator for Comelit Devices.""" _hw_version: str @@ -81,7 +94,7 @@ class ComelitBaseCoordinator(DataUpdateCoordinator[dict[str, Any]]): hw_version=self._hw_version, ) - async def _async_update_data(self) -> dict[str, Any]: + async def _async_update_data(self) -> T: """Update device data.""" _LOGGER.debug("Polling Comelit %s host: %s", self._device, self._host) try: @@ -93,11 +106,13 @@ class ComelitBaseCoordinator(DataUpdateCoordinator[dict[str, Any]]): raise ConfigEntryAuthFailed from err @abstractmethod - async def _async_update_system_data(self) -> dict[str, Any]: + async def _async_update_system_data(self) -> T: """Class method for updating data.""" -class ComelitSerialBridge(ComelitBaseCoordinator): +class ComelitSerialBridge( + ComelitBaseCoordinator[dict[str, dict[int, ComelitSerialBridgeObject]]] +): """Queries Comelit Serial Bridge.""" _hw_version = "20003101" @@ -115,12 +130,14 @@ class ComelitSerialBridge(ComelitBaseCoordinator): self.api = ComeliteSerialBridgeApi(host, port, pin) super().__init__(hass, entry, BRIDGE, host) - async def _async_update_system_data(self) -> dict[str, Any]: + async def _async_update_system_data( + self, + ) -> dict[str, dict[int, ComelitSerialBridgeObject]]: """Specific method for updating data.""" return await self.api.get_all_devices() -class ComelitVedoSystem(ComelitBaseCoordinator): +class ComelitVedoSystem(ComelitBaseCoordinator[AlarmDataObject]): """Queries Comelit VEDO system.""" _hw_version = "VEDO IP" @@ -138,6 +155,13 @@ class ComelitVedoSystem(ComelitBaseCoordinator): self.api = ComelitVedoApi(host, port, pin) super().__init__(hass, entry, VEDO, host) - async def _async_update_system_data(self) -> dict[str, Any]: + async def _async_update_system_data( + self, + ) -> AlarmDataObject: """Specific method for updating data.""" - return await self.api.get_all_areas_and_zones() + data = await self.api.get_all_areas_and_zones() + + return AlarmDataObject( + alarm_areas=cast(dict[int, ComelitVedoAreaObject], data[ALARM_AREAS]), + alarm_zones=cast(dict[int, ComelitVedoZoneObject], data[ALARM_ZONES]), + ) diff --git a/homeassistant/components/comelit/humidifier.py b/homeassistant/components/comelit/humidifier.py index 5daf2297782..da6d44b1bbe 100644 --- a/homeassistant/components/comelit/humidifier.py +++ b/homeassistant/components/comelit/humidifier.py @@ -16,8 +16,8 @@ from homeassistant.components.humidifier import ( HumidifierEntity, HumidifierEntityFeature, ) -from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ServiceValidationError +from homeassistant.core import HomeAssistant, callback +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback from homeassistant.helpers.update_coordinator import CoordinatorEntity @@ -122,61 +122,32 @@ class ComelitHumidifierEntity(CoordinatorEntity[ComelitSerialBridge], Humidifier self._active_action = active_action self._set_command = set_command - @property - def _humidifier(self) -> list[Any]: - """Return humidifier device data.""" + @callback + def _handle_coordinator_update(self) -> None: + """Handle updated data from the coordinator.""" + device = self.coordinator.data[CLIMATE][self._device.index] + if not isinstance(device.val, list): + raise HomeAssistantError("Invalid clima data") + # CLIMATE has a 2 item tuple: # - first for Clima # - second for Humidifier - return self.coordinator.data[CLIMATE][self._device.index].val[1] + values = device.val[1] - @property - def _api_mode(self) -> str: - """Return device mode.""" - # Values from API: "O", "L", "U" - return self._humidifier[2] + _active = values[1] + _mode = values[2] # Values from API: "O", "L", "U" + _automatic = values[3] == HumidifierComelitMode.AUTO - @property - def _api_active(self) -> bool: - "Return device active/idle." - return self._humidifier[1] + self._attr_action = HumidifierAction.IDLE + if _mode == HumidifierComelitMode.OFF: + self._attr_action = HumidifierAction.OFF + if _active and _mode == self._active_mode: + self._attr_action = self._active_action - @property - def _api_automatic(self) -> bool: - """Return device in automatic/manual mode.""" - return self._humidifier[3] == HumidifierComelitMode.AUTO - - @property - def target_humidity(self) -> float: - """Set target humidity.""" - return self._humidifier[4] / 10 - - @property - def current_humidity(self) -> float: - """Return current humidity.""" - return self._humidifier[0] / 10 - - @property - def is_on(self) -> bool | None: - """Return true is humidifier is on.""" - return self._api_mode == self._active_mode - - @property - def mode(self) -> str | None: - """Return current mode.""" - return MODE_AUTO if self._api_automatic else MODE_NORMAL - - @property - def action(self) -> HumidifierAction | None: - """Return current action.""" - - if self._api_mode == HumidifierComelitMode.OFF: - return HumidifierAction.OFF - - if self._api_active and self._api_mode == self._active_mode: - return self._active_action - - return HumidifierAction.IDLE + self._attr_current_humidity = values[0] / 10 + self._attr_is_on = _mode == self._active_mode + self._attr_mode = MODE_AUTO if _automatic else MODE_NORMAL + self._attr_target_humidity = values[4] / 10 async def async_set_humidity(self, humidity: int) -> None: """Set new target humidity.""" diff --git a/homeassistant/components/comelit/sensor.py b/homeassistant/components/comelit/sensor.py index 9200d99262f..3d57d9dca9c 100644 --- a/homeassistant/components/comelit/sensor.py +++ b/homeassistant/components/comelit/sensor.py @@ -5,7 +5,7 @@ from __future__ import annotations from typing import Final, cast from aiocomelit import ComelitSerialBridgeObject, ComelitVedoZoneObject -from aiocomelit.const import ALARM_ZONES, BRIDGE, OTHER, AlarmZoneState +from aiocomelit.const import BRIDGE, OTHER, AlarmZoneState from homeassistant.components.sensor import ( SensorDeviceClass, @@ -82,7 +82,7 @@ async def async_setup_vedo_entry( coordinator = cast(ComelitVedoSystem, config_entry.runtime_data) entities: list[ComelitVedoSensorEntity] = [] - for device in coordinator.data[ALARM_ZONES].values(): + for device in coordinator.data["alarm_zones"].values(): entities.extend( ComelitVedoSensorEntity( coordinator, device, config_entry.entry_id, sensor_desc @@ -119,9 +119,12 @@ class ComelitBridgeSensorEntity(CoordinatorEntity[ComelitSerialBridge], SensorEn @property def native_value(self) -> StateType: """Sensor value.""" - return getattr( - self.coordinator.data[OTHER][self._device.index], - self.entity_description.key, + return cast( + StateType, + getattr( + self.coordinator.data[OTHER][self._device.index], + self.entity_description.key, + ), ) @@ -139,7 +142,7 @@ class ComelitVedoSensorEntity(CoordinatorEntity[ComelitVedoSystem], SensorEntity ) -> None: """Init sensor entity.""" self._api = coordinator.api - self._zone = zone + self._zone_index = zone.index super().__init__(coordinator) # Use config_entry.entry_id as base for unique_id # because no serial number or mac is available @@ -151,7 +154,7 @@ class ComelitVedoSensorEntity(CoordinatorEntity[ComelitVedoSystem], SensorEntity @property def _zone_object(self) -> ComelitVedoZoneObject: """Zone object.""" - return self.coordinator.data[ALARM_ZONES][self._zone.index] + return self.coordinator.data["alarm_zones"][self._zone_index] @property def available(self) -> bool: @@ -164,4 +167,4 @@ class ComelitVedoSensorEntity(CoordinatorEntity[ComelitVedoSystem], SensorEntity if (status := self._zone_object.human_status) == AlarmZoneState.UNKNOWN: return None - return status.value + return cast(str, status.value) diff --git a/homeassistant/components/comelit/switch.py b/homeassistant/components/comelit/switch.py index e89ee74c1be..f6e5b192c38 100644 --- a/homeassistant/components/comelit/switch.py +++ b/homeassistant/components/comelit/switch.py @@ -77,7 +77,4 @@ class ComelitSwitchEntity(CoordinatorEntity[ComelitSerialBridge], SwitchEntity): @property def is_on(self) -> bool: """Return True if switch is on.""" - return ( - self.coordinator.data[self._device.type][self._device.index].status - == STATE_ON - ) + return self.coordinator.data[OTHER][self._device.index].status == STATE_ON diff --git a/mypy.ini b/mypy.ini index c69401b8605..520fad7d738 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1115,6 +1115,16 @@ disallow_untyped_defs = true warn_return_any = true warn_unreachable = true +[mypy-homeassistant.components.comelit.*] +check_untyped_defs = true +disallow_incomplete_defs = true +disallow_subclassing_any = true +disallow_untyped_calls = true +disallow_untyped_decorators = true +disallow_untyped_defs = true +warn_return_any = true +warn_unreachable = true + [mypy-homeassistant.components.command_line.*] check_untyped_defs = true disallow_incomplete_defs = true diff --git a/tests/components/comelit/const.py b/tests/components/comelit/const.py index 3151b83d175..20324765a0b 100644 --- a/tests/components/comelit/const.py +++ b/tests/components/comelit/const.py @@ -6,6 +6,8 @@ from aiocomelit import ( ComelitVedoZoneObject, ) from aiocomelit.const import ( + ALARM_AREAS, + ALARM_ZONES, CLIMATE, COVER, IRRIGATION, @@ -63,7 +65,7 @@ BRIDGE_DEVICE_QUERY = { } VEDO_DEVICE_QUERY = { - "aree": { + ALARM_AREAS: { 0: ComelitVedoAreaObject( index=0, name="Area0", @@ -80,7 +82,7 @@ VEDO_DEVICE_QUERY = { human_status=AlarmAreaState.UNKNOWN, ) }, - "zone": { + ALARM_ZONES: { 0: ComelitVedoZoneObject( index=0, name="Zone0", diff --git a/tests/components/comelit/snapshots/test_diagnostics.ambr b/tests/components/comelit/snapshots/test_diagnostics.ambr index b9891eb3209..c4544f38f52 100644 --- a/tests/components/comelit/snapshots/test_diagnostics.ambr +++ b/tests/components/comelit/snapshots/test_diagnostics.ambr @@ -86,7 +86,7 @@ 'device_info': dict({ 'devices': list([ dict({ - 'aree': list([ + 'alarm_areas': list([ dict({ '0': dict({ 'alarm': False, @@ -106,7 +106,7 @@ ]), }), dict({ - 'zone': list([ + 'alarm_zones': list([ dict({ '0': dict({ 'human_status': 'rest',