fix(review): parse battery flags via to_int, release routes on removal

ecowitt_support
SchiZzA 2026-07-26 15:53:03 +02:00
parent d2ae26eab5
commit 3cdaffc81f
No known key found for this signature in database
9 changed files with 154 additions and 19 deletions

View File

@ -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.")

View File

@ -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 = {

View File

@ -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

View File

@ -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)

View File

@ -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.

View File

@ -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,16 +247,8 @@ def battery_level(battery: int | str | None) -> UnitOfBat:
1: UnitOfBat.NORMAL,
}
if (battery is None) or (battery == ""):
return UnitOfBat.UNKNOWN
vi: int
if isinstance(battery, int):
vi = battery
else:
try:
vi = int(battery)
except ValueError:
vi = to_int(battery)
if vi is None:
return UnitOfBat.UNKNOWN
return level_map.get(vi, UnitOfBat.UNKNOWN)

View File

@ -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"})

View File

@ -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")

View File

@ -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