From 5927b4221f10d9d62329ef67449a1e7359ee7f61 Mon Sep 17 00:00:00 2001 From: Balloob Bot Date: Wed, 2 Sep 2026 11:18:56 +0200 Subject: [PATCH] Get the stiebel_eltron Modbus unit from the modbus integration (#180200) Co-authored-by: Paulus Schoutsen Co-authored-by: Claude Opus 5 --- .../components/stiebel_eltron/__init__.py | 34 +++--- .../components/stiebel_eltron/config_flow.py | 29 +++-- .../components/stiebel_eltron/coordinator.py | 8 +- .../components/stiebel_eltron/manifest.json | 3 +- tests/components/stiebel_eltron/conftest.py | 22 ++-- .../stiebel_eltron/test_config_flow.py | 50 +++++--- tests/components/stiebel_eltron/test_init.py | 107 +++++++++++------- 7 files changed, 145 insertions(+), 108 deletions(-) diff --git a/homeassistant/components/stiebel_eltron/__init__.py b/homeassistant/components/stiebel_eltron/__init__.py index 41933a733e42..a49d03eb840d 100644 --- a/homeassistant/components/stiebel_eltron/__init__.py +++ b/homeassistant/components/stiebel_eltron/__init__.py @@ -1,19 +1,20 @@ """The component for STIEBEL ELTRON heat pumps with ISGWeb Modbus module.""" -import logging - -from modbus_connection import ModbusError -from modbus_connection.pymodbus import connect_tcp +from modbus_connection import ModbusTcpParams from pystiebeleltron import StiebelEltronModbusError, get_controller_model +from homeassistant.components.modbus import async_get_unit from homeassistant.const import CONF_HOST, CONF_PORT, Platform from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.exceptions import ( + ConfigEntryError, + ConfigEntryNotReady, + HomeAssistantError, +) from .const import DEFAULT_PORT, UNIT_ID from .coordinator import StiebelEltronConfigEntry, StiebelEltronDataCoordinator -_LOGGER = logging.getLogger(__name__) _PLATFORMS: list[Platform] = [Platform.CLIMATE] @@ -26,27 +27,24 @@ async def async_setup_entry( port = entry.data.get(CONF_PORT, DEFAULT_PORT) try: - connection = await connect_tcp(host, port=port) - except ModbusError as exception: - raise ConfigEntryNotReady("Could not connect to device") from exception - entry.async_on_unload(connection.close) + unit = async_get_unit( + hass, entry, ModbusTcpParams(host=host, port=port), UNIT_ID + ) + # Another integration already holds this host and port with link settings + # that cannot be honoured on one connection. + except HomeAssistantError as exception: + raise ConfigEntryError(str(exception)) from exception try: - model = await get_controller_model(connection.for_unit(UNIT_ID)) + model = await get_controller_model(unit) except StiebelEltronModbusError as exception: raise ConfigEntryNotReady("Could not read controller model") from exception - coordinator = StiebelEltronDataCoordinator(hass, entry, model, connection, host) + coordinator = StiebelEltronDataCoordinator(hass, entry, model, unit, host) entry.runtime_data = coordinator await coordinator.async_config_entry_first_refresh() - entry.async_on_unload( - connection.on_connection_lost( - lambda: hass.config_entries.async_schedule_reload(entry.entry_id) - ) - ) - await hass.config_entries.async_forward_entry_setups(entry, _PLATFORMS) return True diff --git a/homeassistant/components/stiebel_eltron/config_flow.py b/homeassistant/components/stiebel_eltron/config_flow.py index 0bbe59b0a5ee..4e07dc185605 100644 --- a/homeassistant/components/stiebel_eltron/config_flow.py +++ b/homeassistant/components/stiebel_eltron/config_flow.py @@ -3,13 +3,15 @@ import logging from typing import Any, override -from modbus_connection import ModbusError -from modbus_connection.pymodbus import connect_tcp +from modbus_connection import ModbusTcpParams from pystiebeleltron import StiebelEltronModbusError, get_controller_model import voluptuous as vol +from homeassistant.components.modbus import async_get_temporary_unit from homeassistant.config_entries import ConfigFlow, ConfigFlowResult from homeassistant.const import CONF_HOST, CONF_PORT +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.device_registry import format_mac from homeassistant.helpers.selector import ( NumberSelector, @@ -36,15 +38,18 @@ STEP_USER_DATA_SCHEMA = vol.Schema( ) -async def check_controller_model(host: str, port: int) -> str | None: +async def check_controller_model( + hass: HomeAssistant, host: str, port: int +) -> str | None: """Check if the controller model is valid.""" try: - connection = await connect_tcp(host, port=port) - try: - await get_controller_model(connection.for_unit(UNIT_ID)) - finally: - await connection.close() - except StiebelEltronModbusError, ModbusError: + async with async_get_temporary_unit( + hass, ModbusTcpParams(host=host, port=port), UNIT_ID + ) as unit: + await get_controller_model(unit) + # HomeAssistantError: another integration already holds this host and port + # with link settings that cannot be honoured on one connection. + except StiebelEltronModbusError, HomeAssistantError: _LOGGER.debug("Cannot connect to Stiebel Eltron device", exc_info=True) return "cannot_connect" except Exception: @@ -69,7 +74,7 @@ class StiebelEltronConfigFlow(ConfigFlow, domain=DOMAIN): self._abort_if_unique_id_configured(updates={CONF_HOST: discovery_info.ip}) self._async_abort_entries_match({CONF_HOST: discovery_info.ip}) - error = await check_controller_model(discovery_info.ip, DEFAULT_PORT) + error = await check_controller_model(self.hass, discovery_info.ip, DEFAULT_PORT) if error is not None: return self.async_abort(reason=error) @@ -104,7 +109,7 @@ class StiebelEltronConfigFlow(ConfigFlow, domain=DOMAIN): {CONF_HOST: user_input[CONF_HOST], CONF_PORT: user_input[CONF_PORT]} ) error = await check_controller_model( - user_input[CONF_HOST], user_input[CONF_PORT] + self.hass, user_input[CONF_HOST], user_input[CONF_PORT] ) if error is not None: errors["base"] = error @@ -129,7 +134,7 @@ class StiebelEltronConfigFlow(ConfigFlow, domain=DOMAIN): {CONF_HOST: user_input[CONF_HOST], CONF_PORT: user_input[CONF_PORT]} ) error = await check_controller_model( - user_input[CONF_HOST], user_input[CONF_PORT] + self.hass, user_input[CONF_HOST], user_input[CONF_PORT] ) if error is not None: errors["base"] = error diff --git a/homeassistant/components/stiebel_eltron/coordinator.py b/homeassistant/components/stiebel_eltron/coordinator.py index 8e1f6dde65b4..5331689049bf 100644 --- a/homeassistant/components/stiebel_eltron/coordinator.py +++ b/homeassistant/components/stiebel_eltron/coordinator.py @@ -4,7 +4,7 @@ from datetime import timedelta import logging from typing import override -from modbus_connection import ModbusConnection, ModbusError +from modbus_connection import ModbusError, ModbusUnit from pystiebeleltron import ControllerModel from pystiebeleltron.lwz import LwzStiebelEltronAPI @@ -13,7 +13,7 @@ from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed -from .const import ATTR_MANUFACTURER, DEFAULT_SCAN_INTERVAL, DOMAIN, UNIT_ID +from .const import ATTR_MANUFACTURER, DEFAULT_SCAN_INTERVAL, DOMAIN _LOGGER: logging.Logger = logging.getLogger(__package__) @@ -28,7 +28,7 @@ class StiebelEltronDataCoordinator(DataUpdateCoordinator[None]): hass: HomeAssistant, entry: StiebelEltronConfigEntry, model: ControllerModel, - connection: ModbusConnection, + unit: ModbusUnit, host: str, ) -> None: """Initialize the StiebelEltronDataCoordinator.""" @@ -42,7 +42,7 @@ class StiebelEltronDataCoordinator(DataUpdateCoordinator[None]): # the register values), so there is nothing to diff against. always_update=True, ) - self.api_client = LwzStiebelEltronAPI(connection.for_unit(UNIT_ID)) + self.api_client = LwzStiebelEltronAPI(unit) self.device_info = DeviceInfo( identifiers={(DOMAIN, entry.entry_id)}, configuration_url=f"http://{host}", diff --git a/homeassistant/components/stiebel_eltron/manifest.json b/homeassistant/components/stiebel_eltron/manifest.json index 4aaee654aa56..29c9fd19563b 100644 --- a/homeassistant/components/stiebel_eltron/manifest.json +++ b/homeassistant/components/stiebel_eltron/manifest.json @@ -3,6 +3,7 @@ "name": "STIEBEL ELTRON", "codeowners": ["@fucm", "@ThyMYthOS"], "config_flow": true, + "dependencies": ["modbus"], "dhcp": [ { "hostname": "servicewelt*" @@ -11,7 +12,7 @@ "documentation": "https://www.home-assistant.io/integrations/stiebel_eltron", "integration_type": "device", "iot_class": "local_polling", - "loggers": ["pymodbus", "pystiebeleltron"], + "loggers": ["pystiebeleltron"], "quality_scale": "silver", "requirements": ["pystiebeleltron==0.7.0"] } diff --git a/tests/components/stiebel_eltron/conftest.py b/tests/components/stiebel_eltron/conftest.py index dcef3caf1a6d..f58277898a2c 100644 --- a/tests/components/stiebel_eltron/conftest.py +++ b/tests/components/stiebel_eltron/conftest.py @@ -1,7 +1,7 @@ """Common fixtures for the STIEBEL ELTRON tests.""" from collections.abc import AsyncGenerator, Generator -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import MagicMock, patch from modbus_connection.mock import MockModbusConnection from pystiebeleltron import ControllerModel @@ -32,20 +32,16 @@ def mock_get_controller_model() -> Generator[MagicMock]: @pytest.fixture(autouse=True) -async def mock_connect_tcp( +async def mock_modbus_connection_class( mock_modbus_connection: MockModbusConnection, -) -> AsyncGenerator[AsyncMock]: - """Patch connect_tcp to return the in-memory mock connection.""" +) -> AsyncGenerator[MagicMock]: + """Let the modbus integration hand out units on the in-memory connection.""" await mock_modbus_connection.connect() - connect = AsyncMock(return_value=mock_modbus_connection) - with ( - patch("homeassistant.components.stiebel_eltron.connect_tcp", new=connect), - patch( - "homeassistant.components.stiebel_eltron.config_flow.connect_tcp", - new=connect, - ), - ): - yield connect + with patch( + "homeassistant.components.modbus.connection.ModbusConnection", + return_value=mock_modbus_connection, + ) as mock_connection_cls: + yield mock_connection_cls @pytest.fixture(autouse=True) diff --git a/tests/components/stiebel_eltron/test_config_flow.py b/tests/components/stiebel_eltron/test_config_flow.py index b325d90380a3..5653da2c0ff9 100644 --- a/tests/components/stiebel_eltron/test_config_flow.py +++ b/tests/components/stiebel_eltron/test_config_flow.py @@ -2,11 +2,12 @@ from unittest.mock import MagicMock -from modbus_connection import ModbusError +from modbus_connection import ModbusTcpParams from pystiebeleltron import ControllerModel, StiebelEltronModbusError import pytest -from homeassistant.components.stiebel_eltron.const import DOMAIN +from homeassistant.components.modbus import async_get_unit +from homeassistant.components.stiebel_eltron.const import DOMAIN, UNIT_ID from homeassistant.config_entries import SOURCE_DHCP, SOURCE_RECONFIGURE, SOURCE_USER from homeassistant.const import CONF_HOST, CONF_PORT from homeassistant.core import HomeAssistant @@ -42,28 +43,16 @@ async def test_full_flow(hass: HomeAssistant) -> None: assert result["data"] == USER_INPUT -@pytest.mark.parametrize( - ("failing_fixture", "side_effect"), - [ - pytest.param( - "mock_get_controller_model", StiebelEltronModbusError, id="model_read" - ), - pytest.param("mock_connect_tcp", ModbusError, id="connect"), - ], -) async def test_form_cannot_connect( hass: HomeAssistant, - request: pytest.FixtureRequest, - failing_fixture: str, - side_effect: type[Exception], + mock_get_controller_model: MagicMock, ) -> None: - """Test we handle a cannot connect error while opening or reading the device.""" - failing_mock = request.getfixturevalue(failing_fixture) + """Test we handle a cannot connect error while reading the device.""" result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": SOURCE_USER} ) - failing_mock.side_effect = side_effect + mock_get_controller_model.side_effect = StiebelEltronModbusError result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -73,7 +62,7 @@ async def test_form_cannot_connect( assert result["type"] is FlowResultType.FORM assert result["errors"] == {"base": "cannot_connect"} - failing_mock.side_effect = None + mock_get_controller_model.side_effect = None result = await hass.config_entries.flow.async_configure( result["flow_id"], @@ -83,6 +72,31 @@ async def test_form_cannot_connect( assert result["type"] is FlowResultType.CREATE_ENTRY +async def test_form_conflicting_link_settings(hass: HomeAssistant) -> None: + """Test we handle the device being held with incompatible link settings.""" + other_entry = MockConfigEntry(domain="modbus") + other_entry.add_to_hass(hass) + async_get_unit( + hass, + other_entry, + ModbusTcpParams( + host=USER_INPUT[CONF_HOST], port=USER_INPUT[CONF_PORT], framer="rtu" + ), + UNIT_ID, + ) + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER} + ) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + USER_INPUT, + ) + + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": "cannot_connect"} + + async def test_form_unknown_exception( hass: HomeAssistant, mock_get_controller_model: MagicMock, diff --git a/tests/components/stiebel_eltron/test_init.py b/tests/components/stiebel_eltron/test_init.py index 5f93cc84e25c..dec8f65e7b31 100644 --- a/tests/components/stiebel_eltron/test_init.py +++ b/tests/components/stiebel_eltron/test_init.py @@ -1,17 +1,28 @@ """Tests for the STIEBEL ELTRON integration.""" -from unittest.mock import AsyncMock, MagicMock, patch +from datetime import timedelta +from typing import Any +from unittest.mock import MagicMock, patch -from modbus_connection import ModbusError, ModbusTimeoutError +from freezegun.api import FrozenDateTimeFactory +from modbus_connection import ModbusError, ModbusTcpParams from modbus_connection.mock import MockModbusConnection from pystiebeleltron import StiebelEltronModbusError +import pytest -from homeassistant.components.stiebel_eltron.const import DOMAIN +from homeassistant.components.modbus import async_get_unit +from homeassistant.components.stiebel_eltron.const import ( + DEFAULT_SCAN_INTERVAL, + DOMAIN, + UNIT_ID, +) from homeassistant.config_entries import ConfigEntryState -from homeassistant.const import CONF_HOST, CONF_PORT +from homeassistant.const import CONF_HOST, CONF_PORT, STATE_UNAVAILABLE from homeassistant.core import HomeAssistant -from tests.common import MockConfigEntry +from tests.common import MockConfigEntry, async_fire_time_changed + +CLIMATE_ENTITY_ID = "climate.stiebel_eltron_lwz" async def test_async_setup_entry_success( @@ -26,55 +37,62 @@ async def test_async_setup_entry_success( assert mock_config_entry.state is ConfigEntryState.LOADED -async def test_async_setup_entry_with_custom_port( +@pytest.mark.parametrize( + ("entry_data", "expected_params"), + [ + pytest.param( + {CONF_HOST: "192.168.1.100", CONF_PORT: 5020}, + ModbusTcpParams(host="192.168.1.100", port=5020), + id="custom_port", + ), + pytest.param( + {CONF_HOST: "192.168.1.100"}, + ModbusTcpParams(host="192.168.1.100", port=502), + id="default_port", + ), + ], +) +async def test_async_setup_entry_requests_unit( hass: HomeAssistant, - mock_connect_tcp: AsyncMock, + mock_modbus_connection_class: MagicMock, + entry_data: dict[str, Any], + expected_params: ModbusTcpParams, ) -> None: - """Test setup with custom port.""" + """Test the unit is taken on a connection with the configured host and port.""" config_entry = MockConfigEntry( domain=DOMAIN, title="Stiebel Eltron", - data={CONF_HOST: "192.168.1.100", CONF_PORT: 5020}, + data=entry_data, ) config_entry.add_to_hass(hass) result = await hass.config_entries.async_setup(config_entry.entry_id) assert result is True - mock_connect_tcp.assert_called_once_with("192.168.1.100", port=5020) + mock_modbus_connection_class.assert_called_once_with(expected_params) -async def test_async_setup_entry_without_port( - hass: HomeAssistant, - mock_connect_tcp: AsyncMock, -) -> None: - """Test setup without port (should use default).""" - config_entry = MockConfigEntry( - domain=DOMAIN, - title="Stiebel Eltron", - data={CONF_HOST: "192.168.1.100"}, - ) - config_entry.add_to_hass(hass) - - result = await hass.config_entries.async_setup(config_entry.entry_id) - - assert result is True - mock_connect_tcp.assert_called_once_with("192.168.1.100", port=502) - - -async def test_async_setup_entry_cannot_connect( +async def test_async_setup_entry_conflicting_link_settings( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_connect_tcp: AsyncMock, ) -> None: - """Test setup retries when the connection cannot be opened.""" - mock_connect_tcp.side_effect = ModbusTimeoutError("could not connect") + """Test setup fails with a reason when the device is held over other settings.""" + other_entry = MockConfigEntry(domain="modbus") + other_entry.add_to_hass(hass) + async_get_unit( + hass, + other_entry, + ModbusTcpParams(host="1.1.1.1", port=502, framer="rtu"), + UNIT_ID, + ) mock_config_entry.add_to_hass(hass) result = await hass.config_entries.async_setup(mock_config_entry.entry_id) assert result is False - assert mock_config_entry.state is ConfigEntryState.SETUP_RETRY + assert mock_config_entry.state is ConfigEntryState.SETUP_ERROR + assert mock_config_entry.reason is not None + assert "different link settings" in mock_config_entry.reason async def test_async_setup_entry_modbus_error( @@ -82,7 +100,7 @@ async def test_async_setup_entry_modbus_error( mock_config_entry: MockConfigEntry, mock_get_controller_model: MagicMock, ) -> None: - """Test setup retries when reading the controller model fails.""" + """Test setup retries when the device cannot be reached or read.""" mock_config_entry.add_to_hass(hass) mock_get_controller_model.side_effect = StiebelEltronModbusError() @@ -109,22 +127,27 @@ async def test_async_setup_entry_coordinator_update_fails( assert mock_modbus_connection.connected is False -async def test_connection_lost_reloads_entry( +async def test_entities_unavailable_when_update_fails( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_modbus_connection: MockModbusConnection, + mock_lwz_api: MagicMock, + freezer: FrozenDateTimeFactory, ) -> None: - """Test a lost connection schedules a reload of the config entry.""" + """Test the entities go unavailable when the device stops answering.""" mock_config_entry.add_to_hass(hass) await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - with patch.object( - hass.config_entries, "async_schedule_reload" - ) as mock_schedule_reload: - mock_modbus_connection.simulate_connection_lost() + assert (state := hass.states.get(CLIMATE_ENTITY_ID)) + assert state.state != STATE_UNAVAILABLE - mock_schedule_reload.assert_called_once_with(mock_config_entry.entry_id) + mock_lwz_api.async_update.side_effect = ModbusError("update failed") + freezer.tick(timedelta(seconds=DEFAULT_SCAN_INTERVAL)) + async_fire_time_changed(hass) + await hass.async_block_till_done() + + assert (state := hass.states.get(CLIMATE_ENTITY_ID)) + assert state.state == STATE_UNAVAILABLE async def test_unload_entry_closes_connection(