From cafd7d5afaa51300b83f98ded549d0a08bb79bd3 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Wed, 29 Jul 2026 02:14:13 +0000 Subject: [PATCH] refactor(registry): drop oneUiVersion from device-type detection oneUiVersion looks like the signal you'd want -- the device naming its own type, '7.0 Dishwasher' -- and it was the first thing detection consulted. It never earned the position: - Only 7 of 49 fixtures report it at all. - All 7 resolve to the same registry from their modelNum board token alone. - No device-support issue has ever been fixed by adding a mapping for it. Every one went through modelNum. The alias keys it needed in _REGISTRY_BY_KEY ('airpurifier', 'air_conditioner', 'hood') were speculative when the registries were first written and never used since. So it bought a key-normalizing helper (_type_key), a lookup with a suffix fallback (for_device), three alias keys, and a second config-flow step whose only reason to exist was phrasing a sentence about oneUiVersion -- for a signal that has never once been decisive. Remove it from detection. It stays in diagnostics, where it's genuinely useful: it names the firmware generation ('7.0 Air conditioner' is Tizen Lite), which matters when triaging an issue. Detection order was also duplicated in four places -- the coordinator, the config flow's probe, the golden-regression harness, and the skill -- which is how the harness and the shipped order drift apart. Collapse it into by_type.resolve(resources), and call that everywhere. The two "appliance type not recognized" config steps become one. They differed only in whether they blamed a missing oneUiVersion, which is not a distinction a user can act on, and never was. Verified by the full suite (795 passing), including every golden regression -- so entity output is byte-identical for all 49 device fixtures. TestOneUiVersionIsNotConsulted locks in the premise rather than just the outcome: for every dump that reports a oneUiVersion, the model strings alone must still reach a registry. If a future device breaks that, the test says so instead of the device silently losing half its entities. Also note in requirements-dev.txt that Python 3.13 resolves the pinned harness floor -- 3.12 and older resolve nothing and fail the whole install. --- .claude/skills/adding-device-support/SKILL.md | 45 ++--- custom_components/localthings/config_flow.py | 37 +--- custom_components/localthings/coordinator.py | 28 ++- .../localthings/registry/by_type/__init__.py | 93 +++++----- .../registry/by_type/air_purifier.py | 12 +- .../localthings/registry/by_type/cooktop.py | 5 +- .../registry/capabilities/air_dresser.py | 4 +- .../registry/capabilities/washer.py | 7 +- .../localthings/translations/en.json | 6 +- .../localthings/translations/nl.json | 6 +- requirements-dev.txt | 5 +- tests/localthings/conftest.py | 2 - tests/localthings/test_config_flow.py | 66 +++----- tests/test_airconditioner_capabilities.py | 42 ++--- tests/test_by_type.py | 160 +++++++++--------- tests/test_end_to_end_v2.py | 16 +- tests/test_golden_regression.py | 21 +-- 17 files changed, 229 insertions(+), 326 deletions(-) diff --git a/.claude/skills/adding-device-support/SKILL.md b/.claude/skills/adding-device-support/SKILL.md index b377185..e92df86 100644 --- a/.claude/skills/adding-device-support/SKILL.md +++ b/.claude/skills/adding-device-support/SKILL.md @@ -5,8 +5,8 @@ description: >- /device/0 diagnostics dump. Use when a device-support issue lands, a device raises the "incomplete capability coverage" repair, a diagnostics JSON needs triaging, or you're mapping OCF resources to HA entities. Covers reading dumps, - routing an unrecognized board family to a registry (oneUiVersion, modelNum - board tokens, resource signatures), + routing an unrecognized board family to a registry (modelNum board tokens, + resource signatures), OCF-standard vs vendor hrefs, the diagnostic/config/normal entity taxonomy, preferring dynamic (device-reported) select options over hardcoded lists, ensuring every href is bound or ignored, and locking it in with a fixture + @@ -49,15 +49,7 @@ discovery = importlib.import_module('custom_components.localthings.registry.disc adapter = importlib.import_module('custom_components.localthings.registry.adapter') resources = json.load(open('dump.json'))['data']['resources'] -info = resources.get('/information/vs/0', {}) -one_ui = resources.get('/otninformation/vs/0', {}).get('swVersionInfo', {}).get('oneUiVersion', '') -# Same three-stage order the coordinator uses -- see §3. -reg = ( - (by_type.for_device(one_ui) if one_ui else None) - or by_type.for_device_by_model(info.get('x.com.samsung.da.modelNum', ''), - info.get('x.com.samsung.da.description', '')) - or by_type.for_device_by_resources(resources) -) +reg = by_type.resolve(resources) # the same entry point the coordinator uses unbound = [] bound = discovery.discover(resources, reg.capabilities, reg.pattern_capabilities, log=unbound.append) state = adapter.flatten(bound, resources) # {entity_key: value} @@ -71,21 +63,30 @@ regenerate a golden. ## 3. Route the device to a registry — add a row, never a branch -If `for_device*` returns `None`, the device falls back to common capabilities -and loses roughly **half** its entities (measured across the fixture corpus: -843 of 1510 bound entities survive). So routing is the first thing to fix, and -`registry/by_type/__init__.py` is deliberately kept boring: +If detection returns `None`, the device falls back to common capabilities and +loses roughly **half** its entities (measured across the fixture corpus: 843 of +1510 bound entities survive). So routing is the first thing to fix, and +`registry/by_type/__init__.py` is deliberately kept boring. -1. **`for_device(one_ui_version)`** — `/otninformation/vs/0`'s - `swVersionInfo.oneUiVersion`, e.g. `'7.0 Dishwasher'`. The device naming - its own type, so it's tried first — but only a minority of hardware - reports it, so never assume it exists. -2. **`for_device_by_model(model_num, description)`** — the workhorse. Both +`resolve(resources)` is the only entry point — the coordinator, the config +flow's probe and the golden-regression harness all call it, so the order can't +drift between what ships and what the tests assert. Two stages: + +1. **`for_device_by_model(model_num, description)`** — the primary path. Both fields come from `/information/vs/0`. Board-family tokens are matched against `modelNum` first, then `description`, then the fuzzy two-letter consumer-model prefix. -3. **`for_device_by_resources(resources)`** — last resort for boards that - report no `/information/vs/0` at all. Needs a *distinctive* signature. +2. **`for_device_by_resources(resources)`** — for boards that report no + `/information/vs/0` at all. Needs a *distinctive* signature. + +**`oneUiVersion` is not consulted.** It looks like the obvious signal — the +device naming its own type, `'7.0 Dishwasher'` — and it used to be stage one. +But only a minority of hardware reports it, every device that does is already +typed by its modelNum board token (`TestOneUiVersionIsNotConsulted` checks that +against the whole corpus), and no device-support issue was ever fixed by adding +a mapping for it. Don't reintroduce it as a detection stage; it stays in +diagnostics as a firmware-generation marker (`'7.0 Air conditioner'` means +Tizen Lite), which is useful when triaging. ### Adding a board family diff --git a/custom_components/localthings/config_flow.py b/custom_components/localthings/config_flow.py index 37a8595..c2b50b1 100644 --- a/custom_components/localthings/config_flow.py +++ b/custom_components/localthings/config_flow.py @@ -214,9 +214,7 @@ def _probe_and_validate(host: str, ca_cert_pem: str, ca_key_pem: str) -> dict: import cbor2 from smartthings_local.protocol.dtls_session import DtlsCoapSession from .registry.batch import parse_device0_batch - from .registry.by_type import ( - for_device, for_device_by_model, for_device_by_resources, - ) + from .registry.by_type import resolve as resolve_registry _LOGGER.debug("Fetching Samsung cloud UUID from %s", _SAMSUNG_CLOUD_HOST) try: @@ -275,25 +273,12 @@ def _probe_and_validate(host: str, ca_cert_pem: str, ca_key_pem: str) -> dict: ) if not serial or _is_placeholder_serial(serial): serial = f"{host}:{port}" - one_ui_version = ( - resources - .get('/otninformation/vs/0', {}) - .get('swVersionInfo', {}) - .get('oneUiVersion', '') - ) - info_resource = resources.get('/information/vs/0', {}) - recognized_registry = ( - for_device(one_ui_version) if one_ui_version else None - ) or for_device_by_model( - info_resource.get('x.com.samsung.da.modelNum', ''), - info_resource.get('x.com.samsung.da.description', ''), - ) or for_device_by_resources(resources) + recognized_registry = resolve_registry(resources) return { "port": port, "serial": serial, "leaf_cert_pem": fullchain_pem, "leaf_key_pem": leaf_key_pem, - "one_ui_version": one_ui_version, "device_type_recognized": recognized_registry is not None, } except CannotConnect: @@ -407,27 +392,9 @@ class LocalThingsConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Shown only when the probe already knows the device type is unrecognized.""" if user_input is not None: return self._create_entry(self._pending_info) - - if not self._pending_info["one_ui_version"]: - return await self.async_step_confirm_unknown_type_no_version() - return self.async_show_form( step_id="confirm_unknown_type", data_schema=vol.Schema({}), - description_placeholders={ - "one_ui_version": self._pending_info["one_ui_version"], - }, - ) - - async def async_step_confirm_unknown_type_no_version( - self, user_input: dict[str, Any] | None = None - ) -> FlowResult: - """Confirm an unknown appliance that did not report oneUiVersion.""" - if user_input is not None: - return self._create_entry(self._pending_info) - return self.async_show_form( - step_id="confirm_unknown_type_no_version", - data_schema=vol.Schema({}), ) diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index 65c728a..11d1cc0 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -23,7 +23,7 @@ from smartthings_local.protocol.dtls_session import DtlsCoapSession from smartthings_local.ocf.state_cache import StateCache from .registry.batch import parse_device0_batch -from .registry.by_type import for_device, for_device_by_model, for_device_by_resources +from .registry.by_type import resolve as resolve_registry from .registry.capabilities.common import ( merge_items_field, merge_options_field, @@ -352,19 +352,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # ------------------------------------------------------------------ def _run_discovery(self, resources: dict[str, dict]) -> None: - one_ui = (resources.get('/otninformation/vs/0', {}) - .get('swVersionInfo', {}) - .get('oneUiVersion', '')) - self.one_ui_version = one_ui + # Reported for diagnostics only -- it names the firmware generation + # ('7.0 Air conditioner' is Tizen Lite), which is useful when triaging + # an issue. It does not route: only a minority of hardware reports it + # at all, and every device that does is already typed by its modelNum. + self.one_ui_version = (resources.get('/otninformation/vs/0', {}) + .get('swVersionInfo', {}) + .get('oneUiVersion', '')) info = resources.get('/information/vs/0', {}) - reg = for_device(one_ui) if one_ui else None - if reg is None: - reg = for_device_by_model( - info.get('x.com.samsung.da.modelNum', ''), - info.get('x.com.samsung.da.description', ''), - ) - if reg is None: - reg = for_device_by_resources(resources) + reg = resolve_registry(resources) unbound: list[str] = [] hot, warm = set(), set() @@ -374,13 +370,14 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): elif tier == 'warm': warm.add(href) + model_num = info.get('x.com.samsung.da.modelNum', '') if reg is not None: - self._log.debug("device type: %s (oneUiVersion=%r)", reg.name, one_ui) + self._log.debug("device type: %s (modelNum=%r)", reg.name, model_num) bound = discover(resources, reg.capabilities, reg.pattern_capabilities, log=unbound.append, tier_log=_tier_log) self.device_type_name = reg.name else: - self._log.warning("unknown device type oneUiVersion=%r; using common caps", one_ui) + self._log.warning("unknown device type modelNum=%r; using common caps", model_num) bound = discover(resources, CAPABILITIES, log=unbound.append, tier_log=_tier_log) self.device_type_name = None self.bound = bound @@ -393,7 +390,6 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): ident = self._identity device_type = reg.name.replace('_', ' ').title() if reg else 'Appliance' - model_num = info.get('x.com.samsung.da.modelNum', '') model = model_num.split('|', 1)[0] if model_num else (ident.model if ident else '') name = f"Samsung {device_type} ({model})" if model else f"Samsung {device_type}" mfr = (ident.manufacturer if ident else '') or 'Samsung' diff --git a/custom_components/localthings/registry/by_type/__init__.py b/custom_components/localthings/registry/by_type/__init__.py index 416e977..9f860bd 100644 --- a/custom_components/localthings/registry/by_type/__init__.py +++ b/custom_components/localthings/registry/by_type/__init__.py @@ -10,17 +10,17 @@ from . import ( ) __all__ = [ - 'DeviceRegistry', '_type_key', 'for_device', 'for_device_by_model', + 'DeviceRegistry', 'resolve', 'for_device_by_model', 'for_device_by_resources', '_board_tokens', ] +# One entry per registry, no aliases: every key here is reachable from +# `_BOARD_TOKEN_TO_KEY`, `_CONSUMER_PREFIX_TO_KEY`, or `for_device_by_resources`. _REGISTRY_BY_KEY: dict[str, DeviceRegistry] = { 'air_dresser': air_dresser.REGISTRY, 'air_purifier': air_purifier.REGISTRY, - 'airpurifier': air_purifier.REGISTRY, 'airconditioner': airconditioner.REGISTRY, - 'air_conditioner': airconditioner.REGISTRY, 'cooktop': cooktop.REGISTRY, 'dehumidifier': dehumidifier.REGISTRY, 'dishwasher': dishwasher.REGISTRY, @@ -28,7 +28,6 @@ _REGISTRY_BY_KEY: dict[str, DeviceRegistry] = { 'induction_cooktop': induction_cooktop.REGISTRY, 'microwave': microwave.REGISTRY, 'oven': oven.REGISTRY, - 'hood': range_hood.REGISTRY, 'range': _range.REGISTRY, 'range_hood': range_hood.REGISTRY, 'refrigerator': refrigerator.REGISTRY, @@ -38,48 +37,6 @@ _REGISTRY_BY_KEY: dict[str, DeviceRegistry] = { } -def _type_key(one_ui_version: str) -> str: - """Convert oneUiVersion string to registry key. - - Args: - one_ui_version: String like '7.0 Dishwasher' or 'Oven'. - - Returns: - Lowercase key with version prefix stripped and spaces/hyphens converted to underscores. - - Examples: - '7.0 Dishwasher' -> 'dishwasher' - '7.0 French Door Refrigerator' -> 'french_door_refrigerator' - 'Oven' -> 'oven' - """ - if ' ' in one_ui_version: - # Strip version prefix: everything before and including the first space - suffix = one_ui_version.split(' ', 1)[-1] - else: - suffix = one_ui_version - - return suffix.lower().replace(' ', '_').replace('-', '_') - - -def for_device(one_ui_version: str) -> Optional[DeviceRegistry]: - """Return the DeviceRegistry for the given oneUiVersion string, or None if unknown. - - Args: - one_ui_version: Device's oneUiVersion string (e.g., '7.0 Dishwasher'). - - Returns: - DeviceRegistry if a matching registry exists, None otherwise. - """ - key = _type_key(one_ui_version) - if key in _REGISTRY_BY_KEY: - return _REGISTRY_BY_KEY[key] - # Suffix fallback: e.g. "french_door_refrigerator" ends with "_refrigerator" - for rkey, reg in _REGISTRY_BY_KEY.items(): - if key.endswith(f'_{rkey}'): - return reg - return None - - # Consumer-model prefix (first two letters of the '_'-delimited token in # `description` right before any '/board-info' suffix) -> registry key. # NOT derived from `modelNum` -- washer and dryer share the same 'DA_WM_' @@ -214,9 +171,10 @@ def _consumer_model_key(description: str) -> Optional[str]: def for_device_by_model(model_num: str, description: str) -> Optional[DeviceRegistry]: - """Fallback device-type detection for hardware that never reports - oneUiVersion (confirmed for washers -- their /otninformation/vs/0 has - no swVersionInfo key at all). + """Device-type detection from /information/vs/0's model strings. + + The primary path: the board named in `modelNum` determines the resource + surface, which is what a registry describes. Three passes, narrowest evidence first: @@ -252,11 +210,13 @@ def for_device_by_model(model_num: str, description: str) -> Optional[DeviceRegi def for_device_by_resources(resources: dict[str, dict]) -> Optional[DeviceRegistry]: """Detect a device family from a distinctive local-resource signature. - Some newer cooktops omit both ``oneUiVersion`` and - ``/information/vs/0``. Their mode resource still identifies them: it - contains a DeviceType option and multiple per-burner OperationState - options. Require both shapes so an oven's unrelated ``/mode/vs/0`` is - not misclassified. + For boards that ship no ``/information/vs/0`` at all, leaving + `for_device_by_model` nothing to read. Some newer cooktops are the + original case: their mode resource still identifies them, carrying a + DeviceType option and multiple per-burner OperationState options. + + Require two independent shapes for every signature here, never one, so + an unrelated family's ``/mode/vs/0`` isn't misclassified. """ mode = resources.get('/mode/vs/0', {}) options = mode.get('x.com.samsung.da.options') or () @@ -292,3 +252,28 @@ def for_device_by_resources(resources: dict[str, dict]) -> Optional[DeviceRegist return _REGISTRY_BY_KEY['range'] return _REGISTRY_BY_KEY['oven'] return None + + +def resolve(resources: dict[str, dict]) -> Optional[DeviceRegistry]: + """Device type for a parsed /device/0 dump, or None if unrecognized. + + The single entry point for detection -- the coordinator, the config + flow's probe and the golden-regression harness all call this, so the + order can't drift between what ships and what the tests assert. + + Model strings first (`for_device_by_model`), then a distinctive resource + signature (`for_device_by_resources`) for boards that report no + /information/vs/0 at all. + + `/otninformation/vs/0`'s oneUiVersion is deliberately not consulted. It + reads like the obvious signal -- the device naming its own type, e.g. + '7.0 Dishwasher' -- but only a minority of hardware populates it, every + device that does is already typed by its modelNum board token, and no + device-support issue has ever been fixed by adding a mapping for it. It + is still reported in diagnostics as a firmware-generation marker. + """ + info = resources.get('/information/vs/0', {}) + return for_device_by_model( + info.get('x.com.samsung.da.modelNum', ''), + info.get('x.com.samsung.da.description', ''), + ) or for_device_by_resources(resources) diff --git a/custom_components/localthings/registry/by_type/air_purifier.py b/custom_components/localthings/registry/by_type/air_purifier.py index cf87fba..683496e 100644 --- a/custom_components/localthings/registry/by_type/air_purifier.py +++ b/custom_components/localthings/registry/by_type/air_purifier.py @@ -4,16 +4,16 @@ Spans three board generations sharing this one registry (see capabilities/air_purifier.py's module docstring for the per-href match_fn discriminators that keep them from colliding): -- ARTIK051_TVTL-class (issue #56). Reports no oneUiVersion; resolved via - for_device_by_model's '_TVTL_' modelNum token (see registry.py). -- TP1X_DA-AC-AIR-class (issue #130). Self-reports oneUiVersion "7.0 Air - purifier", resolved via for_device(). Adds real fan-mode control plus +- ARTIK051_TVTL-class (issue #56). Resolved via the 'TVTL' modelNum board + token (see by_type/__init__.py). +- TP1X_DA-AC-AIR-class (issue #130). Resolved via the 'AIR' board token. + Adds real fan-mode control plus display/HEPA-filter/pet-filter/sound resources the older family never reported; reuses airconditioner.DISPLAY_LIGHT and airconditioner.MUTE_ONCE for /light/vs/0 and /option/muteonce/vs/0, which are identical shapes on the shared DA-AC- board family. -- A-VTWW-TP2-21-COMMON-class (issue #151). Reports no oneUiVersion and no - existing modelNum token; falls back to unknown until routed here. Its fan +- A-VTWW-TP2-21-COMMON-class (issue #151). Resolved via the 'VTWW' board + token, added for it. Its fan is WIND_STRENGTH_FAN on /wind/strength/vs/0 rather than FAN on /mode/vs/0 -- see that capability's comment. diff --git a/custom_components/localthings/registry/by_type/cooktop.py b/custom_components/localthings/registry/by_type/cooktop.py index 91fc189..aa4ee84 100644 --- a/custom_components/localthings/registry/by_type/cooktop.py +++ b/custom_components/localthings/registry/by_type/cooktop.py @@ -4,9 +4,8 @@ Named 'gas_cooktop' (not 'cooktop') so its diagnostics/device-info label doesn't collide with the unrelated induction_cooktop family (issue #86, by_type/induction_cooktop.py) -- two different OCF surfaces that happen to share the English word "cooktop". The `_REGISTRY_BY_KEY['cooktop']` lookup -key is unchanged: it's relied on by real devices reporting -oneUiVersion "Cooktop" (for_device), the legacy ARTIK051 modelNum rule -(for_device_by_model), and the resource-signature fallback +key is unchanged: it's relied on by the legacy ARTIK051 'CT' modelNum token +(for_device_by_model) and the resource-signature fallback (for_device_by_resources) alike. """ diff --git a/custom_components/localthings/registry/capabilities/air_dresser.py b/custom_components/localthings/registry/capabilities/air_dresser.py index 3fcfb11..f60ea30 100644 --- a/custom_components/localthings/registry/capabilities/air_dresser.py +++ b/custom_components/localthings/registry/capabilities/air_dresser.py @@ -1,8 +1,8 @@ """Capabilities specific to the AirDresser family (Samsung DA_DF-class, issues #162/#157). -This board family reports no oneUiVersion and no /information/vs/0 token -any existing family routes on, so it gets its own device type -- but most +This board family carried no /information/vs/0 token any existing family +routed on, so it gets its own device type (the 'DF' board token) -- but most of the resources it exposes are already handled by the shared laundry machinery: diff --git a/custom_components/localthings/registry/capabilities/washer.py b/custom_components/localthings/registry/capabilities/washer.py index a006c16..b1dded1 100644 --- a/custom_components/localthings/registry/capabilities/washer.py +++ b/custom_components/localthings/registry/capabilities/washer.py @@ -2,9 +2,10 @@ front-load washers). Resources verified against two live WW90DG6U25LEU4 dumps (Table_02 course -family). Washers never report `oneUiVersion` -- see -`registry/by_type/__init__.py`'s `for_device_by_model()` for the fallback -detection this device type requires. +family). Washers share the `DA_WM_` laundry board with dryers, so their +`modelNum` can't tell the two apart -- see `registry/by_type/__init__.py`'s +`_CONSUMER_PREFIX_TO_KEY` for the `description`-based detection this device +type requires. The shared laundry surface -- power/kids-lock/remote-control OCF+vendor fallback pairs, buzzer, energy meter, job-beginning-status, and the diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index e92e5ec..6fb19fb 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -1068,11 +1068,7 @@ }, "confirm_unknown_type": { "title": "Appliance type not recognized", - "description": "This appliance reported oneUiVersion \"{one_ui_version}\", which isn't a recognized type. It'll still be added, but only with common capabilities (power, alarms, etc. where present) rather than the full set for its family. You can help add full support afterward by downloading diagnostics for this device (Settings > Devices & Services > this device > the menu > Download diagnostics) and filing them in a new issue. Submit to add it anyway." - }, - "confirm_unknown_type_no_version": { - "title": "Appliance type not recognized", - "description": "This appliance did not report a oneUiVersion and its type couldn't be recognized. It'll still be added, but only with common capabilities (power, alarms, etc. where present) rather than the full set for its family. You can help add full support afterward by downloading diagnostics for this device (Settings > Devices & Services > this device > the menu > Download diagnostics) and filing them in a new issue. Submit to add it anyway." + "description": "This appliance's type couldn't be recognized. It'll still be added, but only with common capabilities (power, alarms, etc. where present) rather than the full set for its family. You can help add full support afterward by downloading diagnostics for this device (Settings > Devices & Services > this device > the menu > Download diagnostics) and filing them in a new issue. Submit to add it anyway." } }, "error": { diff --git a/custom_components/localthings/translations/nl.json b/custom_components/localthings/translations/nl.json index 4e53716..af8800b 100644 --- a/custom_components/localthings/translations/nl.json +++ b/custom_components/localthings/translations/nl.json @@ -1068,11 +1068,7 @@ }, "confirm_unknown_type": { "title": "Apparaattype niet herkend", - "description": "Dit apparaat meldt oneUiVersion \"{one_ui_version}\", wat niet als apparaattype wordt herkend. Het apparaat wordt toch toegevoegd, maar alleen met algemene mogelijkheden (zoals voeding en alarmen, voor zover aanwezig), in plaats van alle mogelijkheden voor deze apparaatfamilie. Je kunt daarna helpen volledige ondersteuning toe te voegen door diagnostische gegevens voor dit apparaat te downloaden (Instellingen > Apparaten & diensten > dit apparaat > het menu > Diagnostische gegevens downloaden) en deze bij een nieuw issue te voegen. Kies Verzenden om het apparaat toch toe te voegen." - }, - "confirm_unknown_type_no_version": { - "title": "Apparaattype niet herkend", - "description": "Dit apparaat meldt geen oneUiVersion en het apparaattype kon niet worden herkend. Het apparaat wordt toch toegevoegd, maar alleen met algemene mogelijkheden (zoals voeding en alarmen, voor zover aanwezig), in plaats van alle mogelijkheden voor deze apparaatfamilie. Je kunt daarna helpen volledige ondersteuning toe te voegen door diagnostische gegevens voor dit apparaat te downloaden (Instellingen > Apparaten & diensten > dit apparaat > het menu > Diagnostische gegevens downloaden) en deze bij een nieuw issue te voegen. Kies Verzenden om het apparaat toch toe te voegen." + "description": "Het apparaattype van dit apparaat kon niet worden herkend. Het apparaat wordt toch toegevoegd, maar alleen met algemene mogelijkheden (zoals voeding en alarmen, voor zover aanwezig), in plaats van alle mogelijkheden voor deze apparaatfamilie. Je kunt daarna helpen volledige ondersteuning toe te voegen door diagnostische gegevens voor dit apparaat te downloaden (Instellingen > Apparaten & diensten > dit apparaat > het menu > Diagnostische gegevens downloaden) en deze bij een nieuw issue te voegen. Kies Verzenden om het apparaat toch toe te voegen." } }, "error": { diff --git a/requirements-dev.txt b/requirements-dev.txt index e5a378b..f47c26c 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,6 +1,7 @@ # Test harness — pulls in home-assistant, pytest, and pytest-socket at the -# matching versions. Current Home Assistant requires Python >= 3.14; pip will -# resolve the newest home-assistant your interpreter supports. +# matching versions. pip resolves the newest home-assistant your interpreter +# supports: Python 3.13 gets 0.13.316 (the floor below), 3.14 gets newer. On +# 3.12 or older nothing resolves and the whole install fails. pytest-homeassistant-custom-component>=0.13.316 # Integration runtime deps, needed to import the component under test diff --git a/tests/localthings/conftest.py b/tests/localthings/conftest.py index a5a5e62..cb72715 100644 --- a/tests/localthings/conftest.py +++ b/tests/localthings/conftest.py @@ -105,7 +105,6 @@ def mock_probe(): 'serial': MOCK_SERIAL, 'leaf_cert_pem': MOCK_LEAF_CERT_PEM, 'leaf_key_pem': MOCK_LEAF_KEY_PEM, - 'one_ui_version': '7.0 Refrigerator', 'device_type_recognized': True, }, ) as m: @@ -122,7 +121,6 @@ def mock_probe_unknown_type(): 'serial': MOCK_SERIAL, 'leaf_cert_pem': MOCK_LEAF_CERT_PEM, 'leaf_key_pem': MOCK_LEAF_KEY_PEM, - 'one_ui_version': '9.0 Space Heater', 'device_type_recognized': False, }, ) as m: diff --git a/tests/localthings/test_config_flow.py b/tests/localthings/test_config_flow.py index 20f4270..6808b7d 100644 --- a/tests/localthings/test_config_flow.py +++ b/tests/localthings/test_config_flow.py @@ -211,7 +211,6 @@ async def test_unknown_type_shows_confirmation_step( ) assert result['type'] == FlowResultType.FORM assert result['step_id'] == 'confirm_unknown_type' - assert result['description_placeholders']['one_ui_version'] == '9.0 Space Heater' result = await hass.config_entries.flow.async_configure( result['flow_id'], {}, @@ -220,37 +219,24 @@ async def test_unknown_type_shows_confirmation_step( assert result['data'][CONF_HOST] == MOCK_HOST -async def test_unknown_type_without_version_uses_localized_step( - hass: HomeAssistant, +async def test_unknown_type_step_description_makes_no_version_claim( + hass: HomeAssistant, mock_probe_unknown_type ) -> None: - """No English placeholder sentinel leaks into a translated description.""" - probe_result = { - 'port': MOCK_PORT, - 'serial': MOCK_SERIAL, - 'leaf_cert_pem': 'leaf cert', - 'leaf_key_pem': 'leaf key', - 'one_ui_version': '', - 'device_type_recognized': False, - } - with patch( - 'custom_components.localthings.config_flow._probe_and_validate', - return_value=probe_result, - ): - result = await hass.config_entries.flow.async_init( - DOMAIN, context={'source': 'user'} - ) - result = await hass.config_entries.flow.async_configure( - result['flow_id'], - { - CONF_HOST: MOCK_HOST, - CONF_CA_CERT_PEM: MOCK_CA_CERT_PEM, - CONF_CA_KEY_PEM: MOCK_CA_KEY_PEM, - }, - ) + """One confirmation step covers every unrecognized device. It used to be + two, differing only in whether they blamed a missing oneUiVersion -- a + distinction that stopped existing when detection stopped reading it.""" + import json + from pathlib import Path - assert result['type'] == FlowResultType.FORM - assert result['step_id'] == 'confirm_unknown_type_no_version' - assert not result.get('description_placeholders') + steps = json.loads( + (Path(__file__).parents[2] / 'custom_components' / 'localthings' + / 'translations' / 'en.json').read_text() + )['config']['step'] + + assert 'confirm_unknown_type_no_version' not in steps + description = steps['confirm_unknown_type']['description'] + assert 'oneUiVersion' not in description + assert '{' not in description # no unfilled placeholder async def test_duplicate_device_aborted(hass: HomeAssistant, mock_probe) -> None: @@ -279,9 +265,8 @@ async def test_duplicate_device_aborted(hass: HomeAssistant, mock_probe) -> None def test_probe_marks_washer_as_recognized(monkeypatch): - """A washer's probe response (no oneUiVersion) must still resolve via - the modelNum/description fallback so setup doesn't warn about an - unrecognized device type.""" + """A washer reports no oneUiVersion at all -- its consumer-model code + must still resolve so setup doesn't warn about an unrecognized type.""" from custom_components.localthings import config_flow device0 = [ @@ -298,19 +283,8 @@ def test_probe_marks_washer_as_recognized(monkeypatch): from custom_components.localthings.registry.batch import parse_device0_batch resources = parse_device0_batch(device0) - info_resource = resources.get('/information/vs/0', {}) - one_ui_version = ( - resources.get('/otninformation/vs/0', {}).get('swVersionInfo', {}).get('oneUiVersion', '') - ) - from custom_components.localthings.registry.by_type import for_device, for_device_by_model - recognized = bool( - (one_ui_version and for_device(one_ui_version) is not None) - or for_device_by_model( - info_resource.get('x.com.samsung.da.modelNum', ''), - info_resource.get('x.com.samsung.da.description', ''), - ) is not None - ) - assert recognized is True + from custom_components.localthings.registry.by_type import resolve + assert resolve(resources) is not None async def test_options_flow_init_shows_menu(hass: HomeAssistant) -> None: diff --git a/tests/test_airconditioner_capabilities.py b/tests/test_airconditioner_capabilities.py index 6c3e46e..c2dad10 100644 --- a/tests/test_airconditioner_capabilities.py +++ b/tests/test_airconditioner_capabilities.py @@ -6,7 +6,7 @@ climate entity itself lives in climate.py (imports homeassistant) and is not importable here -- consistent with how the other HA platform files are untested. """ from custom_components.localthings.registry.adapter import flatten -from custom_components.localthings.registry.by_type import for_device, for_device_by_model +from custom_components.localthings.registry.by_type import for_device_by_model from custom_components.localthings.registry.capabilities import airconditioner from custom_components.localthings.registry.discovery import discover from custom_components.localthings.registry.entities import ClimateDesc, SelectDesc @@ -24,18 +24,12 @@ def _ac(): def _resolve(name): - """Mirror the coordinator's detection order: oneUiVersion first, modelNum - fallback second (needed for issue #37's board, which reports neither - oneUiVersion nor a '_PRAC_' modelNum token).""" + """Mirror the coordinator's detection: board tokens in modelNum.""" resources = _load_device(name) - otn = resources.get('/otninformation/vs/0', {}) - one_ui = otn.get('swVersionInfo', {}).get('oneUiVersion', '') info = resources['/information/vs/0'] - reg = for_device(one_ui) if one_ui else None - if reg is None: - reg = for_device_by_model( - info['x.com.samsung.da.modelNum'], info['x.com.samsung.da.description'], - ) + reg = for_device_by_model( + info['x.com.samsung.da.modelNum'], info['x.com.samsung.da.description'], + ) return reg, resources @@ -151,9 +145,7 @@ def test_climate_consumed_hrefs_declared_as_coverage(): # --------------------------------------------------------------------------- def _ac_tp1x(): - resources = _load_device('airconditioner_tp1x_da_ac_rac_01011') - one_ui = resources['/otninformation/vs/0']['swVersionInfo']['oneUiVersion'] - return for_device(one_ui), resources + return _resolve('airconditioner_tp1x_da_ac_rac_01011') def test_tp1x_resolves_to_airconditioner_registry(): @@ -209,10 +201,11 @@ def test_tp2x_rac_20k_no_unbound_hrefs(): assert unbound == [] -def test_tp1x_rac_model_resolves_via_one_ui_version(): - """TP1X_DA-AC-RAC-01001_0000 (issue #38) self-reports oneUiVersion - '7.0 Air conditioner' -- resolved via for_device(), not the modelNum - fallback.""" +def test_tp1x_rac_model_resolves_via_rac_token(): + """TP1X_DA-AC-RAC-01001_0000 (issue #38). It self-reports oneUiVersion + '7.0 Air conditioner', which used to be what resolved it; the hyphenated + '-RAC-' board token types it now, so firmware that omits oneUiVersion (the + cool-only variant, issue #91) lands on the same registry.""" reg, _ = _resolve('airconditioner_tp1x_rac') assert reg is not None and reg.name == 'airconditioner' @@ -280,17 +273,18 @@ def test_current_limit_is_read_only(): # --------------------------------------------------------------------------- # TP1X_DA-AC-RAC-01001 cool-only global variant (issue #91). Same modelNum as # the issue #38 board above, but its /otninformation/vs/0 ships no -# swVersionInfo block, so oneUiVersion is empty and detection must fall back -# to the hyphenated '-RAC-' modelNum token (the older '_RAC_' underscore match -# doesn't fire on this DA-AC-RAC spelling). Adds /stepcontrol/vs/0 and +# swVersionInfo block, so it is typed purely by its 'RAC' modelNum token -- +# which the tokenizer reads out of the hyphenated 'DA-AC-RAC' spelling and the +# underscored 'TP2X_RAC_20K' one alike. Adds /stepcontrol/vs/0 and # /remotedeviceinfo/vs/0 (both ignored) and exposes the WindFree preset via # the Nano/NanoSleep convenient-mode codes. Its panel light is carried inside # /mode/vs/0's options blob instead of a dedicated /light/vs/0 switch. # --------------------------------------------------------------------------- -def test_tp1x_rac_coolonly_resolves_via_hyphenated_model_fallback(): - """Empty oneUiVersion -> resolved by the '-RAC-' modelNum token, not - for_device(). Guards the regression where this unit loaded as 'unknown'.""" +def test_tp1x_rac_coolonly_resolves_via_hyphenated_model_token(): + """This unit reports no oneUiVersion at all -- the 'RAC' board token is + the only thing that types it. Guards the regression where it loaded as + 'unknown'.""" resources = _load_device('airconditioner_tp1x_rac_coolonly') otn = resources.get('/otninformation/vs/0', {}) assert otn.get('swVersionInfo', {}).get('oneUiVersion', '') == '' diff --git a/tests/test_by_type.py b/tests/test_by_type.py index 301eec2..1d21435 100644 --- a/tests/test_by_type.py +++ b/tests/test_by_type.py @@ -1,88 +1,28 @@ """Tests for samsung_appliance/registry/by_type.""" import pytest -from custom_components.localthings.registry.by_type import _type_key, for_device, DeviceRegistry - - -class TestTypeKey: - """Tests for _type_key() function.""" - - def test_type_key_strips_version_prefix(self): - """'7.0 Dishwasher' -> 'dishwasher'""" - assert _type_key("7.0 Dishwasher") == "dishwasher" - - def test_type_key_preserves_spaces_as_underscores(self): - """'7.0 French Door Refrigerator' -> 'french_door_refrigerator'""" - assert _type_key("7.0 French Door Refrigerator") == "french_door_refrigerator" - - def test_type_key_no_space_returns_lowercase(self): - """'Oven' -> 'oven' (no space in string)""" - assert _type_key("Oven") == "oven" - - -class TestForDevice: - """Tests for for_device() function.""" - - def test_for_device_returns_dishwasher_registry(self): - """for_device("7.0 Dishwasher") returns a non-None DeviceRegistry.""" - registry = for_device("7.0 Dishwasher") - assert registry is not None - assert isinstance(registry, DeviceRegistry) - assert registry.name == "dishwasher" - - def test_for_device_unknown_returns_none(self): - """for_device("7.0 Toaster") returns None for unknown device type.""" - registry = for_device("7.0 Toaster") - assert registry is None - - def test_for_device_suffix_fallback(self): - """for_device("7.0 French Door Refrigerator") resolves via suffix fallback.""" - registry = for_device("7.0 French Door Refrigerator") - assert registry is not None - assert isinstance(registry, DeviceRegistry) - assert registry.name == 'refrigerator' - - def test_for_device_returns_cooktop_registry(self): - """The registry's own .name is 'gas_cooktop' (disambiguated from - induction_cooktop), but the lookup key devices route through stays - 'cooktop' -- oneUiVersion "Cooktop" still resolves here.""" - registry = for_device('7.0 Cooktop') - assert registry is not None - assert registry.name == 'gas_cooktop' - - def test_for_device_returns_range_hood_registry(self): - registry = for_device('7.0 Range Hood') - assert registry is not None - assert registry.name == 'range_hood' +from custom_components.localthings.registry.by_type import ( + DeviceRegistry, _REGISTRY_BY_KEY, +) class TestDeviceRegistries: """Tests for device registries themselves.""" - def test_dishwasher_registry_has_no_dup_hrefs(self): - """All caps in dishwasher registry have unique hrefs (or meet disambiguation rule).""" - registry = for_device("7.0 Dishwasher") - assert registry is not None + def test_every_key_maps_to_a_device_registry(self): + for key, registry in _REGISTRY_BY_KEY.items(): + assert isinstance(registry, DeviceRegistry), key - # Each href should map to exactly one cap (or multiple with rt_filter/match_fn) - for href, caps in registry.capabilities.items(): - if len(caps) > 1: - # If multiple caps share an href, all must have rt_filter or match_fn - for cap in caps: - assert cap.rt_filter is not None or cap.match_fn is not None, \ - f"href {href!r} has multiple caps but {cap!r} lacks rt_filter and match_fn" - - def test_refrigerator_registry_has_no_dup_hrefs(self): - """All caps in refrigerator registry have unique hrefs (or meet disambiguation rule).""" - registry = for_device("7.0 Refrigerator") - assert registry is not None - - # Each href should map to exactly one cap (or multiple with rt_filter/match_fn) - for href, caps in registry.capabilities.items(): - if len(caps) > 1: - # If multiple caps share an href, all must have rt_filter or match_fn - for cap in caps: - assert cap.rt_filter is not None or cap.match_fn is not None, \ - f"href {href!r} has multiple caps but {cap!r} lacks rt_filter and match_fn" + def test_no_registry_has_ambiguous_hrefs(self): + """An href carrying more than one capability needs every one of them + to declare a discriminator, or discovery would bind both.""" + for key, registry in _REGISTRY_BY_KEY.items(): + for href, caps in registry.capabilities.items(): + if len(caps) > 1: + for cap in caps: + assert cap.rt_filter is not None or cap.match_fn is not None, ( + f"{key}: href {href!r} has multiple caps but {cap!r} " + f"lacks rt_filter and match_fn" + ) class TestWasherRegistry: @@ -554,6 +494,72 @@ class TestBoardTokenAmbiguity: ) +class TestOneUiVersionIsNotConsulted: + """oneUiVersion used to be the first detection stage. It named the type + directly ('7.0 Dishwasher'), but only a minority of hardware reports it, + every device that does is already typed by its modelNum board token, and + no device-support issue was ever fixed by adding a mapping for it. It is + still reported in diagnostics as a firmware-generation marker.""" + + def test_resolve_ignores_a_recognizable_one_ui_version(self): + from custom_components.localthings.registry.by_type import resolve + resources = { + '/otninformation/vs/0': {'swVersionInfo': {'oneUiVersion': '7.0 Dishwasher'}}, + '/information/vs/0': { + 'x.com.samsung.da.modelNum': 'SOME-UNKNOWN-BOARD', + 'x.com.samsung.da.description': 'SOME-UNKNOWN-BOARD', + }, + } + assert resolve(resources) is None + + def test_resolve_types_every_one_ui_reporting_fixture_without_it( + self, all_device_fixtures + ): + """The claim above, checked: for every dump that reports a + oneUiVersion, the model strings alone reach a registry.""" + from custom_components.localthings.registry.by_type import resolve + seen = 0 + for name, resources in all_device_fixtures.items(): + one_ui = (resources.get('/otninformation/vs/0', {}) + .get('swVersionInfo', {}).get('oneUiVersion', '')) + if not one_ui: + continue + seen += 1 + assert resolve(resources) is not None, ( + f"{name} reports oneUiVersion {one_ui!r} and nothing else types it" + ) + assert seen, "no fixture reports oneUiVersion -- has the corpus changed?" + + +class TestResolve: + def test_prefers_model_strings_over_resource_signature(self, all_device_fixtures): + """Every fixture with usable model strings resolves the same way + through `resolve` as through `for_device_by_model` directly.""" + from custom_components.localthings.registry.by_type import ( + resolve, for_device_by_model, + ) + for name, resources in all_device_fixtures.items(): + info = resources.get('/information/vs/0', {}) + by_model = for_device_by_model( + info.get('x.com.samsung.da.modelNum', ''), + info.get('x.com.samsung.da.description', ''), + ) + if by_model is not None: + assert resolve(resources) is by_model, name + + def test_falls_back_to_resource_signature(self, all_device_fixtures): + """The three dumps with no /information/vs/0 still type.""" + from custom_components.localthings.registry.by_type import resolve + for name in ('cooktop', 'range_ne63a6511', 'range_no_info'): + resources = all_device_fixtures[name] + assert '/information/vs/0' not in resources, name + assert resolve(resources) is not None, name + + def test_returns_none_for_an_unrecognizable_dump(self): + from custom_components.localthings.registry.by_type import resolve + assert resolve({'/some/unknown/vs/0': {}}) is None + + class TestForDeviceByResources: def test_na9300k_without_one_ui_or_information_is_cooktop(self): from custom_components.localthings.registry.by_type import for_device_by_resources diff --git a/tests/test_end_to_end_v2.py b/tests/test_end_to_end_v2.py index 9a9ad80..119ad01 100644 --- a/tests/test_end_to_end_v2.py +++ b/tests/test_end_to_end_v2.py @@ -1,23 +1,23 @@ import pytest from tests.conftest import _load_device -from custom_components.localthings.registry.by_type import for_device, _type_key +from custom_components.localthings.registry.by_type import for_device_by_model from custom_components.localthings.registry.discovery import discover from custom_components.localthings.registry.adapter import flatten -@pytest.mark.parametrize('name,expected_type_key', [ +@pytest.mark.parametrize('name,expected_type', [ ('dishwasher', 'dishwasher'), ('refrigerator', 'refrigerator'), ]) -def test_full_pipeline_v2(name, expected_type_key): +def test_full_pipeline_v2(name, expected_type): resources = _load_device(name) - otn = resources.get('/otninformation/vs/0', {}) - one_ui = otn.get('swVersionInfo', {}).get('oneUiVersion', '') - assert _type_key(one_ui) == expected_type_key - - reg = for_device(one_ui) + info = resources['/information/vs/0'] + reg = for_device_by_model( + info['x.com.samsung.da.modelNum'], info['x.com.samsung.da.description'], + ) assert reg is not None + assert reg.name == expected_type bound = discover(resources, reg.capabilities, reg.pattern_capabilities) assert bound diff --git a/tests/test_golden_regression.py b/tests/test_golden_regression.py index b7ed571..cffbcb3 100644 --- a/tests/test_golden_regression.py +++ b/tests/test_golden_regression.py @@ -7,22 +7,10 @@ GOLDEN = Path(__file__).parent / 'fixtures' / 'golden' def _new_state_keys(name, resources): - from custom_components.localthings.registry.by_type import ( - for_device, for_device_by_model, for_device_by_resources, - ) + from custom_components.localthings.registry.by_type import resolve from custom_components.localthings.registry.discovery import discover from custom_components.localthings.registry.adapter import flatten - otn = resources.get('/otninformation/vs/0', {}) - one_ui = otn.get('swVersionInfo', {}).get('oneUiVersion', '') - info = resources.get('/information/vs/0', {}) - reg = for_device(one_ui) if one_ui else None - if reg is None: - reg = for_device_by_model( - info.get('x.com.samsung.da.modelNum', ''), - info.get('x.com.samsung.da.description', ''), - ) - if reg is None: - reg = for_device_by_resources(resources) + reg = resolve(resources) if reg is None: from custom_components.localthings.registry.registry import CAPABILITIES caps, pats = CAPABILITIES, [] @@ -422,7 +410,7 @@ def test_registry_reproduces_golden_state_keys_for_tp1x_rac(): def test_registry_reproduces_golden_state_keys_for_tp1x_rac_coolonly(): """TP1X_DA-AC-RAC-01001 cool-only global variant (issue #91) whose /otninformation/vs/0 ships no swVersionInfo block -- resolves via the - hyphenated '-RAC-' modelNum fallback rather than for_device().""" + 'RAC' board token in its modelNum.""" from tests.conftest import _load_device resources = _load_device('airconditioner_tp1x_rac_coolonly') golden = json.loads((GOLDEN / 'airconditioner_tp1x_rac_coolonly.json').read_text()) @@ -692,7 +680,8 @@ def test_registry_reproduces_golden_state_keys_for_microwave_me7500d_lamp_high() def test_registry_reproduces_golden_state_keys_for_air_purifier_tp1x_da_ac_air(): """TP1X_DA-AC-AIR-01031_0000 (issue #130) self-reports oneUiVersion - '7.0 Air purifier' and resolves via for_device() onto the existing + '7.0 Air purifier' (unused for routing) and resolves via its 'AIR' + board token onto the existing air_purifier registry (shared with the older ARTIK051_TVTL family via per-href match_fn discrimination -- see capabilities/air_purifier.py). Its /mode/vs/0 reports modes/supportedModes directly (Smart/Max/Mid/