diff --git a/custom_components/localthings/entity.py b/custom_components/localthings/entity.py index b15d166..cbdbca1 100644 --- a/custom_components/localthings/entity.py +++ b/custom_components/localthings/entity.py @@ -24,10 +24,15 @@ def _is_included(bound: BoundEntity, coordinator: 'LocalThingsCoordinator') -> b A stub rep (is_stub_rep — /device/0's "resource exists, no data fetched yet" marker) is included anyway so it can be populated by sub-polls. A - genuinely empty {} rep is the device's confirmed (if empty) answer, not - a stub, and gates the entity off like any other missing field — else a - resource this model never populates would spawn a phantom always- - "unknown" entity every session (issue #127). + genuinely empty {} rep is included too by this default gate -- whether + empty means "not populated yet" or "permanently unsupported" needs + per-field domain knowledge this generic gate doesn't have: /alarms/vs/0's + {} is fridge.py's documented *normal* no-alarm state (see + _active_alarm_codes), not an absence signal, and it's far from the only + resource like that. Only a capability whose author has actually verified + a field is genuinely never populated on unsupported hardware opts into + stricter gating with its own is_stub_rep-based exists_fn (see + common.ENERGY_METER, issue #127) -- this default stays permissive. """ rep = coordinator.last_resources.get(bound.href) if rep is None: @@ -35,7 +40,7 @@ def _is_included(bound: BoundEntity, coordinator: 'LocalThingsCoordinator') -> b if bound.desc.exists_fn is not None: return bound.desc.exists_fn(rep, coordinator.last_resources) if bound.desc.field: - if is_stub_rep(rep): + if not rep or is_stub_rep(rep): return True return bound.desc.field in rep return True # rep_fn or no-field entities (ButtonDesc) are always included diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index 775a318..ac60523 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -503,7 +503,7 @@ AI_ENERGY_LEVEL = Capability( href='/energy/ailevel/vs/0', poll_tier='cold', entities=( - # No `not rep` stub carve-out on either side, unlike most exists_fn + # No is_stub_rep carve-out on either side, unlike most exists_fn # gates in this file -- entity creation only ever runs once, against # whichever snapshot happens to be current the moment platforms are # set up (see entity._is_included / __init__.py's diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index 5a834a1..2050a70 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -399,13 +399,20 @@ class TestAiEnergyLevelStubDoesNotDecideThePlatform: if a stub carve-out let one of them win at setup time while the other wins once real data lands, flatten() would feed the already-instantiated entity a value shaped for the other platform (e.g. a bool into a Select - expecting a string option). Neither side gets a `not rep` carve-out, so - an unfetched stub can never win entity creation for either platform -- + expecting a string option). Neither side gets an is_stub_rep carve-out, + so an unfetched stub can never win entity creation for either platform -- the entity simply doesn't appear until a reload happens with real data, same as any other exists_fn-gated entity in this codebase that's unlucky on first-poll timing, instead of appearing as the wrong widget type.""" - def test_neither_widget_exists_on_empty_stub_rep(self): + def test_neither_widget_exists_on_true_stub_rep(self): + switch = _ai_energy_level_desc('SwitchDesc') + select = _ai_energy_level_desc('SelectDesc') + stub = {'href': '/energy/ailevel/vs/0'} + assert switch.exists_fn(stub, {}) is False + assert select.exists_fn(stub, {}) is False + + def test_neither_widget_exists_on_genuinely_empty_rep(self): switch = _ai_energy_level_desc('SwitchDesc') select = _ai_energy_level_desc('SelectDesc') assert switch.exists_fn({}, {}) is False diff --git a/tests/test_entity.py b/tests/test_entity.py new file mode 100644 index 0000000..1e0538b --- /dev/null +++ b/tests/test_entity.py @@ -0,0 +1,79 @@ +"""Tests for entity._is_included -- the one-time entity-creation gate run +per platform at async_setup_entry (see sensor.py etc.), separate from +adapter.flatten()'s per-poll state values. issue #127 review: this default +gate has to stay permissive on a genuinely empty {} rep, not just a true +is_stub_rep stub -- a resource like /alarms/vs/0 is validly empty in its +normal, no-alarm state (fridge._active_alarm_codes), so excluding the +entity there would silently drop working sensors on first-poll timing, +not just fix phantom ones. +""" +from custom_components.localthings.entity import _is_included +from custom_components.localthings.registry.capability import Capability +from custom_components.localthings.registry.discovery import BoundEntity +from custom_components.localthings.registry.entities import SensorDesc + + +class _FakeCoordinator: + def __init__(self, last_resources): + self.last_resources = last_resources + + +def _bound(desc, href): + capability = Capability(href=href, entities=(desc,)) + return BoundEntity(href=href, capability=capability, desc=desc) + + +class TestDefaultFieldGate: + """No explicit exists_fn -- the field-presence default in entity.py.""" + + def _bound(self): + return _bound(SensorDesc(key='x', field='x.com.samsung.da.value'), '/x/vs/0') + + def test_included_when_field_present(self): + bound = self._bound() + coord = _FakeCoordinator({'/x/vs/0': {'x.com.samsung.da.value': '1'}}) + assert _is_included(bound, coord) is True + + def test_excluded_when_field_absent_from_populated_rep(self): + bound = self._bound() + coord = _FakeCoordinator({'/x/vs/0': {'x.com.samsung.da.other': '1'}}) + assert _is_included(bound, coord) is False + + def test_included_on_true_stub(self): + bound = self._bound() + coord = _FakeCoordinator({'/x/vs/0': {'href': '/x/vs/0'}}) + assert _is_included(bound, coord) is True + + def test_included_on_genuinely_empty_rep(self): + """The default gate stays permissive on a real {} -- see + /alarms/vs/0, whose empty rep is the normal no-alarm state, not a + signal the hardware is unsupported. Only a capability-specific + exists_fn (e.g. common.ENERGY_METER) opts into excluding on + confirmed-empty, after verifying that's actually safe for its field.""" + bound = self._bound() + coord = _FakeCoordinator({'/x/vs/0': {}}) + assert _is_included(bound, coord) is True + + def test_excluded_when_href_missing_from_resources(self): + bound = self._bound() + coord = _FakeCoordinator({}) + assert _is_included(bound, coord) is False + + +class TestExplicitExistsFnGate: + def test_explicit_exists_fn_overrides_default_field_gate(self): + desc = SensorDesc(key='x', field='x.com.samsung.da.value', + exists_fn=lambda rep, resources: rep.get('flag') is True) + bound = _bound(desc, '/x/vs/0') + coord = _FakeCoordinator({'/x/vs/0': {'flag': True}}) + assert _is_included(bound, coord) is True + coord = _FakeCoordinator({'/x/vs/0': {'flag': False, 'x.com.samsung.da.value': '1'}}) + assert _is_included(bound, coord) is False + + +class TestNoFieldEntities: + def test_rep_fn_entity_always_included(self): + desc = SensorDesc(key='x', rep_fn=lambda rep: rep.get('x.com.samsung.da.value')) + bound = _bound(desc, '/x/vs/0') + coord = _FakeCoordinator({'/x/vs/0': {}}) + assert _is_included(bound, coord) is True