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

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)
+    return 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)

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]
+    incomfort_coordinator = entry.runtime_data
-    async_add_entities(
+    heaters = incomfort_coordinator.data.heaters
-        IncomfortFailed(incomfort_data.client, h) for h in incomfort_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"

homeassistant/components/incomfort/climate.py

@@ -4,38 +4,34 @@ from __future__ import annotations
 
 from typing import Any
 
-from incomfortclient import (
+from incomfortclient import Heater as InComfortHeater, Room as InComfortRoom
-    Gateway as InComfortGateway,
-    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)
+        InComfortClimate(incomfort_coordinator, h, r) for h in heaters for r in h.rooms
-        for h in incomfort_data.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."""

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"
 

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

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

homeassistant/components/incomfort/models.py

@@ -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)

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)
+        IncomfortSensor(incomfort_coordinator, heater, description)
-        for heater in incomfort_data.heaters
+        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)}"

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 Heater as InComfortHeater
-from incomfortclient import Gateway as InComfortGateway, 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]
+    incomfort_coordinator = entry.runtime_data
-    async_add_entities(
+    heaters = incomfort_coordinator.data.heaters
-        IncomfortWaterHeater(incomfort_data.client, h) for h in incomfort_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}")

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()]

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