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.
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
@@ -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():
|
||||
|
||||
Reference in New Issue
Block a user