diff --git a/custom_components/localthings/registry/capabilities/range_hood.py b/custom_components/localthings/registry/capabilities/range_hood.py index f579c0f..8854dc4 100644 --- a/custom_components/localthings/registry/capabilities/range_hood.py +++ b/custom_components/localthings/registry/capabilities/range_hood.py @@ -181,12 +181,18 @@ HOOD_FILTER = Capability( # After Run (issue #147): the hood keeps the fan running at low speed for a -# while after it's switched off, to clear residual cooking smoke. No +# while after it's switched off, to clear residual cooking smoke -- a +# feature a user actively watches and cancels, not passive diagnostics, so +# none of the three entities below carry entity_category. No # supported-values list is advertised for activationState, so it's modeled # read-only (monitoring, not an invented "enable" write) per the 'don't # guess' rule; runningCancel's only observed value is the command name # itself ('Cancel'), the same self-describing command-field shape as -# operational.STOP_BUTTON. +# operational.STOP_BUTTON. runningProgress is left a bare passthrough for the +# same 'don't guess' reason -- the only observed sample is "0", with no +# range or supported-values field to confirm it's a percentage rather than a +# minutes/seconds count, and unit + state_class='measurement' would commit +# it to long-term statistics under a possibly-wrong unit. AFTER_RUN = Capability( href='/afterrun/vs/0', poll_tier='warm', @@ -195,17 +201,12 @@ AFTER_RUN = Capability( key='after_run_active', field='x.com.samsung.da.activationState', icon='mdi:fan-clock', - entity_category='diagnostic', value_fn=lambda value: str(value).lower() == 'on', ), SensorDesc( key='after_run_progress', field='x.com.samsung.da.runningProgress', - unit='%', - state_class='measurement', icon='mdi:fan-clock', - entity_category='diagnostic', - value_fn=int_or_none, ), ButtonDesc( key='after_run_cancel', diff --git a/custom_components/localthings/registry/capabilities/water_purifier.py b/custom_components/localthings/registry/capabilities/water_purifier.py index 08b7346..019517c 100644 --- a/custom_components/localthings/registry/capabilities/water_purifier.py +++ b/custom_components/localthings/registry/capabilities/water_purifier.py @@ -105,6 +105,29 @@ FAVORITE_CAPACITY = Capability( # Coffee-capable variant (issue #107) -- a "favorite" supported-list select # for the hot water dispensed alongside brewing, same shape as # FAVORITE_CAPACITY above. + + +def _status_lock_definitely_lacks_hotwater_field(resources: dict) -> bool: + """Three-way read of /status/lock/vs/0's hotwaterLock field, favouring + LOCK.hotwater_lock (the primary descriptor) whenever the outcome is + still ambiguous: + + - href entirely absent from this device -> definitely no clash, the + switchHotwater fallback below may claim the entity. + - href present but an unfetched stub ({}) -> outcome pending, *not* a + confirmed absence. LOCK's own exists_fn optimistically includes itself + through a stub (matching entity.py's default), so returning True here + too would register both descriptors -- as SwitchDescs sharing one key, + with identical unique_ids -- until the next poll resolves it. + - href present and fetched -> the real answer.""" + rep = resources.get('/status/lock/vs/0') + if rep is None: + return True + if not rep: + return False + return 'x.com.samsung.da.hotwaterLock' not in rep + + FAVORITE_HOTWATER = Capability( href='/favorite/hotwater/vs/0', poll_tier='cold', @@ -114,15 +137,26 @@ FAVORITE_HOTWATER = Capability( # hot-water lock as LOCK.hotwater_lock below, just surfaced through # this href on boards that don't populate /status/lock/vs/0's # hotwaterLock field. Shares that descriptor's key so only one "Hot - # water lock" entity ever appears; exists_fn activates this fallback - # only when the primary field is absent, so a board reporting both - # can't collide. + # water lock" entity ever appears. + # + # Both halves of this fallback pair need an exists_fn, not just this + # one: adapter.flatten() (the coordinator.data source every entity's + # is_on reads) only ever honours exists_fn, never entity.py's + # implicit "require own field present" default that gates plain + # registration. Two same-keyed descriptors with only one of them + # gated still both land in flatten()'s output dict -- whichever is + # processed last silently wins, decided by device-reported href + # order, not by which one is actually correct. So this exists_fn + # also re-asserts its own field's presence (switchHotwater), the + # gate a bare `field=` used to get for free before it had to share a + # key with LOCK's descriptor. SwitchDesc(key='hotwater_lock', field='x.com.samsung.da.switchHotwater', device_class='lock', entity_category='config', value_fn=lambda v: v != 'Unlocked', - exists_fn=lambda rep, resources: 'x.com.samsung.da.hotwaterLock' not in ( - resources.get('/status/lock/vs/0') or {}), + exists_fn=lambda rep, resources: ( + 'x.com.samsung.da.switchHotwater' in rep + and _status_lock_definitely_lacks_hotwater_field(resources)), write_fn=lambda p, rep, href=None: ( ['favorite', 'hotwater', 'vs', '0'], {'x.com.samsung.da.switchHotwater': 'Locked' if p == 'On' else 'Unlocked'})), @@ -160,10 +194,19 @@ LOCK = Capability( href='/status/lock/vs/0', poll_tier='warm', entities=( + # Shares its key with FAVORITE_HOTWATER's switchHotwater fallback + # above (issue #144); see the comment there for why this half also + # needs an explicit exists_fn now that the two share a key in + # adapter.flatten()'s output. A stub rep ({}) still counts as + # "present" here (matches entity.py's own default for a field-less + # gate) since the alternative -- treating an unfetched resource as + # confirmed-absent -- is what let both descriptors register at once. SwitchDesc(key='hotwater_lock', field='x.com.samsung.da.hotwaterLock', device_class='lock', entity_category='config', value_fn=lambda v: v != 'Unlocked', + exists_fn=lambda rep, resources: ( + not rep or 'x.com.samsung.da.hotwaterLock' in rep), write_fn=lambda p, rep, href=None: ( ['status', 'lock', 'vs', '0'], {'x.com.samsung.da.hotwaterLock': 'Locked' if p == 'On' else 'Unlocked'})), diff --git a/tests/test_range_hood_capabilities.py b/tests/test_range_hood_capabilities.py index 7727d49..bca96fa 100644 --- a/tests/test_range_hood_capabilities.py +++ b/tests/test_range_hood_capabilities.py @@ -62,7 +62,7 @@ def test_range_hood_fixture_values(): assert state['air_sensing_state'] == 'NonProcessing' assert state['last_air_sensing_level'] == 'Kr2' assert state['after_run_active'] is False - assert state['after_run_progress'] == 0 + assert state['after_run_progress'] == '0' def test_one_composite_fan_is_bound(): diff --git a/tests/test_water_purifier_capabilities.py b/tests/test_water_purifier_capabilities.py index bf5c9dc..0e2ef53 100644 --- a/tests/test_water_purifier_capabilities.py +++ b/tests/test_water_purifier_capabilities.py @@ -216,14 +216,69 @@ def test_favorite_hotwater_switch_is_a_lock_not_an_enable_flag(): assert lock.value_fn('Locked') is True -def test_favorite_hotwater_lock_fallback_only_activates_when_primary_absent(): - """The switchHotwater-based lock and LOCK's hotwaterLock-based lock share - the 'hotwater_lock' key so only one entity is ever registered (issue - #144). This fixture has no hotwaterLock field, so the fallback must be - active; a board reporting both must not.""" - lock = _desc_coffee_by_href('hotwater_lock', '/favorite/hotwater/vs/0') - assert lock.exists_fn({}, {'/status/lock/vs/0': {}}) is True - assert lock.exists_fn({}, {'/status/lock/vs/0': {'x.com.samsung.da.hotwaterLock': 'Unlocked'}}) is False +def test_favorite_hotwater_lock_wins_in_flattened_state(): + """adapter.flatten() -- the actual source of coordinator.data every + switch's is_on reads -- only honours exists_fn, never entity.py's + implicit own-field-presence default. So it's not enough for the + *registered* entity to resolve correctly (test_expected_state_keys_present + territory); the shared 'hotwater_lock' key in the flattened dict itself + must reflect the live switchHotwater value, not a stale phantom from + LOCK's ungated hotwaterLock read (issue #144). This fixture's + switchHotwater reads 'Unlocked'; without exists_fn on *both* sides of the + pair, LOCK's descriptor computes None != 'Unlocked' == True regardless, + and flatten() would pick whichever of the two entities happens to be + processed last.""" + state = _state_coffee() + assert state['hotwater_lock'] is False + + +def test_exactly_one_hotwater_lock_descriptor_exists_per_resource_state(): + """Both LOCK.hotwater_lock and FAVORITE_HOTWATER's switchHotwater + fallback are always bound on this fixture (their hrefs are both always + present) -- discrimination happens entirely in exists_fn. Exactly one of + the two must ever pass, regardless of iteration order, or two switch + entities would be registered with the same unique_id.""" + bound, resources = _bound_coffee() + candidates = [b for b in bound if b.desc.key == 'hotwater_lock'] + assert len(candidates) == 2 + included = [b for b in candidates + if b.desc.exists_fn(resources.get(b.href) or {}, resources)] + assert len(included) == 1 + assert included[0].href == '/favorite/hotwater/vs/0' + + +def test_hotwater_lock_fallback_gating_across_status_lock_states(): + """The fallback (FAVORITE_HOTWATER's switchHotwater descriptor) must + activate only once /status/lock/vs/0 is confirmed to lack hotwaterLock -- + never while that resource is an unfetched stub ({}), and never when it + does carry the field. LOCK's own descriptor is the mirror image.""" + fallback = _desc_coffee_by_href('hotwater_lock', '/favorite/hotwater/vs/0') + primary = _desc_coffee_by_href('hotwater_lock', '/status/lock/vs/0') + own_rep = {'x.com.samsung.da.switchHotwater': 'Unlocked'} + + # /status/lock/vs/0 absent entirely -- device genuinely lacks it. + assert fallback.exists_fn(own_rep, {}) is True + + # /status/lock/vs/0 present but not yet fetched (a stub): outcome + # pending, so the fallback must defer to LOCK rather than assume absence. + stub_resources = {'/status/lock/vs/0': {}} + assert fallback.exists_fn(own_rep, stub_resources) is False + assert primary.exists_fn({}, stub_resources) is True + + # /status/lock/vs/0 fetched and confirmed to lack hotwaterLock (this + # fixture's actual shape) -- the fallback wins. + absent_resources = {'/status/lock/vs/0': {'x.com.samsung.da.coldwaterLock': 'Unlocked'}} + assert fallback.exists_fn(own_rep, absent_resources) is True + assert primary.exists_fn(absent_resources['/status/lock/vs/0'], absent_resources) is False + + # /status/lock/vs/0 fetched and does carry hotwaterLock -- primary wins. + present_resources = {'/status/lock/vs/0': {'x.com.samsung.da.hotwaterLock': 'Unlocked'}} + assert fallback.exists_fn(own_rep, present_resources) is False + assert primary.exists_fn(present_resources['/status/lock/vs/0'], present_resources) is True + + # The fallback also re-asserts its own field, since it no longer gets + # that check for free once it shares LOCK's key (issue #144 review). + assert fallback.exists_fn({}, {}) is False def test_favorite_hotwater_temperature_options_come_from_live_supported_list():