Fix range hood fan power targeting and kimchi mode write validation
- _speed_zero_is_off now keys off the hood resource's own settableMinFanSpeed/supportedFanSpeed fields instead of asking whether the device has any power resource at all, so a combi appliance's cavity /power/0 can no longer be toggled off by turning off just the vent fan. - async_turn_on() no longer resets an already-running fan to its lowest speed when called without a percentage. - _has_separate_power() reads through the O(1) resource cache instead of copying the full resource snapshot on every property access. - _kimchi_mode_write rejects values the compartment didn't advertise in supportMode instead of writing them blind. - kimchi_ripening_status no longer lowercases its value, since it has no enum catalog entry to translate the lowercased token back through. - Documented why KIMCHI_DOOR_GENERIC isn't deduped against the /doors/vs/0 aggregate fallback on the one fixture that reports both. Adds regression tests for the combi-appliance power targeting, the already-on turn_on no-op, kimchi mode write validation, and a kimchi select display/write casing round-trip; a translation-coverage guard for kimchi_zone_mode codes mirroring the existing AC preset one.
This commit is contained in:
@@ -37,6 +37,8 @@ POWER_HREF = '/power/0'
|
||||
POWER_VS_HREF = '/power/vs/0'
|
||||
_FAN_SPEED_FIELD = 'x.com.samsung.da.hood.fanSpeed'
|
||||
_SUPPORTED_FAN_SPEED_FIELD = 'x.com.samsung.da.hood.supportedFanSpeed'
|
||||
_MIN_FAN_SPEED_FIELD = 'x.com.samsung.da.hood.settableMinFanSpeed'
|
||||
_OFF_SPEED_CODE = '0'
|
||||
|
||||
_MODES_FIELD = 'x.com.samsung.da.modes'
|
||||
_SUPPORTED_MODES_FIELD = 'x.com.samsung.da.supportedModes'
|
||||
@@ -65,12 +67,20 @@ class LocalThingsRangeHoodFan(LocalThingsEntity, FanEntity):
|
||||
"""A hood fan combining sibling power and fan-speed resources.
|
||||
|
||||
Some boards that reuse this capability (built-in microwave vent fans,
|
||||
issues #137/#142) report no sibling `/power/0` or `/power/vs/0` resource
|
||||
at all -- fan speed 0 is itself the off state there, with no separate
|
||||
power toggle to write. `_has_separate_power` detects that shape and
|
||||
switches every method below to drive off/on purely through the
|
||||
fanSpeed field, including '0' in the ordered speed codes as the off
|
||||
step instead of assuming every advertised code is an active speed.
|
||||
issues #137/#142) report no sibling `/power/0` or `/power/vs/0`
|
||||
resource at all -- fan speed 0 is itself the off state there, with no
|
||||
separate power toggle to write. `_speed_zero_is_off` detects that
|
||||
shape from the hood resource's own settableMinFanSpeed/
|
||||
supportedFanSpeed fields and switches every method below to drive
|
||||
off/on purely through the fanSpeed field, including '0' in the
|
||||
ordered speed codes as the off step instead of assuming every
|
||||
advertised code is an active speed.
|
||||
|
||||
This is deliberately not the same question as `_has_separate_power`,
|
||||
which only proves *some* power resource exists on the device -- on a
|
||||
combi appliance (e.g. an over-the-range microwave) that resource can
|
||||
belong to the cavity, not the vent fan, and toggling it from here
|
||||
would turn off the whole appliance instead of just the fan.
|
||||
"""
|
||||
|
||||
_enable_turn_on_off_backwards_compatibility = False
|
||||
@@ -88,8 +98,19 @@ class LocalThingsRangeHoodFan(LocalThingsEntity, FanEntity):
|
||||
return self.coordinator.resource(href) or {}
|
||||
|
||||
def _has_separate_power(self) -> bool:
|
||||
resources = self.coordinator.last_resources
|
||||
return POWER_HREF in resources or POWER_VS_HREF in resources
|
||||
return bool(self._rep(POWER_HREF)) or bool(self._rep(POWER_VS_HREF))
|
||||
|
||||
def _speed_zero_is_off(self) -> bool:
|
||||
"""Whether fan speed '0' is itself this hood's off step, with no
|
||||
separate power resource to toggle. The board says so directly:
|
||||
settableMinFanSpeed '0', or '0' inside supportedFanSpeed. The
|
||||
standalone hood's codes start at 14 and it carries a real /power
|
||||
resource instead, so this is False there."""
|
||||
rep = self._rep(self._bound.href)
|
||||
return (
|
||||
str(rep.get(_MIN_FAN_SPEED_FIELD, '')) == _OFF_SPEED_CODE
|
||||
or _OFF_SPEED_CODE in self._all_speed_codes()
|
||||
)
|
||||
|
||||
def _all_speed_codes(self) -> list[str]:
|
||||
rep = self._rep(self._bound.href)
|
||||
@@ -97,14 +118,14 @@ class LocalThingsRangeHoodFan(LocalThingsEntity, FanEntity):
|
||||
|
||||
def _active_speed_codes(self) -> list[str]:
|
||||
codes = self._all_speed_codes()
|
||||
if self._has_separate_power():
|
||||
# Power is carried by the separate /power resource. fanSpeed
|
||||
# retains the selected setting while power is off (as the
|
||||
# lamp's `current` field does), so every advertised code is an
|
||||
# active ordered speed.
|
||||
return codes
|
||||
# No separate power resource: '0' is the off step, not a speed.
|
||||
return [code for code in codes if code != '0']
|
||||
if self._speed_zero_is_off():
|
||||
# No separate power resource: '0' is the off step, not a speed.
|
||||
return [code for code in codes if code != _OFF_SPEED_CODE]
|
||||
# Power is carried by the separate /power resource. fanSpeed
|
||||
# retains the selected setting while power is off (as the
|
||||
# lamp's `current` field does), so every advertised code is an
|
||||
# active ordered speed.
|
||||
return codes
|
||||
|
||||
def _power_payload(self, enabled: bool) -> tuple[str, bool, str]:
|
||||
"""Target whichever power resource this hood actually exposes."""
|
||||
@@ -114,9 +135,9 @@ class LocalThingsRangeHoodFan(LocalThingsEntity, FanEntity):
|
||||
|
||||
@property
|
||||
def is_on(self) -> bool:
|
||||
if not self._has_separate_power():
|
||||
if self._speed_zero_is_off():
|
||||
current = str(self._rep(self._bound.href).get(_FAN_SPEED_FIELD, '0'))
|
||||
return current not in ('', '0')
|
||||
return current not in ('', _OFF_SPEED_CODE)
|
||||
rep = self._rep(POWER_HREF)
|
||||
if 'value' in rep:
|
||||
return bool(rep.get('value'))
|
||||
@@ -142,10 +163,14 @@ class LocalThingsRangeHoodFan(LocalThingsEntity, FanEntity):
|
||||
self, percentage: int | None = None, preset_mode: str | None = None,
|
||||
**kwargs,
|
||||
) -> None:
|
||||
if not self._has_separate_power():
|
||||
if self._speed_zero_is_off():
|
||||
if percentage is not None:
|
||||
await self.async_set_percentage(percentage)
|
||||
return
|
||||
if self.is_on:
|
||||
# Already running: no percentage given means "just turn on",
|
||||
# not "reset to the lowest speed".
|
||||
return
|
||||
codes = self._active_speed_codes()
|
||||
if codes:
|
||||
await self.coordinator.async_send_command(self._bound, ('speed', codes[0]))
|
||||
@@ -157,8 +182,8 @@ class LocalThingsRangeHoodFan(LocalThingsEntity, FanEntity):
|
||||
await self.async_set_percentage(percentage)
|
||||
|
||||
async def async_turn_off(self, **kwargs) -> None:
|
||||
if not self._has_separate_power():
|
||||
await self.coordinator.async_send_command(self._bound, ('speed', '0'))
|
||||
if self._speed_zero_is_off():
|
||||
await self.coordinator.async_send_command(self._bound, ('speed', _OFF_SPEED_CODE))
|
||||
return
|
||||
await self.coordinator.async_send_command(
|
||||
self._bound, self._power_payload(False),
|
||||
@@ -171,7 +196,7 @@ class LocalThingsRangeHoodFan(LocalThingsEntity, FanEntity):
|
||||
codes = self._active_speed_codes()
|
||||
if not codes:
|
||||
return
|
||||
if self._has_separate_power() and not self.is_on:
|
||||
if not self._speed_zero_is_off() and not self.is_on:
|
||||
await self.coordinator.async_send_command(
|
||||
self._bound, self._power_payload(True),
|
||||
)
|
||||
|
||||
@@ -14,7 +14,7 @@ shape a standalone range hood reports it in -- reused directly from
|
||||
range_hood.py rather than duplicated. Unlike a standalone hood, this dump
|
||||
has no sibling `/power/0` or `/power/vs/0` resource; fan.py's
|
||||
LocalThingsRangeHoodFan falls back to treating fan speed 0 as off in that
|
||||
case (see its `_has_separate_power` check).
|
||||
case (see its `_speed_zero_is_off` check).
|
||||
"""
|
||||
from ..capabilities import common, ignored, microwave, oven, range_hood
|
||||
from ._base import DeviceRegistry, _build
|
||||
|
||||
@@ -675,7 +675,7 @@ DOOR_GENERIC = Capability(
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _kimchi_mode_write(p, rep, href=None):
|
||||
if not href:
|
||||
if not href or p not in (rep.get('x.com.samsung.da.supportMode') or ()):
|
||||
return None
|
||||
return [s for s in href.strip('/').split('/') if s], {
|
||||
'x.com.samsung.da.currentMode': p,
|
||||
@@ -697,8 +697,7 @@ KIMCHI_ZONE = Capability(
|
||||
SensorDesc(key='ripening_status', field='x.com.samsung.da.ripeStatus',
|
||||
use_instance_name=True, icon='mdi:progress-clock',
|
||||
translation_key='kimchi_ripening_status',
|
||||
entity_category='diagnostic',
|
||||
value_fn=lambda v: v.lower() if isinstance(v, str) else v),
|
||||
entity_category='diagnostic'),
|
||||
SensorDesc(key='ripening_remaining', field='x.com.samsung.da.ripeRemaintime',
|
||||
use_instance_name=True, icon='mdi:timer-sand',
|
||||
translation_key='kimchi_ripening_remaining',
|
||||
@@ -721,6 +720,13 @@ KIMCHI_DOOR_GENERIC = Capability(
|
||||
strip_prefix_in_key=True,
|
||||
poll_tier='hot',
|
||||
entities=(
|
||||
# Not deduped against DOORS_FALLBACK below: on the one reporter
|
||||
# (refrigerator_tp2x_ref_20k_kimchi) this binds alongside, the
|
||||
# /doors/vs/0 aggregate carries a single generic item (id "4", no
|
||||
# /door/<instance> siblings for DOORS_FALLBACK's match_fn to see)
|
||||
# that doesn't share this compartment's "top" instance numbering --
|
||||
# a distinct main-cabinet door, not this kimchi drawer's own contact
|
||||
# switch reported twice.
|
||||
BinarySensorDesc(key='open', rep_fn=_door_open_state,
|
||||
translation_key='instance_open',
|
||||
use_instance_name=True, device_class='door'),
|
||||
|
||||
@@ -241,25 +241,85 @@ class TestKimchiZone:
|
||||
|
||||
def test_write_derives_path_from_href(self):
|
||||
desc = fridge.KIMCHI_ZONE.entities[0]
|
||||
rep = {'x.com.samsung.da.supportMode': ['KIMCHI_STORAGE_COLD']}
|
||||
path, body = desc.write_fn(
|
||||
'KIMCHI_STORAGE_COLD', {}, href='/status/kimchi/middle/vs/0')
|
||||
'KIMCHI_STORAGE_COLD', rep, href='/status/kimchi/middle/vs/0')
|
||||
assert path == ['status', 'kimchi', 'middle', 'vs', '0']
|
||||
assert body == {'x.com.samsung.da.currentMode': 'KIMCHI_STORAGE_COLD'}
|
||||
|
||||
def test_write_without_href_is_rejected(self):
|
||||
desc = fridge.KIMCHI_ZONE.entities[0]
|
||||
assert desc.write_fn('KIMCHI_STORAGE_COLD', {}) is None
|
||||
rep = {'x.com.samsung.da.supportMode': ['KIMCHI_STORAGE_COLD']}
|
||||
assert desc.write_fn('KIMCHI_STORAGE_COLD', rep) is None
|
||||
|
||||
def test_ripening_status_is_lowercased(self):
|
||||
def test_write_rejects_value_outside_supportmode(self):
|
||||
"""A value the compartment never advertised is rejected rather than
|
||||
written blind -- this write path is unconfirmed against real
|
||||
hardware (module docstring above KIMCHI_ZONE), so a bad value here
|
||||
is a food-safety-adjacent outcome, not just a cosmetic one."""
|
||||
desc = fridge.KIMCHI_ZONE.entities[0]
|
||||
rep = {'x.com.samsung.da.supportMode': ['KIMCHI_STORAGE_COLD']}
|
||||
assert desc.write_fn(
|
||||
'KIMCHI_STORAGE_WARM', rep, href='/status/kimchi/middle/vs/0',
|
||||
) is None
|
||||
|
||||
def test_ripening_status_passes_through_device_value(self):
|
||||
"""No device_class='enum' catalog entry exists for this sensor, so
|
||||
lowercasing it would only make the raw device token un-translatable
|
||||
by HA -- pass the device's own casing straight through instead."""
|
||||
desc = next(e for e in fridge.KIMCHI_ZONE.entities if e.key == 'ripening_status')
|
||||
assert desc.value_fn('Off') == 'off'
|
||||
assert desc.value_fn(None) is None
|
||||
assert desc.value_fn('Off') == 'Off'
|
||||
|
||||
def test_door_reuses_open_state_helper(self):
|
||||
desc = fridge.KIMCHI_DOOR_GENERIC.entities[0]
|
||||
assert desc.rep_fn({'x.com.samsung.da.openState': 'Open'}) is True
|
||||
assert desc.rep_fn({'x.com.samsung.da.openState': 'Close'}) is False
|
||||
|
||||
async def test_zone_mode_select_round_trips_through_display_casing(self):
|
||||
"""kimchi_zone_mode's displayed value (lowercase, catalog-translated)
|
||||
and the raw device code it writes back can silently drift apart --
|
||||
this is the one place that casing conversion could break. Runs
|
||||
through the real discovery/select pipeline against the tp2x_ref_20k
|
||||
kimchi fixture rather than a hand-built descriptor, so it also
|
||||
catches use_instance_name key derivation going wrong."""
|
||||
from custom_components.localthings.registry.adapter import flatten
|
||||
from custom_components.localthings.registry.by_type import refrigerator
|
||||
from custom_components.localthings.registry.discovery import discover
|
||||
from custom_components.localthings.registry.entities import SelectDesc
|
||||
from custom_components.localthings.select import LocalThingsSelect
|
||||
from tests.conftest import _load_device
|
||||
|
||||
resources = _load_device('refrigerator_tp2x_ref_20k_kimchi')
|
||||
bound = discover(
|
||||
resources, refrigerator.REGISTRY.capabilities,
|
||||
refrigerator.REGISTRY.pattern_capabilities,
|
||||
)
|
||||
mode_bound = next(
|
||||
b for b in bound
|
||||
if isinstance(b.desc, SelectDesc) and b.href == '/status/kimchi/middle/vs/0'
|
||||
)
|
||||
|
||||
class _FakeCoordinator:
|
||||
device_serial = 'TEST-SERIAL'
|
||||
|
||||
def __init__(self, resources, data):
|
||||
self.last_resources = resources
|
||||
self.data = data
|
||||
self.commands = []
|
||||
|
||||
async def async_send_command(self, bound, value):
|
||||
self.commands.append(value)
|
||||
|
||||
coordinator = _FakeCoordinator(resources, flatten(bound, resources))
|
||||
entity = LocalThingsSelect(coordinator, mode_bound)
|
||||
|
||||
assert entity.current_option == 'kimchi_storage_normal'
|
||||
assert 'kimchi_storage_cold' in entity.options
|
||||
|
||||
await entity.async_select_option('kimchi_storage_cold')
|
||||
|
||||
assert coordinator.commands == ['KIMCHI_STORAGE_COLD']
|
||||
|
||||
|
||||
class TestArtik051AndTp2xFixturesHaveCompleteCoverage:
|
||||
"""issue #20 (ARTIK051_REF_17K) and #26 (TP2X_REF_20K) both triggered
|
||||
|
||||
@@ -615,7 +615,7 @@ def test_registry_reproduces_golden_state_keys_for_microwave_me7500d():
|
||||
a standalone hood this board has no sibling `/power/0` or
|
||||
`/power/vs/0` resource, so fan.py's LocalThingsRangeHoodFan treats
|
||||
fan speed 0 as the off state instead of writing a separate power
|
||||
resource -- see its `_has_separate_power` check. This board also has
|
||||
resource -- see its `_speed_zero_is_off` check. This board also has
|
||||
no `/temperatures/vs/0` or `x.com.samsung.da.hood.autoOperation`
|
||||
field, unlike MW7300B, so `setpoint`/`current_temp_c` and
|
||||
`automatic_operation` are correctly absent here."""
|
||||
|
||||
@@ -98,12 +98,43 @@ async def test_power_write_falls_back_to_vendor_resource():
|
||||
# itself the off state.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_standalone_hood_has_separate_power():
|
||||
"""Explicit converse of the microwave case below: guards the
|
||||
discriminator itself, not just its downstream effects, so a future
|
||||
change to it fails loudly here instead of only via behavioral drift."""
|
||||
entity = _entity(_load_device('range_hood'))
|
||||
assert entity._has_separate_power() is True
|
||||
assert entity._speed_zero_is_off() is False
|
||||
|
||||
|
||||
def test_microwave_vent_fan_has_no_separate_power_resource():
|
||||
resources = _load_device('microwave_me7500d')
|
||||
assert '/power/0' not in resources
|
||||
assert '/power/vs/0' not in resources
|
||||
entity = _microwave_entity(resources)
|
||||
assert entity._has_separate_power() is False
|
||||
assert entity._speed_zero_is_off() is True
|
||||
|
||||
|
||||
async def test_combi_microwave_with_cavity_power_still_treats_zero_speed_as_fan_off():
|
||||
"""A combi over-the-range microwave can report a /power/0 resource for
|
||||
the cavity while the vent fan still has no power resource of its own
|
||||
(settableMinFanSpeed '0' -- same board shape as microwave_me7500d).
|
||||
_speed_zero_is_off must key off the hood resource itself, not merely
|
||||
"some power resource exists on this device", so turning the fan off
|
||||
writes fan speed rather than the shared cavity power resource."""
|
||||
resources = _load_device('microwave_me7500d')
|
||||
resources['/power/0'] = {'value': True}
|
||||
resources['/hood/fanspeed/vs/0']['x.com.samsung.da.hood.fanSpeed'] = '2'
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _microwave_entity(resources, coordinator)
|
||||
|
||||
assert entity._has_separate_power() is True
|
||||
assert entity._speed_zero_is_off() is True
|
||||
|
||||
await entity.async_turn_off()
|
||||
|
||||
assert coordinator.commands[-1][1] == ('speed', '0')
|
||||
|
||||
|
||||
def test_microwave_vent_fan_off_state_excludes_zero_from_speed_codes():
|
||||
@@ -153,3 +184,37 @@ async def test_microwave_vent_fan_set_percentage_writes_speed_only():
|
||||
await entity.async_set_percentage(100)
|
||||
|
||||
assert coordinator.commands == [(entity._bound, ('speed', '4'))]
|
||||
|
||||
|
||||
async def test_microwave_vent_fan_turn_on_with_percentage_writes_speed_directly():
|
||||
resources = _load_device('microwave_me7500d')
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _microwave_entity(resources, coordinator)
|
||||
|
||||
await entity.async_turn_on(percentage=75)
|
||||
|
||||
assert coordinator.commands == [(entity._bound, ('speed', '3'))]
|
||||
|
||||
|
||||
async def test_microwave_vent_fan_turn_on_without_percentage_when_already_on_is_a_noop():
|
||||
"""A scene or automation calling fan.turn_on on an already-running vent
|
||||
fan must not reset it to the lowest speed."""
|
||||
resources = _load_device('microwave_me7500d')
|
||||
resources['/hood/fanspeed/vs/0']['x.com.samsung.da.hood.fanSpeed'] = '3'
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _microwave_entity(resources, coordinator)
|
||||
|
||||
await entity.async_turn_on()
|
||||
|
||||
assert coordinator.commands == []
|
||||
|
||||
|
||||
async def test_microwave_vent_fan_set_percentage_zero_turns_off():
|
||||
resources = _load_device('microwave_me7500d')
|
||||
resources['/hood/fanspeed/vs/0']['x.com.samsung.da.hood.fanSpeed'] = '2'
|
||||
coordinator = _FakeCoordinator(resources)
|
||||
entity = _microwave_entity(resources, coordinator)
|
||||
|
||||
await entity.async_set_percentage(0)
|
||||
|
||||
assert coordinator.commands[-1][1] == ('speed', '0')
|
||||
|
||||
@@ -210,3 +210,27 @@ def test_every_ac_convenient_mode_code_has_a_preset_label():
|
||||
missing.append((path.name, code))
|
||||
assert missing == []
|
||||
|
||||
|
||||
def test_every_kimchi_zone_supportmode_code_has_a_state_label():
|
||||
"""Same guard as the AC preset one above, for KIMCHI_ZONE's
|
||||
kimchi_zone_mode select (fridge.py, issue #26): the write path resolved
|
||||
from options_field is fully dynamic too, so an unlabelled supportMode
|
||||
code across any /status/kimchi/<slot>/vs/0 resource would silently
|
||||
render as its raw device token instead of the translated state.
|
||||
"""
|
||||
state_labels = set(
|
||||
_load("en")["entity"]["select"]["kimchi_zone_mode"]["state"]
|
||||
)
|
||||
fixtures_dir = Path(__file__).parent / "fixtures"
|
||||
missing = []
|
||||
for path in sorted(fixtures_dir.glob("*_device.json")):
|
||||
dump = json.loads(path.read_text())
|
||||
for item in dump.get("device0", []):
|
||||
href = item.get("href", "")
|
||||
if not (href.startswith("/status/kimchi/") and href.endswith("/vs/0")):
|
||||
continue
|
||||
for code in item["rep"].get("x.com.samsung.da.supportMode", []):
|
||||
if code.lower() not in state_labels:
|
||||
missing.append((path.name, href, code))
|
||||
assert missing == []
|
||||
|
||||
|
||||
Reference in New Issue
Block a user