Fix airflow fan power-href preference and unique_id collision risk
Opus review of the previous commit caught two real bugs. The airflow fan's power writes preferred /power/vs/0, copied from the TP1X fan class -- but that order is only harmless there because TP1X never reports /power/0 at all. This family's dumps carry both hrefs, and the power_switch entity is unconditionally bound to /power/0 when present, so the fan was writing to a different resource than power_switch reads/writes, leaving the two entities disagreeing until the next poll. Flipped to prefer /power/0, matching the range hood's fan and common.POWER_GENERIC. Also renamed the new FanDesc's key from 'fan' to 'airflow_fan': BoundEntity unique_ids are derived from key alone, not href, so it collided with air_purifier.FAN's own 'fan' key on the (currently unobserved, but unenforced) possibility of a board reporting both. Added a platform-level test file covering the power-href preference and percentage<->speed-code mapping, mirroring test_range_hood_fan.py's harness.
This commit is contained in:
@@ -253,17 +253,26 @@ class LocalThingsAirflowFan(LocalThingsEntity, FanEntity):
|
||||
return self.coordinator.resource(href) or {}
|
||||
|
||||
def _power_payload(self, enabled: bool) -> tuple[str, bool, str]:
|
||||
"""Same power-href fallback as LocalThingsAirPurifierFan above."""
|
||||
"""Prefer /power/0 like LocalThingsRangeHoodFan above, NOT
|
||||
LocalThingsAirPurifierFan's vs/0-first order -- that order is only
|
||||
harmless for the TP1X board because it never reports /power/0 at
|
||||
all. This family's dumps carry both hrefs, and common.POWER_GENERIC
|
||||
(the power_switch entity) is unconditionally bound to /power/0
|
||||
whenever it's present, so writing here to /power/vs/0 first would
|
||||
leave power_switch and this fan reading/writing two different
|
||||
resources -- disagreeing until the next poll refreshes the other
|
||||
one (the same optimistic-apply lag coordinator.py's own comments
|
||||
warn about)."""
|
||||
resources = self.coordinator.last_resources
|
||||
target = POWER_VS_HREF if POWER_VS_HREF in resources else POWER_HREF
|
||||
target = POWER_HREF if POWER_HREF in resources else POWER_VS_HREF
|
||||
return 'power', enabled, target
|
||||
|
||||
@property
|
||||
def is_on(self) -> bool:
|
||||
power = self._rep(POWER_VS_HREF).get('x.com.samsung.da.power')
|
||||
if power is not None:
|
||||
return str(power).lower() == 'on'
|
||||
return bool(self._rep(POWER_HREF).get('value'))
|
||||
power = self._rep(POWER_HREF)
|
||||
if 'value' in power:
|
||||
return bool(power.get('value'))
|
||||
return str(self._rep(POWER_VS_HREF).get('x.com.samsung.da.power', '')).lower() == 'on'
|
||||
|
||||
@property
|
||||
def speed_count(self) -> int:
|
||||
|
||||
@@ -136,16 +136,6 @@ DEVICE_ACTIVE = Capability(
|
||||
),
|
||||
)
|
||||
|
||||
# Confirmed via issue #56's second, properly-spaced round of diagnostics
|
||||
# (two independent units, 60-90s apart per setting): /airflow/0's `speed` is
|
||||
# a clean, monotonic 0-4 code across Auto/Sleep/Low/Medium/High, so it now
|
||||
# backs a real ordered-speed fan (fan.py's LocalThingsAirflowFan, same
|
||||
# SET_SPEED shape as the range hood's) instead of a read-only sensor --
|
||||
# no named-preset table needed, since HA's percentage steps map onto the
|
||||
# raw 0-4 code directly, the same way the range hood's numeric levels do.
|
||||
# `direction` stays a plain diagnostic: every dump seen (both rounds, both
|
||||
# units) reads 'Off' for it regardless of fan setting, so there's nothing
|
||||
# confirmed to control there yet.
|
||||
def _airflow_fan_write(payload, rep, href=None):
|
||||
kind, value, *args = payload
|
||||
if kind == 'power':
|
||||
@@ -159,11 +149,29 @@ def _airflow_fan_write(payload, rep, href=None):
|
||||
return None
|
||||
|
||||
|
||||
# Confirmed via issue #56's second, properly-spaced round of diagnostics
|
||||
# (two independent units, 60-90s apart per setting): /airflow/0's `speed` is
|
||||
# a clean, monotonic 0-4 code across Auto/Sleep/Low/Medium/High, so it now
|
||||
# backs a real ordered-speed fan (fan.py's LocalThingsAirflowFan, same
|
||||
# SET_SPEED shape as the range hood's) instead of a read-only sensor --
|
||||
# no named-preset table needed, since HA's percentage steps map onto the
|
||||
# raw 0-4 code directly, the same way the range hood's numeric levels do.
|
||||
# `direction` stays a plain diagnostic: every dump seen (both rounds, both
|
||||
# units) reads 'Off' for it regardless of fan setting, so there's nothing
|
||||
# confirmed to control there yet.
|
||||
#
|
||||
# Keyed 'airflow_fan', not 'fan' -- FAN below (bound to the shared
|
||||
# /mode/vs/0 href) also uses 'fan', and BoundEntity's unique_id is built
|
||||
# from key alone (entity.py's _key), not href. FAN and AIRFLOW_GENERIC are
|
||||
# only *empirically* mutually exclusive (every dump seen has one board
|
||||
# generation's shape or the other, never both), not architecturally
|
||||
# enforced the way same-href caps are by _build()'s match_fn check -- a
|
||||
# same key would collide if a future board ever reported both.
|
||||
AIRFLOW_GENERIC = Capability(
|
||||
href='/airflow/0',
|
||||
href=HREF_AIRFLOW,
|
||||
poll_tier='warm',
|
||||
entities=(
|
||||
FanDesc(key='fan', field='speed', write_fn=_airflow_fan_write),
|
||||
FanDesc(key='airflow_fan', field='speed', write_fn=_airflow_fan_write),
|
||||
SensorDesc(key='fan_direction', field='direction',
|
||||
icon='mdi:rotate-3d-variant',
|
||||
entity_category='diagnostic'),
|
||||
|
||||
+1
-1
@@ -1,5 +1,6 @@
|
||||
{
|
||||
"state_keys": [
|
||||
"airflow_fan",
|
||||
"alarm_code",
|
||||
"clean_level",
|
||||
"device_active",
|
||||
@@ -10,7 +11,6 @@
|
||||
"energy_last_month_kwh",
|
||||
"energy_saved_kwh",
|
||||
"energy_this_month_kwh",
|
||||
"fan",
|
||||
"fan_direction",
|
||||
"filter_progress",
|
||||
"fine_dust",
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
"""HA fan-entity mapping tests for the ARTIK051_TVTL air purifier (issue #56)."""
|
||||
|
||||
from custom_components.localthings.fan import LocalThingsAirflowFan
|
||||
from custom_components.localthings.registry.by_type import air_purifier
|
||||
from custom_components.localthings.registry.capabilities.air_purifier import HREF_AIRFLOW
|
||||
from custom_components.localthings.registry.discovery import discover
|
||||
from custom_components.localthings.registry.entities import FanDesc
|
||||
from tests.conftest import _load_device
|
||||
|
||||
|
||||
class _FakeCoordinator:
|
||||
device_serial = 'TEST-AIRFLOW-SERIAL'
|
||||
device_info = {}
|
||||
data = {}
|
||||
|
||||
def __init__(self, resources):
|
||||
self.last_resources = resources
|
||||
self.commands = []
|
||||
|
||||
def resource(self, href):
|
||||
return self.last_resources.get(href, {})
|
||||
|
||||
async def async_send_command(self, bound, payload):
|
||||
self.commands.append((bound, payload))
|
||||
|
||||
|
||||
def _entity(resources, coordinator=None):
|
||||
bound = discover(
|
||||
resources,
|
||||
air_purifier.REGISTRY.capabilities,
|
||||
air_purifier.REGISTRY.pattern_capabilities,
|
||||
)
|
||||
fan_bound = next(
|
||||
item for item in bound
|
||||
if isinstance(item.desc, FanDesc) and item.href == HREF_AIRFLOW
|
||||
)
|
||||
return LocalThingsAirflowFan(
|
||||
coordinator or _FakeCoordinator(resources), fan_bound,
|
||||
)
|
||||
|
||||
|
||||
def test_power_off_maps_to_zero_percent_and_five_retained_speeds():
|
||||
entity = _entity(_load_device('air_purifier'))
|
||||
assert entity.is_on is False
|
||||
assert entity.speed_count == 5
|
||||
assert entity.percentage == 0
|
||||
|
||||
|
||||
def test_active_codes_map_to_ordered_percentages():
|
||||
"""Confirmed via issue #56's second, properly-spaced diagnostics round:
|
||||
/airflow/0's speed is a clean 0-4 code across Auto/Sleep/Low/Medium/High."""
|
||||
resources = _load_device('air_purifier')
|
||||
resources['/power/0']['value'] = True
|
||||
|
||||
for code, expected_percentage in ((0, 20), (1, 40), (2, 60), (3, 80), (4, 100)):
|
||||
resources['/airflow/0']['speed'] = code
|
||||
assert _entity(resources).percentage == expected_percentage
|
||||
|
||||
|
||||
async def test_power_write_prefers_standard_resource_when_both_exist():
|
||||
"""Both /power/0 and /power/vs/0 are present on this family's dumps --
|
||||
/power/0 must win, matching common.POWER_GENERIC's own preference (the
|
||||
power_switch entity is unconditionally bound to /power/0 whenever it's
|
||||
present, so writing here to /power/vs/0 first would leave the two
|
||||
entities disagreeing until the next poll)."""
|
||||
resources = _load_device('air_purifier')
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _entity(resources, coordinator)
|
||||
|
||||
await entity.async_turn_on()
|
||||
|
||||
assert coordinator.commands[-1][1] == ('power', True, '/power/0')
|
||||
|
||||
|
||||
async def test_power_write_falls_back_to_vendor_resource():
|
||||
resources = _load_device('air_purifier')
|
||||
resources.pop('/power/0')
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _entity(resources, coordinator)
|
||||
|
||||
await entity.async_turn_off()
|
||||
|
||||
assert coordinator.commands[-1][1] == ('power', False, '/power/vs/0')
|
||||
|
||||
|
||||
async def test_set_percentage_writes_raw_speed_code():
|
||||
resources = _load_device('air_purifier')
|
||||
resources['/power/0']['value'] = True
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _entity(resources, coordinator)
|
||||
|
||||
await entity.async_set_percentage(60)
|
||||
|
||||
assert coordinator.commands[-1][1] == ('speed', 2)
|
||||
|
||||
|
||||
async def test_set_percentage_zero_turns_off():
|
||||
resources = _load_device('air_purifier')
|
||||
resources['/power/0']['value'] = True
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _entity(resources, coordinator)
|
||||
|
||||
await entity.async_set_percentage(0)
|
||||
|
||||
assert coordinator.commands[-1][1] == ('power', False, '/power/0')
|
||||
@@ -41,7 +41,7 @@ def test_expected_entities_present():
|
||||
for key in (
|
||||
'power_switch', 'alarm_code', 'dust', 'fine_dust', 'super_fine_dust',
|
||||
'odor', 'clean_level', 'filter_progress', 'device_active',
|
||||
'diagnosis_status', 'fan', 'fan_direction',
|
||||
'diagnosis_status', 'airflow_fan', 'fan_direction',
|
||||
'display_light', 'operating_mode',
|
||||
):
|
||||
assert key in state, key
|
||||
@@ -121,7 +121,7 @@ def test_airflow_fan_write_contract():
|
||||
(two independent units, 60-90s apart per setting): /airflow/0's `speed`
|
||||
is a clean, monotonic 0-4 code, so the write is a plain int passthrough
|
||||
-- no named-preset table needed (see fan.py's LocalThingsAirflowFan)."""
|
||||
fan_desc = next(e for e in air_purifier.AIRFLOW_GENERIC.entities if e.key == 'fan')
|
||||
fan_desc = next(e for e in air_purifier.AIRFLOW_GENERIC.entities if e.key == 'airflow_fan')
|
||||
assert fan_desc.write_fn(('speed', 3), {}) == (['airflow', '0'], {'speed': 3})
|
||||
assert fan_desc.write_fn(('power', True, '/power/vs/0'), {}) == (
|
||||
['power', 'vs', '0'], {'x.com.samsung.da.power': 'On'},
|
||||
@@ -136,5 +136,5 @@ def test_airflow_speed_reads_from_dump():
|
||||
fan_direction stays a plain diagnostic (every dump seen reads 'Off'
|
||||
regardless of fan setting)."""
|
||||
state = _state()
|
||||
assert state['fan'] == 0
|
||||
assert state['airflow_fan'] == 0
|
||||
assert state['fan_direction'] == 'Off'
|
||||
|
||||
@@ -126,8 +126,10 @@ def test_no_catalog_carries_unresolved_core_references():
|
||||
|
||||
|
||||
# The hood fan is its device's primary feature: fan.py sets _attr_name = None
|
||||
# so it presents as the device itself, and never reads a catalog name.
|
||||
UNNAMED_DESCRIPTORS = {("fan", "fan")}
|
||||
# so it presents as the device itself, and never reads a catalog name. Same
|
||||
# for the ARTIK051 air-purifier's airflow_fan (issue #56) -- ordered speed
|
||||
# levels, no presets, same _attr_name = None treatment.
|
||||
UNNAMED_DESCRIPTORS = {("fan", "fan"), ("fan", "airflow_fan")}
|
||||
|
||||
|
||||
def test_every_descriptor_has_an_entity_catalog_entry():
|
||||
|
||||
Reference in New Issue
Block a user