From 15279066b5b2ce6eda790beefb2a0b54ffffce10 Mon Sep 17 00:00:00 2001 From: kkqq9320 Date: Tue, 4 Aug 2026 15:25:12 +0900 Subject: [PATCH] review: move the state_class into the shared tuple's fourth column The frozenset was a parallel structure for a per-row fact, and the comment above the tuple already described it as a fourth column -- so the comment promised the right shape and the code did something else. Fixed to the shape the comment described: _AIR_QUALITY_SENSORS carries state_class per row and the comprehension unpacks it, with _RECORDED_AIR_QUALITY and its duplicated rationale block deleted. air_monitor imports the same rows and now unpacks four, but discards the fourth. That board (issue #210) has stamped all five readings as `measurement` since it was added; consuming the column would silently drop long-term statistics for Odor and CleanLevel on shipped devices, which is a behaviour change this branch has no evidence to make. The grade/concentration split stays scoped to the air purifier. test_shared_sensor_tuple_keeps_its_three_column_shape asserted the premise this replaces -- that widening the tuple breaks air_monitor's import -- so it is replaced rather than renumbered: one test that the rows carry their own state_class, and one that air_monitor still imports and still stamps all five. --- .../registry/capabilities/air_monitor.py | 8 +++- .../registry/capabilities/air_purifier.py | 38 ++++--------------- ...est_air_purifier_air_quality_statistics.py | 23 ++++++++--- 3 files changed, 31 insertions(+), 38 deletions(-) diff --git a/custom_components/localthings/registry/capabilities/air_monitor.py b/custom_components/localthings/registry/capabilities/air_monitor.py index 451ef43..0ed9aa8 100644 --- a/custom_components/localthings/registry/capabilities/air_monitor.py +++ b/custom_components/localthings/registry/capabilities/air_monitor.py @@ -39,6 +39,12 @@ from ..entities import BinarySensorDesc, SensorDesc, SwitchDesc, TimeDesc from .air_purifier import _AIR_QUALITY_SENSORS from .common import int_or_none, sensor_item_value +# _AIR_QUALITY_SENSORS' fourth column (state_class) is deliberately discarded +# here: air_purifier leaves Odor/CleanLevel unstamped because they read as +# graded indices on that family, while this board has stamped all five as +# `measurement` since it was added (issue #210). Consuming the column would +# silently drop long-term statistics for two sensors on shipped devices, so +# the shared rows supply only the key/icon/type here. SENSORS = Capability( href="/sensors/vs/0", poll_tier="warm", @@ -51,7 +57,7 @@ SENSORS = Capability( state_class="measurement", value_fn=lambda items, t=sensor_type: sensor_item_value(items, t), ) - for key, icon, sensor_type in _AIR_QUALITY_SENSORS + for key, icon, sensor_type, _ in _AIR_QUALITY_SENSORS ), SensorDesc( key="co2", diff --git a/custom_components/localthings/registry/capabilities/air_purifier.py b/custom_components/localthings/registry/capabilities/air_purifier.py index b497628..8490d8c 100644 --- a/custom_components/localthings/registry/capabilities/air_purifier.py +++ b/custom_components/localthings/registry/capabilities/air_purifier.py @@ -107,37 +107,13 @@ def _has_top_level_modes(rep, resources): # reading is a µg/m³ concentration, and the dumps never say so. That's a # separate call from making the series recordable at all. _AIR_QUALITY_SENSORS = ( - ("dust", "mdi:blur", "Dust"), - ("fine_dust", "mdi:blur", "FineDust"), - ("super_fine_dust", "mdi:blur", "SuperFineDust"), - ("odor", "mdi:scent", "Odor"), - ("clean_level", "mdi:air-filter", "CleanLevel"), + ("dust", "mdi:blur", "Dust", "measurement"), + ("fine_dust", "mdi:blur", "FineDust", "measurement"), + ("super_fine_dust", "mdi:blur", "SuperFineDust", "measurement"), + ("odor", "mdi:scent", "Odor", None), + ("clean_level", "mdi:air-filter", "CleanLevel", None), ) -# Sensors that get a state_class, which is what makes Home Assistant keep -# long-term statistics -- without one a reading only lives in the short-term -# recorder history and is dropped at the next purge (10 days by default), so it -# can't back a long-range air-quality graph. Kept as a key set rather than a -# fourth column because air_monitor.py imports the tuple above and unpacks it -# as a triple. -# -# Only the three particulate readings are listed. They fall monotonically with -# particle size on three independent board families -- 11/9/5 on ARTIK051_TVTL -# (issue #56), 10/9/6 on AVT-WW-TP1 (issue #190), 18/14/9 on the range hood -- -# which is concentration behaviour, and an average over time is meaningful for -# it. Odor and CleanLevel read 0-2 on every fixture and look like graded -# indices instead, where the mean of a grade isn't obviously meaningful. -# -# Note air_monitor.SENSORS does stamp all five, and its docstring says it is -# "matching air_purifier.AIR_QUALITY's existing precedent" -- a precedent this -# module did not actually set. Extending to all five here is a one-line change -# if consistency is preferred over the grade/concentration split. -# -# Deliberately no device_class/unit: pm1/pm25/pm10 would assert the reading is -# a µg/m³ concentration, and no dump says so. That is a separate call from -# making the series recordable at all. -_RECORDED_AIR_QUALITY = frozenset({"dust", "fine_dust", "super_fine_dust"}) - AIR_QUALITY = Capability( href="/sensors/vs/0", poll_tier="warm", @@ -146,10 +122,10 @@ AIR_QUALITY = Capability( key=key, field="x.com.samsung.da.items", icon=icon, - state_class="measurement" if key in _RECORDED_AIR_QUALITY else None, + state_class=state_class, value_fn=lambda items, t=sensor_type: sensor_item_value(items, t), ) - for key, icon, sensor_type in _AIR_QUALITY_SENSORS + for key, icon, sensor_type, state_class in _AIR_QUALITY_SENSORS ), ) diff --git a/tests/test_air_purifier_air_quality_statistics.py b/tests/test_air_purifier_air_quality_statistics.py index 74376fa..48c97b0 100644 --- a/tests/test_air_purifier_air_quality_statistics.py +++ b/tests/test_air_purifier_air_quality_statistics.py @@ -7,6 +7,7 @@ possible -- that is the bug this guards against reappearing. """ from custom_components.localthings.registry.capabilities import air_purifier +from custom_components.localthings.registry.entities import SensorDesc PARTICULATE = ("dust", "fine_dust", "super_fine_dust") GRADED = ("odor", "clean_level") @@ -39,18 +40,28 @@ def test_no_unit_or_device_class_is_asserted(): assert desc.device_class is None, key -def test_shared_sensor_tuple_keeps_its_three_column_shape(): - """air_monitor.py imports _AIR_QUALITY_SENSORS and unpacks it as a triple, - so widening the tuple here breaks that module's import outright.""" +def test_state_class_comes_from_the_shared_tuples_fourth_column(): + """The rows carry their own state_class rather than a parallel lookup, so + a new sensor can't be added here without deciding the question.""" for row in air_purifier._AIR_QUALITY_SENSORS: - assert len(row) == 3, row + assert len(row) == 4, row + assert row[3] in ("measurement", None), row -def test_air_monitor_still_imports(): - """Guard the coupling above end to end, not just by row width.""" +def test_air_monitor_keeps_stamping_every_shared_sensor(): + """air_monitor imports _AIR_QUALITY_SENSORS and discards the fourth column + on purpose: that board (issue #210) has stamped all five as `measurement` + since it was added, and consuming the column would silently drop long-term + statistics for Odor/CleanLevel there. Guards the import end to end and the + deliberate divergence together.""" from custom_components.localthings.registry.capabilities import air_monitor assert air_monitor.SENSORS.href == "/sensors/vs/0" + for key in PARTICULATE + GRADED: + desc = next( + d for d in air_monitor.SENSORS.entities if d.key == key and isinstance(d, SensorDesc) + ) + assert desc.state_class == "measurement", key def test_every_air_quality_sensor_still_reads_a_plain_int():