From 7cb08fdabc7ece2ccb6f061b93b0e5ddb7fa7736 Mon Sep 17 00:00:00 2001 From: Erwin Douna Date: Wed, 3 Jun 2026 23:31:18 +0200 Subject: [PATCH] Incomfort refactor coordinator (#160953) Co-authored-by: Joost Lekkerkerker --- .../components/incomfort/__init__.py | 90 ++----------- .../components/incomfort/config_flow.py | 17 ++- .../components/incomfort/coordinator.py | 120 +++++++++++++----- .../components/incomfort/diagnostics.py | 9 +- homeassistant/components/incomfort/errors.py | 33 ----- .../components/incomfort/strings.json | 4 +- tests/components/incomfort/conftest.py | 12 +- tests/components/incomfort/test_climate.py | 3 +- tests/components/incomfort/test_init.py | 13 +- 9 files changed, 136 insertions(+), 165 deletions(-) delete mode 100644 homeassistant/components/incomfort/errors.py diff --git a/homeassistant/components/incomfort/__init__.py b/homeassistant/components/incomfort/__init__.py index d6375024653e..7871483f73cb 100644 --- a/homeassistant/components/incomfort/__init__.py +++ b/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() diff --git a/homeassistant/components/incomfort/config_flow.py b/homeassistant/components/incomfort/config_flow.py index a660c8b8e0a5..0c39db2b9236 100644 --- a/homeassistant/components/incomfort/config_flow.py +++ b/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: diff --git a/homeassistant/components/incomfort/coordinator.py b/homeassistant/components/incomfort/coordinator.py index 52598bc27b5f..12f1255bb051 100644 --- a/homeassistant/components/incomfort/coordinator.py +++ b/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 diff --git a/homeassistant/components/incomfort/diagnostics.py b/homeassistant/components/incomfort/diagnostics.py index 29ba123bf3fb..a6115b29e324 100644 --- a/homeassistant/components/incomfort/diagnostics.py +++ b/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, diff --git a/homeassistant/components/incomfort/errors.py b/homeassistant/components/incomfort/errors.py deleted file mode 100644 index c367916d6c77..000000000000 --- a/homeassistant/components/incomfort/errors.py +++ /dev/null @@ -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" diff --git a/homeassistant/components/incomfort/strings.json b/homeassistant/components/incomfort/strings.json index 27b0c3feeabe..99aa18e5d3aa 100644 --- a/homeassistant/components/incomfort/strings.json +++ b/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%]" }, diff --git a/tests/components/incomfort/conftest.py b/tests/components/incomfort/conftest.py index f557e086fcf2..6afd89644a5e 100644 --- a/tests/components/incomfort/conftest.py +++ b/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 diff --git a/tests/components/incomfort/test_climate.py b/tests/components/incomfort/test_climate.py index a4c97d88e348..43d997daa250 100644 --- a/tests/components/incomfort/test_climate.py +++ b/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() diff --git a/tests/components/incomfort/test_init.py b/tests/components/incomfort/test_init.py index 92ce0afa4486..3fdd0c891a5c 100644 --- a/tests/components/incomfort/test_init.py +++ b/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