fix: address final review findings

- for_device(): suffix fallback for descriptive names
  ("french_door_refrigerator" → refrigerator registry)
- Capability.href_prefix: replaces rt_filter for pattern caps;
  DOOR_GENERIC now uses href_prefix='/door/' (batch reps have no rt)
- discover(): apply href_prefix gate before rt_filter in pattern loop
- Remove dead bound_hrefs variable from discover()
- Remove vestigial default_observe_port from RuntimeDescriptor
This commit is contained in:
Marc Billow
2026-06-30 16:33:36 -05:00
parent 6f681622c2
commit 1c4cb2e272
8 changed files with 26 additions and 11 deletions
-2
View File
@@ -37,7 +37,6 @@ def _encode(cfg: dict) -> bytes:
class RuntimeDescriptor:
name: str
seed_path: list[str]
default_observe_port: int
active_interval_s: float = 5.0
idle_interval_s: float = 30.0
is_active: Optional[Callable[[dict], bool]] = None
@@ -208,7 +207,6 @@ def build_runtime_descriptor(bound, *, topic_prefix, ha_prefix, device_name,
return RuntimeDescriptor(
name=name,
seed_path=['device', '0'],
default_observe_port=default_port or 0,
active_interval_s=active_interval_s,
idle_interval_s=idle_interval_s,
is_active=_make_is_active(bound),
@@ -48,4 +48,10 @@ def for_device(one_ui_version: str) -> Optional[DeviceRegistry]:
DeviceRegistry if a matching registry exists, None otherwise.
"""
key = _type_key(one_ui_version)
return _REGISTRY_BY_KEY.get(key)
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
@@ -32,6 +32,10 @@ def _build(caps: list[Capability]) -> dict[str, list[Capability]]:
for cap in caps:
if cap.href is None:
raise ValueError(f"Use pattern_capabilities for href=None caps")
if cap.href_prefix is not None:
raise ValueError(
f"href_prefix is only valid for pattern caps (href=None); "
f"cap with href={cap.href!r} must not set href_prefix")
out.setdefault(cap.href, []).append(cap)
# Validate that multi-cap hrefs have proper discrimination
@@ -353,7 +353,7 @@ def _door_key(href: str) -> str:
DOOR_GENERIC = Capability(
href=None,
rt_filter='oic.r.door',
href_prefix='/door/',
poll_tier='hot',
key_fn=_door_key,
entities=(
+1
View File
@@ -13,6 +13,7 @@ class Capability:
entities: tuple[SamsungEntityDescription, ...] = ()
poll_tier: str = 'warm' # 'hot' | 'warm' | 'cold'
rt_filter: Optional[str] = None # bind only if rt_filter in rep.get('rt', ())
href_prefix: Optional[str] = None # pattern caps only: bind only if href starts with this
match_fn: Optional[Callable[[dict, dict], bool]] = None # match_fn(rep, resources) -> bool
key_fn: Optional[Callable[[str], str]] = None # href -> entity key (pattern caps)
# Rare optional hooks — only operational-state-style resources use these.
+2 -2
View File
@@ -37,7 +37,6 @@ def discover(
log: Optional[Callable[[str], None]] = None,
) -> list[BoundEntity]:
out: list[BoundEntity] = []
bound_hrefs: set[str] = set()
for href, rep in resources.items():
if not isinstance(rep, dict):
@@ -58,11 +57,12 @@ def discover(
matched = True
if matched:
bound_hrefs.add(href)
continue
# Pattern cap fallback — first matching pattern wins
for cap in pattern_caps:
if cap.href_prefix and not href.startswith(cap.href_prefix):
continue
if cap.rt_filter is not None and cap.rt_filter not in rts:
continue
if cap.match_fn is not None and not cap.match_fn(rep, resources):
+7
View File
@@ -34,6 +34,13 @@ class TestForDevice:
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'
class TestDeviceRegistries:
"""Tests for device registries themselves."""
+4 -5
View File
@@ -55,9 +55,9 @@ def test_refrigerator_registry_discovers_expected_keys():
# ---------------------------------------------------------------------------
def test_door_generic_binds_unknown_door_href():
"""/door/wine/0 with oic.r.door rt binds via DOOR_GENERIC with key door_wine_open."""
"""/door/wine/0 binds via DOOR_GENERIC (href_prefix='/door/') with key door_wine_open."""
resources = {
'/door/wine/0': {'rt': ['oic.r.door'], 'openState': 'Open'},
'/door/wine/0': {'openState': 'Open'},
}
bound = discover(resources, {}, [fridge.DOOR_GENERIC])
assert len(bound) == 1
@@ -66,10 +66,9 @@ def test_door_generic_binds_unknown_door_href():
def test_door_generic_does_not_bind_doors_aggregate():
"""/doors/vs/0 with Samsung-proprietary rt is not bound by DOOR_GENERIC."""
"""/doors/vs/0 does not start with '/door/' so DOOR_GENERIC must not bind it."""
resources = {
'/doors/vs/0': {
'rt': ['x.com.samsung.da.doors'],
'x.com.samsung.da.items': [],
},
}
@@ -80,7 +79,7 @@ def test_door_generic_does_not_bind_doors_aggregate():
def test_door_generic_does_not_bind_known_door():
"""/door/cooler/0 is claimed by DOOR_FRIDGE exact cap; DOOR_GENERIC must not also bind it."""
resources = {
'/door/cooler/0': {'rt': ['oic.r.door'], 'openState': 'Closed'},
'/door/cooler/0': {'openState': 'Closed'},
}
reg = refrigerator.REGISTRY
bound = discover(resources, reg.capabilities, [fridge.DOOR_GENERIC])