Merge 2827eac158 into 7582f1e9b4

custom_components/sws12500/__init__.py

@@ -3,16 +3,15 @@
 Architecture overview
 ---------------------
 This integration is *push-based*: the weather station calls our HTTP endpoint and we
-receive a query payload. We do not poll the station.
+receive an HTTP payload. We do not poll the station.
 
 Key building blocks:
 - `WeatherDataUpdateCoordinator` acts as an in-memory "data bus" for the latest payload.
   On each webhook request we call `async_set_updated_data(...)` and all `CoordinatorEntity`
   sensors get notified and update their states.
-- `hass.data[DOMAIN][entry_id]` is a per-entry *dict* that stores runtime state
-  (coordinator instance, options snapshot, and sensor platform callbacks). Keeping this
-  structure consistent is critical; mixing different value types under the same key can
-  break listener wiring and make the UI appear "frozen".
+- `entry.runtime_data` stores per-entry runtime state (coordinator instance, options
+  snapshot, and sensor platform callbacks). Shared aiohttp route registrations stay
+  under `hass.data[DOMAIN]["routes"]` because they must survive a config-entry reload.
 
 Auto-discovery
 --------------
@@ -99,9 +98,9 @@ def register_path(
 ) -> bool:
     """Register webhook paths.
 
-    We register both possible endpoints and use an internal dispatcher (`Routes`) to
-    enable exactly one of them. This lets us toggle WSLink mode without re-registering
-    routes on the aiohttp router.
+    We register the supported station endpoints and use an internal dispatcher
+    (`Routes`) to enable only the configured ingress path. This lets us toggle
+    protocols without re-registering routes on the aiohttp router.
     """
 
     hass.data.setdefault(DOMAIN, {})
@@ -160,6 +159,7 @@ def register_path(
             sticky=True,
         )
     else:
+        routes.activate()
         routes.set_ingress_observer(coordinator_h.record_dispatch)
         _LOGGER.info("We have already registered routes: %s", routes.show_enabled())
     return True
@@ -195,6 +195,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool:
 
     if routes is not None:
         _LOGGER.debug("We have routes registered, will try to switch dispatcher.")
+        routes.activate()
         routes.switch_route(coordinator.received_data, DEFAULT_URL if not _wslink else WSLINK_URL, enabled=_legacy)
         routes.set_ecowitt_enabled(_ecowitt_path, coordinator.received_ecowitt_data, _ecowitt_enabled)
         # Rebind the sticky health route to the new coordinator so /station/health
@@ -270,4 +271,13 @@ async def async_unload_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool
     aiohttp routes stay registered and the dispatcher is re-wired on the next setup.
     """
 
-    return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
+    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):
+            routes.deactivate()
+        setattr(entry, "runtime_data", None)
+
+    return unload_ok

custom_components/sws12500/config_flow.py

@@ -42,6 +42,15 @@ from .const import (
 _PASSWORD_SELECTOR = selector.TextSelector(selector.TextSelectorConfig(type=selector.TextSelectorType.PASSWORD))
 
 
+def _is_empty(value: Any) -> bool:
+    return not isinstance(value, str) or value == ""
+
+
+def _validate_ecowitt_webhook(user_input: dict[str, Any], errors: dict[str, str]) -> None:
+    if user_input.get(ECOWITT_ENABLED) and _is_empty(user_input.get(ECOWITT_WEBHOOK_ID, "")):
+        errors[ECOWITT_WEBHOOK_ID] = "ecowitt_webhook_required"
+
+
 class ConfigOptionsFlowHandler(OptionsFlow):
     """Handle WeatherStation ConfigFlow."""
 
@@ -160,9 +169,9 @@ class ConfigOptionsFlowHandler(OptionsFlow):
             # legacy one while Ecowitt is active would corrupt those entities.
             if self.ecowitt.get(ECOWITT_ENABLED):
                 errors["base"] = ERROR_MUTUALLY_EXCLUSIVE
-            elif user_input[API_ID] in INVALID_CREDENTIALS or user_input.get(API_ID, "") == "":
+            elif user_input[API_ID] in INVALID_CREDENTIALS or _is_empty(user_input.get(API_ID, "")):
                 errors[API_ID] = "valid_credentials_api"
-            elif user_input[API_KEY] in INVALID_CREDENTIALS or user_input.get(API_KEY, "") == "":
+            elif user_input[API_KEY] in INVALID_CREDENTIALS or _is_empty(user_input.get(API_KEY, "")):
                 errors[API_KEY] = "valid_credentials_key"
             elif user_input[API_KEY] == user_input[API_ID]:
                 errors["base"] = "valid_credentials_match"
@@ -251,11 +260,12 @@ class ConfigOptionsFlowHandler(OptionsFlow):
             webhook = secrets.token_hex(8)
 
         if user_input is not None:
+            _validate_ecowitt_webhook(user_input, errors)
             # Both endpoints remap onto the same internal sensor keys, so enabling
             # Ecowitt while the legacy endpoint is active would corrupt those entities.
             if user_input.get(ECOWITT_ENABLED) and self.user_data.get(LEGACY_ENABLED):
                 errors["base"] = ERROR_MUTUALLY_EXCLUSIVE
-            else:
+            if not errors:
                 return self.async_create_entry(title=DOMAIN, data=self.retain_data(user_input))
 
         url: URL = URL(get_url(self.hass))
@@ -358,9 +368,9 @@ class ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
         if user_input is None:
             return self.async_show_form(step_id="pws", data_schema=vol.Schema(self.pws_schema), errors=errors)
 
-        if user_input[API_ID] in INVALID_CREDENTIALS:
+        if user_input[API_ID] in INVALID_CREDENTIALS or _is_empty(user_input.get(API_ID, "")):
             errors[API_ID] = "valid_credentials_api"
-        elif user_input[API_KEY] in INVALID_CREDENTIALS:
+        elif user_input[API_KEY] in INVALID_CREDENTIALS or _is_empty(user_input.get(API_KEY, "")):
             errors[API_KEY] = "valid_credentials_key"
         elif user_input[API_KEY] == user_input[API_ID]:
             errors["base"] = "valid_credentials_match"
@@ -381,8 +391,22 @@ class ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
     async def async_step_ecowitt(self, user_input: Any = None) -> ConfigFlowResult:
         """Ecowitt stations setup."""
 
-        if user_input is None:
-            webhook = secrets.token_hex(8)
+        errors: dict[str, str] = {}
+
+        if user_input is not None:
+            _validate_ecowitt_webhook(user_input, errors)
+            if not errors:
+                options: dict[str, Any] = {
+                    **user_input,
+                    LEGACY_ENABLED: False,
+                    WSLINK: False,
+                    API_ID: "",
+                    API_KEY: "",
+                }
+
+                return self.async_create_entry(title=DOMAIN, data=options, options=options)
+
+        webhook = user_input.get(ECOWITT_WEBHOOK_ID, "") if user_input is not None else secrets.token_hex(8)
         url: URL = URL(get_url(self.hass))
         host = url.host or "UNKNOWN"
 
@@ -399,16 +423,8 @@ class ConfigFlowHandler(ConfigFlow, domain=DOMAIN):
                 "port": str(url.port),
                 "webhook_id": webhook,
             },
+            errors=errors,
         )
-        options: dict[str, Any] = {
-            **user_input,
-            LEGACY_ENABLED: False,
-            WSLINK: False,
-            API_ID: "",
-            API_KEY: "",
-        }
-
-        return self.async_create_entry(title=DOMAIN, data=options, options=options)
 
     @staticmethod
     @callback

custom_components/sws12500/health_coordinator.py

@@ -71,8 +71,8 @@ def _configured_protocol(config: SWSConfigEntry) -> str:
     """Return the primary configured protocol (wu / wslink / ecowitt).
 
     The legacy PWS/WSLink endpoint takes precedence when enabled; otherwise an
-    Ecowitt-only setup reports "ecowitt". (Legacy and Ecowitt can be enabled at the
-    same time; this just labels the primary protocol for the summary.)
+    Ecowitt-only setup reports "ecowitt". If an old or externally edited config
+    has both flags enabled, legacy still wins because that is the effective route.
     """
     if checked_or(config.options.get(LEGACY_ENABLED), bool, True):
         return "wslink" if checked_or(config.options.get(WSLINK), bool, False) else "wu"
@@ -235,8 +235,8 @@ class HealthCoordinator(DataUpdateCoordinator):
         reason = ingress.get("reason")
 
         # A WU vs WSLink mismatch means the station is misconfigured for the legacy
-        # endpoint. Ecowitt coexists with the legacy endpoint, so it never counts as a
-        # mismatch - it is a valid protocol whenever a payload arrives on its route.
+        # endpoint. Ecowitt has its own route and is considered valid only when an
+        # accepted Ecowitt payload arrives there.
         legacy_mismatch = (
             last_protocol in _LEGACY_PROTOCOLS
             and configured_protocol in _LEGACY_PROTOCOLS

custom_components/sws12500/routes.py

@@ -3,10 +3,10 @@
 Why this dispatcher exists
 --------------------------
 Home Assistant registers aiohttp routes on startup. Re-registering or removing routes at runtime
-is awkward and error-prone (and can raise if routes already exist). This integration supports two
-different push endpoints (legacy WU-style vs WSLink). To allow switching between them without
-touching the aiohttp router, we register both routes once and use this in-process dispatcher to
-decide which one is currently enabled.
+is awkward and error-prone (and can raise if routes already exist). This integration supports
+multiple station push endpoints. To allow switching between them without touching the aiohttp
+router, we register routes once and use this in-process dispatcher to decide which one is
+currently enabled.
 
 Important note:
 - Each route stores a *bound method* handler (e.g. `coordinator.received_data`). That means the
@@ -63,6 +63,16 @@ class Routes:
         """Initialize dispatcher storage."""
         self.routes: dict[str, RouteInfo] = {}
         self._ingress_observer: IngressObserver | None = None
+        self.active: bool = True
+
+    def activate(self) -> None:
+        """Allow registered routes to dispatch to their configured handlers."""
+        self.active = True
+
+    def deactivate(self) -> None:
+        """Stop registered routes from dispatching to config-entry handlers."""
+        self.active = False
+        self._ingress_observer = None
 
     def _resolve_route(self, request: Request) -> RouteInfo | None:
         """Find the matching RouteInfo for a request.
@@ -141,6 +151,12 @@ class Routes:
                 self._ingress_observer(request, False, "route_not_registered")
             return await unregistered(request)
 
+        if not self.active:
+            _LOGGER.debug("Route (%s):%s received while integration is not loaded.", request.method, request.path)
+            if self._ingress_observer is not None:
+                self._ingress_observer(request, False, "integration_unloaded")
+            return Response(text="Integration is not loaded.", status=503)
+
         if self._ingress_observer is not None:
             self._ingress_observer(
                 request,
@@ -196,6 +212,9 @@ class Routes:
     def show_enabled(self) -> str:
         """Return a human-readable description of the currently enabled route."""
 
+        if not self.active:
+            return "No routes are enabled."
+
         enabled_routes = {
             f"Dispatcher enabled for ({route.route.method}):{route.url_path}, with handler: {route.handler}"
             for route in self.routes.values()
@@ -207,7 +226,7 @@ class Routes:
 
     def path_enabled(self, url_path: str) -> bool:
         """Return whether any route registered for `url_path` is enabled."""
-        return any(route.enabled for route in self.routes.values() if route.url_path == url_path)
+        return self.active and any(route.enabled for route in self.routes.values() if route.url_path == url_path)
 
     def snapshot(self) -> dict[str, Any]:
         """Return a compact routing snapshot for diagnostics."""
@@ -215,7 +234,7 @@ class Routes:
             key: {
                 "path": route.url_path,
                 "method": route.route.method,
-                "enabled": route.enabled,
+                "enabled": self.active and route.enabled,
                 "sticky": route.sticky,
             }
             for key, route in self.routes.items()

custom_components/sws12500/strings.json

@@ -4,6 +4,7 @@
       "valid_credentials_api": "Provide valid API ID.",
       "valid_credentials_key": "Provide valid API KEY.",
       "valid_credentials_match": "API ID and API KEY should not be the same.",
+      "ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
       "protocols_mutually_exclusive": "The legacy (PWS/WSLink) and Ecowitt endpoints cannot be enabled at the same time - they feed the same sensor entities, which would mix up units and blank readings. Disable one of them first."
     },
     "step": {
@@ -50,6 +51,7 @@
       "valid_credentials_api": "Provide valid API ID.",
       "valid_credentials_key": "Provide valid API KEY.",
       "valid_credentials_match": "API ID and API KEY should not be the same.",
+      "ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
       "windy_id_required": "Windy API ID is required if you want to enable this function.",
       "windy_pw_required": "Windy API password is required if you want to enable this function.",
       "windy_key_required": "Windy station ID and password are required if you want to enable this function.",

custom_components/sws12500/sws12500

@@ -1 +0,0 @@
-../dev/custom_components/sws12500

custom_components/sws12500/translations/cs.json

@@ -4,6 +4,7 @@
       "valid_credentials_api": "Vyplňte platné API ID.",
       "valid_credentials_key": "Vyplňte platný API KEY.",
       "valid_credentials_match": "API ID a API KEY nesmějí být stejné!",
+      "ecowitt_webhook_required": "Před zapnutím dat Ecowitt zadejte webhook ID.",
       "protocols_mutually_exclusive": "Starý endpoint (PWS/WSLink) a Ecowitt nelze zapnout současně – plní stejné entity senzorů, což by pomíchalo jednotky a mazalo naměřené hodnoty. Nejdřív jeden z nich vypni."
     },
     "step": {
@@ -50,6 +51,7 @@
       "valid_credentials_api": "Vyplňte platné API ID",
       "valid_credentials_key": "Vyplňte platný API KEY",
       "valid_credentials_match": "API ID a API KEY nesmějí být stejné!",
+      "ecowitt_webhook_required": "Před zapnutím dat Ecowitt zadejte webhook ID.",
       "windy_id_required": "Je vyžadováno Windy ID, pokud chcete aktivovat přeposílání dat na Windy",
       "windy_pw_required": "Je vyžadován Windy KEY, pokud chcete aktivovat přeposílání dat na Windy",
       "windy_key_required": "Pro aktivaci je vyžadováno Windy ID i heslo stanice.",

custom_components/sws12500/translations/en.json

@@ -4,6 +4,7 @@
       "valid_credentials_api": "Provide valid API ID.",
       "valid_credentials_key": "Provide valid API KEY.",
       "valid_credentials_match": "API ID and API KEY should not be the same.",
+      "ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
       "protocols_mutually_exclusive": "The legacy (PWS/WSLink) and Ecowitt endpoints cannot be enabled at the same time - they feed the same sensor entities, which would mix up units and blank readings. Disable one of them first."
     },
     "step": {
@@ -50,6 +51,7 @@
       "valid_credentials_api": "Provide valid API ID.",
       "valid_credentials_key": "Provide valid API KEY.",
       "valid_credentials_match": "API ID and API KEY should not be the same.",
+      "ecowitt_webhook_required": "Provide a webhook ID before enabling Ecowitt data.",
       "windy_id_required": "Windy API ID is required if you want to enable this function.",
       "windy_pw_required": "Windy API password is required if you want to enable this function.",
       "windy_key_required": "Windy station ID and password are required if you want to enable this function.",

tests/test_config_flow.py

@@ -157,6 +157,35 @@ async def test_config_flow_user_invalid_credentials_match(
     assert result2["errors"]["base"] == "valid_credentials_match"
 
 
+@pytest.mark.parametrize(
+    ("user_input", "field", "error"),
+    [
+        ({API_ID: "", API_KEY: "ok_key", WSLINK: False, DEV_DBG: False}, API_ID, "valid_credentials_api"),
+        ({API_ID: "ok_id", API_KEY: "", WSLINK: False, DEV_DBG: False}, API_KEY, "valid_credentials_key"),
+    ],
+)
+@pytest.mark.asyncio
+async def test_config_flow_user_rejects_empty_pws_credentials(
+    hass,
+    enable_custom_integrations,
+    user_input,
+    field,
+    error,
+) -> None:
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    form = await hass.config_entries.flow.async_configure(
+        result["flow_id"], user_input={"next_step_id": "pws"}
+    )
+
+    result2 = await hass.config_entries.flow.async_configure(form["flow_id"], user_input=user_input)
+
+    assert result2["type"] == "form"
+    assert result2["step_id"] == "pws"
+    assert result2["errors"][field] == error
+
+
 @pytest.mark.asyncio
 async def test_options_flow_init_menu(hass, enable_custom_integrations) -> None:
     """Options flow shows menu with expected steps."""
@@ -399,6 +428,16 @@ async def test_options_flow_ecowitt_uses_get_url_placeholders_and_webhook_defaul
         assert placeholders["port"] == "8123"
         assert placeholders["webhook_id"]  # generated
 
+        bad = await hass.config_entries.options.async_configure(
+            init["flow_id"],
+            user_input={
+                ECOWITT_WEBHOOK_ID: "",
+                ECOWITT_ENABLED: True,
+            },
+        )
+        assert bad["type"] == "form"
+        assert bad["errors"][ECOWITT_WEBHOOK_ID] == "ecowitt_webhook_required"
+
         done = await hass.config_entries.options.async_configure(
             init["flow_id"],
             user_input={
@@ -454,6 +493,16 @@ async def test_config_flow_ecowitt_initial_setup(hass, enable_custom_integration
         assert placeholders["url"] == "example.local"
         assert placeholders["webhook_id"]
 
+        bad = await hass.config_entries.flow.async_configure(
+            form["flow_id"],
+            user_input={
+                ECOWITT_WEBHOOK_ID: "",
+                ECOWITT_ENABLED: True,
+            },
+        )
+        assert bad["type"] == "form"
+        assert bad["errors"][ECOWITT_WEBHOOK_ID] == "ecowitt_webhook_required"
+
         done = await hass.config_entries.flow.async_configure(
             form["flow_id"],
             user_input={

tests/test_init.py

@@ -5,7 +5,7 @@ These tests rely on `pytest-homeassistant-custom-component` to provide:
 - `MockConfigEntry` helper for config entries
 
 They validate that the integration can set up a config entry and that the
-coordinator is created and stored in `hass.data`.
+coordinator is stored on `entry.runtime_data`.
 
 Note:
 This integration registers aiohttp routes via `hass.http.app.router`. In this
@@ -46,7 +46,7 @@ def config_entry() -> MockConfigEntry:
 async def test_async_setup_entry_creates_runtime_state(
     hass, config_entry: MockConfigEntry, monkeypatch
 ):
-    """Setting up a config entry should succeed and populate hass.data."""
+    """Setting up a config entry should succeed and populate runtime state."""
     config_entry.add_to_hass(hass)
 
     # `async_setup_entry` calls `register_path`, which needs `hass.http`.

tests/test_integration_lifecycle.py

@@ -369,6 +369,35 @@ async def test_async_unload_entry_returns_true_on_success(hass_with_http):
     hass_with_http.config_entries.async_unload_platforms.assert_awaited_once()
 
 
+@pytest.mark.asyncio
+async def test_async_unload_entry_deactivates_shared_routes(hass_with_http):
+    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)
+    entry.runtime_data = SWSRuntimeData(
+        coordinator=coordinator,
+        health_coordinator=coordinator_health,
+        last_options=dict(entry.options),
+    )
+
+    routes = hass_with_http.data[DOMAIN]["routes"]
+    assert routes.path_enabled(DEFAULT_URL) is True
+
+    hass_with_http.config_entries.async_unload_platforms = AsyncMock(return_value=True)
+
+    ok = await async_unload_entry(hass_with_http, entry)
+
+    assert ok is True
+    assert routes.path_enabled(DEFAULT_URL) is False
+    assert getattr(entry, "runtime_data", None) is None
+
+    response = await routes.dispatch(SimpleNamespace(method="GET", path=DEFAULT_URL))
+    assert response.status == 503
+
+
 @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"})