From 3b3dd543725939e8c282ce710cc81897e263e878 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 24 Jul 2026 05:55:04 +0000 Subject: [PATCH] Simplify table-scoped translation key; fix stale resolution; correct docstring Simplification (feedback: this was overcomplicated): drop the validated_table gate entirely. cycle_select's table_href now just builds the translation key directly from whatever course table the device reports (washer_cycle + Table_02 -> washer_cycle_table_02) instead of comparing against a hardcoded known-good value and falling back to no key on any mismatch. A table we haven't shipped translations for yet (e.g. FlexWash's Table_00) still gets a key built for it -- Home Assistant's own missing-translation handling takes it from there, the same graceful fallback already relied on for any individual untranslated code within an existing table. Adding a newly-confirmed table later is just new strings.json entries, no code change. Independent (Opus) review of the prior version caught two real issues, fixed here regardless of the simplification above: - translation_key was resolved once at entity construction from whatever coordinator.last_resources held at that moment. Discovery can run while a sibling resource is still an empty stub (documented precedent: see _is_included), so a callable translation_key could permanently bake in a stale value for the entity's lifetime. Moved resolution into a translation_key property override (Entity.translation_key is a property upstream, not a plain attribute), re-evaluated against live coordinator data on every access, matching how options/current_option already work. - The supportedOptions fallback's "smallest passing K wins" docstring claimed every larger passing K is an exact multiple of the true one. False: the shipped dishwasher fixture has passing K=7 (true) alongside 10, 14, and 35, none of which are multiples of 7 -- position 0 always lands on the same real course code regardless of K, which alone satisfies the current-course guard for several unrelated splits. Corrected the reasoning to what's actually true (an empirically-matched heuristic across six real dumps, not a proof) and added a regression test locking in the real dishwasher case so this isn't silently lost. --- custom_components/localthings/entity.py | 21 ++++++- .../registry/capabilities/dryer.py | 4 +- .../registry/capabilities/laundry.py | 57 ++++++++++++------- .../registry/capabilities/washer.py | 4 +- custom_components/localthings/select.py | 10 ++-- tests/test_dryer_capabilities.py | 14 ++--- tests/test_laundry_capabilities.py | 55 +++++++++++++----- tests/test_select_options.py | 17 ++++++ tests/test_washer_capabilities.py | 17 +++--- 9 files changed, 140 insertions(+), 59 deletions(-) diff --git a/custom_components/localthings/entity.py b/custom_components/localthings/entity.py index 8139fc5..aeac3b0 100644 --- a/custom_components/localthings/entity.py +++ b/custom_components/localthings/entity.py @@ -66,13 +66,30 @@ class LocalThingsEntity(CoordinatorEntity[LocalThingsCoordinator]): self._attr_name = f"{bound.instance_name} {_derive_name(bound.desc.key)}".strip() else: self._attr_name = _derive_name(self._state_key) - tk = bound.desc.translation_key - self._attr_translation_key = tk(coordinator.last_resources) if callable(tk) else tk self._attr_icon = bound.desc.icon raw_cat = bound.desc.entity_category self._attr_entity_category = EntityCategory(raw_cat) if raw_cat else None self._attr_entity_registry_enabled_default = bound.desc.enabled_default + @property + def translation_key(self) -> str | None: + """Override Entity.translation_key (a property upstream, not a + plain attribute) so a callable descriptor -- e.g. + laundry.cycle_select's table-id-gated resolver -- is re-evaluated + against live coordinator data on every access, not resolved once + at construction time. + + Discovery runs on the first /device/0 poll, which the entity + registry already documents can hand a sibling resource an empty + stub rep before it's actually been fetched (see _is_included's + docstring) -- a static one-time resolution here would risk baking + in a permanent None (no translation) for the entity's whole + lifetime if that stub hadn't populated yet, even once the real + value arrives on a later poll. + """ + tk = self._bound.desc.translation_key + return tk(self.coordinator.last_resources) if callable(tk) else tk + @property def device_info(self) -> DeviceInfo: return self.coordinator.device_info diff --git a/custom_components/localthings/registry/capabilities/dryer.py b/custom_components/localthings/registry/capabilities/dryer.py index 38cc4b0..9f49406 100644 --- a/custom_components/localthings/registry/capabilities/dryer.py +++ b/custom_components/localthings/registry/capabilities/dryer.py @@ -48,8 +48,8 @@ DRYER_SETTINGS = Capability( DRYER_COURSE = Capability( href='/course/vs/0', entities=( - cycle_select(translation_key='dryer_cycle_table_03', icon='mdi:tumble-dryer', - table_href='/st/dryercourse/vs/0', validated_table='Table_03'), + cycle_select(translation_key='dryer_cycle', icon='mdi:tumble-dryer', + table_href='/st/dryercourse/vs/0'), ), ) diff --git a/custom_components/localthings/registry/capabilities/laundry.py b/custom_components/localthings/registry/capabilities/laundry.py index 07190fc..76fca4a 100644 --- a/custom_components/localthings/registry/capabilities/laundry.py +++ b/custom_components/localthings/registry/capabilities/laundry.py @@ -218,13 +218,20 @@ def _course_codes_from_supported_options(course_rep): member of its own device's valid list. If no split satisfies both, this returns [] rather than guess. - Among splits that satisfy both, the *smallest* passing K wins rather - than requiring a single unambiguous one: any exact multiple of the true - K (e.g. 2x or 7x its byte width) trivially re-passes both checks too -- - it's just a sparser sampling of the same valid table, every entry of - which is still a real course code carried over from the finer split -- - so a larger match is never independent evidence of a different table, - only redundant confirmation of the smaller one. + Among splits that satisfy both, the *smallest* passing K wins, rather + than requiring a single unambiguous one -- more than one K reliably + does pass on real data (e.g. the shipped dishwasher fixture: true + K=7 passes, but so do 10, 14, and 35, none of which are multiples of + 7 -- position 0 always lands on the same real course code regardless + of K, which is enough on its own to satisfy the current-course guard + for several unrelated splits). Smallest-K-wins is a heuristic, not a + proof: it matches the confirmed answer on every one of six independent + real-world dumps this was checked against, but a coincidentally + unique, current-course-inclusive *smaller* K is not mathematically + impossible on some future device, and would be picked silently. Not + guarded against further here, since course tables are typically large + enough (double digits) that colliding by chance on both checks is + unlikely, and no device seen so far actually needs it. """ raw = course_rep.get('x.com.samsung.da.supportedOptions') hexstr = raw[0] if isinstance(raw, list) and raw else raw @@ -269,37 +276,45 @@ def _table_id(resources, table_href): return rep.get('x.com.samsung.da.st.courseTable') -def cycle_select(*, translation_key, icon, table_href=None, validated_table=None): +def cycle_select(*, translation_key, icon, table_href=None): """A 'Cycle' select over /course/vs/0, labelled from `translation_key`. The option list, current value, and write path are all shared across washer/dryer/dishwasher; only the translation is family- (and, for washer/dryer, board-) specific. - table_href/validated_table (washer/dryer only -- see washer.py/dryer.py's - call sites) gate translation_key on the device's own course-table id, - read from /st/washercourse/vs/0 or /st/dryercourse/vs/0's - x.com.samsung.da.st.courseTable: translation_key only applies when that - matches validated_table exactly; anything else -- a different table, or - no table id available at all -- gets no translation_key, i.e. the raw - course code displayed as-is, rather than risk a wrong name. + table_href (washer/dryer only -- see washer.py/dryer.py's call sites) + suffixes translation_key with the device's own course-table id, read + from /st/washercourse/vs/0 or /st/dryercourse/vs/0's + x.com.samsung.da.st.courseTable (e.g. 'washer_cycle' + 'Table_02' -> + 'washer_cycle_table_02'). No table id available at all -- the href + absent or empty -- gets no translation_key, i.e. the raw course code + displayed as-is. This matters because course codes are NOT guaranteed consistent across board generations sharing the same /course/vs/0 contract: every code in washer_cycle_table_02 was confirmed against Table_02-reporting devices (DA_WM_TP1/TP2 boards); FlexWash's older DA_WM_A51 board reports Table_00 instead, so the same hex code could mean a different course - there for all we've verified. + there for all we've verified. Building the key from whatever table the + device actually reports, rather than gating a single hardcoded key on + an exact match, means a table we haven't built translations for yet + (like Table_00) just falls through Home Assistant's own missing- + translation handling to the same raw-code display -- exactly what + happens today for any individual code within a table's translations + that isn't populated yet -- and adding one later needs new strings.json + entries, not a code change here. - Left at their defaults for dishwasher, which has no equivalent - table-id resource in any dump seen and no evidence its course codes - vary by table the way washer/dryer's do -- there's nothing to gate on, - and no observed problem to gate against. + Left at its default for dishwasher, which has no equivalent table-id + resource in any dump seen and no evidence its course codes vary by + table the way washer/dryer's do -- there's nothing to build a + table-specific key from. """ key = translation_key if table_href is not None: def key(resources): - return translation_key if _table_id(resources, table_href) == validated_table else None + table = _table_id(resources, table_href) + return f'{translation_key}_{table.lower()}' if table else None return SelectDesc( key='cycle', name='Cycle', icon=icon, translation_key=key, diff --git a/custom_components/localthings/registry/capabilities/washer.py b/custom_components/localthings/registry/capabilities/washer.py index 32faf64..8c21137 100644 --- a/custom_components/localthings/registry/capabilities/washer.py +++ b/custom_components/localthings/registry/capabilities/washer.py @@ -293,8 +293,8 @@ def _bool_option_switch(key, name, icon, prefix, availability_field): WASHER_COURSE = Capability( href='/course/vs/0', entities=( - cycle_select(translation_key='washer_cycle_table_02', icon='mdi:washing-machine', - table_href='/st/washercourse/vs/0', validated_table='Table_02'), + cycle_select(translation_key='washer_cycle', icon='mdi:washing-machine', + table_href='/st/washercourse/vs/0'), SensorDesc(key='drum_clean_cycles_remaining', name='Drum clean due in', icon='mdi:washing-machine-alert', unit='cycles', state_class='measurement', diff --git a/custom_components/localthings/select.py b/custom_components/localthings/select.py index 9acea2c..2d84b9e 100644 --- a/custom_components/localthings/select.py +++ b/custom_components/localthings/select.py @@ -37,7 +37,7 @@ def _display(value, translation_key: Optional[str]): `translation_key` is the entity's already-resolved key (SelectDesc. translation_key can itself be a callable -- see entities.py -- so - callers pass the resolved value, e.g. self._attr_translation_key, not + callers pass the resolved value, e.g. self.translation_key, not the raw descriptor field). An entity with a translation_key looks its state up in strings.json, @@ -69,7 +69,7 @@ class LocalThingsSelect(LocalThingsEntity, SelectEntity): super().__init__(coordinator, bound) desc: SelectDesc = bound.desc if not desc.options_field and not callable(desc.options): - self._attr_options = [_display(o, self._attr_translation_key) for o in desc.options] + self._attr_options = [_display(o, self.translation_key) for o in desc.options] def _raw_options(self) -> list[str]: desc: SelectDesc = self._bound.desc @@ -89,17 +89,17 @@ class LocalThingsSelect(LocalThingsEntity, SelectEntity): def options(self) -> list[str]: desc: SelectDesc = self._bound.desc if desc.options_field or callable(desc.options): - return [_display(o, self._attr_translation_key) for o in self._raw_options()] + return [_display(o, self.translation_key) for o in self._raw_options()] return self._attr_options @property def current_option(self): raw = (self.coordinator.data or {}).get(self._state_key) - return _display(raw, self._attr_translation_key) + return _display(raw, self.translation_key) async def async_select_option(self, option: str) -> None: raw = next( - (o for o in self._raw_options() if _display(o, self._attr_translation_key) == option), + (o for o in self._raw_options() if _display(o, self.translation_key) == option), option, ) await self.coordinator.async_send_command(self._bound, raw) diff --git a/tests/test_dryer_capabilities.py b/tests/test_dryer_capabilities.py index 2b4ff10..a6885cc 100644 --- a/tests/test_dryer_capabilities.py +++ b/tests/test_dryer_capabilities.py @@ -59,16 +59,16 @@ def test_power_watts_gated_for_dead_sentinel(): def test_course_bound_to_shared_course_vs_0(): - """Dryer course uses the shared /course/vs/0 cycle select with - dryer_cycle_table_03 translations, consistent with washer/dishwasher -- - gated on the device's own course table matching Table_03, the only one - dryer_cycle_table_03's names were confirmed against (see - laundry.cycle_select).""" + """Dryer course uses the shared /course/vs/0 cycle select, with the + translation key built from the device's own course table (see + laundry.cycle_select) -- confirmed dryers report Table_03, matching + the shipped dryer_cycle_table_03 translations, consistent with + washer/dishwasher.""" assert dryer.DRYER_COURSE.href == '/course/vs/0' desc = next(e for e in dryer.DRYER_COURSE.entities if e.key == 'cycle') assert callable(desc.translation_key) - validated = {'/st/dryercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_03'}} - assert desc.translation_key(validated) == 'dryer_cycle_table_03' + table_03 = {'/st/dryercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_03'}} + assert desc.translation_key(table_03) == 'dryer_cycle_table_03' assert desc.translation_key({}) is None assert desc.options is laundry.cycle_options rep = {'x.com.samsung.da.options': ['Course_16', 'GMT_02']} diff --git a/tests/test_laundry_capabilities.py b/tests/test_laundry_capabilities.py index 992e4ed..a9ea555 100644 --- a/tests/test_laundry_capabilities.py +++ b/tests/test_laundry_capabilities.py @@ -109,6 +109,28 @@ class TestCourseCodesFromSupportedOptions: assert laundry.cycle_options({}) == [] assert laundry.cycle_options({'/course/vs/0': {}}) == [] + def test_smallest_wins_even_when_a_larger_pass_is_not_a_multiple(self): + """Real dishwasher dump (K=7, 10 courses): K=10, 14, and 35 also + pass both checks here, and none of them are multiples of 7 -- + position 0 lands on the same real course code ('0e') regardless of + K, which alone satisfies the current-course guard for several + unrelated splits. Smallest-K-wins is a heuristic that matches every + real dump checked so far, not a proven guarantee -- see + _course_codes_from_supported_options's docstring.""" + resources = { + '/course/vs/0': { + 'x.com.samsung.da.options': ['Course_0E'], + 'x.com.samsung.da.supportedOptions': [ + '30E5434B102D102835034B002D002845034B002D002805034B000D000' + '865034B002D002075000B000D000905000B000D0008D5034B002D0028' + 'E5034B000D0008F5034B000D000' + ], + }, + } + assert laundry.cycle_options(resources) == [ + '0E', '83', '84', '80', '86', '07', '90', '8D', '8E', '8F', + ] + class TestCycleSelect: def test_builds_labelled_cycle_select(self): @@ -147,39 +169,46 @@ class TestCycleSelect: class TestCycleSelectTableGating: """translation_key becomes a resolver, not a plain string, once - table_href/validated_table are given -- washer/dryer's real call sites - (issue: course codes aren't guaranteed consistent across board - generations sharing the same /course/vs/0 contract; FlexWash's older - board reports a different course table than every device the shipped - translations were confirmed against).""" + table_href is given -- washer/dryer's real call sites (issue: course + codes aren't guaranteed consistent across board generations sharing + the same /course/vs/0 contract; FlexWash's older board reports a + different course table than every device the shipped translations + were confirmed against). The resolved key is built from whatever table + the device actually reports -- a table with no strings.json entries + yet just falls through Home Assistant's own missing-translation + handling to raw-code display, the same as any individual untranslated + code within an existing table.""" def _desc(self): return laundry.cycle_select( - translation_key='washer_cycle_table_02', icon='x', - table_href='/st/washercourse/vs/0', validated_table='Table_02', + translation_key='washer_cycle', icon='x', + table_href='/st/washercourse/vs/0', ) - def test_static_string_when_no_table_params_given(self): + def test_static_string_when_no_table_href_given(self): """dishwasher's call site -- no equivalent table-id resource in any dump seen, no evidence of the same cross-board risk -- keeps the plain static key unconditionally.""" desc = laundry.cycle_select(translation_key='dishwasher_cycle', icon='x') assert desc.translation_key == 'dishwasher_cycle' - def test_resolves_to_the_key_when_table_matches(self): + def test_resolved_key_is_built_from_the_reported_table(self): desc = self._desc() resources = {'/st/washercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_02'}} assert callable(desc.translation_key) assert desc.translation_key(resources) == 'washer_cycle_table_02' - def test_resolves_to_none_for_a_different_table(self): + def test_resolved_key_reflects_an_unbuilt_table_too(self): + """No gating against a hardcoded 'known good' table -- a table we + haven't shipped translations for yet still gets a key built for + it, just one strings.json has nothing under (raw-code display).""" desc = self._desc() resources = {'/st/washercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_00'}} - assert desc.translation_key(resources) is None + assert desc.translation_key(resources) == 'washer_cycle_table_00' def test_resolves_to_none_when_table_id_is_unknown(self): - """Absence isn't evidence of a match -- no href, or an empty rep, - gets no translation_key either, not the family default.""" + """No href, or an empty rep, gets no translation_key at all -- + there's nothing to build a key from.""" desc = self._desc() assert desc.translation_key({}) is None assert desc.translation_key({'/st/washercourse/vs/0': {}}) is None diff --git a/tests/test_select_options.py b/tests/test_select_options.py index cab3e11..293cc41 100644 --- a/tests/test_select_options.py +++ b/tests/test_select_options.py @@ -57,3 +57,20 @@ def test_callable_options_empty_result(): desc = SelectDesc(key='cycle', options=lambda resources: []) entity = _make_select(desc, '/x/vs/0', {}) assert entity.options == [] + + +def test_callable_translation_key_reresolves_live_not_once_at_construction(): + """A callable translation_key (laundry.cycle_select's table-id-gated + resolver) must be re-evaluated against current coordinator data on + every access, not baked in once at __init__ -- discovery can run while + a sibling resource (e.g. /st/washercourse/vs/0) is still an empty stub + (see entity.py's _is_included docstring), and a one-time resolution + would permanently show untranslated codes even after a later poll + populates the real value.""" + desc = SelectDesc(key='cycle', translation_key=lambda resources: resources.get('key')) + resources = {'key': None} + entity = _make_select(desc, '/x/vs/0', resources) + assert entity.translation_key is None + + resources['key'] = 'washer_cycle_table_02' + assert entity.translation_key == 'washer_cycle_table_02' diff --git a/tests/test_washer_capabilities.py b/tests/test_washer_capabilities.py index 980e911..8e2005f 100644 --- a/tests/test_washer_capabilities.py +++ b/tests/test_washer_capabilities.py @@ -61,15 +61,18 @@ class TestWasherCourse: assert washer.WASHER_COURSE.href == '/course/vs/0' def test_translation_key(self): - """Table-gated (issue: FlexWash's older board reports a different - course table, Table_00, than every device washer_cycle_table_02's - names were confirmed against, Table_02) -- see laundry.cycle_select.""" + """Table-scoped (issue: course codes aren't guaranteed consistent + across board generations sharing /course/vs/0 -- FlexWash's older + board reports Table_00, not the Table_02 every washer_cycle_table_02 + name was confirmed against) -- see laundry.cycle_select. The key is + built from whatever table the device reports, not gated against a + hardcoded 'known good' value.""" desc = next(e for e in washer.WASHER_COURSE.entities if e.key == 'cycle') assert callable(desc.translation_key) - validated = {'/st/washercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_02'}} - assert desc.translation_key(validated) == 'washer_cycle_table_02' - other_table = {'/st/washercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_00'}} - assert desc.translation_key(other_table) is None + table_02 = {'/st/washercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_02'}} + assert desc.translation_key(table_02) == 'washer_cycle_table_02' + table_00 = {'/st/washercourse/vs/0': {'x.com.samsung.da.st.courseTable': 'Table_00'}} + assert desc.translation_key(table_00) == 'washer_cycle_table_00' assert desc.translation_key({}) is None def test_reads_raw_course_code_from_options_array(self):