review: unfold AI Purify into one entity per field
Review feedback on #268. The largest change is that the sensing-mode select no longer folds two device fields into one control. periodicSensingActivationState and autoExeState are independent knobs, and the appliance presents them that way -- its own UI has an on/off for AI Purify separately from the three mode choices. Folding them lost two things: a configured action was invisible while the feature was off, and no select option could toggle the feature without also overwriting the action. The switch was not the duplicate it looked like. So the switch now owns periodicSensingActivationState alone, and the select owns autoExeState alone. That resolves the hardcoded-options finding at the source rather than working around it: the select reads supportedAutoExeState via options_field -- the same shape SOUND_MODE already uses for supportedModes -- instead of carrying a typed-in tuple, so a board advertising a fourth action is accepted on both the options list and the write path. _sensing_mode, _sensing_mode_write and _SENSING_MODE_BODIES are all gone with the fold. Option slugs are now the advertised values lowercased (off / airpurify / alarm) rather than invented names. The catalog carries the labels, so the two 'off's stay distinguishable in the UI: the switch's means the unit isn't sampling, the select's means it samples and doesn't act on the reading -- what the app calls "sensing only". Also from the review: * _interval_minutes checks `is None` so a reported 0 stays 0, and native_min drops to 0 since sub-30s values round there. oven.cook_time and operational's delay hours are the precedent -- both convert a device time value and floor at zero. The Number-rather-than-Select choice is now stated in the write helper: the app offers three fixed intervals, but this resource advertises no supported-values or range field (supportedAutoExeState sits right beside it, so the board does advertise constraints where it has them) and it accepted 60 s, six times finer than the app's smallest choice. * _skip_time_write no longer splices a malformed half back onto the wire. The read side already refuses one it can't parse; the write side now zeroes it to match. * air_sensing_state and last_air_sensing_level lose enabled_default=False, matching range_hood.AIR_LEVEL_CHECK. Hiding two of three read-only keys while claiming key parity with that capability -- and leaving the third visible -- had no justification behind it. * The catalog-parity test drops periodic_air_sensing from its key set: that key is a SwitchDesc here and a BinarySensorDesc on the hood, so the two live in different platform catalogs and are worded differently. The claim now covers only the three read-only sensor keys, where it holds. * Tests route through the descriptors (_desc(key).write_fn / .value_fn) rather than module-private helpers, matching test_air_monitor_capabilities. startSensingOnce stays unbound, now explicitly rather than by omission -- the module comment records it as deferred. It looks like a one-shot "sense now" button, but this board acknowledges writes it discards, and nothing has confirmed the side effect yet. Goldens are untouched: the key set is unchanged, only sensing_mode's value moves from the folded slug to the raw autoExeState.
This commit is contained in:
1 parent
412fff9b99
commit
a5484f746a
7 files changed
+217
-193
No files matched your search
@@ -3,8 +3,8 @@ sensing engine (issues #84 and #190).
|
||||
|
||||
The resource is reported by three of this registry's four board families, so
|
||||
the read assertions run against each family's own fixture; the write contracts
|
||||
were exercised on AVT-WW-TP1-23-AXX500 hardware and are asserted here at the
|
||||
body level.
|
||||
were exercised on AVT-WW-TP1-23-AXX500 hardware and are asserted here through
|
||||
the descriptors that carry them.
|
||||
"""
|
||||
|
||||
import datetime
|
||||
@@ -19,6 +19,12 @@ from tests.conftest import _load_device
|
||||
# ARTIK051_TVTL family, issue #56) has no such href and is deliberately absent.
|
||||
FAMILIES = ("air_purifier_avt_ww", "air_purifier_vtww", "air_purifier_tp1x_da_ac_air")
|
||||
|
||||
HREF = ["airlevelcheck", "vs", "0"]
|
||||
|
||||
|
||||
def _rep(fixture):
|
||||
return _load_device(fixture)["/airlevelcheck/vs/0"]
|
||||
|
||||
|
||||
def _state(fixture):
|
||||
resources = _load_device(fixture)
|
||||
@@ -63,14 +69,6 @@ def test_tvtl_family_is_untouched():
|
||||
assert key not in state, key
|
||||
|
||||
|
||||
def test_sensing_interval_only_where_the_field_exists():
|
||||
"""TP1X_DA-AC-AIR (issue #130) omits periodicSensingInterval; the other two
|
||||
report it. The entity must follow the field, not the href."""
|
||||
assert "sensing_interval" in _state("air_purifier_avt_ww")
|
||||
assert "sensing_interval" in _state("air_purifier_vtww")
|
||||
assert "sensing_interval" not in _state("air_purifier_tp1x_da_ac_air")
|
||||
|
||||
|
||||
def test_no_unbound_hrefs_on_any_reporting_family():
|
||||
for fixture in FAMILIES:
|
||||
resources = _load_device(fixture)
|
||||
@@ -81,78 +79,87 @@ def test_no_unbound_hrefs_on_any_reporting_family():
|
||||
assert unbound == [], f"{fixture}: {unbound}"
|
||||
|
||||
|
||||
def test_sensing_mode_folds_toggle_and_action():
|
||||
"""Off beats any pending auto-action; On with no action is 'sensing only'."""
|
||||
assert (
|
||||
air_purifier._sensing_mode(
|
||||
{
|
||||
"x.com.samsung.da.periodicSensingActivationState": "Off",
|
||||
"x.com.samsung.da.autoExeState": "Airpurify",
|
||||
}
|
||||
)
|
||||
== "off"
|
||||
)
|
||||
assert (
|
||||
air_purifier._sensing_mode(
|
||||
{
|
||||
"x.com.samsung.da.periodicSensingActivationState": "On",
|
||||
"x.com.samsung.da.autoExeState": "Off",
|
||||
}
|
||||
)
|
||||
== "sensing_only"
|
||||
)
|
||||
assert (
|
||||
air_purifier._sensing_mode(
|
||||
{
|
||||
"x.com.samsung.da.periodicSensingActivationState": "On",
|
||||
"x.com.samsung.da.autoExeState": "Airpurify",
|
||||
}
|
||||
)
|
||||
== "auto_purify"
|
||||
)
|
||||
assert (
|
||||
air_purifier._sensing_mode(
|
||||
{
|
||||
"x.com.samsung.da.periodicSensingActivationState": "On",
|
||||
"x.com.samsung.da.autoExeState": "Alarm",
|
||||
}
|
||||
)
|
||||
== "st_alarm"
|
||||
)
|
||||
# --- the two knobs are separate entities, not one folded control -------------
|
||||
|
||||
|
||||
def test_sensing_mode_write_sets_both_fields_in_one_body():
|
||||
href, body = air_purifier._sensing_mode_write("auto_purify", {})
|
||||
assert href == ["airlevelcheck", "vs", "0"]
|
||||
assert body == {
|
||||
"x.com.samsung.da.periodicSensingActivationState": "On",
|
||||
"x.com.samsung.da.autoExeState": "Airpurify",
|
||||
}
|
||||
# 'off' only needs the toggle -- the pending action is preserved.
|
||||
assert air_purifier._sensing_mode_write("off", {})[1] == {
|
||||
"x.com.samsung.da.periodicSensingActivationState": "Off"
|
||||
}
|
||||
assert air_purifier._sensing_mode_write("nonsense", {}) is None
|
||||
def test_activation_and_action_are_separate_entities():
|
||||
"""The resource carries an on/off and an action as independent fields, and
|
||||
the appliance's own UI presents them that way. Folding them into one
|
||||
control would make a configured action invisible while the feature is off,
|
||||
and would leave no way to toggle the feature without overwriting it."""
|
||||
assert _desc("periodic_air_sensing").field == (
|
||||
"x.com.samsung.da.periodicSensingActivationState"
|
||||
)
|
||||
assert _desc("sensing_mode").field == "x.com.samsung.da.autoExeState"
|
||||
|
||||
|
||||
def test_sensing_mode_write_does_not_mutate_the_shared_body_table():
|
||||
before = dict(air_purifier._SENSING_MODE_BODIES["auto_purify"])
|
||||
_, body = air_purifier._sensing_mode_write("auto_purify", {})
|
||||
body["x.com.samsung.da.autoExeState"] = "clobbered"
|
||||
assert air_purifier._SENSING_MODE_BODIES["auto_purify"] == before
|
||||
def test_action_options_come_from_the_device_not_a_table():
|
||||
"""supportedAutoExeState is advertised on every reporting fixture, so the
|
||||
select reads it rather than carrying a typed-in tuple -- a board adding a
|
||||
fourth action is then accepted rather than rejected."""
|
||||
desc = _desc("sensing_mode")
|
||||
assert desc.options_field == "x.com.samsung.da.supportedAutoExeState"
|
||||
assert not desc.options, "options must come from the device, not a static tuple"
|
||||
for fixture in FAMILIES:
|
||||
assert _rep(fixture)["x.com.samsung.da.supportedAutoExeState"] == [
|
||||
"Off",
|
||||
"Airpurify",
|
||||
"Alarm",
|
||||
], fixture
|
||||
|
||||
|
||||
def test_action_write_sends_the_raw_advertised_value():
|
||||
"""select.py maps the chosen option back to the device's own casing before
|
||||
calling write_fn, so the body is the advertised token verbatim."""
|
||||
for raw in ("Off", "Airpurify", "Alarm"):
|
||||
href, body = _desc("sensing_mode").write_fn(raw, {})
|
||||
assert href == HREF
|
||||
assert body == {"x.com.samsung.da.autoExeState": raw}
|
||||
|
||||
|
||||
def test_activation_write_leaves_the_action_alone():
|
||||
"""Toggling the feature must not disturb autoExeState -- that's what makes
|
||||
the switch able to do something the action select can't."""
|
||||
for payload, expected in (("On", "On"), ("Off", "Off")):
|
||||
href, body = _desc("periodic_air_sensing").write_fn(payload, {})
|
||||
assert href == HREF
|
||||
assert body == {"x.com.samsung.da.periodicSensingActivationState": expected}
|
||||
assert "x.com.samsung.da.autoExeState" not in body
|
||||
|
||||
|
||||
# --- interval ----------------------------------------------------------------
|
||||
|
||||
|
||||
def test_sensing_interval_only_where_the_field_exists():
|
||||
"""TP1X_DA-AC-AIR (issue #130) omits periodicSensingInterval; the other two
|
||||
report it. The entity must follow the field, not the href."""
|
||||
assert "sensing_interval" in _state("air_purifier_avt_ww")
|
||||
assert "sensing_interval" in _state("air_purifier_vtww")
|
||||
assert "sensing_interval" not in _state("air_purifier_tp1x_da_ac_air")
|
||||
|
||||
|
||||
def test_interval_is_minutes_in_the_ui_and_seconds_on_the_wire():
|
||||
assert air_purifier._interval_minutes("600") == 10
|
||||
assert air_purifier._interval_minutes(None) is None
|
||||
assert air_purifier._interval_write(10, {})[1] == {
|
||||
"x.com.samsung.da.periodicSensingInterval": "600"
|
||||
}
|
||||
desc = _desc("sensing_interval")
|
||||
assert desc.value_fn("600") == 10
|
||||
assert desc.write_fn(10, {})[1] == {"x.com.samsung.da.periodicSensingInterval": "600"}
|
||||
|
||||
|
||||
def test_interval_keeps_zero_distinct_from_unknown():
|
||||
"""`if secs` would fold a reported 0 into None; sub-30s values round to 0,
|
||||
which is why native_min is 0 rather than 1."""
|
||||
desc = _desc("sensing_interval")
|
||||
assert desc.value_fn("0") == 0
|
||||
assert desc.value_fn("20") == 0
|
||||
assert desc.value_fn(None) is None
|
||||
assert desc.native_min == 0
|
||||
|
||||
|
||||
# --- skip window -------------------------------------------------------------
|
||||
|
||||
|
||||
def test_skip_time_splits_the_hhmmhhmm_window():
|
||||
read_start = air_purifier._skip_time_read("start")
|
||||
read_end = air_purifier._skip_time_read("end")
|
||||
read_start = _desc("sensing_skip_start").value_fn
|
||||
read_end = _desc("sensing_skip_end").value_fn
|
||||
# Issue #190's unit ships a real window: 03:00-23:00.
|
||||
assert read_start("03002300") == datetime.time(3, 0)
|
||||
assert read_end("03002300") == datetime.time(23, 0)
|
||||
@@ -166,27 +173,36 @@ def test_skip_time_splits_the_hhmmhhmm_window():
|
||||
|
||||
def test_skip_time_write_preserves_the_other_half():
|
||||
rep = {"x.com.samsung.da.periodicSensingSkipTime": "03002300"}
|
||||
_, body = air_purifier._skip_time_write("start")(datetime.time(7, 30), rep)
|
||||
_, body = _desc("sensing_skip_start").write_fn(datetime.time(7, 30), rep)
|
||||
assert body == {"x.com.samsung.da.periodicSensingSkipTime": "07302300"}
|
||||
_, body = air_purifier._skip_time_write("end")(datetime.time(22, 5), rep)
|
||||
_, body = _desc("sensing_skip_end").write_fn(datetime.time(22, 5), rep)
|
||||
assert body == {"x.com.samsung.da.periodicSensingSkipTime": "03002205"}
|
||||
# A board that has never had a window set still round-trips.
|
||||
_, body = air_purifier._skip_time_write("end")(datetime.time(1, 2), {})
|
||||
_, body = _desc("sensing_skip_end").write_fn(datetime.time(1, 2), {})
|
||||
assert body == {"x.com.samsung.da.periodicSensingSkipTime": "00000102"}
|
||||
|
||||
|
||||
def test_periodic_sensing_and_skip_switch_bodies():
|
||||
assert air_purifier._periodic_sensing_write("On", {})[1] == {
|
||||
"x.com.samsung.da.periodicSensingActivationState": "On"
|
||||
}
|
||||
assert air_purifier._periodic_sensing_write("Off", {})[1] == {
|
||||
"x.com.samsung.da.periodicSensingActivationState": "Off"
|
||||
}
|
||||
assert air_purifier._skip_status_write("On", {})[1] == {
|
||||
def test_skip_time_write_normalizes_a_half_it_cannot_parse():
|
||||
"""Padding alone would splice a malformed half straight back onto the wire.
|
||||
The read side already refuses one, so the write side zeroes it instead of
|
||||
echoing junk to the device."""
|
||||
junk = {"x.com.samsung.da.periodicSensingSkipTime": "0730zzzz"}
|
||||
_, body = _desc("sensing_skip_start").write_fn(datetime.time(8, 0), junk)
|
||||
assert body == {"x.com.samsung.da.periodicSensingSkipTime": "08000000"}
|
||||
junk = {"x.com.samsung.da.periodicSensingSkipTime": "zzzz2200"}
|
||||
_, body = _desc("sensing_skip_end").write_fn(datetime.time(23, 0), junk)
|
||||
assert body == {"x.com.samsung.da.periodicSensingSkipTime": "00002300"}
|
||||
|
||||
|
||||
def test_skip_status_switch_body():
|
||||
assert _desc("periodic_sensing_skip_status").write_fn("On", {})[1] == {
|
||||
"x.com.samsung.da.periodicSensingSkipStatus": "On"
|
||||
}
|
||||
|
||||
|
||||
# --- read-only diagnostics ---------------------------------------------------
|
||||
|
||||
|
||||
def test_last_sensing_time_reads_as_utc():
|
||||
state = _state("air_purifier_avt_ww")
|
||||
assert state["last_air_sensing_time"].tzinfo is not None
|
||||
@@ -194,9 +210,14 @@ def test_last_sensing_time_reads_as_utc():
|
||||
|
||||
|
||||
def test_read_only_keys_match_the_range_hood_capability():
|
||||
"""Same href, same fields -- the keys are shared deliberately so both
|
||||
families read from one translation catalog entry. If either side renames
|
||||
one, this catches the drift."""
|
||||
"""The three read-only sensor keys are shared with the hood deliberately,
|
||||
so both families read from one translation catalog entry each. If either
|
||||
side renames one, this catches the drift.
|
||||
|
||||
periodic_air_sensing is excluded: it's a SwitchDesc here and a
|
||||
BinarySensorDesc on the hood, so the two sit in different platform
|
||||
catalogs and are worded differently -- see the next test.
|
||||
"""
|
||||
from custom_components.localthings.registry.capabilities import range_hood
|
||||
|
||||
hood = {d.key for d in range_hood.AIR_LEVEL_CHECK.entities}
|
||||
@@ -205,7 +226,6 @@ def test_read_only_keys_match_the_range_hood_capability():
|
||||
"air_sensing_state",
|
||||
"last_air_sensing_time",
|
||||
"last_air_sensing_level",
|
||||
"periodic_air_sensing",
|
||||
} <= hood & ours
|
||||
|
||||
|
||||
@@ -222,3 +242,10 @@ def test_periodic_air_sensing_is_writable_here_and_read_only_on_hoods():
|
||||
ours = _desc("periodic_air_sensing")
|
||||
assert isinstance(ours, SwitchDesc)
|
||||
assert ours.write_fn is not None
|
||||
|
||||
|
||||
def test_read_only_diagnostics_match_the_hood_on_visibility():
|
||||
"""The hood leaves all three enabled; asserting key parity with it while
|
||||
hiding two of them would be a quiet divergence."""
|
||||
for key in ("air_sensing_state", "last_air_sensing_time", "last_air_sensing_level"):
|
||||
assert _desc(key).enabled_default is True, key
|
||||
Reference in new issue
Block a user