Distinguish not-yet-fetched stub reps from confirmed-empty ones (issue #127)
parse_device0_batch used to collapse /device/0's {"href": "..."} "no data
yet" marker into a plain {}, indistinguishable from a resource the device
had actually polled and confirmed empty. Every exists_fn using the "not
rep or ..." stub carve-out (and entity._is_included's default field-gate)
then treated both the same way, creating phantom always-"unknown" entities
for any resource a model simply doesn't support (e.g. GSzabados's fridge's
/energy/consumption/vs/0).
is_stub_rep() now recognizes only the literal {"href": ...} marker as a
stub; a genuine {} is treated as the device's real (if empty) answer and
gates the entity off like any other missing field. Updated the energy
meter, self-check error, cooktop burner, and range-hood auto-operation
exists_fn call sites, plus three golden fixtures that had baked the
phantom-entity behavior in as "expected".
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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',
|
||||
),
|
||||
),
|
||||
|
||||
-6
@@ -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"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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) == {}
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user