fix(fridge,observe): merge partial cache updates, name ice makers dynamically
Issue #27: the flex-zone/cooler-drawer select vanished after a device stopped including supportedOptions on an update for /mode/vs/0. ObserveManager.apply() handed reps straight to StateCache.apply_rep, which fully replaces the cached rep -- so a partial update (missing a field the select's exists_fn/options_field gate on) silently erased data a fuller update had previously supplied. apply() now merges incoming reps onto whatever's already cached instead. Also give ice-maker entities (and any future pattern-cap instance) a device-given display name instead of the href-derived "Icemaker One"/"Icemaker Two": Capability.name_field lets a pattern capability read and normalize an instance name (e.g. iceMaker.name's "CUBED_ICE") for use as the entity name prefix, independent of the stable key/unique_id.
This commit is contained in:
8 files changed
+164
-5
No files matched your search
@@ -57,7 +57,15 @@ class LocalThingsEntity(CoordinatorEntity[LocalThingsCoordinator]):
|
||||
self._bound = bound
|
||||
self._state_key = _key(bound)
|
||||
self._attr_unique_id = f"{DOMAIN}_{coordinator.device_serial}_{self._state_key}"
|
||||
self._attr_name = bound.desc.name if bound.desc.name is not None else _derive_name(self._state_key)
|
||||
if bound.desc.name is not None:
|
||||
self._attr_name = bound.desc.name
|
||||
elif bound.instance_name:
|
||||
# A device-given instance name (e.g. an ice maker's "Cubed
|
||||
# Ice") takes the place of the href-derived instance label,
|
||||
# keeping the same entity-specific suffix (issue #27).
|
||||
self._attr_name = f"{bound.instance_name} {_derive_name(bound.desc.key)}".strip()
|
||||
else:
|
||||
self._attr_name = _derive_name(self._state_key)
|
||||
self._attr_translation_key = bound.desc.translation_key
|
||||
self._attr_icon = bound.desc.icon
|
||||
raw_cat = bound.desc.entity_category
|
||||
|
||||
@@ -92,11 +92,26 @@ class ObserveManager:
|
||||
return True
|
||||
|
||||
def apply(self, href: str, rep: dict, source: str) -> bool:
|
||||
"""Gate a StateCache.apply_rep call through the write-settle guard."""
|
||||
"""Gate a StateCache.apply_rep call through the write-settle guard.
|
||||
|
||||
Merges the incoming rep onto whatever's already cached for this
|
||||
href rather than handing it to StateCache.apply_rep verbatim --
|
||||
apply_rep does a full replace, and Samsung devices don't always
|
||||
repeat every field on every update (issue #27: a /mode/vs/0
|
||||
OBSERVE notify -- and, on at least one Bespoke fridge, even the
|
||||
/device/0 sweep entry for that href -- can carry just `modes`,
|
||||
omitting `supportedOptions`/`supportedModes` entirely). A full
|
||||
replace would silently wipe fields a select entity's
|
||||
exists_fn/options_field gates on the moment one partial update
|
||||
comes through, even though nothing about the device's actual
|
||||
supported options changed.
|
||||
"""
|
||||
if self._is_settling(href):
|
||||
self.log.debug("dropping %s update for %s (settling)", source, href)
|
||||
return False
|
||||
return self.cache.apply_rep(href, rep, source=source)
|
||||
prior = self.cache.get(href)
|
||||
merged = {**prior, **rep} if prior else rep
|
||||
return self.cache.apply_rep(href, merged, source=source)
|
||||
|
||||
def on_notification(self, href: str, payload: bytes) -> None:
|
||||
"""Wired as DtlsCoapSession.on_notification. Runs on the DTLS
|
||||
|
||||
@@ -100,6 +100,12 @@ ICEMAKER_NIGHTTIME = Capability(
|
||||
# Icemaker (generic — covers /icemaker/one/vs/0, /icemaker/two/vs/0)
|
||||
# /icemaker/status/vs/0 is kept as exact-href cap and binds first.
|
||||
# /icemaker/nighttime/vs/0 is excluded by match_fn (lacks iceMaker.state).
|
||||
#
|
||||
# Entity names are derived from x.com.samsung.da.iceMaker.name ("CUBED_ICE",
|
||||
# "ICE_BITES") via name_field, not the href's "one"/"two" segment -- these two
|
||||
# ice makers are independent on/off toggles that can both be enabled at once
|
||||
# (issue #27), so they stay separate entities rather than a single ice-type
|
||||
# select, but users still want them labeled with the device's own names.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _icemaker_write(field):
|
||||
@@ -112,6 +118,7 @@ ICEMAKER_GENERIC = Capability(
|
||||
href=None,
|
||||
href_prefix='/icemaker/',
|
||||
match_fn=lambda rep, resources: 'x.com.samsung.da.iceMaker.state' in rep,
|
||||
name_field='x.com.samsung.da.iceMaker.name',
|
||||
poll_tier='warm',
|
||||
entities=(
|
||||
SensorDesc(key='making_status',
|
||||
|
||||
@@ -15,6 +15,11 @@ class Capability:
|
||||
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
|
||||
strip_prefix_in_key: bool = False # strip href_prefix segs before building key_override
|
||||
# Rep field holding this instance's device-given name (e.g. an ice
|
||||
# maker's "CUBED_ICE"/"ICE_BITES"), normalized and used as the display
|
||||
# name prefix in place of the href-derived instance label. Does not
|
||||
# affect key_override/unique_id -- only what's shown in the UI.
|
||||
name_field: Optional[str] = None
|
||||
match_fn: Optional[Callable[[dict, dict], bool]] = None # match_fn(rep, resources) -> bool
|
||||
# Rare optional hook — only operational-state-style resources use this.
|
||||
on_observation: Optional[Callable[[dict, dict], None]] = None
|
||||
|
||||
@@ -27,6 +27,19 @@ class BoundEntity:
|
||||
desc: SamsungEntityDescription
|
||||
instance: str = ''
|
||||
key_override: Optional[str] = None
|
||||
instance_name: Optional[str] = None
|
||||
|
||||
|
||||
def _instance_name(cap: Capability, rep: dict) -> Optional[str]:
|
||||
"""Normalize `cap.name_field`'s raw value ("CUBED_ICE" -> "Cubed Ice")
|
||||
for use as a display-name prefix, or None if the cap doesn't declare
|
||||
one or the device didn't report it."""
|
||||
if not cap.name_field:
|
||||
return None
|
||||
raw = rep.get(cap.name_field)
|
||||
if not isinstance(raw, str) or not raw:
|
||||
return None
|
||||
return raw.replace('_', ' ').title()
|
||||
|
||||
|
||||
def instance_suffix(href: str) -> str:
|
||||
@@ -58,9 +71,11 @@ def discover(
|
||||
if cap.match_fn is not None and not cap.match_fn(rep, resources):
|
||||
continue
|
||||
inst = instance_suffix(href)
|
||||
inst_name = _instance_name(cap, rep)
|
||||
for desc in cap.entities:
|
||||
out.append(BoundEntity(href=href, capability=cap,
|
||||
desc=desc, instance=inst))
|
||||
desc=desc, instance=inst,
|
||||
instance_name=inst_name))
|
||||
matched = True
|
||||
|
||||
if matched:
|
||||
@@ -75,13 +90,15 @@ def discover(
|
||||
if cap.match_fn is not None and not cap.match_fn(rep, resources):
|
||||
continue
|
||||
inst = instance_suffix(href)
|
||||
inst_name = _instance_name(cap, rep)
|
||||
# Auto-derive key prefix from href segments (skip digits and 'vs')
|
||||
src = href[len(cap.href_prefix):] if (cap.strip_prefix_in_key and cap.href_prefix) else href
|
||||
segs = [s for s in src.strip('/').split('/') if s and not s.isdigit() and s != 'vs']
|
||||
for desc in cap.entities:
|
||||
key_override = '_'.join(segs) + '_' + desc.key
|
||||
out.append(BoundEntity(href=href, capability=cap, desc=desc,
|
||||
instance=inst, key_override=key_override))
|
||||
instance=inst, key_override=key_override,
|
||||
instance_name=inst_name))
|
||||
matched = True
|
||||
break
|
||||
|
||||
|
||||
@@ -34,6 +34,28 @@ def test_apply_writes_through_when_not_settling():
|
||||
assert mgr.cache.get('/oven/vs/0') == {'a': 1}
|
||||
|
||||
|
||||
def test_apply_merges_partial_update_onto_prior_rep():
|
||||
"""Regression test for issue #27: a Bespoke fridge's /mode/vs/0 notify
|
||||
(and even a later sweep entry for that href) can carry only `modes`,
|
||||
omitting `supportedOptions` entirely. A full replace would wipe
|
||||
`supportedOptions` from the cache the moment that partial update
|
||||
arrives, even though the device's supported options didn't change --
|
||||
which is exactly what made the flex-zone select disappear."""
|
||||
mgr = _manager()
|
||||
full = {
|
||||
'x.com.samsung.da.modes': ['CVN_CONVERTIBLE_ZONE', 'CV_FDR_MEAT'],
|
||||
'x.com.samsung.da.supportedOptions': ['CV_FDR_WINE', 'CV_FDR_MEAT'],
|
||||
}
|
||||
mgr.apply('/mode/vs/0', full, source='poll')
|
||||
|
||||
partial = {'x.com.samsung.da.modes': ['CVN_CONVERTIBLE_ZONE', 'WATERFILTER_ENABLE']}
|
||||
mgr.apply('/mode/vs/0', partial, source='observe')
|
||||
|
||||
cached = mgr.cache.get('/mode/vs/0')
|
||||
assert cached['x.com.samsung.da.modes'] == ['CVN_CONVERTIBLE_ZONE', 'WATERFILTER_ENABLE']
|
||||
assert cached['x.com.samsung.da.supportedOptions'] == ['CV_FDR_WINE', 'CV_FDR_MEAT']
|
||||
|
||||
|
||||
def test_apply_drops_update_during_settle_window():
|
||||
mgr = _manager()
|
||||
mgr.cache.apply_rep('/oven/vs/0', {'a': 1}, source='seed')
|
||||
|
||||
@@ -158,6 +158,46 @@ def test_discover_match_fn_decline_is_not_logged_as_gap():
|
||||
assert seen == []
|
||||
|
||||
|
||||
def test_discover_pattern_cap_reads_name_field():
|
||||
"""A pattern cap's name_field normalizes a device-given instance name
|
||||
(e.g. an ice maker's "CUBED_ICE") for use in the entity's display name,
|
||||
independent of the href-derived key (issue #27)."""
|
||||
cap = Capability(
|
||||
href=None,
|
||||
href_prefix='/icemaker/',
|
||||
name_field='x.com.samsung.da.iceMaker.name',
|
||||
entities=(BinarySensorDesc(key='enabled', field='x.com.samsung.da.iceMaker.state'),),
|
||||
)
|
||||
resources = {
|
||||
'/icemaker/one/vs/0': {
|
||||
'x.com.samsung.da.iceMaker.state': 'On',
|
||||
'x.com.samsung.da.iceMaker.name': 'CUBED_ICE',
|
||||
},
|
||||
'/icemaker/two/vs/0': {
|
||||
'x.com.samsung.da.iceMaker.state': 'On',
|
||||
'x.com.samsung.da.iceMaker.name': 'ICE_BITES',
|
||||
},
|
||||
}
|
||||
bound = discover(resources, {}, pattern_caps=[cap])
|
||||
names = {b.href: b.instance_name for b in bound}
|
||||
assert names == {
|
||||
'/icemaker/one/vs/0': 'Cubed Ice',
|
||||
'/icemaker/two/vs/0': 'Ice Bites',
|
||||
}
|
||||
|
||||
|
||||
def test_discover_pattern_cap_name_field_absent_leaves_instance_name_none():
|
||||
cap = Capability(
|
||||
href=None,
|
||||
href_prefix='/icemaker/',
|
||||
name_field='x.com.samsung.da.iceMaker.name',
|
||||
entities=(BinarySensorDesc(key='enabled', field='x.com.samsung.da.iceMaker.state'),),
|
||||
)
|
||||
resources = {'/icemaker/one/vs/0': {'x.com.samsung.da.iceMaker.state': 'On'}}
|
||||
bound = discover(resources, {}, pattern_caps=[cap])
|
||||
assert bound[0].instance_name is None
|
||||
|
||||
|
||||
def test_discover_rt_filter_gates_binding():
|
||||
"""Cap with rt_filter must not bind a rep whose rt list does not match."""
|
||||
oven_mode_cap = Capability(
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
"""Tests for LocalThingsEntity's display-name derivation
|
||||
(custom_components/localthings/entity.py) -- the explicit-name,
|
||||
device-given-instance-name, and href-derived fallbacks.
|
||||
"""
|
||||
from custom_components.localthings.entity import LocalThingsEntity
|
||||
from custom_components.localthings.registry.capability import Capability
|
||||
from custom_components.localthings.registry.discovery import BoundEntity
|
||||
from custom_components.localthings.registry.entities import BinarySensorDesc
|
||||
|
||||
|
||||
class _FakeCoordinator:
|
||||
device_serial = 'TEST-SERIAL'
|
||||
|
||||
def __init__(self, last_resources=None):
|
||||
self.last_resources = last_resources or {}
|
||||
|
||||
|
||||
def _make_entity(desc, href='/x/vs/0', key_override=None, instance='', instance_name=None):
|
||||
capability = Capability(href=href, entities=(desc,))
|
||||
bound = BoundEntity(href=href, capability=capability, desc=desc,
|
||||
instance=instance, key_override=key_override,
|
||||
instance_name=instance_name)
|
||||
return LocalThingsEntity(_FakeCoordinator(), bound)
|
||||
|
||||
|
||||
def test_explicit_name_wins_over_everything():
|
||||
desc = BinarySensorDesc(key='enabled', name='Explicit Name')
|
||||
entity = _make_entity(desc, instance_name='Cubed Ice')
|
||||
assert entity._attr_name == 'Explicit Name'
|
||||
|
||||
|
||||
def test_instance_name_prefixes_the_derived_suffix():
|
||||
"""Issue #27: an ice maker's device-given name ("Cubed Ice") replaces
|
||||
the href-derived instance label ("Icemaker One") as the name prefix,
|
||||
keeping the same entity-specific suffix."""
|
||||
desc = BinarySensorDesc(key='enabled')
|
||||
entity = _make_entity(desc, key_override='icemaker_one_enabled',
|
||||
instance_name='Cubed Ice')
|
||||
assert entity._attr_name == 'Cubed Ice Enabled'
|
||||
|
||||
|
||||
def test_no_instance_name_falls_back_to_derived_state_key():
|
||||
desc = BinarySensorDesc(key='enabled')
|
||||
entity = _make_entity(desc, key_override='icemaker_one_enabled')
|
||||
assert entity._attr_name == 'Icemaker One Enabled'
|
||||
Reference in new issue
Block a user