diff --git a/custom_components/localthings/entity.py b/custom_components/localthings/entity.py index c5ddf9e..8a8fc3d 100644 --- a/custom_components/localthings/entity.py +++ b/custom_components/localthings/entity.py @@ -8,7 +8,7 @@ from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.const import EntityCategory from .registry.adapter import _key -from .registry.discovery import BoundEntity +from .registry.discovery import BoundEntity, _snake_to_title from .const import DOMAIN from .coordinator import LocalThingsCoordinator @@ -44,7 +44,7 @@ def _derive_name(state_key: str) -> str: instance number with a space: "door_cooler_open1" → "Door Cooler Open 1". """ name = re.sub(r'(\d+)$', lambda m: f' {m.group()}' if int(m.group()) > 0 else '', state_key) - return name.replace('_', ' ').title().strip() + return _snake_to_title(name).strip() class LocalThingsEntity(CoordinatorEntity[LocalThingsCoordinator]): diff --git a/custom_components/localthings/observe.py b/custom_components/localthings/observe.py index b3b4544..e3c5aad 100644 --- a/custom_components/localthings/observe.py +++ b/custom_components/localthings/observe.py @@ -69,6 +69,7 @@ class ObserveManager: self.last_mode_change_wall = time.time() self._settle_until: dict[str, float] = {} self._settle_lock = threading.Lock() + self._cache_lock = threading.Lock() self.subscribed_hrefs: set[str] = set() self._notified: set[str] = set() self._last_notify_ts: float | None = None @@ -105,13 +106,22 @@ class ObserveManager: exists_fn/options_field gates on the moment one partial update comes through, even though nothing about the device's actual supported options changed. + + `apply()` is the sole path StateCache mutations flow through in + this component (poll, sweep, and OBSERVE notify all funnel here), + so `_cache_lock` serializes the read-then-write across those + threads (DTLS reader for notifies, executor threads for poll/ + sweep) -- without it, two concurrent updates for the same href + could each read the same prior rep and the second writer would + silently lose the first's fields, reintroducing the exact bug + this merge fixes. """ if self._is_settling(href): self.log.debug("dropping %s update for %s (settling)", source, href) return False - prior = self.cache.get(href) - merged = {**prior, **rep} if prior else rep - return self.cache.apply_rep(href, merged, source=source) + with self._cache_lock: + merged = {**(self.cache.get(href) or {}), **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 diff --git a/custom_components/localthings/registry/discovery.py b/custom_components/localthings/registry/discovery.py index f7c5392..891c2ae 100644 --- a/custom_components/localthings/registry/discovery.py +++ b/custom_components/localthings/registry/discovery.py @@ -30,6 +30,12 @@ class BoundEntity: instance_name: Optional[str] = None +def _snake_to_title(s: str) -> str: + """'CUBED_ICE'/'cubed_ice' -> 'Cubed Ice'. Shared with entity.py's + _derive_name, which applies the same transform to a state key.""" + return s.replace('_', ' ').title() + + 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 @@ -39,7 +45,7 @@ def _instance_name(cap: Capability, rep: dict) -> Optional[str]: raw = rep.get(cap.name_field) if not isinstance(raw, str) or not raw: return None - return raw.replace('_', ' ').title() + return _snake_to_title(raw) def instance_suffix(href: str) -> str: @@ -50,6 +56,18 @@ def instance_suffix(href: str) -> str: return '' +def _bind(cap: Capability, href: str, inst: str, inst_name: Optional[str], + key_prefix: Optional[str] = None) -> list[BoundEntity]: + """Build one BoundEntity per entity on `cap`, sharing the instance/ + key-prefix/instance-name computed once by the caller.""" + return [ + BoundEntity(href=href, capability=cap, desc=desc, instance=inst, + key_override=f'{key_prefix}_{desc.key}' if key_prefix else None, + instance_name=inst_name) + for desc in cap.entities + ] + + def discover( resources: dict[str, dict], registry: dict[str, list[Capability]], @@ -71,11 +89,7 @@ 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, - instance_name=inst_name)) + out.extend(_bind(cap, href, inst, _instance_name(cap, rep))) matched = True if matched: @@ -90,15 +104,10 @@ 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_name=inst_name)) + out.extend(_bind(cap, href, inst, _instance_name(cap, rep), '_'.join(segs))) matched = True break