From 5adc68455820de586879fae56f8b7fed094e57f1 Mon Sep 17 00:00:00 2001 From: SchiZzA Date: Sat, 25 Jul 2026 23:24:45 +0200 Subject: [PATCH] refactor(sws12500): centralize battery sensor classification --- .../sws12500/battery_sensors_def.py | 36 ++++++- custom_components/sws12500/const.py | 15 ++- custom_components/sws12500/sensors_wslink.py | 16 +-- tests/test_battery_classification.py | 100 ++++++++++++++++++ 4 files changed, 150 insertions(+), 17 deletions(-) create mode 100644 tests/test_battery_classification.py diff --git a/custom_components/sws12500/battery_sensors_def.py b/custom_components/sws12500/battery_sensors_def.py index 9fa0cde..f067419 100644 --- a/custom_components/sws12500/battery_sensors_def.py +++ b/custom_components/sws12500/battery_sensors_def.py @@ -1,14 +1,29 @@ -"""Battery sensors templates. +"""Battery sensor templates. -We create a sensor template here. -Actually loaded sensors are gated in coordinator. +Entity descriptions for both battery kinds are *generated* from the classification +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 +) diff --git a/custom_components/sws12500/const.py b/custom_components/sws12500/const.py index ec40092..8668712 100644 --- a/custom_components/sws12500/const.py +++ b/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 -# (e.g. T9_BATTERY) are plain sensors, not binary ones. +# How the station reports each battery decides which entity it becomes. The two tuples +# 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) diff --git a/custom_components/sws12500/sensors_wslink.py b/custom_components/sws12500/sensors_wslink.py index 659b24d..ae4df0f 100644 --- a/custom_components/sws12500/sensors_wslink.py +++ b/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( - key=T9_BATTERY, - translation_key=T9_BATTERY, - device_class=SensorDeviceClass.BATTERY, - native_unit_of_measurement=PERCENTAGE, - state_class=SensorStateClass.MEASUREMENT, - suggested_display_precision=0, - value_fn=battery_5step_to_pct, - ), + # 0-5 level batteries are generated from BATTERY_NON_BINARY so the classification + # lives in exactly one place (see battery_sensors_def). + *BATTERY_LEVEL_SENSORS, ) diff --git a/tests/test_battery_classification.py b/tests/test_battery_classification.py new file mode 100644 index 0000000..90532d1 --- /dev/null +++ b/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"