Incomfort refactor coordinator (#160953)

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

homeassistant/components/incomfort/__init__.py

@@ -1,21 +1,12 @@
 """Support for an Intergas boiler via an InComfort/Intouch Lan2RF gateway."""
 
-from aiohttp import ClientResponseError
-from incomfortclient import InvalidGateway, InvalidHeaterList
+from incomfortclient import Gateway as InComfortGateway
 
-from homeassistant.const import Platform
-from homeassistant.core import HomeAssistant, callback
-from homeassistant.exceptions import ConfigEntryAuthFailed
-from homeassistant.helpers import device_registry as dr
+from homeassistant.const import CONF_HOST, Platform
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers.aiohttp_client import async_get_clientsession
 
-from .const import DOMAIN
-from .coordinator import (
-    InComfortConfigEntry,
-    InComfortData,
-    InComfortDataCoordinator,
-    async_connect_gateway,
-)
-from .errors import InComfortTimeout, InComfortUnknownError, NoHeaters, NotFound
+from .coordinator import InComfortConfigEntry, InComfortDataCoordinator
 
 PLATFORMS = (
     Platform.WATER_HEATER,
@@ -27,75 +18,16 @@ PLATFORMS = (
 INTEGRATION_TITLE = "Intergas InComfort/Intouch Lan2RF gateway"
 
 
-@callback
-def async_cleanup_stale_devices(
-    hass: HomeAssistant,
-    entry: InComfortConfigEntry,
-    data: InComfortData,
-    gateway_device: dr.DeviceEntry,
-) -> None:
-    """Cleanup stale heater devices and climates."""
-    heater_serial_numbers = {heater.serial_no for heater in data.heaters}
-    device_registry = dr.async_get(hass)
-    device_entries = device_registry.devices.get_devices_for_config_entry_id(
-        entry.entry_id
-    )
-    stale_heater_serial_numbers: list[str] = [
-        device_entry.serial_number
-        for device_entry in device_entries
-        if device_entry.id != gateway_device.id
-        and device_entry.serial_number is not None
-        and device_entry.serial_number not in heater_serial_numbers
-    ]
-    if not stale_heater_serial_numbers:
-        return
-    cleanup_devices: list[str] = []
-    # Find stale heater and climate devices
-    for serial_number in stale_heater_serial_numbers:
-        cleanup_list = [f"{serial_number}_{index}" for index in range(1, 4)]
-        cleanup_list.append(serial_number)
-        cleanup_identifiers = [{(DOMAIN, cleanup_id)} for cleanup_id in cleanup_list]
-        cleanup_devices.extend(
-            device_entry.id
-            for device_entry in device_entries
-            if device_entry.identifiers in cleanup_identifiers
-        )
-    for device_id in cleanup_devices:
-        device_registry.async_remove_device(device_id)
-
-
 async def async_setup_entry(hass: HomeAssistant, entry: InComfortConfigEntry) -> bool:
     """Set up a config entry."""
-    try:
-        data = await async_connect_gateway(hass, dict(entry.data))
-        for heater in data.heaters:
-            await heater.update()
-    except InvalidHeaterList as exc:
-        raise NoHeaters from exc
-    except InvalidGateway as exc:
-        raise ConfigEntryAuthFailed(
-            translation_domain=DOMAIN, translation_key="incorrect_credentials"
-        ) from exc
-    except ClientResponseError as exc:
-        if exc.status == 404:
-            raise NotFound from exc
-        raise InComfortUnknownError from exc
-    except TimeoutError as exc:
-        raise InComfortTimeout from exc
 
-    # Register discovered gateway device
-    device_registry = dr.async_get(hass)
-    gateway_device = device_registry.async_get_or_create(
-        config_entry_id=entry.entry_id,
-        identifiers={(DOMAIN, entry.entry_id)},
-        connections={(dr.CONNECTION_NETWORK_MAC, entry.unique_id)}
-        if entry.unique_id is not None
-        else set(),
-        manufacturer="Intergas",
-        name="RFGateway",
+    credentials = dict(entry.data)
+    hostname = credentials.pop(CONF_HOST)
+    client = InComfortGateway(
+        hostname, **credentials, session=async_get_clientsession(hass)
     )
-    async_cleanup_stale_devices(hass, entry, data, gateway_device)
-    coordinator = InComfortDataCoordinator(hass, entry, data)
+
+    coordinator = InComfortDataCoordinator(hass, entry, client)
     entry.runtime_data = coordinator
     await coordinator.async_config_entry_first_refresh()
 

homeassistant/components/incomfort/config_flow.py

@@ -4,7 +4,11 @@ from collections.abc import Mapping
 import logging
 from typing import Any, override
 
-from incomfortclient import InvalidGateway, InvalidHeaterList
+from incomfortclient import (
+    Gateway as InComfortGateway,
+    InvalidGateway,
+    InvalidHeaterList,
+)
 import voluptuous as vol
 
 from homeassistant.config_entries import (
@@ -17,6 +21,7 @@ from homeassistant.config_entries import (
 from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.data_entry_flow import AbortFlow
+from homeassistant.helpers.aiohttp_client import async_get_clientsession
 from homeassistant.helpers.device_registry import format_mac
 from homeassistant.helpers.selector import (
     BooleanSelector,
@@ -28,7 +33,7 @@ from homeassistant.helpers.selector import (
 from homeassistant.helpers.service_info.dhcp import DhcpServiceInfo
 
 from .const import CONF_LEGACY_SETPOINT_STATUS, DOMAIN
-from .coordinator import InComfortConfigEntry, async_connect_gateway
+from .coordinator import InComfortConfigEntry
 
 _LOGGER = logging.getLogger(__name__)
 TITLE = "Intergas InComfort/Intouch Lan2RF gateway"
@@ -81,7 +86,13 @@ async def async_try_connect_gateway(
 ) -> dict[str, str] | None:
     """Try to connect to the Lan2RF gateway."""
     try:
-        await async_connect_gateway(hass, config)
+        client = InComfortGateway(
+            hostname=config[CONF_HOST],
+            username=config.get(CONF_USERNAME),
+            password=config.get(CONF_PASSWORD),
+            session=async_get_clientsession(hass),
+        )
+        await client.heaters()
     except InvalidGateway:
         return {"base": "auth_error"}
     except InvalidHeaterList:

homeassistant/components/incomfort/coordinator.py

@@ -2,21 +2,22 @@
 
 from dataclasses import dataclass, field
 from datetime import timedelta
+from http import HTTPStatus
 import logging
-from typing import Any, override
+from typing import override
 
 from aiohttp import ClientResponseError
 from incomfortclient import (
     Gateway as InComfortGateway,
     Heater as InComfortHeater,
+    InvalidGateway,
     InvalidHeaterList,
 )
 
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.const import CONF_HOST
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
 from homeassistant.exceptions import ConfigEntryAuthFailed
-from homeassistant.helpers.aiohttp_client import async_get_clientsession
+from homeassistant.helpers import device_registry as dr
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
 from .const import DOMAIN
@@ -36,20 +37,41 @@ class InComfortData:
     heaters: list[InComfortHeater] = field(default_factory=list)
 
 
-async def async_connect_gateway(
+@callback
+def async_cleanup_stale_devices(
     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)
+    entry: InComfortConfigEntry,
+    data: InComfortData,
+    gateway_device: dr.DeviceEntry,
+) -> None:
+    """Cleanup stale heater devices and climates."""
+    heater_serial_numbers = {heater.serial_no for heater in data.heaters}
+    device_registry = dr.async_get(hass)
+    device_entries = device_registry.devices.get_devices_for_config_entry_id(
+        entry.entry_id
     )
-    heaters = await client.heaters()
-
-    return InComfortData(client=client, heaters=heaters)
+    stale_heater_serial_numbers: list[str] = [
+        device_entry.serial_number
+        for device_entry in device_entries
+        if device_entry.id != gateway_device.id
+        and device_entry.serial_number is not None
+        and device_entry.serial_number not in heater_serial_numbers
+    ]
+    if not stale_heater_serial_numbers:
+        return
+    cleanup_devices: list[str] = []
+    # Find stale heater and climate devices
+    for serial_number in stale_heater_serial_numbers:
+        cleanup_list = [f"{serial_number}_{index}" for index in range(1, 4)]
+        cleanup_list.append(serial_number)
+        cleanup_identifiers = [{(DOMAIN, cleanup_id)} for cleanup_id in cleanup_list]
+        cleanup_devices.extend(
+            device_entry.id
+            for device_entry in device_entries
+            if device_entry.identifiers in cleanup_identifiers
+        )
+    for device_id in cleanup_devices:
+        device_registry.async_remove_device(device_id)
 
 
 class InComfortDataCoordinator(DataUpdateCoordinator[InComfortData]):
@@ -61,10 +83,9 @@ class InComfortDataCoordinator(DataUpdateCoordinator[InComfortData]):
         self,
         hass: HomeAssistant,
         config_entry: InComfortConfigEntry,
-        incomfort_data: InComfortData,
+        client: InComfortGateway,
     ) -> None:
         """Initialize coordinator."""
-        self.unique_id = config_entry.unique_id
         super().__init__(
             hass,
             _LOGGER,
@@ -72,28 +93,65 @@ class InComfortDataCoordinator(DataUpdateCoordinator[InComfortData]):
             name="InComfort datacoordinator",
             update_interval=timedelta(seconds=UPDATE_INTERVAL),
         )
-        self.incomfort_data = incomfort_data
+        self.client = client
+        self.unique_id = config_entry.unique_id
 
     @override
     async def _async_update_data(self) -> InComfortData:
-        """Fetch data from API endpoint."""
+        """Fetch data from Incomfort."""
         try:
-            for heater in self.incomfort_data.heaters:
+            heaters = await self.client.heaters()
+            for heater in heaters:
                 await heater.update()
-        except ClientResponseError as exc:
-            if exc.status == 401:
-                raise ConfigEntryAuthFailed(
-                    translation_domain=DOMAIN, translation_key="incorrect_credentials"
-                ) from exc
+        except InvalidGateway as exc:
+            raise ConfigEntryAuthFailed(
+                translation_domain=DOMAIN,
+                translation_key="invalid_auth",
+            ) from exc
+        except TimeoutError as exc:
             raise UpdateFailed(
                 translation_domain=DOMAIN,
-                translation_key="update_failed_with_error_message",
-                translation_placeholders={"error": exc.message},
+                translation_key="timeout_error",
+            ) from exc
+        except ClientResponseError as exc:
+            if exc.status == HTTPStatus.UNAUTHORIZED:
+                raise ConfigEntryAuthFailed(
+                    translation_domain=DOMAIN,
+                    translation_key="invalid_auth",
+                ) from exc
+            _LOGGER.exception("Error communicating with InComfort gateway")
+            raise UpdateFailed(
+                translation_domain=DOMAIN,
+                translation_key="unknown",
             ) from exc
         except InvalidHeaterList as exc:
             raise UpdateFailed(
                 translation_domain=DOMAIN,
-                translation_key="update_failed_with_error_message",
-                translation_placeholders={"error": exc.message},
+                translation_key="no_heaters",
             ) from exc
-        return self.incomfort_data
+
+        incomfort_data = InComfortData(
+            client=self.client,
+            heaters=heaters,
+        )
+
+        # Register discovered gateway device
+        # Respect this as it is. Maybe later...
+        device_registry = dr.async_get(self.hass)
+        gateway_device = device_registry.async_get_or_create(
+            config_entry_id=self.config_entry.entry_id,
+            identifiers={(DOMAIN, self.config_entry.entry_id)},
+            connections={(dr.CONNECTION_NETWORK_MAC, self.config_entry.unique_id)}
+            if self.config_entry.unique_id is not None
+            else set(),
+            manufacturer="Intergas",
+            name="RFGateway",
+        )
+        async_cleanup_stale_devices(
+            self.hass,
+            self.config_entry,
+            incomfort_data,
+            gateway_device,
+        )
+
+        return incomfort_data

homeassistant/components/incomfort/diagnostics.py

@@ -27,15 +27,14 @@ def _async_get_diagnostics(
     redacted_config = async_redact_data(entry.data | entry.options, REDACT_CONFIG)
     coordinator = entry.runtime_data
 
-    nr_heaters = len(coordinator.incomfort_data.heaters)
+    nr_heaters = len(coordinator.data.heaters)
     status: dict[str, Any] = {
-        f"heater_{n}": coordinator.incomfort_data.heaters[n].status
-        for n in range(nr_heaters)
+        f"heater_{n}": coordinator.data.heaters[n].status for n in range(nr_heaters)
     }
     for n in range(nr_heaters):
         status[f"heater_{n}"]["rooms"] = {
-            m: dict(coordinator.incomfort_data.heaters[n].rooms[m].status)
-            for m in range(len(coordinator.incomfort_data.heaters[n].rooms))
+            m: dict(coordinator.data.heaters[n].rooms[m].status)
+            for m in range(len(coordinator.data.heaters[n].rooms))
         }
     return {
         "config": redacted_config,

homeassistant/components/incomfort/errors.py

@@ -1,33 +0,0 @@
-"""Exceptions raised by Intergas InComfort integration."""
-
-from homeassistant.exceptions import ConfigEntryNotReady, HomeAssistantError
-
-from .const import DOMAIN
-
-
-class NotFound(HomeAssistantError):
-    """Raise exception if no Lan2RF Gateway was found."""
-
-    translation_domain = DOMAIN
-    translation_key = "not_found"
-
-
-class NoHeaters(ConfigEntryNotReady):
-    """Raise exception if no heaters are found."""
-
-    translation_domain = DOMAIN
-    translation_key = "no_heaters"
-
-
-class InComfortTimeout(ConfigEntryNotReady):
-    """Raise exception if no heaters are found."""
-
-    translation_domain = DOMAIN
-    translation_key = "timeout_error"
-
-
-class InComfortUnknownError(ConfigEntryNotReady):
-    """Raise exception if no heaters are found."""
-
-    translation_domain = DOMAIN
-    translation_key = "unknown"

homeassistant/components/incomfort/strings.json

@@ -131,7 +131,9 @@
     }
   },
   "exceptions": {
-    "incorrect_credentials": { "message": "Incorrect credentials." },
+    "invalid_auth": {
+      "message": "[%key:component::incomfort::config::error::auth_error%]"
+    },
     "no_heaters": {
       "message": "[%key:component::incomfort::config::error::no_heaters%]"
     },

tests/components/incomfort/conftest.py

@@ -171,9 +171,15 @@ def mock_incomfort(
                 setattr(self, key, value)
             self.rooms = [MockRoom()]
 
-    with patch(
-        "homeassistant.components.incomfort.coordinator.InComfortGateway", MagicMock()
-    ) as patch_gateway:
+    mock_cls = MagicMock()
+    with (
+        patch(
+            "homeassistant.components.incomfort.InComfortGateway", mock_cls
+        ) as patch_gateway,
+        patch(
+            "homeassistant.components.incomfort.config_flow.InComfortGateway", mock_cls
+        ),
+    ):
         patch_gateway().heaters = AsyncMock()
         patch_gateway().heaters.return_value = [MockHeater()]
         patch_gateway().mock_heater_status = mock_heater_status

tests/components/incomfort/test_climate.py

@@ -6,7 +6,6 @@ import pytest
 from syrupy.assertion import SnapshotAssertion
 
 from homeassistant.components import climate
-from homeassistant.components.incomfort.coordinator import InComfortData
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import ATTR_ENTITY_ID, ATTR_TEMPERATURE, Platform
 from homeassistant.core import HomeAssistant
@@ -77,7 +76,7 @@ async def test_target_temp(
     state = hass.states.get("climate.thermostat_1")
     assert state is not None
 
-    incomfort_data: InComfortData = mock_config_entry.runtime_data.incomfort_data
+    incomfort_data = mock_config_entry.runtime_data.data
 
     with patch.object(
         incomfort_data.heaters[0].rooms[0], "set_override", AsyncMock()

tests/components/incomfort/test_init.py

@@ -2,14 +2,14 @@
 
 from datetime import timedelta
 from typing import Any
-from unittest.mock import AsyncMock, MagicMock, patch
+from unittest.mock import MagicMock, patch
 
 from aiohttp import ClientResponseError, RequestInfo
 from freezegun.api import FrozenDateTimeFactory
 from incomfortclient import InvalidGateway, InvalidHeaterList
 import pytest
 
-from homeassistant.components.incomfort import DOMAIN
+from homeassistant.components.incomfort.const import DOMAIN
 from homeassistant.components.incomfort.coordinator import UPDATE_INTERVAL
 from homeassistant.config_entries import ConfigEntry, ConfigEntryState
 from homeassistant.const import STATE_UNAVAILABLE
@@ -188,7 +188,7 @@ async def test_coordinator_update_fails(
                 None,
                 status=404,
             ),
-            ConfigEntryState.SETUP_ERROR,
+            ConfigEntryState.SETUP_RETRY,
         ),
         (
             ClientResponseError(
@@ -215,11 +215,8 @@ async def test_entry_setup_fails(
     config_entry_state: ConfigEntryState,
 ) -> None:
     """Test the incomfort coordinator entry setup fails."""
-    with patch(
-        "homeassistant.components.incomfort.async_connect_gateway",
-        AsyncMock(side_effect=exc),
-    ):
-        await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    mock_incomfort().heaters.side_effect = exc
+    await hass.config_entries.async_setup(mock_config_entry.entry_id)
     state = hass.states.get("sensor.boiler_pressure")
     assert state is None
     assert mock_config_entry.state is config_entry_state