From 2da58e965f473c4c7ed84c3c53891055ef1b039d Mon Sep 17 00:00:00 2001 From: rhammen <75572839+rhammen@users.noreply.github.com> Date: Sat, 4 Jul 2026 18:01:48 +0200 Subject: [PATCH] =?UTF-8?q?fix(coordinator):=20=F0=9F=90=9B=20validate=20s?= =?UTF-8?q?erial=20number=20instead=20of=20masking=20missing=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove the unused catch_luxtronik_errors decorator (defined and tested, but never wired into any write path) instead of leaving dead code that looked like it protected something. - Make async_shutdown explicitly disconnect the client via a public Luxtronik.disconnect() (executor job), instead of relying on `del` + GC/__del__ timing to eventually close the socket. - Split write and identification failures out of UpdateFailed into dedicated LuxtronikWriteError / LuxtronikSerialNumberError exceptions, since UpdateFailed is meant to signal a failed update_method, not an arbitrary write or a missing serial number. - get_sensor() now tolerates coordinator.data being unpopulated instead of raising AttributeError; serial_number raises LuxtronikSerialNumberError instead of silently falling back to "" - since it feeds unique_id, an empty fallback risks colliding device/entry identifiers. - connect_and_get_coordinator() now checks last_update_success after the config-flow path's async_refresh() (which swallows failures, unlike async_config_entry_first_refresh()), converting a silent failed refresh into the existing, well-handled LuxtronikConnectionError. - Propagate LuxtronikSerialNumberError through config_flow with a dedicated "cannot_identify" abort reason/form error (all 5 locales), instead of surfacing as a generic "unknown" error. - Add regression test coverage for the above, plus the previously untested `except AbortFlow: raise` guard in async_step_dhcp and the reload_on_update=False safeguard that keeps DHCP rediscovery from re-triggering HA's "has an update listener" reload deprecation warning. Co-Authored-By: Claude Sonnet 5 --- custom_components/luxtronik2/config_flow.py | 60 ++++++--- custom_components/luxtronik2/coordinator.py | 71 ++++++----- custom_components/luxtronik2/lux_helper.py | 4 + .../luxtronik2/translations/cs.json | 2 + .../luxtronik2/translations/de.json | 2 + .../luxtronik2/translations/en.json | 2 + .../luxtronik2/translations/nl.json | 2 + .../luxtronik2/translations/pl.json | 2 + tests/test_config_flow.py | 114 +++++++++++++++++- tests/test_coordinator.py | 67 +++++----- tests/test_lux_helper.py | 14 +++ 11 files changed, 261 insertions(+), 79 deletions(-) diff --git a/custom_components/luxtronik2/config_flow.py b/custom_components/luxtronik2/config_flow.py index 5a18017e..9846b1fc 100644 --- a/custom_components/luxtronik2/config_flow.py +++ b/custom_components/luxtronik2/config_flow.py @@ -32,6 +32,7 @@ from .coordinator import ( LuxtronikConnectionError, LuxtronikCoordinator, + LuxtronikSerialNumberError, connect_and_get_coordinator, ) from .lux_helper import discover @@ -75,16 +76,27 @@ async def _discover_devices(self) -> list[tuple[str, int | None]]: async def _set_unique_id_or_abort( self, coordinator: LuxtronikCoordinator, config: dict[str, Any] - ) -> bool: - """Set unique ID and abort if already configured.""" + ) -> ConfigFlowResult | None: + """Set unique ID, returning an abort result if the flow should stop. + + Returns None if the flow should proceed to create/update the entry. + """ try: await self.async_set_unique_id(coordinator.unique_id) self._abort_if_unique_id_configured() - - return True + return None except AbortFlow: LOGGER.debug("Device already configured: %s", config[CONF_HOST]) - return False + return self.async_abort(reason="already_configured") + except LuxtronikSerialNumberError as err: + LOGGER.error("Could not identify device at %s: %s", config[CONF_HOST], err) + return self.async_abort( + reason="cannot_identify", + description_placeholders={ + "host": config[CONF_HOST], + "error": str(err), + }, + ) def _create_entry( self, config: dict[str, Any], coordinator: LuxtronikCoordinator @@ -206,8 +218,8 @@ async def async_step_select_devices( }, ) - if not await self._set_unique_id_or_abort(coordinator, config): - return self.async_abort(reason="already_configured") + if abort_result := await self._set_unique_id_or_abort(coordinator, config): + return abort_result return self._create_entry(config, coordinator) @@ -239,10 +251,10 @@ async def async_step_manual_entry( }, ) - if not await self._set_unique_id_or_abort( + if abort_result := await self._set_unique_id_or_abort( # pragma: no cover coordinator, config - ): # pragma: no cover - return self.async_abort(reason="already_configured") + ): + return abort_result return self._create_entry(config, coordinator) @@ -360,7 +372,16 @@ async def async_step_dhcp( }, ) - await self.async_set_unique_id(coordinator.unique_id) + try: + await self.async_set_unique_id(coordinator.unique_id) + except LuxtronikSerialNumberError as err: + LOGGER.error( + "Could not identify DHCP-discovered device at %s: %s", host, err + ) + return self.async_abort( + reason="cannot_identify", + description_placeholders={"host": host, "error": str(err)}, + ) self._abort_if_unique_id_configured( updates={CONF_HOST: host, CONF_PORT: int(port or DEFAULT_PORT)}, # Our own update listener (see __init__.py) already reloads the @@ -425,12 +446,12 @@ async def async_step_reconfigure( LOGGER.exception("Unexpected exception during reconfigure connect") errors["base"] = "unknown" else: - LOGGER.debug( - "Reconfigure connected and refreshed successfully; entry unique_id=%s, coordinator unique_id=%s", - reconfigure_entry.unique_id, - coordinator.unique_id, - ) try: + LOGGER.debug( + "Reconfigure connected and refreshed successfully; entry unique_id=%s, coordinator unique_id=%s", + reconfigure_entry.unique_id, + coordinator.unique_id, + ) await self.async_set_unique_id(coordinator.unique_id) if reconfigure_entry.unique_id is None: LOGGER.debug( @@ -454,6 +475,13 @@ async def async_step_reconfigure( err, ) raise + except LuxtronikSerialNumberError as err: + LOGGER.error( + "Reconfigure could not identify device at %s: %s", + config[CONF_HOST], + err, + ) + errors["base"] = "cannot_identify" except Exception: # pylint: disable=broad-except LOGGER.exception("Unexpected exception during reconfigure update") errors["base"] = "unknown" diff --git a/custom_components/luxtronik2/coordinator.py b/custom_components/luxtronik2/coordinator.py index d2b78caf..379e0242 100644 --- a/custom_components/luxtronik2/coordinator.py +++ b/custom_components/luxtronik2/coordinator.py @@ -4,13 +4,12 @@ from __future__ import annotations import asyncio -from collections.abc import Awaitable, Callable, Coroutine, Mapping +from collections.abc import Callable, Mapping from datetime import timedelta -from functools import wraps import operator import re from types import MappingProxyType -from typing import Any, Concatenate, Final, ParamSpec, TypeVar +from typing import Any, Final from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TIMEOUT @@ -53,30 +52,6 @@ # endregion Imports -_LuxtronikCoordinatorT = TypeVar("_LuxtronikCoordinatorT", bound="LuxtronikCoordinator") -_P = ParamSpec("_P") - - -def catch_luxtronik_errors( - func: Callable[Concatenate[_LuxtronikCoordinatorT, _P], Awaitable[None]], -) -> Callable[Concatenate[_LuxtronikCoordinatorT, _P], Coroutine[Any, Any, None]]: - """Catch Luxtronik errors.""" - - @wraps(func) - async def wrapper( - self: _LuxtronikCoordinatorT, - *args: _P.args, - **kwargs: _P.kwargs, - ) -> None: - """Catch Luxtronik errors and log message.""" - try: - await func(self, *args, **kwargs) - except Exception as err: # pylint: disable=broad-except - LOGGER.error("Command error: %s", err) - await self.async_request_refresh() - - return wrapper - class LuxtronikCoordinator(DataUpdateCoordinator[LuxtronikCoordinatorData]): """Representation of a Luxtronik Coordinator.""" @@ -160,7 +135,7 @@ async def async_write(self, parameter: str, value: Any) -> LuxtronikCoordinatorD return self.data except Exception as err: - raise UpdateFailed(f"Write error: {err}") from err + raise LuxtronikWriteError(f"Write error: {err}") from err @staticmethod async def connect( # pragma: no cover @@ -318,8 +293,21 @@ def _is_version_not_compatible( @property def serial_number(self) -> str: - """Return the serial number.""" + """Return the serial number. + + Raises: + LuxtronikSerialNumberError: if the serial number date (P0874) is + unavailable. This feeds `unique_id`, so it must be treated as + a hard error rather than silently identifying the device + with an empty/placeholder value. + + """ serial_number_date = self.get_value(LP.P0874_SERIAL_NUMBER) + if serial_number_date is None: + raise LuxtronikSerialNumberError( + "Serial number (P0874) is not available - coordinator data " + "may not be populated yet" + ) serial_number_model = self.get_value(LP.P0875_SERIAL_NUMBER_MODEL) serial_number_hex = ( hex(int(serial_number_model)) if serial_number_model is not None else "0" @@ -558,6 +546,8 @@ def get_sensor_by_id(self, group_sensor_id: str): def get_sensor(self, group: str, sensor_id: str): """Get sensor by configured sensor ID from coordinator data.""" + if self.data is None: + return None if group == CONF_PARAMETERS: return self.data.parameters.get(sensor_id) if group == CONF_CALCULATIONS: @@ -618,6 +608,7 @@ async def async_shutdown(self) -> None: """Make sure a coordinator is shut down as well as its connection.""" await super().async_shutdown() if hasattr(self, "client") and self.client is not None: + await self.hass.async_add_executor_job(self.client.disconnect) del self.client @@ -633,6 +624,19 @@ def __init__(self, host: str, port: int, original: Exception): self.original = original +class LuxtronikWriteError(HomeAssistantError): + """Raised when writing a parameter to the Luxtronik device fails.""" + + +class LuxtronikSerialNumberError(HomeAssistantError): + """Raised when the heatpump's serial number cannot be determined. + + The serial number feeds `unique_id`, so treating it as "not available + yet" and falling back to an empty value would risk two different + devices ending up with colliding (empty) device/entry identifiers. + """ + + _OVERRIDES_APPLIED = False @@ -674,6 +678,15 @@ async def connect_and_get_coordinator( ) else: await coordinator.async_refresh() + if not coordinator.last_update_success: + # async_refresh() (unlike async_config_entry_first_refresh()) + # swallows update failures instead of raising. Without this + # check we'd hand back a coordinator with no data, and the + # failure would only surface later as a confusing error when + # something (e.g. unique_id) tries to read a sensor value. + raise RuntimeError( + f"Initial data refresh did not succeed for {host}:{port}" + ) LOGGER.debug( "Initial coordinator refresh completed for %s:%s via direct config", host, diff --git a/custom_components/luxtronik2/lux_helper.py b/custom_components/luxtronik2/lux_helper.py index d44e9497..721613f9 100644 --- a/custom_components/luxtronik2/lux_helper.py +++ b/custom_components/luxtronik2/lux_helper.py @@ -228,6 +228,10 @@ def __del__(self): """Luxtronik helper descructor.""" self._disconnect() + def disconnect(self) -> None: + """Explicitly close the connection to the heatpump.""" + self._disconnect() + def _disconnect(self): if self._socket is not None: if not _is_socket_closed(self._socket): diff --git a/custom_components/luxtronik2/translations/cs.json b/custom_components/luxtronik2/translations/cs.json index a9406615..5dea8eba 100644 --- a/custom_components/luxtronik2/translations/cs.json +++ b/custom_components/luxtronik2/translations/cs.json @@ -1012,6 +1012,7 @@ "config": { "abort": { "cannot_connect": "Nepoda\u0159ilo se p\u0159ipojit k Luxtronik na adrese {host}.\\nChyba: {connect_error}", + "cannot_identify": "P\u0159ipojeno k za\u0159\u00edzen\u00ed na adrese {host}, ale nepoda\u0159ilo se p\u0159e\u010d\u00edst jeho s\u00e9riov\u00e9 \u010d\u00edslo pro identifikaci.\\nChyba: {error}", "already_configured": "Toto za\u0159\u00edzen\u00ed je ji\u017e nakonfigurov\u00e1no.", "reconfigure_successful": "Konfigurace \u00fasp\u011b\u0161n\u011b aktualizov\u00e1na.", "unique_id_mismatch": "Za\u0159\u00edzen\u00ed na nov\u00e9 adrese m\u00e1 jinou identitu ne\u017e aktu\u00e1ln\u011b nakonfigurovan\u00e9 za\u0159\u00edzen\u00ed.", @@ -1026,6 +1027,7 @@ "address": "Neplatn\u00e1 vzd\u00e1len\u00e1 adresa. Adresa mus\u00ed b\u00fdt IP adresa nebo rozpoznateln\u00fd n\u00e1zev hostitele.", "address_in_use": "Zvolen\u00fd port je ji\u017e v tomto syst\u00e9mu pou\u017e\u00edv\u00e1n.", "cannot_connect": "P\u0159ipojen\u00ed se nezda\u0159ilo. Zkontrolujte pros\u00edm host a port.", + "cannot_identify": "P\u0159ipojeno k za\u0159\u00edzen\u00ed, ale nepoda\u0159ilo se p\u0159e\u010d\u00edst jeho s\u00e9riov\u00e9 \u010d\u00edslo pro identifikaci.", "unknown": "Neo\u010dek\u00e1van\u00e1 chyba" }, "step": { diff --git a/custom_components/luxtronik2/translations/de.json b/custom_components/luxtronik2/translations/de.json index 87ff67e4..67f1e0eb 100644 --- a/custom_components/luxtronik2/translations/de.json +++ b/custom_components/luxtronik2/translations/de.json @@ -1007,6 +1007,7 @@ "config": { "abort": { "cannot_connect": "Verbindung zu Luxtronik unter {host} fehlgeschlagen.\\nFehler: {connect_error}", + "cannot_identify": "Verbindung zum Ger\u00e4t unter {host} hergestellt, aber die Seriennummer zur Identifizierung konnte nicht gelesen werden.\\nFehler: {error}", "already_configured": "Dieses Ger\u00e4t ist bereits konfiguriert.", "reconfigure_successful": "Konfiguration erfolgreich aktualisiert.", "unique_id_mismatch": "Das Ger\u00e4t an der neuen Adresse hat eine andere Identit\u00e4t als das aktuell konfigurierte Ger\u00e4t.", @@ -1021,6 +1022,7 @@ "address": "Ung\u00fcltige Remote-Adresse. Die Adresse muss eine IP-Adresse oder ein aufl\u00f6sbarer Hostname sein.", "address_in_use": "Der gew\u00e4hlte Port wird bereits auf diesem System verwendet.", "cannot_connect": "Verbindung fehlgeschlagen. Bitte Host und Port \u00fcberpr\u00fcfen.", + "cannot_identify": "Verbindung zum Ger\u00e4t hergestellt, aber die Seriennummer zur Identifizierung konnte nicht gelesen werden.", "unknown": "Unerwarteter Fehler" }, "step": { diff --git a/custom_components/luxtronik2/translations/en.json b/custom_components/luxtronik2/translations/en.json index e3421ffa..a8f42180 100644 --- a/custom_components/luxtronik2/translations/en.json +++ b/custom_components/luxtronik2/translations/en.json @@ -742,6 +742,7 @@ "config": { "abort": { "cannot_connect": "Failed to connect to Luxtronik at {host}.\\nError: {connect_error}", + "cannot_identify": "Connected to the device at {host}, but could not read its serial number to identify it.\\nError: {error}", "already_configured": "This device is already configured.", "reconfigure_successful": "Configuration updated successfully.", "unique_id_mismatch": "The device at the new address has a different identity than the currently configured device.", @@ -756,6 +757,7 @@ "address": "Invalid remote-address insert. The address has to be an IP address or a resolvable hostname.", "address_in_use": "The chosen listen port is already in use on this system.", "cannot_connect": "Failed to connect. Please check the host and port.", + "cannot_identify": "Connected to the device, but could not read its serial number to identify it.", "unknown": "Unexpected error" }, "step": { diff --git a/custom_components/luxtronik2/translations/nl.json b/custom_components/luxtronik2/translations/nl.json index 1e78bb43..836ff0de 100644 --- a/custom_components/luxtronik2/translations/nl.json +++ b/custom_components/luxtronik2/translations/nl.json @@ -997,6 +997,7 @@ "config": { "abort": { "cannot_connect": "De verbinding met Luxtronik op {host} is mislukt.\nFout: {connect_error}", + "cannot_identify": "Verbonden met het apparaat op {host}, maar het serienummer kon niet worden gelezen om het te identificeren.\nFout: {error}", "already_configured": "Dit apparaat is al geconfigureerd.", "reconfigure_successful": "Configuratie succesvol bijgewerkt.", "unique_id_mismatch": "Het apparaat op het nieuwe adres heeft een andere identiteit dan het momenteel geconfigureerde apparaat.", @@ -1011,6 +1012,7 @@ "address": "Ongeldig extern adres. Het adres moet een IP-adres of een oplosbare hostnaam zijn.", "address_in_use": "De gekozen poort is al in gebruik op dit systeem.", "cannot_connect": "Verbinding mislukt. Controleer de host en poort.", + "cannot_identify": "Verbonden met het apparaat, maar het serienummer kon niet worden gelezen om het te identificeren.", "unknown": "Onverwachte fout" }, "step": { diff --git a/custom_components/luxtronik2/translations/pl.json b/custom_components/luxtronik2/translations/pl.json index e1eb9cdd..1c3304a6 100644 --- a/custom_components/luxtronik2/translations/pl.json +++ b/custom_components/luxtronik2/translations/pl.json @@ -1025,6 +1025,7 @@ "config": { "abort": { "cannot_connect": "Nie uda\u0142o si\u0119 po\u0142\u0105czy\u0107 z Luxtronik pod adresem {host}.\\nB\u0142\u0105d: {connect_error}", + "cannot_identify": "Po\u0142\u0105czono z urz\u0105dzeniem pod adresem {host}, ale nie uda\u0142o si\u0119 odczyta\u0107 jego numeru seryjnego w celu identyfikacji.\\nB\u0142\u0105d: {error}", "already_configured": "To urz\u0105dzenie jest ju\u017c skonfigurowane.", "reconfigure_successful": "Konfiguracja zosta\u0142a pomy\u015blnie zaktualizowana.", "unique_id_mismatch": "Urz\u0105dzenie pod nowym adresem ma inn\u0105 to\u017csamo\u015b\u0107 ni\u017c aktualnie skonfigurowane urz\u0105dzenie.", @@ -1039,6 +1040,7 @@ "address": "Nieprawid\u0142owy adres zdalny. Adres musi by\u0107 adresem IP lub rozpoznawaln\u0105 nazw\u0105 hosta.", "address_in_use": "Wybrany port jest ju\u017c u\u017cywany w tym systemie.", "cannot_connect": "Po\u0142\u0105czenie nie powiod\u0142o si\u0119. Sprawd\u017a host i port.", + "cannot_identify": "Po\u0142\u0105czono z urz\u0105dzeniem, ale nie uda\u0142o si\u0119 odczyta\u0107 jego numeru seryjnego w celu identyfikacji.", "unknown": "Nieoczekiwany b\u0142\u0105d" }, "step": { diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 800946ad..5e9c2674 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -3,7 +3,7 @@ from __future__ import annotations from typing import Any -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TIMEOUT from homeassistant.data_entry_flow import AbortFlow @@ -23,7 +23,10 @@ DEFAULT_TIMEOUT, DOMAIN, ) -from custom_components.luxtronik2.coordinator import LuxtronikConnectionError +from custom_components.luxtronik2.coordinator import ( + LuxtronikConnectionError, + LuxtronikSerialNumberError, +) def _mock_coordinator(): @@ -114,11 +117,11 @@ async def test_sets_unique_id(self): flow._abort_if_unique_id_configured = MagicMock() coord = _mock_coordinator() result = await flow._set_unique_id_or_abort(coord, {CONF_HOST: "1.2.3.4"}) - assert result is True + assert result is None flow.async_set_unique_id.assert_awaited_once_with(coord.unique_id) @pytest.mark.asyncio - async def test_returns_false_on_abort(self): + async def test_returns_abort_result_on_already_configured(self): flow = LuxtronikFlowHandler() flow.async_set_unique_id = AsyncMock() flow._abort_if_unique_id_configured = MagicMock( @@ -126,7 +129,20 @@ async def test_returns_false_on_abort(self): ) coord = _mock_coordinator() result = await flow._set_unique_id_or_abort(coord, {CONF_HOST: "1.2.3.4"}) - assert result is False + assert result is not None + assert result["reason"] == "already_configured" + + @pytest.mark.asyncio + async def test_returns_abort_result_on_serial_number_error(self): + flow = LuxtronikFlowHandler() + flow.async_set_unique_id = AsyncMock() + coord = MagicMock() + type(coord).unique_id = PropertyMock( + side_effect=LuxtronikSerialNumberError("no serial number") + ) + result = await flow._set_unique_id_or_abort(coord, {CONF_HOST: "1.2.3.4"}) + assert result is not None + assert result["reason"] == "cannot_identify" # =========================================================================== @@ -349,6 +365,41 @@ async def test_dhcp_discovery_creates_entry(self): ): await flow.async_step_dhcp(self._make_dhcp_info()) flow.async_create_entry.assert_called_once() + # Regression guard: our own update listener (__init__.py) already + # reloads the entry on data changes. If this ever goes back to the + # default (reload_on_update=True), core schedules a second reload and + # logs HA's "has an update listener and should use it for scheduling + # a reload" deprecation warning (removed in 2026.12.0). + assert ( + flow._abort_if_unique_id_configured.call_args.kwargs["reload_on_update"] + is False + ) + + @pytest.mark.asyncio + async def test_dhcp_already_configured_after_connect_reraises_abort(self): + """AbortFlow raised by _abort_if_unique_id_configured after a successful + connect must propagate (via the `except AbortFlow: raise` guard added in + c8f0795) instead of being swallowed by the catch-all handler and + reported as an "unknown" error. + """ + flow = LuxtronikFlowHandler() + flow.hass = MagicMock() + flow.hass.async_add_executor_job = AsyncMock(return_value=[("1.2.3.4", 8889)]) + flow._async_current_entries = MagicMock(return_value=[]) + flow.async_set_unique_id = AsyncMock() + flow._abort_if_unique_id_configured = MagicMock( + side_effect=AbortFlow("already_configured") + ) + coord = _mock_coordinator() + with ( + patch( + "custom_components.luxtronik2.config_flow.connect_and_get_coordinator", + new_callable=AsyncMock, + return_value=coord, + ), + pytest.raises(AbortFlow, match="already_configured"), + ): + await flow.async_step_dhcp(self._make_dhcp_info()) @pytest.mark.asyncio async def test_dhcp_no_match_uses_default_port(self): @@ -385,6 +436,31 @@ async def test_dhcp_connection_error_aborts(self): await flow.async_step_dhcp(self._make_dhcp_info()) flow.async_abort.assert_called_once() + @pytest.mark.asyncio + async def test_dhcp_cannot_identify_aborts(self): + flow = LuxtronikFlowHandler() + flow.hass = MagicMock() + flow.hass.async_add_executor_job = AsyncMock(return_value=[("1.2.3.4", 8889)]) + flow._async_current_entries = MagicMock(return_value=[]) + flow.async_set_unique_id = AsyncMock( + side_effect=LuxtronikSerialNumberError("no serial number") + ) + flow.async_abort = MagicMock(return_value={"type": "abort"}) + coord = _mock_coordinator() + with patch( + "custom_components.luxtronik2.config_flow.connect_and_get_coordinator", + new_callable=AsyncMock, + return_value=coord, + ): + await flow.async_step_dhcp(self._make_dhcp_info()) + flow.async_abort.assert_called_with( + reason="cannot_identify", + description_placeholders={ + "host": "1.2.3.4", + "error": "no serial number", + }, + ) + @pytest.mark.asyncio async def test_dhcp_unknown_error_aborts(self): flow = LuxtronikFlowHandler() @@ -663,6 +739,34 @@ async def test_update_exception_shows_unknown_error(self): flow.async_show_form.assert_called_once() assert flow.async_show_form.call_args[1]["errors"] == {"base": "unknown"} + @pytest.mark.asyncio + async def test_serial_number_error_shows_cannot_identify_error(self): + flow = LuxtronikFlowHandler() + flow.hass = MagicMock() + entry = MagicMock() + entry.data = { + CONF_HOST: "1.2.3.4", + CONF_PORT: 8889, + CONF_TIMEOUT: DEFAULT_TIMEOUT, + CONF_MAX_DATA_LENGTH: DEFAULT_MAX_DATA_LENGTH, + } + flow._get_reconfigure_entry = MagicMock(return_value=entry) + flow.async_show_form = MagicMock(return_value={"type": "form"}) + flow.async_set_unique_id = AsyncMock( + side_effect=LuxtronikSerialNumberError("no serial number") + ) + coord = _mock_coordinator() + with patch( + "custom_components.luxtronik2.config_flow.connect_and_get_coordinator", + new_callable=AsyncMock, + return_value=coord, + ): + await flow.async_step_reconfigure({CONF_HOST: "5.6.7.8", CONF_PORT: 8889}) + flow.async_show_form.assert_called_once() + assert flow.async_show_form.call_args[1]["errors"] == { + "base": "cannot_identify" + } + @pytest.mark.asyncio async def test_successful_reconfigure(self): flow = LuxtronikFlowHandler() diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 8bd123ef..bd903557 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -27,7 +27,8 @@ from custom_components.luxtronik2.coordinator import ( LuxtronikConnectionError, LuxtronikCoordinator, - catch_luxtronik_errors, + LuxtronikSerialNumberError, + LuxtronikWriteError, ) from custom_components.luxtronik2.model import ( LuxtronikCoordinatorData, @@ -224,6 +225,11 @@ def test_serial_number(self): assert "20230101" in sn assert "ff" in sn.lower() # hex(255) = 0xff + def test_serial_number_missing_date_raises(self): + coord = _make_coordinator() + with pytest.raises(LuxtronikSerialNumberError): + _ = coord.serial_number + def test_room_thermostat_type(self): coord = _make_coordinator(parameters={"ID_Einst_RFVEinb_akt": 4}) thermostat_type = coord.room_thermostat_type @@ -665,6 +671,11 @@ def test_get_sensor_unknown_group(self): coord = _make_coordinator() assert coord.get_sensor("unknown_group", "some_key") is None + def test_get_sensor_no_data_yet(self): + coord = _make_coordinator() + coord.data = None + assert coord.get_sensor("parameters", "some_key") is None + # =========================================================================== # async operations @@ -726,33 +737,6 @@ async def test_async_shutdown(self): assert not hasattr(coord, "client") or coord.client is None -# =========================================================================== -# catch_luxtronik_errors decorator -# =========================================================================== - - -class TestCatchLuxtronikErrors: - @pytest.mark.asyncio - async def test_catches_exception_and_refreshes(self): - @catch_luxtronik_errors - async def failing_method(self): - raise ValueError("test error") - - coord = _make_coordinator_direct() - await failing_method(coord) - coord.async_request_refresh.assert_awaited_once() - - @pytest.mark.asyncio - async def test_calls_refresh_on_success(self): - @catch_luxtronik_errors - async def success_method(self): - pass - - coord = _make_coordinator_direct() - await success_method(coord) - coord.async_request_refresh.assert_awaited_once() - - # =========================================================================== # _async_update_data (direct coordinator) # =========================================================================== @@ -808,7 +792,7 @@ async def test_write_error(self): coord.hass.async_add_executor_job = AsyncMock( side_effect=Exception("write fail") ) - with pytest.raises(UpdateFailed): + with pytest.raises(LuxtronikWriteError): await coord.async_write("param", 1) @@ -821,12 +805,17 @@ class TestAsyncShutdownDirect: @pytest.mark.asyncio async def test_shutdown_with_client(self): coord = _make_coordinator_direct() + coord.hass.async_add_executor_job = AsyncMock( + side_effect=lambda fn, *a, **kw: fn(*a, **kw) + ) + client = coord.client with patch( "homeassistant.helpers.update_coordinator.DataUpdateCoordinator.async_shutdown", new_callable=AsyncMock, ): await coord.async_shutdown() assert not hasattr(coord, "client") + client.disconnect.assert_called_once() @pytest.mark.asyncio async def test_shutdown_without_client(self): @@ -1146,6 +1135,26 @@ async def test_initial_refresh_uses_async_refresh_for_dict_config(self): coordinator.async_refresh.assert_awaited_once() coordinator.async_config_entry_first_refresh.assert_not_awaited() + @pytest.mark.asyncio + async def test_initial_refresh_failure_raises_connection_error(self): + """async_refresh() swallows failures; connect_and_get_coordinator must not.""" + from custom_components.luxtronik2.coordinator import connect_and_get_coordinator + + config = {CONF_HOST: "192.168.1.100", CONF_PORT: DEFAULT_PORT} + coordinator = MagicMock() + coordinator.async_refresh = AsyncMock() + coordinator.last_update_success = False + + with ( + patch( + "custom_components.luxtronik2.coordinator.LuxtronikCoordinator.connect", + new_callable=AsyncMock, + return_value=coordinator, + ), + pytest.raises(LuxtronikConnectionError), + ): + await connect_and_get_coordinator(MagicMock(), config) + @pytest.mark.asyncio async def test_initial_refresh_uses_config_entry_first_refresh_for_config_entry( self, diff --git a/tests/test_lux_helper.py b/tests/test_lux_helper.py index 83891057..a6d96084 100644 --- a/tests/test_lux_helper.py +++ b/tests/test_lux_helper.py @@ -166,6 +166,20 @@ def test_disconnect_when_no_socket(self): client = Luxtronik("192.168.1.100", DEFAULT_PORT, 10.0, DEFAULT_MAX_DATA_LENGTH) client._disconnect() # should not raise + @patch( + "custom_components.luxtronik2.lux_helper._is_socket_closed", return_value=False + ) + def test_public_disconnect_closes_socket(self, mock_is_closed): + client = Luxtronik("192.168.1.100", DEFAULT_PORT, 10.0, DEFAULT_MAX_DATA_LENGTH) + mock_sock = MagicMock() + mock_sock.fileno.return_value = -1 + client._socket = mock_sock + + client.disconnect() + + mock_sock.close.assert_called_once() + assert client._socket is None + @patch( "custom_components.luxtronik2.lux_helper._is_socket_closed", return_value=False )