bhetherman/localthings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(registry): drop oneUiVersion from device-type detection

Browse Source 
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.
This commit is contained in:
Marc Billow
2026-07-29 02:14:13 +00:00
parent a863df1e59
commit cafd7d5afa
17 changed files with 229 additions and 326 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/localthings/conftest.py
-2
View File
@@ -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:
tests/localthings/test_config_flow.py
+20 -46
View File
@@ -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:
tests/test_airconditioner_capabilities.py
+18 -24
View File
@@ -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', '') == ''
tests/test_by_type.py
+83 -77
View File
@@ -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
tests/test_end_to_end_v2.py
+8 -8
View File
@@ -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
tests/test_golden_regression.py
+5 -16
View File
@@ -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/