From d6639c99d4f41ba693f87e07dea2928fb47a2fae Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Wed, 22 Jul 2026 19:14:52 +0000 Subject: [PATCH 1/4] Bind AI energy level on washer; switch/select split, drop translations Issue #40: /energy/ailevel/vs/0 was unbound on a plain washer. The capability already existed for fridges but was gated off entirely on single-level hardware (the common case), so it's moved to common.py (cross-family, like fridge + washer now) and split into two entities: a switch when supportedAiLevel has exactly one entry (aiLevel is really just an on/off toggle there), and a select otherwise, with '0' (off) synthesized back into the select's options since supportedAiLevel never lists it but it's a real observed value. Also drops the translation_key/strings.json entries -- aiLevel's raw digit values already render fine untranslated, and translating a handful of levels can't cover devices with more. --- .../registry/by_type/refrigerator.py | 2 +- .../localthings/registry/by_type/washer.py | 1 + .../registry/capabilities/common.py | 62 +++++++++++- .../registry/capabilities/fridge.py | 27 ------ custom_components/localthings/strings.json | 6 -- .../localthings/translations/en.json | 6 -- .../golden/refrigerator_tp1x_ref_21k_us.json | 1 + tests/fixtures/golden/washer.json | 1 + tests/fixtures/washer_device.json | 15 +++ tests/test_common_capabilities.py | 97 +++++++++++++++++++ tests/test_fridge_capabilities.py | 47 ++------- tests/test_washer_capabilities.py | 27 ++++++ 12 files changed, 212 insertions(+), 80 deletions(-) diff --git a/custom_components/localthings/registry/by_type/refrigerator.py b/custom_components/localthings/registry/by_type/refrigerator.py index 02e7e44..ecd63f6 100644 --- a/custom_components/localthings/registry/by_type/refrigerator.py +++ b/custom_components/localthings/registry/by_type/refrigerator.py @@ -20,7 +20,7 @@ REGISTRY = DeviceRegistry( dishwasher.DIAGNOSIS, fridge.ICEMAKER_NIGHTTIME, fridge.FLEX_ZONE, - fridge.AI_ENERGY_LEVEL, + common.AI_ENERGY_LEVEL, fridge.REFRIGERATION, fridge.AUTOFILL, fridge.WELCOME_LIGHTING, diff --git a/custom_components/localthings/registry/by_type/washer.py b/custom_components/localthings/registry/by_type/washer.py index 2b84126..ac46cf4 100644 --- a/custom_components/localthings/registry/by_type/washer.py +++ b/custom_components/localthings/registry/by_type/washer.py @@ -19,6 +19,7 @@ REGISTRY = DeviceRegistry( common.ALARMS, common.ENERGY_METER, common.WATER_METER, + common.AI_ENERGY_LEVEL, operational.OPERATIONAL_STATE, dishwasher.DIAGNOSIS, fridge.FIRMWARE_UPDATE, diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index edc3f6c..a7660d2 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -11,7 +11,7 @@ against live device dumps: /filter/waterfilter/vs/0 -> x.com.samsung.da.filterUsage / filterStatus """ from ..capability import Capability -from ..entities import BinarySensorDesc, SensorDesc, SwitchDesc +from ..entities import BinarySensorDesc, SelectDesc, SensorDesc, SwitchDesc def _num(v): @@ -231,3 +231,63 @@ WATER_FILTER = Capability( name='Filter status', icon='mdi:filter-check'), ), ) + +# AI energy-saving level -- '0' is off, and supportedAiLevel lists the +# additional level(s) the device offers ('1' meaning just "on" on most +# hardware, but multi-level boards have been reported). Verified cross-family: +# fridge (issue #21) and washer (issue #40) both expose this href. +# +# supportedAiLevel is a single-entry list on most captured hardware, where a +# select would offer only one real choice against an implicit "off" -- shown +# as a switch instead. '0' itself is never in supportedAiLevel but has been +# observed live as the off value of aiLevel, so the select synthesizes it +# back in as an explicit option rather than leaving no way to turn off. +# +# No translation_key: aiLevel's values are plain digit strings, and +# select.py's _display() already renders an untranslated numeric string +# as-is -- there's nothing a strings.json entry adds that's worth maintaining +# against an unknown, growing number of future levels. + + +def _ai_energy_supported_levels(rep): + """supportedAiLevel as a list -- a stray scalar (e.g. a string) must not + be len()-checked as if it were a list.""" + sl = rep.get('supportedAiLevel') + return list(sl) if isinstance(sl, (list, tuple)) else [] + + +def _ai_energy_level_options(resources): + rep = resources.get('/energy/ailevel/vs/0') or {} + return ['0', *_ai_energy_supported_levels(rep)] + + +def _ai_energy_level_write(p, rep, href=None): + return ['energy', 'ailevel', 'vs', '0'], {'aiLevel': p} + + +def _ai_energy_level_switch_write(p, rep, href=None): + levels = _ai_energy_supported_levels(rep) + on_level = levels[0] if levels else '1' + return ['energy', 'ailevel', 'vs', '0'], {'aiLevel': on_level if p == 'On' else '0'} + + +AI_ENERGY_LEVEL = Capability( + href='/energy/ailevel/vs/0', + poll_tier='cold', + entities=( + SwitchDesc(key='ai_energy_level', field='aiLevel', + name='AI energy level', icon='mdi:leaf', + entity_category='config', + value_fn=lambda v: v != '0', + exists_fn=lambda rep, resources: ( + len(_ai_energy_supported_levels(rep)) == 1), + write_fn=_ai_energy_level_switch_write), + SelectDesc(key='ai_energy_level', field='aiLevel', + name='AI energy level', icon='mdi:leaf', + entity_category='config', + options=_ai_energy_level_options, + exists_fn=lambda rep, resources: ( + not rep or len(_ai_energy_supported_levels(rep)) > 1), + write_fn=_ai_energy_level_write), + ), +) diff --git a/custom_components/localthings/registry/capabilities/fridge.py b/custom_components/localthings/registry/capabilities/fridge.py index 0a44bea..5529526 100644 --- a/custom_components/localthings/registry/capabilities/fridge.py +++ b/custom_components/localthings/registry/capabilities/fridge.py @@ -588,33 +588,6 @@ FLEX_ZONE = Capability( ), ) -# --------------------------------------------------------------------------- -# AI energy-saving level -# -# supportedAiLevel is a single-entry list ('1' only) on some hardware, where -# a select would offer no real choice — gated to >1 supported level. -# --------------------------------------------------------------------------- - -AI_ENERGY_LEVEL = Capability( - href='/energy/ailevel/vs/0', - poll_tier='cold', - entities=( - # Only levels '1'/'2' have translated labels in strings.json/translations; - # a higher supportedAiLevel entry renders as its raw value until added. - SelectDesc(key='ai_energy_level', field='aiLevel', - name='AI energy level', icon='mdi:leaf', - translation_key='ai_energy_level', - entity_category='config', - options_field='supportedAiLevel', - exists_fn=lambda rep, resources: ( - not rep or ( - isinstance((sl := rep.get('supportedAiLevel')), (list, tuple)) - and len(sl) > 1)), - write_fn=lambda p, rep, href=None: ( - ['energy', 'ailevel', 'vs', '0'], {'aiLevel': p})), - ), -) - # --------------------------------------------------------------------------- # Firmware update # --------------------------------------------------------------------------- diff --git a/custom_components/localthings/strings.json b/custom_components/localthings/strings.json index 4bba100..3347a5c 100644 --- a/custom_components/localthings/strings.json +++ b/custom_components/localthings/strings.json @@ -148,12 +148,6 @@ "180": "3 hr", "240": "4 hr" } - }, - "ai_energy_level": { - "state": { - "1": "Level 1", - "2": "Level 2" - } } }, "sensor": { diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index 4bba100..3347a5c 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -148,12 +148,6 @@ "180": "3 hr", "240": "4 hr" } - }, - "ai_energy_level": { - "state": { - "1": "Level 1", - "2": "Level 2" - } } }, "sensor": { diff --git a/tests/fixtures/golden/refrigerator_tp1x_ref_21k_us.json b/tests/fixtures/golden/refrigerator_tp1x_ref_21k_us.json index ed004a4..63b71e8 100644 --- a/tests/fixtures/golden/refrigerator_tp1x_ref_21k_us.json +++ b/tests/fixtures/golden/refrigerator_tp1x_ref_21k_us.json @@ -1,5 +1,6 @@ { "state_keys": [ + "ai_energy_level", "alarm_code", "autofill", "cooler_setpoint", diff --git a/tests/fixtures/golden/washer.json b/tests/fixtures/golden/washer.json index d3a99a1..b48cd1b 100644 --- a/tests/fixtures/golden/washer.json +++ b/tests/fixtures/golden/washer.json @@ -1,5 +1,6 @@ { "state_keys": [ + "ai_energy_level", "alarm_code", "bubble_soak", "buzzer_sound", diff --git a/tests/fixtures/washer_device.json b/tests/fixtures/washer_device.json index 2f2eece..b76d18f 100644 --- a/tests/fixtures/washer_device.json +++ b/tests/fixtures/washer_device.json @@ -44,6 +44,21 @@ "href": "/energy/consumption/0", "rep": {} }, + { + "href": "/energy/ailevel/vs/0", + "rep": { + "aiLevel": "0", + "supportedAiLevel": [ + "1" + ], + "energySavingInfo": [ + { + "aiLevel": "1", + "maxEnergySavingRate": "70" + } + ] + } + }, { "href": "/course/vs/0", "rep": { diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index f535d54..adc1431 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -130,3 +130,100 @@ class TestEnergyMeter: phantom power sensor (the exists_fn replaces the field-presence gate).""" pw = next(e for e in common.ENERGY_METER.entities if e.key == 'power_watts') assert pw.exists_fn({'x.com.samsung.da.cumulativePower': '5'}, {}) is False + + +# --------------------------------------------------------------------------- +# AI energy-saving level. '0' is off; supportedAiLevel lists the additional +# level(s) on offer. A single-entry list (issue #21 fridge, issue #40 washer) +# is really a binary toggle, so it's exposed as a switch instead of a +# one-choice select; multiple entries get a select with '0' synthesized back +# in as the explicit off option (supportedAiLevel never lists '0' itself, but +# it's a real, observed value of aiLevel). +# --------------------------------------------------------------------------- + + +class TestAiEnergyLevelSwitch: + def _desc(self): + return next(e for e in common.AI_ENERGY_LEVEL.entities + if e.__class__.__name__ == 'SwitchDesc') + + def test_href(self): + assert common.AI_ENERGY_LEVEL.href == '/energy/ailevel/vs/0' + + def test_shown_only_with_single_supported_level(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1']}, {}) is True + assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1', '2']}, {}) is False + + def test_hidden_when_supported_level_is_non_list_scalar(self): + """A stray scalar (e.g. a string) must not be len()-checked as if it + were a list -- a 2-char string would otherwise wrongly pass `== 1` + style checks.""" + desc = self._desc() + assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': '1'}, {}) is False + + def test_hidden_on_empty_stub_rep(self): + """Unlike the select, the switch requires a confirmed single-entry + list -- an unfetched stub defaults to the select branch instead (see + TestAiEnergyLevelSelect.test_shown_for_empty_stub_rep).""" + desc = self._desc() + assert desc.exists_fn({}, {}) is False + + def test_value_fn(self): + desc = self._desc() + assert desc.value_fn('0') is False + assert desc.value_fn('1') is True + + def test_write_on_uses_the_single_supported_level(self): + """The on-value is whatever the device calls its one level, not a + hardcoded '1'.""" + desc = self._desc() + path, body = desc.write_fn('On', {'supportedAiLevel': ['2']}) + assert path == ['energy', 'ailevel', 'vs', '0'] + assert body == {'aiLevel': '2'} + + def test_write_off(self): + desc = self._desc() + path, body = desc.write_fn('Off', {'supportedAiLevel': ['1']}) + assert body == {'aiLevel': '0'} + + +class TestAiEnergyLevelSelect: + def _desc(self): + return next(e for e in common.AI_ENERGY_LEVEL.entities + if e.__class__.__name__ == 'SelectDesc') + + def test_shown_only_with_multiple_supported_levels(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1', '2']}, {}) is True + assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1']}, {}) is False + + def test_shown_for_empty_stub_rep(self): + """An empty {} rep is /device/0's not-yet-fetched-stub carve-out -- + defaults to the select so sub-polls can populate it either way.""" + desc = self._desc() + assert desc.exists_fn({}, {}) is True + + def test_no_translation_key(self): + """aiLevel's values are plain digits that render fine untranslated + (select.py's _display()) -- no strings.json entry to maintain + against an unknown number of future levels.""" + desc = self._desc() + assert desc.translation_key is None + + def test_options_synthesize_off(self): + """'0' is never in supportedAiLevel but is a real, observed aiLevel + value -- synthesized back in as the explicit off option.""" + desc = self._desc() + resources = {'/energy/ailevel/vs/0': {'supportedAiLevel': ['1', '2']}} + assert desc.options(resources) == ['0', '1', '2'] + + def test_options_empty_when_resource_missing(self): + desc = self._desc() + assert desc.options({}) == ['0'] + + def test_write(self): + desc = self._desc() + path, body = desc.write_fn('2', {}) + assert path == ['energy', 'ailevel', 'vs', '0'] + assert body == {'aiLevel': '2'} diff --git a/tests/test_fridge_capabilities.py b/tests/test_fridge_capabilities.py index fa0f470..c76ddfe 100644 --- a/tests/test_fridge_capabilities.py +++ b/tests/test_fridge_capabilities.py @@ -267,48 +267,17 @@ class TestSelfCheckError: assert desc.value_fn([]) is None -class TestAiEnergyLevel: - """AI energy-saving level select on /energy/ailevel/vs/0 -- only - exposed when the device actually offers more than one level; hardware - that only ever reports a single supported level keeps this ignored - (see capabilities/ignored.py).""" +class TestRefrigeratorAiEnergyLevelFixtureCoverage: + """AI energy-saving level (common.AI_ENERGY_LEVEL, see + test_common_capabilities.py) is exercised end-to-end here against the + refrigerator fixture/registry. - def test_href(self): - assert fridge.AI_ENERGY_LEVEL.href == '/energy/ailevel/vs/0' - - def test_hidden_with_single_supported_level(self): - desc = fridge.AI_ENERGY_LEVEL.entities[0] - assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1']}, {}) is False - - def test_shown_with_multiple_supported_levels(self): - desc = fridge.AI_ENERGY_LEVEL.entities[0] - assert desc.exists_fn( - {'aiLevel': '1', 'supportedAiLevel': ['1', '2']}, {}) is True - - def test_exists_for_empty_stub_rep(self): - """An empty {} rep is /device/0's not-yet-fetched-stub carve-out -- - must be included-for-now, same as ENERGY_METER's fields.""" - desc = fridge.AI_ENERGY_LEVEL.entities[0] - assert desc.exists_fn({}, {}) is True - - def test_hidden_when_supported_level_is_non_list_scalar(self): - """A stray scalar (e.g. a string) must not be len()-checked as if it - were a list -- a 5-char string would otherwise wrongly pass `> 1`.""" - desc = fridge.AI_ENERGY_LEVEL.entities[0] - assert desc.exists_fn( - {'aiLevel': '1', 'supportedAiLevel': '12'}, {}) is False - - def test_write(self): - desc = fridge.AI_ENERGY_LEVEL.entities[0] - path, body = desc.write_fn('2', {}) - assert path == ['energy', 'ailevel', 'vs', '0'] - assert body == {'aiLevel': '2'} + refrigerator_device.json's supportedAiLevel was extended to two entries + (['1', '2']) specifically to exercise the select branch -- the real + captured TP1X_REF_21K_US dump only ever reports one entry (switch + branch), so the select is synthetic-fixture-only for now.""" def test_synthetic_fixture_has_complete_coverage(self): - """refrigerator_device.json's supportedAiLevel was extended to two - entries (['1', '2']) specifically to exercise this select -- the - real captured TP1X_REF_21K_US dump only ever reports one entry, so - this capability is synthetic-fixture-only for now.""" from custom_components.localthings.registry.adapter import flatten from custom_components.localthings.registry.by_type import refrigerator from custom_components.localthings.registry.discovery import discover diff --git a/tests/test_washer_capabilities.py b/tests/test_washer_capabilities.py index 0a1a3b4..a06243f 100644 --- a/tests/test_washer_capabilities.py +++ b/tests/test_washer_capabilities.py @@ -379,6 +379,33 @@ class TestWashOptionToggleValidation: assert desc.validate_fn('On', rep, _EDIT_COURSE_RESOURCES) is None +class TestAiEnergyLevel: + """Issue #40 -- /energy/ailevel/vs/0 was unbound on a plain washer. + + The capability itself (common.AI_ENERGY_LEVEL) is tested in + test_common_capabilities.py; this just confirms it's wired into the + washer registry and that the fixture's single-entry supportedAiLevel + (['1'], matching the issue's dump) surfaces as a switch, not a select.""" + + def test_fixture_has_complete_coverage(self): + from custom_components.localthings.registry.adapter import flatten + from custom_components.localthings.registry.by_type import washer as washer_registry + from custom_components.localthings.registry.discovery import discover + from tests.conftest import _load_device + + resources = _load_device('washer') + unbound = [] + bound = discover( + resources, + washer_registry.REGISTRY.capabilities, + washer_registry.REGISTRY.pattern_capabilities, + log=unbound.append, + ) + assert unbound == [] + state = flatten(bound, resources) + assert state['ai_energy_level'] is False # aiLevel '0' -- off + + class TestFlexWashAndComboFixturesHaveCompleteCoverage: """FlexWash (issue #19, previously unrecognized entirely) and washer/dryer combo (issue #22, dry_level) dumps must both resolve to From 3d5263d7928a280e170d4761d501c97540f8fb02 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Wed, 22 Jul 2026 19:51:37 +0000 Subject: [PATCH 2/4] Add common.UNIVERSAL/POWER bundles to stop hand-duplicating capabilities FIRMWARE_UPDATE and ALARMS were already copy-pasted into all 6 device-type registries by hand; POWER/KIDS_LOCK/REMOTE_CONTROL into 5 of 6. Consolidate into two bundles in common.py, unpacked via *common.UNIVERSAL / *common.POWER the same way ignored.IGNORED already is: - UNIVERSAL: ALARMS, ENERGY_METER, FIRMWARE_UPDATE (moved from fridge.py), SELF_CHECK (moved from fridge.py), AI_ENERGY_LEVEL, and the kids-lock/ remote-control pairs. Safe everywhere -- discover() only binds a href actually present in a device's dump, so a capability with no known conflicting family is a no-op where the href is absent and a real, wanted entity where it's present. This also broadens AI_ENERGY_LEVEL, ENERGY_METER, and SELF_CHECK to device types they weren't confirmed on before, on the same reasoning. - POWER: just POWER_GENERIC/POWER_VS_FALLBACK, applied to the 5 non-AC registries. Airconditioner keeps its own opt-out: its climate entity already owns /power/0 and /power/vs/0 via a bare, no-entity claim (airconditioner.COVERAGE), and a real power capability on the same href would make _build() raise (a href with multiple caps requires every cap to have rt_filter or match_fn; the bare COVERAGE cap has neither). Full test suite (327 tests, all 6 device-type golden fixtures) passes unchanged -- none of the newly-broadened capabilities bind on any existing fixture, confirming the no-op reasoning held in practice, not just theory. --- .../registry/by_type/airconditioner.py | 17 ++-- .../registry/by_type/dishwasher.py | 13 +-- .../localthings/registry/by_type/dryer.py | 13 +-- .../localthings/registry/by_type/oven.py | 12 +-- .../registry/by_type/refrigerator.py | 13 +-- .../localthings/registry/by_type/washer.py | 14 +-- .../registry/capabilities/airconditioner.py | 5 +- .../registry/capabilities/common.py | 86 ++++++++++++++++++- .../registry/capabilities/fridge.py | 51 +---------- tests/test_common_capabilities.py | 69 +++++++++++++++ tests/test_fridge_capabilities.py | 36 -------- 11 files changed, 180 insertions(+), 149 deletions(-) diff --git a/custom_components/localthings/registry/by_type/airconditioner.py b/custom_components/localthings/registry/by_type/airconditioner.py index 0d5f050..9de96e0 100644 --- a/custom_components/localthings/registry/by_type/airconditioner.py +++ b/custom_components/localthings/registry/by_type/airconditioner.py @@ -3,23 +3,22 @@ The first device whose core controls surface as a single composite HA `climate` entity (see capabilities/airconditioner.py and climate.py). Power/mode/temp/wind are consumed by that entity rather than exposed as separate switches/selects, so -this registry deliberately does NOT include the common POWER caps -- on/off is -the climate entity's HVACMode.OFF / TURN_ON/OFF. +this registry includes *common.UNIVERSAL but deliberately NOT common.POWER -- +on/off is the climate entity's HVACMode.OFF / TURN_ON/OFF, and /power/0 + +/power/vs/0 are already pre-claimed (no-entity) by airconditioner.COVERAGE via +CLIMATE_CONSUMED_HREFS. A real POWER_GENERIC/POWER_VS_FALLBACK cap on either +href alongside that bare claim would make _build() raise (see its docstring). -Reuses common.ALARMS + common.ENERGY_METER, fridge.FIRMWARE_UPDATE (as every -registry does), and dishwasher.DIAGNOSIS for /diagnosis/vs/0. +Reuses dishwasher.DIAGNOSIS for /diagnosis/vs/0. """ -from ..capabilities import airconditioner, common, dishwasher, fridge, ignored +from ..capabilities import airconditioner, common, dishwasher, ignored from ._base import DeviceRegistry, _build REGISTRY = DeviceRegistry( name='airconditioner', capabilities=_build([ *ignored.IGNORED, - common.ALARMS, - common.ENERGY_METER, - fridge.FIRMWARE_UPDATE, - fridge.SELF_CHECK, + *common.UNIVERSAL, dishwasher.DIAGNOSIS, airconditioner.CLIMATE, airconditioner.AIR_PURIFY, diff --git a/custom_components/localthings/registry/by_type/dishwasher.py b/custom_components/localthings/registry/by_type/dishwasher.py index e12dbd7..1f099f3 100644 --- a/custom_components/localthings/registry/by_type/dishwasher.py +++ b/custom_components/localthings/registry/by_type/dishwasher.py @@ -1,19 +1,13 @@ """Dishwasher device registry.""" -from ..capabilities import common, dishwasher, fridge, ignored, laundry, operational +from ..capabilities import common, dishwasher, ignored, laundry, operational from ._base import DeviceRegistry, _build REGISTRY = DeviceRegistry( name='dishwasher', capabilities=_build([ *ignored.IGNORED, - common.POWER_GENERIC, - common.POWER_VS_FALLBACK, - common.KIDS_LOCK_GENERIC, - common.KIDS_LOCK_VS_FALLBACK, - common.REMOTE_CONTROL_GENERIC, - common.REMOTE_CONTROL_VS_FALLBACK, - common.ALARMS, - common.ENERGY_METER, + *common.UNIVERSAL, + *common.POWER, common.WATER_METER, common.WATER_FILTER, operational.OPERATIONAL_STATE, @@ -25,6 +19,5 @@ REGISTRY = DeviceRegistry( laundry.DOOR_LED, laundry.SOUND_MODE, laundry.SOUND_VOLUME, - fridge.FIRMWARE_UPDATE, ]), ) diff --git a/custom_components/localthings/registry/by_type/dryer.py b/custom_components/localthings/registry/by_type/dryer.py index 71122f1..81d2891 100644 --- a/custom_components/localthings/registry/by_type/dryer.py +++ b/custom_components/localthings/registry/by_type/dryer.py @@ -9,21 +9,15 @@ objects the washer registry uses -- washer and dryer expose the same DA_WM_-family surface, so they stay consistent instead of each carrying a bespoke variant. """ -from ..capabilities import common, dryer, fridge, ignored, laundry, operational +from ..capabilities import common, dryer, ignored, laundry, operational from ._base import DeviceRegistry, _build REGISTRY = DeviceRegistry( name='dryer', capabilities=_build([ *ignored.IGNORED, - common.POWER_GENERIC, - common.POWER_VS_FALLBACK, - common.KIDS_LOCK_GENERIC, - common.KIDS_LOCK_VS_FALLBACK, - common.REMOTE_CONTROL_GENERIC, - common.REMOTE_CONTROL_VS_FALLBACK, - common.ALARMS, - common.ENERGY_METER, + *common.UNIVERSAL, + *common.POWER, operational.OPERATIONAL_STATE, laundry.DOOR_LED, laundry.SOUND_MODE, @@ -32,6 +26,5 @@ REGISTRY = DeviceRegistry( dryer.DRYER_SETTINGS, dryer.DRYER_COURSE, dryer.DRYER_DIAGNOSIS, - fridge.FIRMWARE_UPDATE, ]), ) diff --git a/custom_components/localthings/registry/by_type/oven.py b/custom_components/localthings/registry/by_type/oven.py index 69a0096..2ae3341 100644 --- a/custom_components/localthings/registry/by_type/oven.py +++ b/custom_components/localthings/registry/by_type/oven.py @@ -1,24 +1,18 @@ """Oven device registry.""" -from ..capabilities import common, fridge, ignored, oven +from ..capabilities import common, ignored, oven from ._base import DeviceRegistry, _build REGISTRY = DeviceRegistry( name='oven', capabilities=_build([ *ignored.IGNORED, - common.POWER_GENERIC, - common.POWER_VS_FALLBACK, - common.KIDS_LOCK_GENERIC, - common.KIDS_LOCK_VS_FALLBACK, - common.REMOTE_CONTROL_GENERIC, - common.REMOTE_CONTROL_VS_FALLBACK, - common.ALARMS, + *common.UNIVERSAL, + *common.POWER, oven.OVEN_CAVITY, oven.OVEN_SETPOINT, oven.OVEN_MODE, oven.OVEN_OPERATIONAL_STATE, oven.OVEN_DOOR, oven.OVEN_CONNECTED, - fridge.FIRMWARE_UPDATE, ]), ) diff --git a/custom_components/localthings/registry/by_type/refrigerator.py b/custom_components/localthings/registry/by_type/refrigerator.py index ecd63f6..b893475 100644 --- a/custom_components/localthings/registry/by_type/refrigerator.py +++ b/custom_components/localthings/registry/by_type/refrigerator.py @@ -6,21 +6,14 @@ REGISTRY = DeviceRegistry( name='refrigerator', capabilities=_build([ *ignored.IGNORED, - common.POWER_GENERIC, - common.POWER_VS_FALLBACK, + *common.UNIVERSAL, + *common.POWER, fridge.STATUS_LOCK, fridge.DOOR_ALERT, - common.KIDS_LOCK_GENERIC, - common.KIDS_LOCK_VS_FALLBACK, - common.REMOTE_CONTROL_GENERIC, - common.REMOTE_CONTROL_VS_FALLBACK, - common.ALARMS, - common.ENERGY_METER, common.WATER_FILTER, dishwasher.DIAGNOSIS, fridge.ICEMAKER_NIGHTTIME, fridge.FLEX_ZONE, - common.AI_ENERGY_LEVEL, fridge.REFRIGERATION, fridge.AUTOFILL, fridge.WELCOME_LIGHTING, @@ -29,11 +22,9 @@ REGISTRY = DeviceRegistry( fridge.SABBATH, fridge.BEVERAGE_ZONE, fridge.PANTRY_ZONE, - fridge.FIRMWARE_UPDATE, fridge.DEFROST_DELAY, fridge.DEFROST_DELAY_NATIVE_DUPLICATE, fridge.DEFROST_BLOCK_STATUS, - fridge.SELF_CHECK, fridge.DOORS_FALLBACK, fridge.TEMPERATURES_FALLBACK, fridge.ICEMAKER_STATUS_FALLBACK, diff --git a/custom_components/localthings/registry/by_type/washer.py b/custom_components/localthings/registry/by_type/washer.py index ac46cf4..c7732d8 100644 --- a/custom_components/localthings/registry/by_type/washer.py +++ b/custom_components/localthings/registry/by_type/washer.py @@ -1,27 +1,19 @@ """Washer device registry.""" -from ..capabilities import common, dishwasher, fridge, ignored, laundry, operational, washer +from ..capabilities import common, dishwasher, ignored, laundry, operational, washer from ._base import DeviceRegistry, _build REGISTRY = DeviceRegistry( name='washer', capabilities=_build([ *ignored.IGNORED, - common.POWER_GENERIC, - common.POWER_VS_FALLBACK, - common.KIDS_LOCK_GENERIC, - common.KIDS_LOCK_VS_FALLBACK, - common.REMOTE_CONTROL_GENERIC, - common.REMOTE_CONTROL_VS_FALLBACK, + *common.UNIVERSAL, + *common.POWER, washer.WASHER_SETTINGS, washer.WASHER_COURSE, laundry.BUZZER_SOUND, laundry.JOB_BEGINNING_STATUS, - common.ALARMS, - common.ENERGY_METER, common.WATER_METER, - common.AI_ENERGY_LEVEL, operational.OPERATIONAL_STATE, dishwasher.DIAGNOSIS, - fridge.FIRMWARE_UPDATE, ]), ) diff --git a/custom_components/localthings/registry/capabilities/airconditioner.py b/custom_components/localthings/registry/capabilities/airconditioner.py index bfcb374..0abc0b7 100644 --- a/custom_components/localthings/registry/capabilities/airconditioner.py +++ b/custom_components/localthings/registry/capabilities/airconditioner.py @@ -219,8 +219,9 @@ _AC_IGNORED = [ # user-actionable state or no documented write contract, so per the # 'don't guess' rule they are ignored rather than modeled. # /option/muteonce/vs/0 and /selfcheck/vs/0 are deliberately NOT here -- - # see MUTE_ONCE and fridge.SELF_CHECK above/in the by_type registry, - # both of which have a confirmed, cleanly modelable contract. + # see MUTE_ONCE above and common.SELF_CHECK (via common.UNIVERSAL) in + # the by_type registry, both of which have a confirmed, cleanly + # modelable contract. '/airlevelcheck/vs/0', # periodic air-quality sensing scheduler plumbing '/aisleep/vs/0', # AI-sleep feedback state (no actionable control) '/availablecontrolsets/vs/0', # opaque hex-encoded control-set bitmap diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index a7660d2..d2b2ac5 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -11,7 +11,9 @@ against live device dumps: /filter/waterfilter/vs/0 -> x.com.samsung.da.filterUsage / filterStatus """ from ..capability import Capability -from ..entities import BinarySensorDesc, SelectDesc, SensorDesc, SwitchDesc +from ..entities import ( + BinarySensorDesc, ButtonDesc, SelectDesc, SensorDesc, SwitchDesc, +) def _num(v): @@ -291,3 +293,85 @@ AI_ENERGY_LEVEL = Capability( write_fn=_ai_energy_level_write), ), ) + +FIRMWARE_UPDATE = Capability( + href='/otninformation/vs/0', + poll_tier='cold', + entities=( + BinarySensorDesc( + key='firmware_update', + field='x.com.samsung.da.newVersionAvailable', + name='Firmware update available', + device_class='update', + entity_category='diagnostic', + value_fn=lambda v: str(v).lower() == 'true' if v is not None else None, + ), + ), +) + +SELF_CHECK = Capability( + href='/selfcheck/vs/0', + poll_tier='cold', + entities=( + SensorDesc(key='selfcheck_status', field='x.com.samsung.da.status', + name='Self-check status', icon='mdi:stethoscope', + entity_category='diagnostic'), + SensorDesc(key='selfcheck_result', field='x.com.samsung.da.result', + name='Self-check result', icon='mdi:clipboard-check-outline', + entity_category='diagnostic'), + # List of error codes from the last self-check; joined for display. + # Not every fridge reports the field, hence the exists_fn. + SensorDesc(key='selfcheck_error', field='x.com.samsung.da.error', + name='Self-check error', icon='mdi:alert-circle-outline', + entity_category='diagnostic', + exists_fn=lambda rep, resources: ( + not rep or 'x.com.samsung.da.error' in rep), + value_fn=lambda v: (', '.join(v) if v else None) if isinstance(v, list) else v), + ButtonDesc(key='selfcheck_start', field='', name='Start self-check', + payload='Start', icon='mdi:play-circle-outline', + entity_category='diagnostic', + write_fn=lambda p, rep, href=None: ( + ['selfcheck', 'vs', '0'], {'x.com.samsung.da.status': p})), + ), +) + +# --------------------------------------------------------------------------- +# Cross-family bundles, unpacked into every by_type registry's _build([...]) +# call the same way ignored.IGNORED is (*common.UNIVERSAL / *common.POWER). +# discover() only binds a capability whose href is actually present in a +# given device's resource dump, so listing one here for a family that +# doesn't expose the href is a no-op, not a phantom entity -- see the +# adding-device-support skill's coverage-discipline section. +# +# UNIVERSAL holds every capability with no known family that both (a) has +# the href and (b) needs to model it some other way -- broadening one of +# these to a new family is a safe, harmless guess (issue #40's AI energy +# level: 2 of 6 families confirmed, blanket-added everywhere else). +# +# POWER is kept separate: airconditioner deliberately does NOT include it. +# AC's climate entity (registry/capabilities/airconditioner.py) already +# owns /power/0 and /power/vs/0 -- they're pre-claimed there via bare, +# no-entity Capability objects (COVERAGE) so a second, real POWER_GENERIC/ +# POWER_VS_FALLBACK cap on the same href would make _build() raise +# (a href with >1 cap must have every cap discriminated by rt_filter/ +# match_fn, and the bare COVERAGE cap has neither). Kids-lock and remote- +# control don't have this conflict -- no AC dump has ever reported those +# hrefs -- so they stay in UNIVERSAL. +# --------------------------------------------------------------------------- + +UNIVERSAL = ( + ALARMS, + ENERGY_METER, + FIRMWARE_UPDATE, + SELF_CHECK, + AI_ENERGY_LEVEL, + KIDS_LOCK_GENERIC, + KIDS_LOCK_VS_FALLBACK, + REMOTE_CONTROL_GENERIC, + REMOTE_CONTROL_VS_FALLBACK, +) + +POWER = ( + POWER_GENERIC, + POWER_VS_FALLBACK, +) diff --git a/custom_components/localthings/registry/capabilities/fridge.py b/custom_components/localthings/registry/capabilities/fridge.py index 5529526..b6d6dd1 100644 --- a/custom_components/localthings/registry/capabilities/fridge.py +++ b/custom_components/localthings/registry/capabilities/fridge.py @@ -19,7 +19,7 @@ import datetime from ..capability import Capability from ..entities import ( - BinarySensorDesc, ButtonDesc, NumberDesc, SelectDesc, SensorDesc, + BinarySensorDesc, NumberDesc, SelectDesc, SensorDesc, SwitchDesc, TimeDesc, ) from .common import normalize_temp_unit @@ -228,36 +228,6 @@ DEFROST_BLOCK_STATUS = Capability( ), ) -# --------------------------------------------------------------------------- -# Self-check diagnostic -# --------------------------------------------------------------------------- - -SELF_CHECK = Capability( - href='/selfcheck/vs/0', - poll_tier='cold', - entities=( - SensorDesc(key='selfcheck_status', field='x.com.samsung.da.status', - name='Self-check status', icon='mdi:stethoscope', - entity_category='diagnostic'), - SensorDesc(key='selfcheck_result', field='x.com.samsung.da.result', - name='Self-check result', icon='mdi:clipboard-check-outline', - entity_category='diagnostic'), - # List of error codes from the last self-check; joined for display. - # Not every fridge reports the field, hence the exists_fn. - SensorDesc(key='selfcheck_error', field='x.com.samsung.da.error', - name='Self-check error', icon='mdi:alert-circle-outline', - entity_category='diagnostic', - exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.error' in rep), - value_fn=lambda v: (', '.join(v) if v else None) if isinstance(v, list) else v), - ButtonDesc(key='selfcheck_start', field='', name='Start self-check', - payload='Start', icon='mdi:play-circle-outline', - entity_category='diagnostic', - write_fn=lambda p, rep, href=None: ( - ['selfcheck', 'vs', '0'], {'x.com.samsung.da.status': p})), - ), -) - # --------------------------------------------------------------------------- # Refrigeration modes (rapid cooling) # --------------------------------------------------------------------------- @@ -588,25 +558,6 @@ FLEX_ZONE = Capability( ), ) -# --------------------------------------------------------------------------- -# Firmware update -# --------------------------------------------------------------------------- - -FIRMWARE_UPDATE = Capability( - href='/otninformation/vs/0', - poll_tier='cold', - entities=( - BinarySensorDesc( - key='firmware_update', - field='x.com.samsung.da.newVersionAvailable', - name='Firmware update available', - device_class='update', - entity_category='diagnostic', - value_fn=lambda v: str(v).lower() == 'true' if v is not None else None, - ), - ), -) - # --------------------------------------------------------------------------- # Generic door pattern capability (href=None — use as pattern_cap only) # --------------------------------------------------------------------------- diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index adc1431..4a38654 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -227,3 +227,72 @@ class TestAiEnergyLevelSelect: path, body = desc.write_fn('2', {}) assert path == ['energy', 'ailevel', 'vs', '0'] assert body == {'aiLevel': '2'} + + +class TestSelfCheckError: + """Self-check diagnostic error list -- surfaced on hardware that reports + x.com.samsung.da.error, joined into a single display string.""" + + def _desc(self): + return next(e for e in common.SELF_CHECK.entities if e.key == 'selfcheck_error') + + def test_exists_when_field_present(self): + desc = self._desc() + assert desc.exists_fn({'x.com.samsung.da.error': ['DA_ERROR_NONE']}, {}) is True + + def test_does_not_exist_when_field_absent(self): + desc = self._desc() + assert desc.exists_fn({'x.com.samsung.da.status': 'Ready'}, {}) is False + + def test_exists_for_empty_stub_rep(self): + """An empty {} rep is /device/0's not-yet-fetched-stub carve-out -- + must be included-for-now, same as ENERGY_METER's fields.""" + desc = self._desc() + assert desc.exists_fn({}, {}) is True + + def test_value_joins_list(self): + desc = self._desc() + assert desc.value_fn(['E1', 'E2']) == 'E1, E2' + + def test_value_passes_through_scalar(self): + desc = self._desc() + assert desc.value_fn('DA_ERROR_NONE') == 'DA_ERROR_NONE' + + def test_value_none_for_empty_list(self): + """An empty error list means no value to show -- None (unknown), + not an empty string.""" + desc = self._desc() + assert desc.value_fn([]) is None + + +# --------------------------------------------------------------------------- +# Cross-family bundles (UNIVERSAL / POWER) -- unpacked into every by_type +# registry's _build([...]) call in place of the hand-duplicated capability +# lists that used to live there. +# --------------------------------------------------------------------------- + + +class TestUniversalAndPowerBundles: + def test_universal_contains_the_no_conflict_capabilities(self): + assert set(common.UNIVERSAL) == { + common.ALARMS, common.ENERGY_METER, common.FIRMWARE_UPDATE, + common.SELF_CHECK, common.AI_ENERGY_LEVEL, + common.KIDS_LOCK_GENERIC, common.KIDS_LOCK_VS_FALLBACK, + common.REMOTE_CONTROL_GENERIC, common.REMOTE_CONTROL_VS_FALLBACK, + } + + def test_power_kept_separate_for_airconditioners_sake(self): + """airconditioner.py deliberately doesn't unpack this bundle -- its + climate entity already owns /power/0 and /power/vs/0 via a bare, + no-entity claim (airconditioner.COVERAGE); a real POWER_GENERIC/ + POWER_VS_FALLBACK cap on the same href would make _build() raise.""" + assert set(common.POWER) == {common.POWER_GENERIC, common.POWER_VS_FALLBACK} + + def test_no_overlap_between_the_two_bundles(self): + assert not (set(common.UNIVERSAL) & set(common.POWER)) + + def test_airconditioner_registry_does_not_include_power(self): + from custom_components.localthings.registry.by_type import airconditioner + bound_caps = {c for caps in airconditioner.REGISTRY.capabilities.values() for c in caps} + assert common.POWER_GENERIC not in bound_caps + assert common.POWER_VS_FALLBACK not in bound_caps diff --git a/tests/test_fridge_capabilities.py b/tests/test_fridge_capabilities.py index c76ddfe..11dc501 100644 --- a/tests/test_fridge_capabilities.py +++ b/tests/test_fridge_capabilities.py @@ -231,42 +231,6 @@ class TestArtik051AndTp2xFixturesHaveCompleteCoverage: assert state['flex_zone_mode'] == 'CV_FDR_BEVERAGE' -class TestSelfCheckError: - """Self-check diagnostic error list -- surfaced on hardware that reports - x.com.samsung.da.error, joined into a single display string.""" - - def _desc(self): - return next(e for e in fridge.SELF_CHECK.entities if e.key == 'selfcheck_error') - - def test_exists_when_field_present(self): - desc = self._desc() - assert desc.exists_fn({'x.com.samsung.da.error': ['DA_ERROR_NONE']}, {}) is True - - def test_does_not_exist_when_field_absent(self): - desc = self._desc() - assert desc.exists_fn({'x.com.samsung.da.status': 'Ready'}, {}) is False - - def test_exists_for_empty_stub_rep(self): - """An empty {} rep is /device/0's not-yet-fetched-stub carve-out -- - must be included-for-now, same as ENERGY_METER's fields.""" - desc = self._desc() - assert desc.exists_fn({}, {}) is True - - def test_value_joins_list(self): - desc = self._desc() - assert desc.value_fn(['E1', 'E2']) == 'E1, E2' - - def test_value_passes_through_scalar(self): - desc = self._desc() - assert desc.value_fn('DA_ERROR_NONE') == 'DA_ERROR_NONE' - - def test_value_none_for_empty_list(self): - """An empty error list means no value to show -- None (unknown), - not an empty string.""" - desc = self._desc() - assert desc.value_fn([]) is None - - class TestRefrigeratorAiEnergyLevelFixtureCoverage: """AI energy-saving level (common.AI_ENERGY_LEVEL, see test_common_capabilities.py) is exercised end-to-end here against the From 61ee2972dc683d24a12d3eb29eb704fdf154dc7b Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Wed, 22 Jul 2026 20:00:01 +0000 Subject: [PATCH 3/4] Trim duplicated comments from /simplify review Consolidate the AC/power-exclusion rationale (previously spelled out nearly verbatim in three places) down to one canonical explanation next to common.POWER, with one-line pointers elsewhere. Dedupe the switch/select test classes' identical _desc() lookup into one shared helper. --- .../registry/by_type/airconditioner.py | 6 ++---- .../registry/capabilities/common.py | 19 ++++++++++--------- tests/test_common_capabilities.py | 16 ++++++++-------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/custom_components/localthings/registry/by_type/airconditioner.py b/custom_components/localthings/registry/by_type/airconditioner.py index 9de96e0..e218de5 100644 --- a/custom_components/localthings/registry/by_type/airconditioner.py +++ b/custom_components/localthings/registry/by_type/airconditioner.py @@ -4,10 +4,8 @@ The first device whose core controls surface as a single composite HA `climate` entity (see capabilities/airconditioner.py and climate.py). Power/mode/temp/wind are consumed by that entity rather than exposed as separate switches/selects, so this registry includes *common.UNIVERSAL but deliberately NOT common.POWER -- -on/off is the climate entity's HVACMode.OFF / TURN_ON/OFF, and /power/0 + -/power/vs/0 are already pre-claimed (no-entity) by airconditioner.COVERAGE via -CLIMATE_CONSUMED_HREFS. A real POWER_GENERIC/POWER_VS_FALLBACK cap on either -href alongside that bare claim would make _build() raise (see its docstring). +on/off is the climate entity's HVACMode.OFF / TURN_ON/OFF. See common.POWER's +own comment in capabilities/common.py for why it's excluded. Reuses dishwasher.DIAGNOSIS for /diagnosis/vs/0. """ diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index d2b2ac5..7a490a1 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -348,15 +348,16 @@ SELF_CHECK = Capability( # these to a new family is a safe, harmless guess (issue #40's AI energy # level: 2 of 6 families confirmed, blanket-added everywhere else). # -# POWER is kept separate: airconditioner deliberately does NOT include it. -# AC's climate entity (registry/capabilities/airconditioner.py) already -# owns /power/0 and /power/vs/0 -- they're pre-claimed there via bare, -# no-entity Capability objects (COVERAGE) so a second, real POWER_GENERIC/ -# POWER_VS_FALLBACK cap on the same href would make _build() raise -# (a href with >1 cap must have every cap discriminated by rt_filter/ -# match_fn, and the bare COVERAGE cap has neither). Kids-lock and remote- -# control don't have this conflict -- no AC dump has ever reported those -# hrefs -- so they stay in UNIVERSAL. +# POWER is kept separate -- airconditioner is the one family that opts out +# of it. Canonical reason (see by_type/airconditioner.py and its test for +# pointers back here, not restatements): AC's climate entity already owns +# /power/0 and /power/vs/0 via bare, no-entity Capability objects +# (airconditioner.COVERAGE), and a second, real POWER_GENERIC/ +# POWER_VS_FALLBACK cap on the same href would make _build() raise (a href +# with >1 cap must have every cap discriminated by rt_filter/match_fn, and +# the bare COVERAGE cap has neither). Kids-lock/remote-control don't have +# this conflict -- no AC dump has ever reported those hrefs -- so they stay +# in UNIVERSAL. # --------------------------------------------------------------------------- UNIVERSAL = ( diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index 4a38654..e852809 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -142,10 +142,14 @@ class TestEnergyMeter: # --------------------------------------------------------------------------- +def _ai_energy_level_desc(cls_name): + return next(e for e in common.AI_ENERGY_LEVEL.entities + if e.__class__.__name__ == cls_name) + + class TestAiEnergyLevelSwitch: def _desc(self): - return next(e for e in common.AI_ENERGY_LEVEL.entities - if e.__class__.__name__ == 'SwitchDesc') + return _ai_energy_level_desc('SwitchDesc') def test_href(self): assert common.AI_ENERGY_LEVEL.href == '/energy/ailevel/vs/0' @@ -190,8 +194,7 @@ class TestAiEnergyLevelSwitch: class TestAiEnergyLevelSelect: def _desc(self): - return next(e for e in common.AI_ENERGY_LEVEL.entities - if e.__class__.__name__ == 'SelectDesc') + return _ai_energy_level_desc('SelectDesc') def test_shown_only_with_multiple_supported_levels(self): desc = self._desc() @@ -282,10 +285,7 @@ class TestUniversalAndPowerBundles: } def test_power_kept_separate_for_airconditioners_sake(self): - """airconditioner.py deliberately doesn't unpack this bundle -- its - climate entity already owns /power/0 and /power/vs/0 via a bare, - no-entity claim (airconditioner.COVERAGE); a real POWER_GENERIC/ - POWER_VS_FALLBACK cap on the same href would make _build() raise.""" + """See common.POWER's own comment for why airconditioner opts out.""" assert set(common.POWER) == {common.POWER_GENERIC, common.POWER_VS_FALLBACK} def test_no_overlap_between_the_two_bundles(self): From 1493467c6313eba9c98335001ee3958f8bd22cdb Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Wed, 22 Jul 2026 20:16:20 +0000 Subject: [PATCH 4/4] Fix AI energy level platform-flip bug found in Opus review The switch and select shared a stub-time asymmetry: only the select had a `not rep` carve-out, so an unfetched-stub rep at the moment platforms are set up (entity creation runs once, ever) would instantiate a Select, while flatten() re-evaluates exists_fn every poll against live data -- once the resource populated to a single-level list, the switch's exists_fn would win instead and feed the already-created Select a bool through their shared 'ai_energy_level' key, which isn't a valid select option. Dropped the stub carve-out from the select's exists_fn so both sides require real, populated data to decide the platform -- on a device that stubs this cold-tier href on its very first poll, the entity now simply doesn't appear until a reload, instead of appearing as the wrong widget type. Added tests for the missing/empty-list supportedAiLevel shapes on both widgets and a regression test locking in the stub behavior. --- .../registry/capabilities/common.py | 18 ++++++- tests/test_common_capabilities.py | 47 ++++++++++++++++--- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index 7a490a1..ef181fc 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -277,6 +277,22 @@ AI_ENERGY_LEVEL = Capability( href='/energy/ailevel/vs/0', poll_tier='cold', entities=( + # No `not rep` stub carve-out on either side, unlike most exists_fn + # gates in this file -- entity creation only ever runs once, against + # whichever snapshot happens to be current the moment platforms are + # set up (see entity._is_included / __init__.py's + # async_config_entry_first_refresh-before-forward-entry-setups + # ordering), while flatten() re-evaluates exists_fn every poll + # against live data. Both descriptors share key='ai_energy_level', + # so if a stub carve-out let one of them win at setup time while the + # other wins once real data lands, flatten() would feed the + # instantiated entity a value shaped for the other platform (e.g. a + # bool into a Select). Requiring real, populated data on both sides + # keeps the entity-creation decision and the live-value decision in + # permanent agreement -- the cost is this entity doesn't appear + # until a reload if the device's very first poll stubs this + # cold-tier href, the same reload already required to fix which + # platform got picked in that case. SwitchDesc(key='ai_energy_level', field='aiLevel', name='AI energy level', icon='mdi:leaf', entity_category='config', @@ -289,7 +305,7 @@ AI_ENERGY_LEVEL = Capability( entity_category='config', options=_ai_energy_level_options, exists_fn=lambda rep, resources: ( - not rep or len(_ai_energy_supported_levels(rep)) > 1), + len(_ai_energy_supported_levels(rep)) > 1), write_fn=_ai_energy_level_write), ), ) diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index e852809..fdd1150 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -166,10 +166,15 @@ class TestAiEnergyLevelSwitch: desc = self._desc() assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': '1'}, {}) is False + def test_hidden_when_supported_level_missing(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '1'}, {}) is False + + def test_hidden_when_supported_level_empty_list(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '0', 'supportedAiLevel': []}, {}) is False + def test_hidden_on_empty_stub_rep(self): - """Unlike the select, the switch requires a confirmed single-entry - list -- an unfetched stub defaults to the select branch instead (see - TestAiEnergyLevelSelect.test_shown_for_empty_stub_rep).""" desc = self._desc() assert desc.exists_fn({}, {}) is False @@ -201,11 +206,17 @@ class TestAiEnergyLevelSelect: assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1', '2']}, {}) is True assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1']}, {}) is False - def test_shown_for_empty_stub_rep(self): - """An empty {} rep is /device/0's not-yet-fetched-stub carve-out -- - defaults to the select so sub-polls can populate it either way.""" + def test_hidden_when_supported_level_is_non_list_scalar(self): desc = self._desc() - assert desc.exists_fn({}, {}) is True + assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': '12'}, {}) is False + + def test_hidden_when_supported_level_missing(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '1'}, {}) is False + + def test_hidden_when_supported_level_empty_list(self): + desc = self._desc() + assert desc.exists_fn({'aiLevel': '0', 'supportedAiLevel': []}, {}) is False def test_no_translation_key(self): """aiLevel's values are plain digits that render fine untranslated @@ -232,6 +243,28 @@ class TestAiEnergyLevelSelect: assert body == {'aiLevel': '2'} +class TestAiEnergyLevelStubDoesNotDecideThePlatform: + """Issue found in review: entity *creation* runs once, against whatever + snapshot is current when platforms are set up (see + __init__.py's async_config_entry_first_refresh-before-forward-entry-setups + ordering), while flatten() re-evaluates exists_fn every poll against live + data. Both descriptors share key='ai_energy_level' (see adapter._key), so + if a stub carve-out let one of them win at setup time while the other + wins once real data lands, flatten() would feed the already-instantiated + entity a value shaped for the other platform (e.g. a bool into a Select + expecting a string option). Neither side gets a `not rep` carve-out, so + an unfetched stub can never win entity creation for either platform -- + the entity simply doesn't appear until a reload happens with real data, + same as any other exists_fn-gated entity in this codebase that's unlucky + on first-poll timing, instead of appearing as the wrong widget type.""" + + def test_neither_widget_exists_on_empty_stub_rep(self): + switch = _ai_energy_level_desc('SwitchDesc') + select = _ai_energy_level_desc('SelectDesc') + assert switch.exists_fn({}, {}) is False + assert select.exists_fn({}, {}) is False + + class TestSelfCheckError: """Self-check diagnostic error list -- surfaced on hardware that reports x.com.samsung.da.error, joined into a single display string."""