diff --git a/custom_components/sws12500/__init__.py b/custom_components/sws12500/__init__.py index 6925cbb..7cdc35e 100644 --- a/custom_components/sws12500/__init__.py +++ b/custom_components/sws12500/__init__.py @@ -66,6 +66,14 @@ PLATFORMS: list[Platform] = [Platform.SENSOR, Platform.BINARY_SENSOR] CONFIG_ENTRY_VERSION: int = 2 +def _shared_routes(hass: HomeAssistant) -> Routes | None: + """Return the route dispatcher shared across entry reloads, if it exists yet.""" + + domain_data = hass.data.get(DOMAIN) + routes = domain_data.get("routes") if isinstance(domain_data, dict) else None + return routes if isinstance(routes, Routes) else None + + async def async_migrate_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool: """Migrate an old config entry. @@ -277,10 +285,25 @@ async def async_unload_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS) if unload_ok: - domain_data = hass.data.get(DOMAIN) - routes = domain_data.get("routes") if isinstance(domain_data, dict) else None - if isinstance(routes, Routes): + if (routes := _shared_routes(hass)) is not None: routes.deactivate() setattr(entry, "runtime_data", None) return unload_ok + + +async def async_remove_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> None: + """Release what the shared dispatcher keeps across an unload. + + Unload is deliberately not the end of the line - the dispatcher keeps the ingress + observer so a payload that lands mid-reload is still recorded, and the next setup + repoints it at the new coordinator. Removal has no next setup, so the references + have to go here or the deleted entry's coordinators stay reachable from + `hass.data[DOMAIN]["routes"]` for the rest of the process. + """ + + del entry # single_config_entry: the shared dispatcher belongs to the one entry + + if (routes := _shared_routes(hass)) is not None: + routes.release() + _LOGGER.debug("Config entry removed; dispatcher references released.") diff --git a/custom_components/sws12500/config_flow.py b/custom_components/sws12500/config_flow.py index 9bb0af7..58da9f0 100644 --- a/custom_components/sws12500/config_flow.py +++ b/custom_components/sws12500/config_flow.py @@ -28,6 +28,7 @@ from .const import ( POCASI_CZ_API_KEY, POCASI_CZ_ENABLED, POCASI_CZ_LOGGER_ENABLED, + POCASI_CZ_SEND_DEFAULT, POCASI_CZ_SEND_INTERVAL, POCASI_CZ_SEND_MINIMUM, SENSORS_TO_LOAD, @@ -143,7 +144,9 @@ class ConfigOptionsFlowHandler(OptionsFlow): POCASI_CZ_API_KEY: self.config_entry.options.get(POCASI_CZ_API_KEY, ""), POCASI_CZ_ENABLED: self.config_entry.options.get(POCASI_CZ_ENABLED, False), POCASI_CZ_LOGGER_ENABLED: self.config_entry.options.get(POCASI_CZ_LOGGER_ENABLED, False), - POCASI_CZ_SEND_INTERVAL: self.config_entry.options.get(POCASI_CZ_SEND_INTERVAL, 30), + POCASI_CZ_SEND_INTERVAL: self.config_entry.options.get( + POCASI_CZ_SEND_INTERVAL, POCASI_CZ_SEND_DEFAULT + ), } self.pocasi_cz_schema = { diff --git a/custom_components/sws12500/const.py b/custom_components/sws12500/const.py index ae3a0b4..c3effed 100644 --- a/custom_components/sws12500/const.py +++ b/custom_components/sws12500/const.py @@ -226,6 +226,7 @@ WINDY_URL = "https://stations.windy.com/api/v2/observation/update" POCASI_CZ_URL: Final = "http://ms.pocasimeteo.cz" POCASI_CZ_SEND_MINIMUM: Final = 12 # minimal time to resend data +POCASI_CZ_SEND_DEFAULT: Final = 30 # used when the option is unset or unreadable # Failed sends in a row before resending is disabled. Timeouts are excluded on purpose - # a slow upstream is transient, see the TimeoutError branch in `PocasiPush`. POCASI_CZ_MAX_RETRIES: Final = 3 diff --git a/custom_components/sws12500/pocasti_cz.py b/custom_components/sws12500/pocasti_cz.py index d5344fc..c9ce483 100644 --- a/custom_components/sws12500/pocasti_cz.py +++ b/custom_components/sws12500/pocasti_cz.py @@ -27,6 +27,7 @@ from .const import ( POCASI_CZ_ENABLED, POCASI_CZ_LOGGER_ENABLED, POCASI_CZ_MAX_RETRIES, + POCASI_CZ_SEND_DEFAULT, POCASI_CZ_SEND_INTERVAL, POCASI_CZ_SUCCESS, POCASI_CZ_UNEXPECTED, @@ -34,7 +35,7 @@ from .const import ( POCASI_INVALID_KEY, WSLINK_URL, ) -from .utils import anonymize, update_options +from .utils import anonymize, to_int, update_options _LOGGER = logging.getLogger(__name__) @@ -55,7 +56,9 @@ class PocasiPush: self.last_status: str = "disabled" if not self.enabled else "idle" self.last_error: str | None = None self.last_attempt_at: str | None = None - self._interval = int(self.config.options.get(POCASI_CZ_SEND_INTERVAL, 30)) + # A stored interval that does not parse falls back to the default rather than + # raising here, which would take the whole config entry setup down. + self._interval = to_int(self.config.options.get(POCASI_CZ_SEND_INTERVAL)) or POCASI_CZ_SEND_DEFAULT self.last_update = dt_util.utcnow() self.next_update = dt_util.utcnow() + timedelta(seconds=self._interval) diff --git a/custom_components/sws12500/routes.py b/custom_components/sws12500/routes.py index d4266fe..ae095c1 100644 --- a/custom_components/sws12500/routes.py +++ b/custom_components/sws12500/routes.py @@ -80,6 +80,23 @@ class Routes: """ self.active = False + def release(self) -> None: + """Drop every config-entry object the dispatcher still holds. + + `deactivate()` deliberately keeps the ingress observer so a payload arriving + while the entry is unloaded is still recorded. Removal is the other case: the + entry is gone for good, and this dispatcher outlives it in `hass.data`, so + holding its coordinators - and through them the removed ConfigEntry - would + keep them alive for the rest of the process. + + Enablement flags are left alone: a re-added entry rebinds every handler + through the usual setup path, and `rebind_handler` only touches sticky routes + that are still enabled. + """ + self._ingress_observer = None + for route in self.routes.values(): + route.handler = unregistered + def _resolve_route(self, request: Request) -> RouteInfo | None: """Find the matching RouteInfo for a request. diff --git a/custom_components/sws12500/utils.py b/custom_components/sws12500/utils.py index ace3718..d9daef7 100644 --- a/custom_components/sws12500/utils.py +++ b/custom_components/sws12500/utils.py @@ -232,11 +232,12 @@ def wind_dir_to_text(deg: float | str | None) -> UnitOfDir | None: return azimut -def battery_level(battery: int | str | None) -> UnitOfBat: +def battery_level(battery: Any) -> UnitOfBat: """Return battery level. - WSLink payload values often arrive as strings (e.g. "0"/"1"), so we accept - both ints and strings and coerce to int before mapping. + Goes through `to_int` like every other payload coercion: values arrive as + strings, sometimes spelled as decimals ("1.0"), and reading one of those as + "unknown" would contradict the binary battery entity fed by the same field. Returns UnitOfBat """ @@ -246,18 +247,10 @@ def battery_level(battery: int | str | None) -> UnitOfBat: 1: UnitOfBat.NORMAL, } - if (battery is None) or (battery == ""): + vi = to_int(battery) + if vi is None: return UnitOfBat.UNKNOWN - vi: int - if isinstance(battery, int): - vi = battery - else: - try: - vi = int(battery) - except ValueError: - return UnitOfBat.UNKNOWN - return level_map.get(vi, UnitOfBat.UNKNOWN) diff --git a/tests/test_integration_lifecycle.py b/tests/test_integration_lifecycle.py index 88b3a3e..1291684 100644 --- a/tests/test_integration_lifecycle.py +++ b/tests/test_integration_lifecycle.py @@ -24,6 +24,7 @@ from custom_components.sws12500.const import ( from custom_components.sws12500.coordinator import IncorrectDataError, WeatherDataUpdateCoordinator from custom_components.sws12500.data import SWSRuntimeData from custom_components.sws12500.health_coordinator import HealthCoordinator +from custom_components.sws12500.routes import unregistered ECOWITT_PATH = ECOWITT_URL_PREFIX + "/{webhook_id}" @@ -425,6 +426,67 @@ async def test_async_unload_entry_deactivates_shared_routes(hass_with_http): assert response.status == 503 +@pytest.mark.asyncio +async def test_async_remove_entry_releases_dispatcher_references(hass_with_http): + """Unload keeps the observer on purpose; removal has to let it go. + + The dispatcher outlives the entry in `hass.data`, so a removed entry whose + coordinators are still referenced there stays alive for the rest of the process. + """ + from custom_components.sws12500 import async_remove_entry + + entry = MockConfigEntry(domain=DOMAIN, data={}, options={API_ID: "id", API_KEY: "key"}) + entry.add_to_hass(hass_with_http) + + coordinator = WeatherDataUpdateCoordinator(hass_with_http, entry) + coordinator_health = HealthCoordinator(hass_with_http, entry) + register_path(hass_with_http, coordinator, coordinator_health, entry) + + routes = hass_with_http.data[DOMAIN]["routes"] + routes.deactivate() + # Unload deliberately keeps the observer so mid-reload ingress is still recorded. + assert routes._ingress_observer is not None + + await async_remove_entry(hass_with_http, entry) + + assert routes._ingress_observer is None + assert all(route.handler is unregistered for route in routes.routes.values()) + + +@pytest.mark.asyncio +async def test_reload_after_unload_still_rebinds_the_observer(hass_with_http, monkeypatch): + """Release must not be reached on the reload path.""" + entry = MockConfigEntry(domain=DOMAIN, data={}, options={API_ID: "id", API_KEY: "key"}) + entry.add_to_hass(hass_with_http) + + coordinator = WeatherDataUpdateCoordinator(hass_with_http, entry) + coordinator_health = HealthCoordinator(hass_with_http, entry) + register_path(hass_with_http, coordinator, coordinator_health, entry) + + routes = hass_with_http.data[DOMAIN]["routes"] + first_observer = routes._ingress_observer + + _mock_health_first_refresh(monkeypatch) + monkeypatch.setattr( + hass_with_http.config_entries, + "async_unload_platforms", + AsyncMock(return_value=True), + ) + monkeypatch.setattr( + hass_with_http.config_entries, + "async_forward_entry_setups", + AsyncMock(return_value=True), + ) + await async_unload_entry(hass_with_http, entry) + + assert await async_setup_entry(hass_with_http, entry) is True + + assert routes.active is True + assert routes._ingress_observer is not None + assert routes._ingress_observer != first_observer + assert routes.path_enabled(HEALTH_URL) is True + + @pytest.mark.asyncio async def test_async_unload_entry_returns_false_on_failure(hass_with_http): entry = MockConfigEntry(domain=DOMAIN, data={}, options={API_ID: "id", API_KEY: "key"}) diff --git a/tests/test_pocasi_push.py b/tests/test_pocasi_push.py index 274f386..1940a15 100644 --- a/tests/test_pocasi_push.py +++ b/tests/test_pocasi_push.py @@ -111,6 +111,26 @@ def notify(monkeypatch): return created +@pytest.mark.parametrize("stored", ["", "abc", None, [], 0]) +def test_unreadable_send_interval_falls_back_to_the_default(hass, stored): + """A corrupted option must not take the whole entry setup down. + + `PocasiPush` is constructed from the coordinator during `async_setup_entry`, so a + raise here means the integration fails to load rather than forwarding late. + """ + from custom_components.sws12500.const import POCASI_CZ_SEND_DEFAULT + + entry = _make_entry() + entry.options[POCASI_CZ_SEND_INTERVAL] = stored + + assert PocasiPush(hass, entry)._interval == POCASI_CZ_SEND_DEFAULT + + +def test_a_stored_send_interval_is_honoured(hass): + entry = _make_entry(interval=45) + assert PocasiPush(hass, entry)._interval == 45 + + @pytest.mark.asyncio async def test_push_data_to_server_missing_api_id_returns_early(monkeypatch, hass): entry = _make_entry(api_id=None, api_key="key") diff --git a/tests/test_utils_more.py b/tests/test_utils_more.py index cad8d75..6eb33f9 100644 --- a/tests/test_utils_more.py +++ b/tests/test_utils_more.py @@ -254,6 +254,19 @@ def test_battery_level_handles_none_empty_invalid_and_known_values(): assert battery_level("2") == UnitOfBat.UNKNOWN +def test_battery_level_accepts_the_decimal_spelling(): + """The station sometimes sends integer fields as decimals. + + Reading `1.0` as UNKNOWN would make the deprecated ENUM battery sensor contradict + the binary battery entity fed by the very same payload field. + """ + assert battery_level("1.0") == UnitOfBat.NORMAL + assert battery_level("0.0") == UnitOfBat.LOW + assert battery_level(1.0) == UnitOfBat.NORMAL + assert battery_level(" ") == UnitOfBat.UNKNOWN + assert battery_level([]) == UnitOfBat.UNKNOWN + + def test_temperature_conversions_round_trip(): # Use a value that is exactly representable in binary-ish floats