fix(subdevices): don't materialize a slot whose only live state is a meter
Issue #214: a single-split ARTIK051_KRAC_18K showed up in HA as two air conditioners. Its /device/1 answers the same unused-slot shape the Pattern A reporter's /device/2 does -- every operational rep empty {} -- but also reports a populated /energy/consumption/vs/1. Running discovery against the reporter's own quoted subdevice block reproduces their diagnostics exactly (21 bound entities, the same six hrefs), and of those 21 the only primary entity with a non-None value is energy_kwh: a lifetime kWh total was the sole thing passing discover_partitioned's liveness gate and materializing the phantom. A single-split AC has one compressor and one energy meter, so a whole-appliance running total appearing under a second index is the appliance's own bookkeeping, not evidence that hardware is installed at that slot. Exclude cumulative meters (HA's total/total_increasing state classes, plus the energy/water/gas device classes for the descriptors that deliberately declare no state class) from the gate. The gate never applies to MAIN, and across the whole fixture corpus every device has at least one non-meter live primary, so no existing device's entities change -- verified by the golden for the new fixture being identical to the plain KRAC one. Also implement async_remove_config_entry_device. A subdevice's HA device outlives the discovery that created it, so a phantom materialized by an earlier release stays in the registry with no way to delete it from the UI -- which is the state the second reporter on that issue is in, with a refrigerator whose diagnostics now report no subdevices at all. Devices the entry currently provides still refuse removal. No automatic pruning: enumeration is one-shot and a real sibling can miss a poll (issue #205 on the reference hardware), so auto-removal would discard a live subdevice's name, area and automations on a transient miss. The new fixture's /device/1 seed is the reporter's verbatim capture; its master half comes from the corpus's other KRAC unit, with the deviations spelled out in seeds_note. Claude-Session: https://claude.ai/code/session_01HzUMLnSWBT64o4BQkVEzXp
This commit is contained in:
9 files changed
+750
-22
No files matched your search
@@ -6,6 +6,7 @@ import logging
|
||||
from homeassistant.config_entries import ConfigEntry
|
||||
from homeassistant.core import HomeAssistant
|
||||
from homeassistant.exceptions import ConfigEntryNotReady
|
||||
from homeassistant.helpers import device_registry as dr
|
||||
|
||||
from .const import DOMAIN, PLATFORMS
|
||||
from .coordinator import LocalThingsCoordinator
|
||||
@@ -25,6 +26,44 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
||||
return True
|
||||
|
||||
|
||||
async def async_remove_config_entry_device(
|
||||
hass: HomeAssistant, entry: ConfigEntry, device: dr.DeviceEntry,
|
||||
) -> bool:
|
||||
"""Allow deleting a device this entry no longer provides (issue #214).
|
||||
|
||||
Defining this at all is what makes Home Assistant offer the "Delete
|
||||
device" action for our devices; without it a device registry entry
|
||||
belonging to a loaded config entry can never be removed from the UI. That
|
||||
matters because a subdevice's HA device outlives the discovery that
|
||||
created it: a candidate that materialized under an older release (issue
|
||||
#214's phantom second air conditioner, born from an unused /device/1 slot
|
||||
reporting the appliance's energy counter -- see
|
||||
registry/subdevices.py's liveness gate) leaves a device entry behind that
|
||||
nothing recreates and nothing cleans up once the gate stops materializing
|
||||
it. Same for a sibling that a firmware update stops exposing.
|
||||
|
||||
Removal is refused for devices this entry *does* currently provide --
|
||||
HA would recreate them on the next entity add, so allowing it would look
|
||||
like the delete silently failed. Deliberately no automatic pruning at
|
||||
discovery time: subdevice enumeration is one-shot and a sibling can fail
|
||||
to answer for a poll (issue #205 is exactly that on the reference
|
||||
hardware), so auto-removal would throw away a real subdevice's name,
|
||||
area and automation references on a transient miss. The user gets the
|
||||
button; the integration doesn't guess.
|
||||
"""
|
||||
coordinator: LocalThingsCoordinator | None = hass.data.get(DOMAIN, {}).get(
|
||||
entry.entry_id
|
||||
)
|
||||
if coordinator is None:
|
||||
# Entry not loaded (or already unloaded) -- nothing is claiming this
|
||||
# device, so there's nothing to protect it from being removed.
|
||||
return True
|
||||
live = set(coordinator.device_info.get('identifiers') or set())
|
||||
for subdevice in coordinator.subdevices:
|
||||
live |= set(coordinator.device_info_for(subdevice).get('identifiers') or set())
|
||||
return not (device.identifiers & live)
|
||||
|
||||
|
||||
async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
|
||||
unloaded = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
|
||||
if unloaded:
|
||||
|
||||
@@ -60,13 +60,21 @@ fridge's second compartment, ...) baked into a registry field before any
|
||||
of those families could use this module at all. `discover_partitioned`
|
||||
instead gates at the *entity* layer, after discovery+flattening: a
|
||||
candidate is only kept if it produced at least one *primary* (no
|
||||
`entity_category`) bound entity whose flattened value isn't `None` -- e.g.
|
||||
the Pattern A reporter's /device/2 does flatten to an `alarm_code` value, but
|
||||
that entity is diagnostic-category and derived from an empty /alarms/vs/2,
|
||||
so it doesn't count. This reuses the same primary/config/diagnostic
|
||||
taxonomy every registry already declares (see the adding-device-support
|
||||
skill's entity-taxonomy section) instead of adding a second, parallel
|
||||
domain-knowledge mechanism.
|
||||
`entity_category`), non-meter bound entity whose flattened value isn't
|
||||
`None` -- e.g. the Pattern A reporter's /device/2 does flatten to an
|
||||
`alarm_code` value, but that entity is diagnostic-category and derived from
|
||||
an empty /alarms/vs/2, so it doesn't count. This reuses the same
|
||||
primary/config/diagnostic taxonomy every registry already declares (see the
|
||||
adding-device-support skill's entity-taxonomy section) instead of adding a
|
||||
second, parallel domain-knowledge mechanism.
|
||||
|
||||
The meter carve-out is issue #214, and it's the same "an unused slot still
|
||||
answers *something*" problem one layer further in: that reporter's
|
||||
single-split ARTIK051_KRAC_18K has a /device/1 whose operational reps are
|
||||
all empty {} -- the /device/2 shape above -- but which also reports a
|
||||
populated /energy/consumption/vs/1, a whole-appliance lifetime kWh counter
|
||||
that materialized the slot as a phantom second air conditioner. See
|
||||
`_has_live_primary_entity`.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -447,22 +455,63 @@ class SkippedSubdevice:
|
||||
hrefs: tuple[str, ...]
|
||||
|
||||
|
||||
# Sensor kinds whose value is a running total the *appliance* keeps rather
|
||||
# than a reading of the subdevice's own hardware -- excluded from the
|
||||
# liveness gate below (issue #214). HA's own running-total state classes
|
||||
# cover most of them; the consumption device classes catch the rest, since a
|
||||
# descriptor may deliberately declare no state_class (common.ENERGY_METER's
|
||||
# monthly totals reset at each billing boundary, so they aren't
|
||||
# `total_increasing`).
|
||||
_METER_STATE_CLASSES = frozenset({'total', 'total_increasing'})
|
||||
_METER_DEVICE_CLASSES = frozenset({'energy', 'water', 'gas'})
|
||||
|
||||
|
||||
def _is_meter(desc) -> bool:
|
||||
"""True for a cumulative consumption/counter descriptor -- see the two
|
||||
constants above. Only SensorDesc carries either attribute; everything
|
||||
else answers False through the getattr defaults."""
|
||||
return (
|
||||
getattr(desc, 'state_class', None) in _METER_STATE_CLASSES
|
||||
or getattr(desc, 'device_class', None) in _METER_DEVICE_CLASSES
|
||||
)
|
||||
|
||||
|
||||
def _has_live_primary_entity(bound, state: dict) -> bool:
|
||||
"""True if flattening `bound` (one candidate subdevice's BoundEntity
|
||||
list) produced at least one non-`None` value for a *primary* entity --
|
||||
`entity_category` unset, HA's own "the user acts on or watches this"
|
||||
tier (see the adding-device-support skill's entity-taxonomy section).
|
||||
tier (see the adding-device-support skill's entity-taxonomy section) --
|
||||
that isn't a cumulative meter (`_is_meter`).
|
||||
|
||||
This is the materialization gate itself (see this module's docstring):
|
||||
the Pattern A reporter's `/device/2` does flatten to one non-`None` value
|
||||
(`alarm_code`), but that entity is `diagnostic`-category and derived
|
||||
from an empty `/alarms/vs/2` -- a config/diagnostic entity reading
|
||||
"something" proves nothing about whether a physical subdevice is actually
|
||||
installed there, so it's deliberately excluded from this check.
|
||||
This is the materialization gate itself (see this module's docstring).
|
||||
Two exclusions, both for the same reason -- the question this answers is
|
||||
"is a physical subdevice installed at this slot?", and neither kind of
|
||||
value can speak to it:
|
||||
|
||||
- **Non-primary entities.** The Pattern A reporter's `/device/2` does
|
||||
flatten to one non-`None` value (`alarm_code`), but that entity is
|
||||
`diagnostic`-category and derived from an empty `/alarms/vs/2` -- a
|
||||
config/diagnostic entity reading "something" proves nothing about
|
||||
whether hardware is there.
|
||||
- **Cumulative meters** (issue #214). An unused slot on the issue #214
|
||||
reporter's ARTIK051_KRAC_18K reports `/energy/consumption/vs/1` with a
|
||||
populated `cumulativePower` while every operational rep on it
|
||||
(`/power/1`, `/mode/1`, `/mode/vs/1`, `/temperature/current/1`,
|
||||
`/temperature/desired/1`, `/airflow/1`, `/humidity/1`) is empty `{}` --
|
||||
i.e. exactly the Pattern A `/device/2` shape plus a lifetime kWh
|
||||
counter. That counter got the slot materialized as a phantom second
|
||||
air conditioner. A single-split AC has one compressor and one energy
|
||||
meter, so a whole-appliance total showing up under a second index is
|
||||
the appliance's own bookkeeping, not evidence of a second indoor unit.
|
||||
A genuinely installed subdevice reports its own operational state too
|
||||
(the Pattern A reporter's real `/device/1` reports power, mode, both
|
||||
temperatures and airflow), and that state is what still passes this
|
||||
gate.
|
||||
"""
|
||||
from .adapter import _key # see discover_partitioned's deferred-import note
|
||||
return any(
|
||||
not b.desc.entity_category and state.get(_key(b)) is not None
|
||||
not b.desc.entity_category and not _is_meter(b.desc)
|
||||
and state.get(_key(b)) is not None
|
||||
for b in bound
|
||||
)
|
||||
|
||||
|
||||
Reference in new issue
Block a user