Address review: make the AddWash master switch's On idempotent

Home Assistant calls turn_on regardless of current state, and applying a
scene re-asserts every captured state, so asserting the alarm on over a
rinse-only AddWashSet_1 rewrote it to _7 -- silently widening the moments
the user picked, with no state change on this switch to point at it. The
write is now refused when the mask is already non-zero.

Distinct from the off-then-on path documented in _add_wash_bit_switch,
where the appliance has no subset left to keep.

Also satisfies the ty check on the new tests: resolve(),
for_device_by_model() and exists_fn are all optional, asserted the way
the other capability tests do.
This commit is contained in:
Marek Tyburec
2026-08-20 06:56:47 +02:00
parent 51bce3c8cf
commit 6c980f927b
2 changed files with 38 additions and 7 deletions
@@ -313,7 +313,16 @@ def _add_wash_alarm_write(p, rep, href=None):
# Gated on the mask being readable, like the per-moment writes: a device # Gated on the mask being readable, like the per-moment writes: a device
# reporting a wider mask than these three bits would otherwise have it # reporting a wider mask than these three bits would otherwise have it
# truncated to 7 here, silently dropping a moment it supports. # truncated to 7 here, silently dropping a moment it supports.
if p not in ("On", "Off") or _add_wash_mask(rep, "AddWashSet") is None: mask = _add_wash_mask(rep, "AddWashSet")
if p not in ("On", "Off") or mask is None:
return None
if p == "On" and mask:
# Already on, so "on" is a no-op rather than a rewrite to 7. Home
# Assistant calls turn_on regardless of current state, so an
# automation asserting the alarm on over a rinse-only mask would
# otherwise widen it to all three moments with no state change on
# this switch to point at. Distinct from the off-then-on case in
# _add_wash_bit_switch, where there is no subset left to keep.
return None return None
return _add_wash_set_write(0b111 if p == "On" else 0) return _add_wash_set_write(0b111 if p == "On" else 0)
+28 -6
View File
@@ -53,9 +53,17 @@ def _options(result):
return body["x.com.samsung.da.options"] return body["x.com.samsung.da.options"]
def _exists(key, rep):
"""Whether `key`'s descriptor gates itself in for `rep`."""
desc = next(e for e in washer.WASHER_COURSE.entities if e.key == key)
assert desc.exists_fn is not None
return desc.exists_fn(rep, {})
def _flatten(fixture): def _flatten(fixture):
resources = _load_device(fixture) resources = _load_device(fixture)
reg = resolve(resources) reg = resolve(resources)
assert reg is not None
return flatten(discover(resources, reg.capabilities, reg.pattern_capabilities), resources) return flatten(discover(resources, reg.capabilities, reg.pattern_capabilities), resources)
@@ -78,6 +86,20 @@ class TestAlarmMasterSwitch:
desc = _desc("add_wash_alarm", SwitchDesc) desc = _desc("add_wash_alarm", SwitchDesc)
assert _options(_write(desc, "Off", _rep("AddWashSet_5"))) == ["AddWashSet_0"] assert _options(_write(desc, "Off", _rep("AddWashSet_5"))) == ["AddWashSet_0"]
@pytest.mark.parametrize("mask", range(1, 8))
def test_on_over_an_alarm_already_on_keeps_the_chosen_moments(self, mask):
"""Home Assistant calls turn_on regardless of current state, so
re-asserting "on" over a rinse-only mask must not widen it to all
three -- this switch reads on either way, so no state change would
point at the loss. Reaching 7 from a subset still means off, then
on."""
desc = _desc("add_wash_alarm", SwitchDesc)
rep = _rep(f"AddWashSet_{mask}")
assert desc.rep_fn(rep) is True
assert _write(desc, "On", rep) is None
assert _options(_write(desc, "Off", rep)) == ["AddWashSet_0"]
assert _options(_write(desc, "On", _rep("AddWashSet_0"))) == ["AddWashSet_7"]
def test_rejects_a_payload_that_is_not_on_or_off(self): def test_rejects_a_payload_that_is_not_on_or_off(self):
desc = _desc("add_wash_alarm", SwitchDesc) desc = _desc("add_wash_alarm", SwitchDesc)
assert _write(desc, "7", _rep("AddWashSet_0")) is None assert _write(desc, "7", _rep("AddWashSet_0")) is None
@@ -182,19 +204,17 @@ class TestCapabilityDetection:
@pytest.mark.parametrize("key,token", ENTITY_TOKENS.items()) @pytest.mark.parametrize("key,token", ENTITY_TOKENS.items())
def test_absent_on_a_washer_that_never_reports_the_token(self, key, token): def test_absent_on_a_washer_that_never_reports_the_token(self, key, token):
desc = next(e for e in washer.WASHER_COURSE.entities if e.key == key) assert _exists(key, _rep("Course_5C")) is False
assert desc.exists_fn(_rep("Course_5C"), {}) is False
@pytest.mark.parametrize("key,token", ENTITY_TOKENS.items()) @pytest.mark.parametrize("key,token", ENTITY_TOKENS.items())
def test_present_once_the_token_appears(self, key, token): def test_present_once_the_token_appears(self, key, token):
desc = next(e for e in washer.WASHER_COURSE.entities if e.key == key) assert _exists(key, _rep(f"{token}_{SAMPLE[token]}")) is True
assert desc.exists_fn(_rep(f"{token}_{SAMPLE[token]}"), {}) is True
def test_a_washer_with_only_the_indicator_gets_only_that_entity(self): def test_a_washer_with_only_the_indicator_gets_only_that_entity(self):
present = { present = {
e.key e.key
for e in washer.WASHER_COURSE.entities for e in washer.WASHER_COURSE.entities
if e.key in ENTITY_TOKENS and e.exists_fn(_rep("AddWashIndicator_On"), {}) if e.key in ENTITY_TOKENS and _exists(e.key, _rep("AddWashIndicator_On"))
} }
assert present == {"add_wash_indicator"} assert present == {"add_wash_indicator"}
@@ -216,12 +236,14 @@ class TestAgainstTheWW6500Dump:
info = _load_device("washer_ww6500")["/information/vs/0"] info = _load_device("washer_ww6500")["/information/vs/0"]
model = info["x.com.samsung.da.modelNum"] model = info["x.com.samsung.da.modelNum"]
assert for_device_by_model(model, info["x.com.samsung.da.description"]).name == "washer" reg = for_device_by_model(model, info["x.com.samsung.da.description"])
assert reg is not None and reg.name == "washer"
assert for_device_by_model(model, "") is None assert for_device_by_model(model, "") is None
def test_no_unbound_hrefs(self): def test_no_unbound_hrefs(self):
resources = _load_device("washer_ww6500") resources = _load_device("washer_ww6500")
reg = resolve(resources) reg = resolve(resources)
assert reg is not None
unbound = [] unbound = []
discover(resources, reg.capabilities, reg.pattern_capabilities, log=unbound.append) discover(resources, reg.capabilities, reg.pattern_capabilities, log=unbound.append)
assert unbound == [] assert unbound == []