fix: block enabling legacy and Ecowitt protocols together

custom_components/sws12500/__init__.py

@@ -39,13 +39,12 @@ from homeassistant.core import HomeAssistant, callback
 from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers.event import async_track_time_interval
 
+from .conflicts import effective_protocols, update_protocol_conflict_issue
 from .const import (
     DEFAULT_URL,
     DOMAIN,
-    ECOWITT_ENABLED,
     ECOWITT_URL_PREFIX,
     HEALTH_URL,
-    LEGACY_ENABLED,
     POCASI_CZ_ENABLED,
     SENSORS_TO_LOAD,
     WINDY_ENABLED,
@@ -81,8 +80,8 @@ def register_path(
         raise ConfigEntryNotReady
 
     _wslink: bool = checked_or(config.options.get(WSLINK), bool, False)
-    _ecowitt_enabled: bool = checked_or(config.options.get(ECOWITT_ENABLED), bool, False)
-    _legacy: bool = checked_or(config.options.get(LEGACY_ENABLED), bool, True)
+    # Legacy and Ecowitt share one entity namespace, so only one may be wired up.
+    _legacy, _ecowitt_enabled = effective_protocols(config)
 
     # Load registred routes
     routes: Routes | None = hass_data.get("routes", None)
@@ -157,8 +156,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool:
         last_options=dict(entry.options),
     )
     _wslink = checked_or(entry.options.get(WSLINK), bool, False)
-    _legacy = checked_or(entry.options.get(LEGACY_ENABLED), bool, True)
-    _ecowitt_enabled = checked_or(entry.options.get(ECOWITT_ENABLED), bool, False)
+    # Legacy and Ecowitt share one entity namespace, so only one may be wired up.
+    _legacy, _ecowitt_enabled = effective_protocols(entry)
     _ecowitt_path = ECOWITT_URL_PREFIX + "/{webhook_id}"
 
     _LOGGER.debug("WS Link is %s", "enabled" if _wslink else "disabled")
@@ -197,6 +196,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: SWSConfigEntry) -> bool:
 
     entry.async_on_unload(entry.add_update_listener(update_listener))
     update_legacy_battery_issue(hass, entry)
+    update_protocol_conflict_issue(hass, entry)
 
     return True
 

custom_components/sws12500/config_flow.py

@@ -14,6 +14,7 @@ from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import selector
 from homeassistant.helpers.network import get_url
 
+from .conflicts import ERROR_MUTUALLY_EXCLUSIVE
 from .const import (
     API_ID,
     API_KEY,
@@ -171,7 +172,11 @@ class ConfigOptionsFlowHandler(OptionsFlow):
             )
 
         if user_input.get(LEGACY_ENABLED):
-            if user_input[API_ID] in INVALID_CREDENTIALS or user_input.get(API_ID, "") == "":
+            # Both endpoints remap onto the same internal sensor keys, so enabling the
+            # 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, "") == "":
                 errors[API_ID] = "valid_credentials_api"
             elif user_input[API_KEY] in INVALID_CREDENTIALS or user_input.get(API_KEY, "") == "":
                 errors[API_KEY] = "valid_credentials_key"
@@ -261,35 +266,38 @@ class ConfigOptionsFlowHandler(OptionsFlow):
         if not (webhook := self.ecowitt.get(ECOWITT_WEBHOOK_ID)):
             webhook = secrets.token_hex(8)
 
-        if user_input is None:
-            url: URL = URL(get_url(self.hass))
+        if user_input is not None:
+            # 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:
+                return self.async_create_entry(title=DOMAIN, data=self.retain_data(user_input))
 
-            host = url.host or "UNKNOWN"
+        url: URL = URL(get_url(self.hass))
+        host = url.host or "UNKNOWN"
 
-            ecowitt_schema = {
-                vol.Required(
-                    ECOWITT_WEBHOOK_ID,
-                    default=webhook,
-                ): str,
-                vol.Optional(
-                    ECOWITT_ENABLED,
-                    default=self.ecowitt.get(ECOWITT_ENABLED, False),
-                ): bool,
-            }
+        ecowitt_schema = {
+            vol.Required(
+                ECOWITT_WEBHOOK_ID,
+                default=webhook,
+            ): str,
+            vol.Optional(
+                ECOWITT_ENABLED,
+                default=self.ecowitt.get(ECOWITT_ENABLED, False),
+            ): bool,
+        }
 
-            return self.async_show_form(
-                step_id="ecowitt",
-                data_schema=vol.Schema(ecowitt_schema),
-                description_placeholders={
-                    "url": host,
-                    "port": str(url.port),
-                    "webhook_id": webhook,
-                },
-                errors=errors,
-            )
-
-        user_input = self.retain_data(user_input)
-        return self.async_create_entry(title=DOMAIN, data=user_input)
+        return self.async_show_form(
+            step_id="ecowitt",
+            data_schema=vol.Schema(ecowitt_schema),
+            description_placeholders={
+                "url": host,
+                "port": str(url.port),
+                "webhook_id": webhook,
+            },
+            errors=errors,
+        )
 
     async def async_step_wslink_port_setup(self, user_input: Any = None) -> ConfigFlowResult:
         """WSLink Addon port setup."""

custom_components/sws12500/conflicts.py

@@ -0,0 +1,90 @@
+"""Protocol conflict handling.
+
+The legacy (PWS/WSLink) and Ecowitt endpoints both remap their payloads onto the *same*
+internal sensor keys (`REMAP_ITEMS` / `REMAP_WSLINK_ITEMS` vs `REMAP_ECOWITT_COMPAT`) and
+push them through the same coordinator. Running both at once is therefore unsound:
+
+1. Units clash. The entity descriptions are chosen by the WSLink flag alone, so Ecowitt's
+   imperial payload (`tempf`, `baromrelin`, `windspeedmph`, ...) is rendered with the
+   WSLink set's metric units - 18 of the 26 Ecowitt-mapped keys disagree.
+2. Payloads blank each other. `async_set_updated_data` *replaces* the coordinator payload,
+   so every Ecowitt push blanks the keys only the legacy protocol reports (and vice versa),
+   flipping those entities to `unknown` on every other push.
+3. One entity, two sources. A single `sensor.outside_temp` would alternate between whatever
+   the two endpoints report.
+
+The initial config flow already keeps them exclusive (the `pws` step disables Ecowitt, the
+`ecowitt` step disables legacy); only the options flow could turn both on. This module is
+the single place that states the rule, so the config flow, the route wiring and the device
+model all agree on it.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Final
+
+from py_typecheck import checked_or
+
+from homeassistant.core import HomeAssistant, callback
+from homeassistant.helpers import issue_registry as ir
+from homeassistant.helpers.issue_registry import IssueSeverity
+
+from .const import DOMAIN, ECOWITT_ENABLED, LEGACY_ENABLED
+
+if TYPE_CHECKING:
+    # Type-only: `data` imports `effective_protocols` from here at runtime, so a
+    # runtime import back into `data` would close an import cycle.
+    from .data import SWSConfigEntry
+
+# Shown in the config/options flow when a submit would enable both protocols.
+ERROR_MUTUALLY_EXCLUSIVE: Final = "protocols_mutually_exclusive"
+
+
+@callback
+def protocols_conflict(entry: SWSConfigEntry) -> bool:
+    """Return whether this entry has both protocols enabled."""
+    return checked_or(entry.options.get(LEGACY_ENABLED), bool, True) and checked_or(
+        entry.options.get(ECOWITT_ENABLED), bool, False
+    )
+
+
+@callback
+def effective_protocols(entry: SWSConfigEntry) -> tuple[bool, bool]:
+    """Return the `(legacy, ecowitt)` state to actually wire up.
+
+    Options written before this rule existed may still have both flags set. Rather than
+    ingesting two protocols into one entity namespace, legacy wins - matching the
+    precedence already documented in `health_coordinator._configured_protocol` - and
+    `update_protocol_conflict_issue` tells the user what happened.
+    """
+    legacy = checked_or(entry.options.get(LEGACY_ENABLED), bool, True)
+    ecowitt = checked_or(entry.options.get(ECOWITT_ENABLED), bool, False)
+
+    if legacy and ecowitt:
+        return True, False
+    return legacy, ecowitt
+
+
+def _issue_id(entry: SWSConfigEntry) -> str:
+    """Return the Repairs issue id for this config entry."""
+    return f"protocol_conflict_{entry.entry_id}"
+
+
+@callback
+def update_protocol_conflict_issue(hass: HomeAssistant, entry: SWSConfigEntry) -> None:
+    """Create or clear the Repairs issue for a legacy/Ecowitt conflict."""
+
+    issue_id = _issue_id(entry)
+
+    if protocols_conflict(entry):
+        ir.async_create_issue(
+            hass,
+            DOMAIN,
+            issue_id=issue_id,
+            is_persistent=True,
+            is_fixable=False,
+            severity=IssueSeverity.ERROR,
+            translation_key="protocol_conflict",
+        )
+    else:
+        ir.async_delete_issue(hass, DOMAIN, issue_id=issue_id)

custom_components/sws12500/data.py

@@ -19,7 +19,8 @@ from homeassistant.helpers.entity import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.util import dt as dt_util
 
-from .const import DOMAIN, ECOWITT_ENABLED, WSLINK
+from .conflicts import effective_protocols
+from .const import DOMAIN, WSLINK
 
 if TYPE_CHECKING:
     from homeassistant.components.binary_sensor import BinarySensorEntityDescription
@@ -69,12 +70,17 @@ def _station_model(entry: SWSConfigEntry) -> str:
     """Return the device model label reflecting the running station type.
 
     Ecowitt (with the learned model when available), else WSLink, else PWS.
+
+    Uses the *effective* protocols so a stale both-enabled config reports the endpoint
+    that is actually wired up, rather than one that is being ignored.
     """
-    if checked_or(entry.options.get(ECOWITT_ENABLED), bool, False):
+    legacy, ecowitt = effective_protocols(entry)
+
+    if ecowitt:
         runtime = getattr(entry, "runtime_data", None)
         model = getattr(runtime, "ecowitt_model", None) if runtime is not None else None
         return f"Ecowitt {model}" if model else "Ecowitt"
-    if checked_or(entry.options.get(WSLINK), bool, False):
+    if legacy and checked_or(entry.options.get(WSLINK), bool, False):
         return "WSLink"
     return "PWS"
 

custom_components/sws12500/translations/cs.json

@@ -3,7 +3,8 @@
     "error": {
       "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é!"
+      "valid_credentials_match": "API ID a API KEY nesmějí být stejné!",
+      "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": {
       "user": {
@@ -54,7 +55,8 @@
       "windy_key_required": "Pro aktivaci je vyžadováno Windy ID i heslo stanice.",
       "pocasi_id_required": "Je vyžadován Počasí ID, pokud chcete aktivovat přeposílání dat na Počasí Meteo CZ",
       "pocasi_key_required": "Klíč k účtu Počasí Meteo je povinný.",
-      "pocasi_send_minimum": "Minimální interval pro přeposílání je 12 sekund."
+      "pocasi_send_minimum": "Minimální interval pro přeposílání je 12 sekund.",
+      "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": {
       "init": {
@@ -551,6 +553,10 @@
     "stale_sensors_detected": {
       "title": "Detekovány nečinné sensory",
       "description": "Tyto senzory jsou nastavené, ale nereportují data: {sensors}. V HA budou viditelné jako Nedostupné. Pokud už nejsou přítomné, můžeš je odstranit přes Nastavení –> Zařízení a Služby –> SWS 12500."
+    },
+    "protocol_conflict": {
+      "title": "Zapnuté dva neslučitelné protokoly",
+      "description": "Současně je zapnutý starý endpoint (PWS/WSLink) i Ecowitt. Oba plní stejné entity senzorů, takže jejich souběh míchá měrné jednotky a hodnoty mezi aktualizacemi mizí. Používá se starý endpoint a data z Ecowittu se ignorují, dokud si nevybereš jeden: jdi do Nastavení -> Zařízení a služby -> SWS 12500 -> Konfigurovat a vypni buď starý endpoint, nebo Ecowitt."
     }
   },
   "notify": {

tests/test_data.py

@@ -4,7 +4,7 @@ from __future__ import annotations
 
 from types import SimpleNamespace
 
-from custom_components.sws12500.const import DOMAIN, ECOWITT_ENABLED, WSLINK
+from custom_components.sws12500.const import DOMAIN, ECOWITT_ENABLED, LEGACY_ENABLED, WSLINK
 from custom_components.sws12500.data import (
     SWSRuntimeData,
     _station_model,
@@ -75,11 +75,11 @@ def test_station_model_ecowitt_without_learned_model():
 
     Covers both the missing-runtime_data and the model-is-None paths.
     """
-    no_runtime = SimpleNamespace(options={ECOWITT_ENABLED: True})
+    no_runtime = SimpleNamespace(options={ECOWITT_ENABLED: True, LEGACY_ENABLED: False})
     assert _station_model(no_runtime) == "Ecowitt"  # type: ignore[arg-type]
 
     runtime_no_model = SimpleNamespace(
-        options={ECOWITT_ENABLED: True},
+        options={ECOWITT_ENABLED: True, LEGACY_ENABLED: False},
         runtime_data=SimpleNamespace(ecowitt_model=None),
     )
     assert _station_model(runtime_no_model) == "Ecowitt"  # type: ignore[arg-type]
@@ -88,19 +88,23 @@ def test_station_model_ecowitt_without_learned_model():
 def test_station_model_ecowitt_with_learned_model():
     """Ecowitt enabled with a learned model -> "Ecowitt <model>"."""
     entry = SimpleNamespace(
-        options={ECOWITT_ENABLED: True},
+        options={ECOWITT_ENABLED: True, LEGACY_ENABLED: False},
         runtime_data=SimpleNamespace(ecowitt_model="GW1000"),
     )
     assert _station_model(entry) == "Ecowitt GW1000"  # type: ignore[arg-type]
 
 
-def test_station_model_ecowitt_takes_precedence_over_wslink():
-    """When both flags are set the ecowitt model wins."""
+def test_station_model_legacy_wins_over_ecowitt_on_conflict():
+    """A stale both-enabled config reports the endpoint that is actually wired up.
+
+    `effective_protocols` resolves the conflict in favour of legacy, so the device model
+    must not advertise an Ecowitt station whose data is being ignored.
+    """
     entry = SimpleNamespace(
-        options={ECOWITT_ENABLED: True, WSLINK: True},
+        options={ECOWITT_ENABLED: True, LEGACY_ENABLED: True, WSLINK: True},
         runtime_data=SimpleNamespace(ecowitt_model="WS3900"),
     )
-    assert _station_model(entry) == "Ecowitt WS3900"  # type: ignore[arg-type]
+    assert _station_model(entry) == "WSLink"  # type: ignore[arg-type]
 
 
 def test_build_device_info_shared_identity():