diff --git a/custom_components/localthings/entity.py b/custom_components/localthings/entity.py index e8c0b9b..b15d166 100644 --- a/custom_components/localthings/entity.py +++ b/custom_components/localthings/entity.py @@ -8,6 +8,7 @@ from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.const import EntityCategory from .registry.adapter import _key +from .registry.batch import is_stub_rep from .registry.discovery import BoundEntity, _snake_to_title from .const import DOMAIN @@ -21,9 +22,12 @@ def _is_included(bound: BoundEntity, coordinator: 'LocalThingsCoordinator') -> b require that field to be present in the resource rep so that optional fields on shared resources don't create phantom entities. - An empty rep ({}) means /device/0 returned a stub for this resource — - the resource exists on the device but data hasn't been fetched yet. - In that case we include the entity so it can be populated by sub-polls. + 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). """ rep = coordinator.last_resources.get(bound.href) if rep is None: @@ -31,7 +35,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 not rep: # stub — resource known to exist, data not yet fetched + if 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/batch.py b/custom_components/localthings/registry/batch.py index 6a90c2e..c7fb3ad 100644 --- a/custom_components/localthings/registry/batch.py +++ b/custom_components/localthings/registry/batch.py @@ -2,8 +2,25 @@ from __future__ import annotations +def is_stub_rep(rep: dict) -> bool: + """True for the device's "resource exists, no data fetched yet" marker -- + an echoed {"href": "..."} with no other fields. + + Distinct from a genuinely empty {} rep, which is the device's confirmed + (if empty) answer -- e.g. an unsupported resource on this model that will + never populate. Conflating the two used to make every field-gated entity + on a permanently-empty resource look like a not-yet-fetched stub forever, + creating phantom always-"unknown" entities (issue #127).""" + return isinstance(rep, dict) and set(rep.keys()) == {'href'} + + def parse_device0_batch(device0: list) -> dict[str, dict]: - """Extract {href: rep} from a /device/0 CBOR list response.""" + """Extract {href: rep} from a /device/0 CBOR list response. + + A stub rep is passed through unchanged rather than collapsed to {} -- + downstream code (entity._is_included, capability exists_fns) uses + is_stub_rep to tell "not fetched yet" apart from a confirmed-empty {}. + """ out = {} for entry in device0[1:]: # skip [0] (device-level rep) if not isinstance(entry, dict): @@ -12,8 +29,6 @@ def parse_device0_batch(device0: list) -> dict[str, dict]: rep = entry.get('rep') if not href: continue - # rep == {"href": "..."} is a stub (resource present, no current data). - # Include it as {} so capabilities still bind and the entity exists. if isinstance(rep, dict): - out[href] = {} if set(rep.keys()) == {'href'} else rep + out[href] = rep return out diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index cd03a91..775a318 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -12,6 +12,7 @@ against live device dumps: """ from datetime import datetime, timezone +from ..batch import is_stub_rep from ..capability import Capability from ..entities import ( BinarySensorDesc, ButtonDesc, SelectDesc, SensorDesc, SwitchDesc, @@ -380,32 +381,35 @@ _DEAD_INSTANTANEOUS_POWER = '-500' ENERGY_METER = Capability( href='/energy/consumption/vs/0', entities=( - # `not rep` keeps the empty-{} stub carve-out (see entity._is_included): + # `is_stub_rep(rep)` keeps the stub carve-out (see entity._is_included): # an explicit exists_fn otherwise bypasses it, which would drop the - # entity when /device/0 returns a not-yet-fetched stub. On a populated - # rep, hide power only for the dead sentinel or an absent field. + # entity when /device/0 returns a not-yet-fetched stub. A genuinely + # empty {} rep is NOT a stub -- it's the device's confirmed (if empty) + # answer, so it falls through to the normal field/sentinel checks like + # any populated rep. On a populated rep, hide power only for the dead + # sentinel or an absent field. SensorDesc(key='power_watts', field='x.com.samsung.da.instantaneousPower', device_class='power', state_class='measurement', unit='W', value_fn=clamp_power, - exists_fn=lambda rep, resources: not rep or ( + exists_fn=lambda rep, resources: is_stub_rep(rep) or ( rep.get('x.com.samsung.da.instantaneousPower') not in (None, _DEAD_INSTANTANEOUS_POWER))), SensorDesc(key='energy_kwh', field='x.com.samsung.da.cumulativePower', device_class='energy', state_class='total_increasing', unit='kWh', value_fn=wh_to_kwh, exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.cumulativePower' in rep)), + is_stub_rep(rep) or 'x.com.samsung.da.cumulativePower' in rep)), # cumulativeConsumption is a second, independently-varying running # total alongside cumulativePower -- some fridges (issue #26) report - # both. Self-gates off where only cumulativePower is present. `not - # rep or` keeps the same empty-{} stub carve-out as power_watts/ + # both. Self-gates off where only cumulativePower is present. The + # `is_stub_rep(rep) or` keeps the same stub carve-out as power_watts/ # energy_kwh above -- without it, an exists_fn permanently drops the # entity if setup happens to land on a not-yet-fetched stub. SensorDesc(key='power_energy_kwh', field='x.com.samsung.da.cumulativeConsumption', device_class='energy', state_class='total_increasing', unit='kWh', value_fn=wh_to_kwh, exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.cumulativeConsumption' in rep)), + is_stub_rep(rep) or 'x.com.samsung.da.cumulativeConsumption' in rep)), # AI Energy Mode's lifetime savings estimate vs. an unoptimized # baseline -- present on some models (e.g. TP1X_REF_21K, issue #21/ # #27) and absent on others (issue #20/#26), unlike cumulativePower. @@ -413,7 +417,7 @@ ENERGY_METER = Capability( device_class='energy', state_class='total_increasing', unit='kWh', value_fn=wh_to_kwh, exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.cumulativeSavedPower' in rep)), + is_stub_rep(rep) or 'x.com.samsung.da.cumulativeSavedPower' in rep)), # Monthly billing-cycle totals -- the completed prior month and the # in-progress current month. Not ever-increasing (each resets at # month boundary), so no state_class. @@ -421,12 +425,12 @@ ENERGY_METER = Capability( device_class='energy', unit='kWh', value_fn=wh_to_kwh, exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.monthlyConsumption' in rep)), + is_stub_rep(rep) or 'x.com.samsung.da.monthlyConsumption' in rep)), SensorDesc(key='energy_this_month_kwh', field='x.com.samsung.da.thismonthlyConsumption', device_class='energy', unit='kWh', value_fn=wh_to_kwh, exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.thismonthlyConsumption' in rep)), + is_stub_rep(rep) or 'x.com.samsung.da.thismonthlyConsumption' in rep)), ), ) @@ -562,7 +566,7 @@ SELF_CHECK = Capability( icon='mdi:alert-circle-outline', entity_category='diagnostic', exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.error' in rep), + is_stub_rep(rep) or 'x.com.samsung.da.error' in rep), value_fn=lambda v: (', '.join(v) if v else None) if isinstance(v, list) else v), ButtonDesc(key='selfcheck_start', field='', payload='Start', icon='mdi:play-circle-outline', diff --git a/custom_components/localthings/registry/capabilities/cooktop.py b/custom_components/localthings/registry/capabilities/cooktop.py index b76138f..96f1cfd 100644 --- a/custom_components/localthings/registry/capabilities/cooktop.py +++ b/custom_components/localthings/registry/capabilities/cooktop.py @@ -9,6 +9,7 @@ cooktop must not be remotely ignited by an automation. import re +from ..batch import is_stub_rep from ..capability import Capability from ..entities import BinarySensorDesc, SensorDesc @@ -97,7 +98,7 @@ COOKTOP_MODE = Capability( options, f'OperationState{slot}' ), exists_fn=lambda rep, resources, slot=slot: ( - not rep or _option_value( + is_stub_rep(rep) or _option_value( rep.get('x.com.samsung.da.options'), f'OperationState{slot}', ) is not None diff --git a/custom_components/localthings/registry/capabilities/range_hood.py b/custom_components/localthings/registry/capabilities/range_hood.py index f3b4eca..58e262b 100644 --- a/custom_components/localthings/registry/capabilities/range_hood.py +++ b/custom_components/localthings/registry/capabilities/range_hood.py @@ -9,6 +9,7 @@ independent fields. from datetime import datetime, timezone +from ..batch import is_stub_rep from ..capability import Capability from ..entities import ( BinarySensorDesc, @@ -103,7 +104,7 @@ HOOD_FAN = Capability( # #137) -- this board has no auto-ventilation mode, unlike the # standalone range hood this capability was written for. exists_fn=lambda rep, resources: ( - not rep or 'x.com.samsung.da.hood.autoOperation' in rep), + is_stub_rep(rep) or 'x.com.samsung.da.hood.autoOperation' in rep), value_fn=lambda value: str(value).lower() == 'on', ), ), diff --git a/tests/fixtures/golden/air_purifier.json b/tests/fixtures/golden/air_purifier.json index 7e049c5..05e3410 100644 --- a/tests/fixtures/golden/air_purifier.json +++ b/tests/fixtures/golden/air_purifier.json @@ -7,18 +7,12 @@ "diagnosis_status", "display_light", "dust", - "energy_kwh", - "energy_last_month_kwh", - "energy_saved_kwh", - "energy_this_month_kwh", "fan_direction", "filter_progress", "fine_dust", "odor", "operating_mode", - "power_energy_kwh", "power_switch", - "power_watts", "super_fine_dust" ] } diff --git a/tests/fixtures/golden/refrigerator_artik051_dongle_ref.json b/tests/fixtures/golden/refrigerator_artik051_dongle_ref.json index 6174968..6c50fe9 100644 --- a/tests/fixtures/golden/refrigerator_artik051_dongle_ref.json +++ b/tests/fixtures/golden/refrigerator_artik051_dongle_ref.json @@ -5,16 +5,10 @@ "defrost_delay", "diagnosis_status", "door_onedoorfreezer_open", - "energy_kwh", - "energy_last_month_kwh", - "energy_saved_kwh", - "energy_this_month_kwh", "firmware_update", "freezer_setpoint", "freezer_temperature", "ice_maker_enabled", - "power_energy_kwh", - "power_watts", "rapid_freezing", "rapid_fridge", "sabbath_mode" diff --git a/tests/fixtures/golden/refrigerator_artik051_dongle_ref_cooler.json b/tests/fixtures/golden/refrigerator_artik051_dongle_ref_cooler.json index 984569c..44d2d96 100644 --- a/tests/fixtures/golden/refrigerator_artik051_dongle_ref_cooler.json +++ b/tests/fixtures/golden/refrigerator_artik051_dongle_ref_cooler.json @@ -8,14 +8,8 @@ "diagnosis_status", "door_cooler_open", "door_onedoorfreezer_open", - "energy_kwh", - "energy_last_month_kwh", - "energy_saved_kwh", - "energy_this_month_kwh", "firmware_update", "ice_maker_enabled", - "power_energy_kwh", - "power_watts", "rapid_freezing", "rapid_fridge", "sabbath_mode" diff --git a/tests/test_batch.py b/tests/test_batch.py new file mode 100644 index 0000000..d707662 --- /dev/null +++ b/tests/test_batch.py @@ -0,0 +1,56 @@ +"""Tests for registry.batch — the /device/0 sweep parser and its stub marker. + +issue #127: a device whose /energy/consumption/vs/0 is permanently +unsupported reports a genuinely empty {} rep for it. The previous parser +collapsed /device/0's own {"href": "..."} "not fetched yet" marker to that +same {} shape, so downstream exists_fn checks couldn't tell "confirmed +empty" apart from "haven't polled it yet" and created phantom always- +"unknown" entities either way. is_stub_rep/parse_device0_batch now keep the +two shapes distinct. +""" +from custom_components.localthings.registry.batch import is_stub_rep, parse_device0_batch + + +class TestIsStubRep: + def test_true_for_bare_href_marker(self): + assert is_stub_rep({'href': '/energy/consumption/vs/0'}) is True + + def test_false_for_genuinely_empty_rep(self): + assert is_stub_rep({}) is False + + def test_false_for_populated_rep(self): + assert is_stub_rep({'x.com.samsung.da.cumulativePower': '58900'}) is False + + def test_false_for_href_plus_data(self): + """A real, populated rep may legitimately echo 'href' alongside + actual fields -- only a rep with *no other keys* is the stub.""" + assert is_stub_rep({'href': '/x/0', 'value': True}) is False + + +class TestParseDevice0Batch: + def test_stub_rep_kept_distinct_from_genuine_empty(self): + device0 = [ + {}, + {'href': '/energy/consumption/vs/0', 'rep': {'href': '/energy/consumption/vs/0'}}, + {'href': '/sabbath/vs/0', 'rep': {}}, + ] + resources = parse_device0_batch(device0) + assert is_stub_rep(resources['/energy/consumption/vs/0']) is True + assert is_stub_rep(resources['/sabbath/vs/0']) is False + assert resources['/sabbath/vs/0'] == {} + + def test_populated_rep_passes_through_unchanged(self): + device0 = [ + {}, + {'href': '/door/cooler/0', 'rep': {'openState': 'Close'}}, + ] + resources = parse_device0_batch(device0) + assert resources['/door/cooler/0'] == {'openState': 'Close'} + + def test_skips_entries_without_href(self): + device0 = [{}, {'rep': {'a': 1}}] + assert parse_device0_batch(device0) == {} + + def test_skips_non_dict_rep(self): + device0 = [{}, {'href': '/x/0', 'rep': 'not-a-dict'}] + assert parse_device0_batch(device0) == {} diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index 8f7bcad..5a834a1 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -253,13 +253,24 @@ class TestEnergyMeter: kwh = next(e for e in common.ENERGY_METER.entities if e.key == 'energy_kwh') assert kwh.exists_fn({'x.com.samsung.da.cumulativePower': '58900'}, {}) is True - def test_both_entities_included_on_empty_stub(self): - """An empty {} rep means the resource exists but data isn't fetched yet - (see entity._is_included) -- include both so sub-polls populate them.""" + def test_both_entities_included_on_true_stub(self): + """A true stub -- /device/0's {"href": "..."} "not fetched yet" + marker (see registry.batch.is_stub_rep) -- means the resource exists + but data isn't fetched yet; include both so sub-polls populate them.""" pw = next(e for e in common.ENERGY_METER.entities if e.key == 'power_watts') kwh = next(e for e in common.ENERGY_METER.entities if e.key == 'energy_kwh') - assert pw.exists_fn({}, {}) is True - assert kwh.exists_fn({}, {}) is True + stub = {'href': '/energy/consumption/vs/0'} + assert pw.exists_fn(stub, {}) is True + assert kwh.exists_fn(stub, {}) is True + + def test_both_entities_hidden_on_genuinely_empty_rep(self): + """A real {} rep (no 'href' key) is the device's confirmed -- if + empty -- answer, not a stub, so a model that never populates this + resource doesn't get a phantom always-"unknown" entity (issue #127).""" + pw = next(e for e in common.ENERGY_METER.entities if e.key == 'power_watts') + kwh = next(e for e in common.ENERGY_METER.entities if e.key == 'energy_kwh') + assert pw.exists_fn({}, {}) is False + assert kwh.exists_fn({}, {}) is False def test_power_watts_hidden_when_field_absent_in_populated_rep(self): """A populated rep that lacks instantaneousPower must not spawn a @@ -416,11 +427,17 @@ class TestSelfCheckError: desc = self._desc() assert desc.exists_fn({'x.com.samsung.da.status': 'Ready'}, {}) is False - def test_exists_for_empty_stub_rep(self): - """An empty {} rep is /device/0's not-yet-fetched-stub carve-out -- - must be included-for-now, same as ENERGY_METER's fields.""" + def test_exists_for_true_stub_rep(self): + """A true stub ({"href": "..."}) is /device/0's not-yet-fetched + marker -- must be included-for-now, same as ENERGY_METER's fields.""" desc = self._desc() - assert desc.exists_fn({}, {}) is True + assert desc.exists_fn({'href': '/selfcheck/vs/0'}, {}) is True + + def test_hidden_for_genuinely_empty_rep(self): + """A real {} rep is the device's confirmed empty answer, not a stub -- + must NOT be force-included (issue #127's phantom-entity pattern).""" + desc = self._desc() + assert desc.exists_fn({}, {}) is False def test_value_joins_list(self): desc = self._desc()