Compare commits

..
Author SHA1 Message Date
Marc Billow 8504682d9b Stop showing permanently-unsupported energy/self-check sensors as Unknown
ENERGY_METER's five sensors (and SELF_CHECK's selfcheck_error, and
cooktop.py's per-burner state) all carried a `not rep or <field check>`
exists_fn -- a deliberate stub carve-out so an entity isn't dropped
just because /device/0's first poll can hand back an empty {} for a
resource that populates moments later (see entity._is_included's
docstring). But an empty {} rep and a permanently-unsupported resource
look identical from content alone: a fridge whose
/energy/consumption/vs/0 is genuinely always {} (issue #78, spotted via
its screenshot showing every energy/power sensor stuck at "Unknown")
got all five sensors created anyway, since `not {}` is True regardless
of which case it actually is.

Drop the `not rep or` prefix from all six call sites, so exists_fn goes
back to a plain field-presence check. This is the same tradeoff
AI_ENERGY_LEVEL already makes deliberately (see
TestAiEnergyLevelStubDoesNotDecideThePlatform) -- an entity that's
unlucky on first-poll timing stays absent until a reload sees real
data, rather than every genuinely-unsupported resource showing a
permanent phantom sensor. entity.py's own default field-presence gate
(no explicit exists_fn) is untouched -- that's a much larger blast
radius across every plain-field descriptor in the codebase and isn't
what issue #78 actually hit.

Regenerates air_purifier's golden fixture, the one existing device
whose /energy/consumption/vs/0 is genuinely empty -- it now correctly
drops the six energy/power keys instead of shipping them as unusable
sensors.
2026-07-27 01:03:01 +00:00
5 changed files with 37 additions and 69 deletions
+4 -35
View File
@@ -103,38 +103,7 @@ sub-polled between summary polls. Pick descriptor types from `entities.py`
as a gap for a human, or ignore it with a documented reason — never invent an
entity on a hunch (`ignored.py`'s rule).
## 5. Never hard-code the one dump's values
A single `/device/0` dump is **one device on one firmware** — its select options,
temperature range/increment, and any other "what values are valid here" data are
**that unit's snapshot**, not the field's universe. Other units of the same model
(different region, firmware, board revision) can support more, fewer, or
differently-stepped values. If the dump reports the live option/range list, wire
the descriptor to read it live — don't transcribe what you saw into a Python
literal:
- **Selects**: use `options_field` (a resource field holding the live options
list, e.g. `supportedWaterTemperature`, `iceType.supported`) so `select.py`
reads the current device's real options every time, not `options=(...)` typed
from the dump. Reach for a callable `options` only when the values require
cross-resource computation the field alone can't give you — a static tuple is
right only for genuinely fixed, spec-defined enums (e.g. an OCF-standard field
with a closed value set), never for vendor `supported*` lists.
- **Number ranges/steps**: use `range_field` (a `[min, max]`-shaped field) or
`native_min_fn`/`native_max_fn`/`step_fn` to read bounds from the live rep —
see `oven.py`'s `_setpoint_bounds`. Only fall back to static `native_min`/
`native_max`/`step` when the dump has no such field and the bound is genuinely
fixed by spec, not just "the only value this one unit happened to report."
- **Anywhere else** a field's presence, count, or shape looks like it could vary
by model/config (course lists, capability flags, supported-mode arrays):
check whether the resource carries its own `supported*` companion field before
assuming the observed value is exhaustive.
When you do hard-code something (a genuinely fixed enum, a spec constant), that's
a judgement call worth a one-line comment saying why it's safe — the default
assumption should be "derive it," not "copy it."
## 6. Names and enum labels live in translations, never in Python
## 5. Names and enum labels live in translations, never in Python
Descriptors have **no `name` field**. Every entity is named from the shipped
catalog, keyed by `translation_key` — which defaults to the descriptor's own
@@ -173,7 +142,7 @@ no `[%key:...%]` resolution (that's Core build tooling). Every other language
must mirror `en.json` key for key — also enforced by
`tests/test_translations.py`.
## 7. Coverage discipline: bound or ignored
## 6. Coverage discipline: bound or ignored
Every href in the dump must resolve, or the repair fires. If a resource isn't
worth an entity, add it to `capabilities/ignored.py` (a no-entity `Capability`)
@@ -187,7 +156,7 @@ friendlier href**.
ignored because washers bind it. When only one family should ignore an href
that another binds, scope the ignore to that family's registry.
## 8. Reuse before writing new code
## 7. Reuse before writing new code
Check `common.py` (generic OCF: power, energy, alarms, water) and `laundry.py`
(shared washer/dryer/dishwasher: buzzer, job status, `cycle_select` + course
@@ -196,7 +165,7 @@ registry uses `fridge.FIRMWARE_UPDATE`; all three laundry families share
`laundry.cycle_select`. If two families hand-roll the same helper, hoist it to a
shared module rather than copying.
## 9. Lock it in
## 8. Lock it in
1. Add a **scrubbed** fixture `tests/fixtures/<type>_device.json`
(`{"device0": [ {devcol rep}, {href, rep}, ... ]}`) — replace serials, MACs,
@@ -237,32 +237,33 @@ _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):
# 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.
# No `not rep` stub carve-out (issue #78: a fridge whose
# /energy/consumption/vs/0 is permanently {} -- it just doesn't
# report energy data -- got all five sensors created anyway and
# stuck at "Unknown" forever, since an empty {} rep and a
# not-yet-fetched stub are indistinguishable from content alone).
# Same tradeoff AI_ENERGY_LEVEL below already makes deliberately:
# an entity that's unlucky on first-poll timing stays absent until
# a reload sees real data, rather than every genuinely-unsupported
# resource showing a permanent phantom sensor.
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: (
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)),
exists_fn=lambda rep, resources: '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/
# energy_kwh above -- without it, an exists_fn permanently drops the
# entity if setup happens to land on a not-yet-fetched stub.
# both. Self-gates off where only cumulativePower is present.
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)),
'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.
@@ -270,7 +271,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)),
'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.
@@ -278,12 +279,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)),
'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)),
'x.com.samsung.da.thismonthlyConsumption' in rep)),
),
)
@@ -414,12 +415,12 @@ SELF_CHECK = Capability(
icon='mdi:clipboard-check-outline',
entity_category='diagnostic'),
# List of error codes from the last self-check; joined for display.
# Not every fridge reports the field, hence the exists_fn.
# Not every fridge reports the field, hence the exists_fn (no stub
# carve-out -- see ENERGY_METER above).
SensorDesc(key='selfcheck_error', field='x.com.samsung.da.error',
icon='mdi:alert-circle-outline',
entity_category='diagnostic',
exists_fn=lambda rep, resources: (
not rep or 'x.com.samsung.da.error' in rep),
exists_fn=lambda rep, resources: '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',
@@ -97,7 +97,7 @@ COOKTOP_MODE = Capability(
options, f'OperationState{slot}'
),
exists_fn=lambda rep, resources, slot=slot: (
not rep or _option_value(
_option_value(
rep.get('x.com.samsung.da.options'),
f'OperationState{slot}',
) is not None
-6
View File
@@ -6,19 +6,13 @@
"diagnosis_status",
"display_light",
"dust",
"energy_kwh",
"energy_last_month_kwh",
"energy_saved_kwh",
"energy_this_month_kwh",
"fan_direction",
"fan_speed_level",
"filter_progress",
"fine_dust",
"odor",
"operating_mode",
"power_energy_kwh",
"power_switch",
"power_watts",
"super_fine_dust"
]
}
+13 -9
View File
@@ -156,13 +156,17 @@ 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_hidden_on_empty_rep(self):
"""Issue #78: a permanently-{} rep (a fridge that just doesn't report
energy data) must not spawn a phantom sensor stuck at "Unknown"
forever -- an empty {} rep and a not-yet-fetched stub are
indistinguishable from content alone, so this deliberately drops the
old stub carve-out (same tradeoff AI_ENERGY_LEVEL already makes,
see TestAiEnergyLevelStubDoesNotDecideThePlatform)."""
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
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
@@ -319,11 +323,11 @@ 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_hidden_for_empty_rep(self):
"""Issue #78: no stub carve-out -- see ENERGY_METER's
test_both_entities_hidden_on_empty_rep for why."""
desc = self._desc()
assert desc.exists_fn({}, {}) is True
assert desc.exists_fn({}, {}) is False
def test_value_joins_list(self):
desc = self._desc()