refactor(sws12500): centralize battery sensor classification

custom_components/sws12500/battery_sensors_def.py

@@ -1,14 +1,29 @@
-"""Battery sensors templates.
+"""Battery sensor templates.
 
-We create a sensor template here.
+Entity descriptions for both battery kinds are *generated* from the classification
-Actually loaded sensors are gated in coordinator.
+tuples in `const`, so those tuples are the single source of truth:
+
+- ``BATTERY_LIST``       -> 0/1 low/normal  -> binary sensors
+- ``BATTERY_NON_BINARY`` -> 0-5 level       -> percentage sensors
+
+Generating both from one place is what prevents the conflict: a key listed in both
+tuples would otherwise get a binary sensor (collapsing 0-5 into low/not-low and
+throwing away four fifths of the reading) *and* a percentage sensor for the same
+battery. `tests/test_battery_classification.py` fails if the tuples ever overlap or
+if a `*_BATTERY` constant is left unclassified.
+
+Which of these are actually loaded is gated in the coordinator by SENSORS_TO_LOAD.
 """
 
 from __future__ import annotations
 
 from homeassistant.components.binary_sensor import BinarySensorDeviceClass, BinarySensorEntityDescription
+from homeassistant.components.sensor import SensorDeviceClass, SensorStateClass
+from homeassistant.const import PERCENTAGE
 
-from .const import BATTERY_LIST
+from .const import BATTERY_LIST, BATTERY_NON_BINARY
+from .sensors_common import WeatherSensorEntityDescription
+from .utils import battery_5step_to_pct
 
 BATTERY_BINARY_SENSORS: tuple[BinarySensorEntityDescription, ...] = tuple(
     BinarySensorEntityDescription(
@@ -18,3 +33,16 @@ BATTERY_BINARY_SENSORS: tuple[BinarySensorEntityDescription, ...] = tuple(
     )
     for key in BATTERY_LIST
 )
+
+BATTERY_LEVEL_SENSORS: tuple[WeatherSensorEntityDescription, ...] = tuple(
+    WeatherSensorEntityDescription(
+        key=key,
+        translation_key=key,
+        device_class=SensorDeviceClass.BATTERY,
+        native_unit_of_measurement=PERCENTAGE,
+        state_class=SensorStateClass.MEASUREMENT,
+        suggested_display_precision=0,
+        value_fn=battery_5step_to_pct,
+    )
+    for key in BATTERY_NON_BINARY
+)

custom_components/sws12500/const.py

@@ -361,8 +361,16 @@ REMAP_WSLINK_ITEMS: dict[str, str] = {
 #
 
 
-# Station reports batteries as 0/1 (low/normal). Sensors reporting a 0-5 level
+# How the station reports each battery decides which entity it becomes. The two tuples
-# (e.g. T9_BATTERY) are plain sensors, not binary ones.
+# below are the single source of truth for that split - `battery_sensors_def` generates
+# both entity description sets from them, so a key cannot end up with two entities.
+#
+# They must stay disjoint, and every `*_BATTERY` constant must appear in exactly one of
+# them; `tests/test_battery_classification.py` enforces both. That matters because the
+# WSLink API has more of each kind still to be implemented (see the TODO block above):
+# `t5lsbat` / `t6c1-7bat` are 0/1, while `t8bat` / `t10bat` / `t11bat` are 0-5.
+
+# Reported as 0/1 (low/normal) -> BinarySensorDeviceClass.BATTERY.
 BATTERY_LIST: Final[tuple[str, ...]] = (
     OUTSIDE_BATTERY,
     INDOOR_BATTERY,
@@ -375,6 +383,9 @@ BATTERY_LIST: Final[tuple[str, ...]] = (
     CH8_BATTERY,
 )
 
+# Reported as a 0-5 level, 5 being full -> percentage SensorDeviceClass.BATTERY.
+BATTERY_NON_BINARY: Final[tuple[str, ...]] = (T9_BATTERY,)
+
 
 CONNECTION_GATED_SENSORS: Final[dict[str, list[str]]] = {
     # Multi-channel temp/humidity probes (CH2 - CH8)

custom_components/sws12500/sensors_wslink.py

@@ -16,6 +16,7 @@ from homeassistant.const import (
     UnitOfVolumetricFlux,
 )
 
+from .battery_sensors_def import BATTERY_LEVEL_SENSORS
 from .const import (
     BARO_PRESSURE,
     CH2_BATTERY,
@@ -54,7 +55,6 @@ from .const import (
     OUTSIDE_TEMP,
     RAIN,
     SOLAR_RADIATION,
-    T9_BATTERY,
     UV,
     VOC,
     WBGT_TEMP,
@@ -69,7 +69,7 @@ from .const import (
     VOCLevel,
 )
 from .sensors_common import WeatherSensorEntityDescription
-from .utils import battery_5step_to_pct, battery_level, to_float, to_int, voc_level_to_text, wind_dir_to_text
+from .utils import battery_level, to_float, to_int, voc_level_to_text, wind_dir_to_text
 
 SENSOR_TYPES_WSLINK: tuple[WeatherSensorEntityDescription, ...] = (
     WeatherSensorEntityDescription(
@@ -542,13 +542,7 @@ SENSOR_TYPES_WSLINK: tuple[WeatherSensorEntityDescription, ...] = (
         icon="mdi:air-filter",
         value_fn=voc_level_to_text,
     ),
-    WeatherSensorEntityDescription(
+    # 0-5 level batteries are generated from BATTERY_NON_BINARY so the classification
-        key=T9_BATTERY,
+    # lives in exactly one place (see battery_sensors_def).
-        translation_key=T9_BATTERY,
+    *BATTERY_LEVEL_SENSORS,
-        device_class=SensorDeviceClass.BATTERY,
-        native_unit_of_measurement=PERCENTAGE,
-        state_class=SensorStateClass.MEASUREMENT,
-        suggested_display_precision=0,
-        value_fn=battery_5step_to_pct,
-    ),
 )

tests/test_battery_classification.py

@@ -0,0 +1,100 @@
+"""Every battery key must be classified exactly once.
+
+The station reports batteries in two incompatible ways:
+
+- 0/1 (low/normal)     -> ``BATTERY_LIST``       -> binary sensors
+- 0-5 level (5 = full) -> ``BATTERY_NON_BINARY`` -> percentage sensors
+
+Both entity description sets are generated from those tuples, so a key appearing in
+both would produce a binary sensor *and* a percentage sensor for the same battery -
+and the binary one would collapse levels 1-5 into a single "not low", silently
+discarding the reading. A key appearing in neither simply never gets an entity.
+
+The WSLink API has more of both kinds still to implement (`t5lsbat`, `t6c1-7bat` are
+0/1; `t8bat`, `t10bat`, `t11bat` are 0-5), so these tests exist to fail the moment a
+new `*_BATTERY` constant is added without deciding which kind it is.
+"""
+
+from __future__ import annotations
+
+from homeassistant.components.binary_sensor import BinarySensorDeviceClass
+from homeassistant.components.sensor import SensorDeviceClass
+from homeassistant.const import PERCENTAGE
+
+from custom_components.sws12500 import const
+from custom_components.sws12500.battery_sensors_def import BATTERY_BINARY_SENSORS, BATTERY_LEVEL_SENSORS
+from custom_components.sws12500.const import BATTERY_LIST, BATTERY_NON_BINARY
+from custom_components.sws12500.sensors_weather import SENSOR_TYPES_WEATHER_API
+from custom_components.sws12500.sensors_wslink import SENSOR_TYPES_WSLINK
+
+
+def _all_battery_constants() -> set[str]:
+    """Every `*_BATTERY` value defined in const.py."""
+    return {value for name, value in vars(const).items() if name.endswith("_BATTERY") and isinstance(value, str)}
+
+
+def test_classifications_are_disjoint() -> None:
+    """A key in both tuples would get two conflicting entities."""
+    overlap = set(BATTERY_LIST) & set(BATTERY_NON_BINARY)
+    assert not overlap, f"battery keys classified as both binary and 0-5: {sorted(overlap)}"
+
+
+def test_every_battery_constant_is_classified() -> None:
+    """Adding a `*_BATTERY` constant must force a decision about its kind."""
+    classified = set(BATTERY_LIST) | set(BATTERY_NON_BINARY)
+    unclassified = _all_battery_constants() - classified
+
+    assert not unclassified, (
+        f"unclassified battery keys: {sorted(unclassified)} - add each to BATTERY_LIST "
+        "(reported as 0/1) or BATTERY_NON_BINARY (reported as a 0-5 level) in const.py"
+    )
+
+
+def test_classification_covers_only_real_constants() -> None:
+    """Neither tuple may list a key that is not an actual battery constant."""
+    stray = (set(BATTERY_LIST) | set(BATTERY_NON_BINARY)) - _all_battery_constants()
+    assert not stray, f"not `*_BATTERY` constants: {sorted(stray)}"
+
+
+# ---------------------------------------------------------------------------
+# The description sets are generated from the tuples, not hand-maintained
+# ---------------------------------------------------------------------------
+
+
+def test_binary_descriptions_generated_from_battery_list() -> None:
+    keys = [desc.key for desc in BATTERY_BINARY_SENSORS]
+    assert keys == list(BATTERY_LIST)
+    assert all(desc.device_class is BinarySensorDeviceClass.BATTERY for desc in BATTERY_BINARY_SENSORS)
+
+
+def test_level_descriptions_generated_from_battery_non_binary() -> None:
+    keys = [desc.key for desc in BATTERY_LEVEL_SENSORS]
+    assert keys == list(BATTERY_NON_BINARY)
+
+    for desc in BATTERY_LEVEL_SENSORS:
+        assert desc.device_class is SensorDeviceClass.BATTERY
+        assert desc.native_unit_of_measurement == PERCENTAGE
+        assert desc.value_fn is not None
+        # 5 is full; the raw level must be rendered as a percentage.
+        assert desc.value_fn("5") == 100
+        assert desc.value_fn("0") == 0
+
+
+def test_level_batteries_are_not_also_plain_sensors() -> None:
+    """A 0-5 battery must appear exactly once in each platform's description set."""
+    for sensor_types in (SENSOR_TYPES_WSLINK, SENSOR_TYPES_WEATHER_API):
+        keys = [desc.key for desc in sensor_types]
+        for key in BATTERY_NON_BINARY:
+            assert keys.count(key) <= 1, f"{key} defined more than once"
+
+
+def test_binary_batteries_are_not_plain_sensors_too() -> None:
+    """0/1 batteries belong to the binary platform only.
+
+    The legacy plain-sensor variants are deprecated (see `legacy.py`) but still
+    present; they must at least not be duplicated.
+    """
+    for sensor_types in (SENSOR_TYPES_WSLINK, SENSOR_TYPES_WEATHER_API):
+        keys = [desc.key for desc in sensor_types]
+        for key in BATTERY_LIST:
+            assert keys.count(key) <= 1, f"{key} defined more than once"