From 9a510cfe321feea11dc6230d02dbaf7c6e8e174c Mon Sep 17 00:00:00 2001 From: Jan Bouwhuis Date: Wed, 5 Jun 2024 10:45:01 +0200 Subject: [PATCH] Add data coordinator to incomfort integration (#118816) * Add data coordinator to incomfort integration * Remove unused code and redundant comment, move entity class * Use freezer * Cleanup snapshot * Use entry.runtime_data * Use freezer, use mock_config_entry * Use tick * Use ConfigEntryError while we do not yet support a re-auth flow, update tests * Use tick with async_fire_time_changed --- .../components/incomfort/__init__.py | 35 ++------- .../components/incomfort/binary_sensor.py | 23 +++--- homeassistant/components/incomfort/climate.py | 29 ++++--- .../components/incomfort/config_flow.py | 2 +- .../components/incomfort/coordinator.py | 75 ++++++++++++++++++ homeassistant/components/incomfort/entity.py | 11 +++ homeassistant/components/incomfort/models.py | 40 ---------- homeassistant/components/incomfort/sensor.py | 21 ++--- .../components/incomfort/water_heater.py | 36 +++------ tests/components/incomfort/conftest.py | 2 +- tests/components/incomfort/test_init.py | 78 ++++++++++++++++++- 11 files changed, 219 insertions(+), 133 deletions(-) create mode 100644 homeassistant/components/incomfort/coordinator.py create mode 100644 homeassistant/components/incomfort/entity.py delete mode 100644 homeassistant/components/incomfort/models.py diff --git a/homeassistant/components/incomfort/__init__.py b/homeassistant/components/incomfort/__init__.py index c6d479cafb5..39e471b7614 100644 --- a/homeassistant/components/incomfort/__init__.py +++ b/homeassistant/components/incomfort/__init__.py @@ -8,17 +8,15 @@ import voluptuous as vol from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME, Platform -from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant, callback +from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant from homeassistant.data_entry_flow import FlowResultType from homeassistant.exceptions import ConfigEntryAuthFailed from homeassistant.helpers import config_validation as cv, issue_registry as ir -from homeassistant.helpers.dispatcher import async_dispatcher_connect -from homeassistant.helpers.entity import Entity from homeassistant.helpers.typing import ConfigType from .const import DOMAIN +from .coordinator import InComfortDataCoordinator, async_connect_gateway from .errors import InConfortTimeout, InConfortUnknownError, NoHeaters, NotFound -from .models import DATA_INCOMFORT, async_connect_gateway CONFIG_SCHEMA = vol.Schema( { @@ -42,6 +40,8 @@ PLATFORMS = ( INTEGRATION_TITLE = "Intergas InComfort/Intouch Lan2RF gateway" +type InComfortConfigEntry = ConfigEntry[InComfortDataCoordinator] + async def _async_import(hass: HomeAssistant, config: ConfigType) -> None: """Import config entry from configuration.yaml.""" @@ -108,7 +108,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: except TimeoutError as exc: raise InConfortTimeout from exc - hass.data.setdefault(DATA_INCOMFORT, {entry.entry_id: data}) + coordinator = InComfortDataCoordinator(hass, data) + entry.runtime_data = coordinator + await coordinator.async_config_entry_first_refresh() await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) return True @@ -116,25 +118,4 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload config entry.""" - unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) - del hass.data[DOMAIN][entry.entry_id] - return unload_ok - - -class IncomfortEntity(Entity): - """Base class for all InComfort entities.""" - - _attr_should_poll = False - _attr_has_entity_name = True - - async def async_added_to_hass(self) -> None: - """Set up a listener when this entity is added to HA.""" - self.async_on_remove( - async_dispatcher_connect( - self.hass, f"{DOMAIN}_{self.unique_id}", self._refresh - ) - ) - - @callback - def _refresh(self) -> None: - self.async_schedule_update_ha_state(force_refresh=True) + return await hass.config_entries.async_unload_platforms(entry, PLATFORMS) diff --git a/homeassistant/components/incomfort/binary_sensor.py b/homeassistant/components/incomfort/binary_sensor.py index f60ce2f4b59..238f1812aa2 100644 --- a/homeassistant/components/incomfort/binary_sensor.py +++ b/homeassistant/components/incomfort/binary_sensor.py @@ -4,28 +4,28 @@ from __future__ import annotations from typing import Any -from incomfortclient import Gateway as InComfortGateway, Heater as InComfortHeater +from incomfortclient import Heater as InComfortHeater from homeassistant.components.binary_sensor import BinarySensorEntity -from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback -from . import DATA_INCOMFORT, IncomfortEntity +from . import InComfortConfigEntry from .const import DOMAIN +from .coordinator import InComfortDataCoordinator +from .entity import IncomfortEntity async def async_setup_entry( hass: HomeAssistant, - entry: ConfigEntry, + entry: InComfortConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: """Set up an InComfort/InTouch binary_sensor entity.""" - incomfort_data = hass.data[DATA_INCOMFORT][entry.entry_id] - async_add_entities( - IncomfortFailed(incomfort_data.client, h) for h in incomfort_data.heaters - ) + incomfort_coordinator = entry.runtime_data + heaters = incomfort_coordinator.data.heaters + async_add_entities(IncomfortFailed(incomfort_coordinator, h) for h in heaters) class IncomfortFailed(IncomfortEntity, BinarySensorEntity): @@ -33,11 +33,12 @@ class IncomfortFailed(IncomfortEntity, BinarySensorEntity): _attr_name = "Fault" - def __init__(self, client: InComfortGateway, heater: InComfortHeater) -> None: + def __init__( + self, coordinator: InComfortDataCoordinator, heater: InComfortHeater + ) -> None: """Initialize the binary sensor.""" - super().__init__() + super().__init__(coordinator) - self._client = client self._heater = heater self._attr_unique_id = f"{heater.serial_no}_failed" diff --git a/homeassistant/components/incomfort/climate.py b/homeassistant/components/incomfort/climate.py index f1487716d01..7e5cbd08f18 100644 --- a/homeassistant/components/incomfort/climate.py +++ b/homeassistant/components/incomfort/climate.py @@ -4,38 +4,34 @@ from __future__ import annotations from typing import Any -from incomfortclient import ( - Gateway as InComfortGateway, - Heater as InComfortHeater, - Room as InComfortRoom, -) +from incomfortclient import Heater as InComfortHeater, Room as InComfortRoom from homeassistant.components.climate import ( ClimateEntity, ClimateEntityFeature, HVACMode, ) -from homeassistant.config_entries import ConfigEntry from homeassistant.const import ATTR_TEMPERATURE, UnitOfTemperature from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback -from . import DATA_INCOMFORT, IncomfortEntity +from . import InComfortConfigEntry from .const import DOMAIN +from .coordinator import InComfortDataCoordinator +from .entity import IncomfortEntity async def async_setup_entry( hass: HomeAssistant, - entry: ConfigEntry, + entry: InComfortConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: """Set up InComfort/InTouch climate devices.""" - incomfort_data = hass.data[DATA_INCOMFORT][entry.entry_id] + incomfort_coordinator = entry.runtime_data + heaters = incomfort_coordinator.data.heaters async_add_entities( - InComfortClimate(incomfort_data.client, h, r) - for h in incomfort_data.heaters - for r in h.rooms + InComfortClimate(incomfort_coordinator, h, r) for h in heaters for r in h.rooms ) @@ -52,12 +48,14 @@ class InComfortClimate(IncomfortEntity, ClimateEntity): _enable_turn_on_off_backwards_compatibility = False def __init__( - self, client: InComfortGateway, heater: InComfortHeater, room: InComfortRoom + self, + coordinator: InComfortDataCoordinator, + heater: InComfortHeater, + room: InComfortRoom, ) -> None: """Initialize the climate device.""" - super().__init__() + super().__init__(coordinator) - self._client = client self._room = room self._attr_unique_id = f"{heater.serial_no}_{room.room_no}" @@ -86,6 +84,7 @@ class InComfortClimate(IncomfortEntity, ClimateEntity): """Set a new target temperature for this zone.""" temperature = kwargs.get(ATTR_TEMPERATURE) await self._room.set_override(temperature) + await self.coordinator.async_refresh() async def async_set_hvac_mode(self, hvac_mode: HVACMode) -> None: """Set new target hvac mode.""" diff --git a/homeassistant/components/incomfort/config_flow.py b/homeassistant/components/incomfort/config_flow.py index bc928997b32..e905f0d743d 100644 --- a/homeassistant/components/incomfort/config_flow.py +++ b/homeassistant/components/incomfort/config_flow.py @@ -16,7 +16,7 @@ from homeassistant.helpers.selector import ( ) from .const import DOMAIN -from .models import async_connect_gateway +from .coordinator import async_connect_gateway TITLE = "Intergas InComfort/Intouch Lan2RF gateway" diff --git a/homeassistant/components/incomfort/coordinator.py b/homeassistant/components/incomfort/coordinator.py new file mode 100644 index 00000000000..a5c8da0c208 --- /dev/null +++ b/homeassistant/components/incomfort/coordinator.py @@ -0,0 +1,75 @@ +"""Datacoordinator for InComfort integration.""" + +from dataclasses import dataclass, field +from datetime import timedelta +import logging +from typing import Any + +from aiohttp import ClientResponseError +from incomfortclient import ( + Gateway as InComfortGateway, + Heater as InComfortHeater, + IncomfortError, +) + +from homeassistant.const import CONF_HOST +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryError +from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed + +_LOGGER = logging.getLogger(__name__) + +UPDATE_INTERVAL = 30 + + +@dataclass +class InComfortData: + """Keep the Intergas InComfort entry data.""" + + client: InComfortGateway + heaters: list[InComfortHeater] = field(default_factory=list) + + +async def async_connect_gateway( + hass: HomeAssistant, + entry_data: dict[str, Any], +) -> InComfortData: + """Validate the configuration.""" + credentials = dict(entry_data) + hostname = credentials.pop(CONF_HOST) + + client = InComfortGateway( + hostname, **credentials, session=async_get_clientsession(hass) + ) + heaters = await client.heaters() + + return InComfortData(client=client, heaters=heaters) + + +class InComfortDataCoordinator(DataUpdateCoordinator[InComfortData]): + """Data coordinator for InComfort entities.""" + + def __init__(self, hass: HomeAssistant, incomfort_data: InComfortData) -> None: + """Initialize coordinator.""" + super().__init__( + hass, + _LOGGER, + name="InComfort datacoordinator", + update_interval=timedelta(seconds=UPDATE_INTERVAL), + ) + self.incomfort_data = incomfort_data + + async def _async_update_data(self) -> InComfortData: + """Fetch data from API endpoint.""" + try: + for heater in self.incomfort_data.heaters: + await heater.update() + except TimeoutError as exc: + raise UpdateFailed from exc + except IncomfortError as exc: + if isinstance(exc.message, ClientResponseError): + if exc.message.status == 401: + raise ConfigEntryError("Incorrect credentials") from exc + raise UpdateFailed from exc + return self.incomfort_data diff --git a/homeassistant/components/incomfort/entity.py b/homeassistant/components/incomfort/entity.py new file mode 100644 index 00000000000..7b4a535bff6 --- /dev/null +++ b/homeassistant/components/incomfort/entity.py @@ -0,0 +1,11 @@ +"""Common entity classes for InComfort integration.""" + +from homeassistant.helpers.update_coordinator import CoordinatorEntity + +from .coordinator import InComfortDataCoordinator + + +class IncomfortEntity(CoordinatorEntity[InComfortDataCoordinator]): + """Base class for all InComfort entities.""" + + _attr_has_entity_name = True diff --git a/homeassistant/components/incomfort/models.py b/homeassistant/components/incomfort/models.py deleted file mode 100644 index 19e4269e0b4..00000000000 --- a/homeassistant/components/incomfort/models.py +++ /dev/null @@ -1,40 +0,0 @@ -"""Models for Intergas InComfort integration.""" - -from dataclasses import dataclass, field -from typing import Any - -from incomfortclient import Gateway as InComfortGateway, Heater as InComfortHeater - -from homeassistant.const import CONF_HOST -from homeassistant.core import HomeAssistant -from homeassistant.helpers.aiohttp_client import async_get_clientsession -from homeassistant.util.hass_dict import HassKey - -from .const import DOMAIN - - -@dataclass -class InComfortData: - """Keep the Intergas InComfort entry data.""" - - client: InComfortGateway - heaters: list[InComfortHeater] = field(default_factory=list) - - -DATA_INCOMFORT: HassKey[dict[str, InComfortData]] = HassKey(DOMAIN) - - -async def async_connect_gateway( - hass: HomeAssistant, - entry_data: dict[str, Any], -) -> InComfortData: - """Validate the configuration.""" - credentials = dict(entry_data) - hostname = credentials.pop(CONF_HOST) - - client = InComfortGateway( - hostname, **credentials, session=async_get_clientsession(hass) - ) - heaters = await client.heaters() - - return InComfortData(client=client, heaters=heaters) diff --git a/homeassistant/components/incomfort/sensor.py b/homeassistant/components/incomfort/sensor.py index a31488603b3..044443c8ac0 100644 --- a/homeassistant/components/incomfort/sensor.py +++ b/homeassistant/components/incomfort/sensor.py @@ -5,22 +5,23 @@ from __future__ import annotations from dataclasses import dataclass from typing import Any -from incomfortclient import Gateway as InComfortGateway, Heater as InComfortHeater +from incomfortclient import Heater as InComfortHeater from homeassistant.components.sensor import ( SensorDeviceClass, SensorEntity, SensorEntityDescription, ) -from homeassistant.config_entries import ConfigEntry from homeassistant.const import UnitOfPressure, UnitOfTemperature from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.util import slugify -from . import DATA_INCOMFORT, IncomfortEntity +from . import InComfortConfigEntry from .const import DOMAIN +from .coordinator import InComfortDataCoordinator +from .entity import IncomfortEntity INCOMFORT_HEATER_TEMP = "CV Temp" INCOMFORT_PRESSURE = "CV Pressure" @@ -63,14 +64,15 @@ SENSOR_TYPES: tuple[IncomfortSensorEntityDescription, ...] = ( async def async_setup_entry( hass: HomeAssistant, - entry: ConfigEntry, + entry: InComfortConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: """Set up InComfort/InTouch sensor entities.""" - incomfort_data = hass.data[DATA_INCOMFORT][entry.entry_id] + incomfort_coordinator = entry.runtime_data + heaters = incomfort_coordinator.data.heaters async_add_entities( - IncomfortSensor(incomfort_data.client, heater, description) - for heater in incomfort_data.heaters + IncomfortSensor(incomfort_coordinator, heater, description) + for heater in heaters for description in SENSOR_TYPES ) @@ -82,15 +84,14 @@ class IncomfortSensor(IncomfortEntity, SensorEntity): def __init__( self, - client: InComfortGateway, + coordinator: InComfortDataCoordinator, heater: InComfortHeater, description: IncomfortSensorEntityDescription, ) -> None: """Initialize the sensor.""" - super().__init__() + super().__init__(coordinator) self.entity_description = description - self._client = client self._heater = heater self._attr_unique_id = f"{heater.serial_no}_{slugify(description.name)}" diff --git a/homeassistant/components/incomfort/water_heater.py b/homeassistant/components/incomfort/water_heater.py index 239ddae3106..e21e2d5100f 100644 --- a/homeassistant/components/incomfort/water_heater.py +++ b/homeassistant/components/incomfort/water_heater.py @@ -5,19 +5,18 @@ from __future__ import annotations import logging from typing import Any -from aiohttp import ClientResponseError -from incomfortclient import Gateway as InComfortGateway, Heater as InComfortHeater +from incomfortclient import Heater as InComfortHeater from homeassistant.components.water_heater import WaterHeaterEntity -from homeassistant.config_entries import ConfigEntry from homeassistant.const import UnitOfTemperature from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import DeviceInfo -from homeassistant.helpers.dispatcher import async_dispatcher_send from homeassistant.helpers.entity_platform import AddEntitiesCallback -from . import DATA_INCOMFORT, IncomfortEntity +from . import InComfortConfigEntry from .const import DOMAIN +from .coordinator import InComfortDataCoordinator +from .entity import IncomfortEntity _LOGGER = logging.getLogger(__name__) @@ -26,14 +25,13 @@ HEATER_ATTRS = ["display_code", "display_text", "is_burning"] async def async_setup_entry( hass: HomeAssistant, - entry: ConfigEntry, + entry: InComfortConfigEntry, async_add_entities: AddEntitiesCallback, ) -> None: """Set up an InComfort/InTouch water_heater device.""" - incomfort_data = hass.data[DATA_INCOMFORT][entry.entry_id] - async_add_entities( - IncomfortWaterHeater(incomfort_data.client, h) for h in incomfort_data.heaters - ) + incomfort_coordinator = entry.runtime_data + heaters = incomfort_coordinator.data.heaters + async_add_entities(IncomfortWaterHeater(incomfort_coordinator, h) for h in heaters) class IncomfortWaterHeater(IncomfortEntity, WaterHeaterEntity): @@ -45,11 +43,12 @@ class IncomfortWaterHeater(IncomfortEntity, WaterHeaterEntity): _attr_should_poll = True _attr_temperature_unit = UnitOfTemperature.CELSIUS - def __init__(self, client: InComfortGateway, heater: InComfortHeater) -> None: + def __init__( + self, coordinator: InComfortDataCoordinator, heater: InComfortHeater + ) -> None: """Initialize the water_heater device.""" - super().__init__() + super().__init__(coordinator) - self._client = client self._heater = heater self._attr_unique_id = heater.serial_no @@ -85,14 +84,3 @@ class IncomfortWaterHeater(IncomfortEntity, WaterHeaterEntity): return f"Fault code: {self._heater.fault_code}" return self._heater.display_text - - async def async_update(self) -> None: - """Get the latest state data from the gateway.""" - try: - await self._heater.update() - - except (ClientResponseError, TimeoutError) as err: - _LOGGER.warning("Update failed, message is: %s", err) - - else: - async_dispatcher_send(self.hass, f"{DOMAIN}_{self.unique_id}") diff --git a/tests/components/incomfort/conftest.py b/tests/components/incomfort/conftest.py index 34c38995895..8c4bc5b2e31 100644 --- a/tests/components/incomfort/conftest.py +++ b/tests/components/incomfort/conftest.py @@ -140,7 +140,7 @@ def mock_incomfort( self.rooms = [MockRoom()] with patch( - "homeassistant.components.incomfort.models.InComfortGateway", MagicMock() + "homeassistant.components.incomfort.coordinator.InComfortGateway", MagicMock() ) as patch_gateway: patch_gateway().heaters = AsyncMock() patch_gateway().heaters.return_value = [MockHeater()] diff --git a/tests/components/incomfort/test_init.py b/tests/components/incomfort/test_init.py index 7c0a8b395a8..47365a836d2 100644 --- a/tests/components/incomfort/test_init.py +++ b/tests/components/incomfort/test_init.py @@ -1,23 +1,93 @@ """Tests for Intergas InComfort integration.""" +from datetime import timedelta from unittest.mock import MagicMock, patch -from syrupy import SnapshotAssertion +from aiohttp import ClientResponseError +from freezegun.api import FrozenDateTimeFactory +from incomfortclient import IncomfortError +import pytest +from homeassistant.components.incomfort.coordinator import UPDATE_INTERVAL from homeassistant.config_entries import ConfigEntry, ConfigEntryState -from homeassistant.const import Platform +from homeassistant.const import STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er +from tests.common import async_fire_time_changed + -@patch("homeassistant.components.incomfort.PLATFORMS", [Platform.SENSOR]) async def test_setup_platforms( hass: HomeAssistant, mock_incomfort: MagicMock, entity_registry: er.EntityRegistry, - snapshot: SnapshotAssertion, mock_config_entry: ConfigEntry, ) -> None: """Test the incomfort integration is set up correctly.""" await hass.config_entries.async_setup(mock_config_entry.entry_id) assert mock_config_entry.state is ConfigEntryState.LOADED + + +async def test_coordinator_updates( + hass: HomeAssistant, + mock_incomfort: MagicMock, + freezer: FrozenDateTimeFactory, + mock_config_entry: ConfigEntry, +) -> None: + """Test the incomfort coordinator is updating.""" + await hass.config_entries.async_setup(mock_config_entry.entry_id) + state = hass.states.get("climate.thermostat_1") + assert state is not None + assert state.attributes["current_temperature"] == 21.4 + mock_incomfort().mock_room_status["room_temp"] = 20.91 + + state = hass.states.get("sensor.boiler_cv_pressure") + assert state is not None + assert state.state == "1.86" + mock_incomfort().mock_heater_status["pressure"] = 1.84 + + freezer.tick(timedelta(seconds=UPDATE_INTERVAL + 5)) + async_fire_time_changed(hass) + await hass.async_block_till_done(wait_background_tasks=True) + + state = hass.states.get("climate.thermostat_1") + assert state is not None + assert state.attributes["current_temperature"] == 20.9 + + state = hass.states.get("sensor.boiler_cv_pressure") + assert state is not None + assert state.state == "1.84" + + +@pytest.mark.parametrize( + "exc", + [ + IncomfortError(ClientResponseError(None, None, status=401)), + IncomfortError(ClientResponseError(None, None, status=500)), + IncomfortError(ValueError("some_error")), + TimeoutError, + ], +) +async def test_coordinator_update_fails( + hass: HomeAssistant, + mock_incomfort: MagicMock, + freezer: FrozenDateTimeFactory, + exc: Exception, + mock_config_entry: ConfigEntry, +) -> None: + """Test the incomfort coordinator update fails.""" + await hass.config_entries.async_setup(mock_config_entry.entry_id) + state = hass.states.get("sensor.boiler_cv_pressure") + assert state is not None + assert state.state == "1.86" + + with patch.object( + mock_incomfort().heaters.return_value[0], "update", side_effect=exc + ): + freezer.tick(timedelta(seconds=UPDATE_INTERVAL + 5)) + async_fire_time_changed(hass) + await hass.async_block_till_done(wait_background_tasks=True) + + state = hass.states.get("sensor.boiler_cv_pressure") + assert state is not None + assert state.state == STATE_UNAVAILABLE