fix: normalize washer dosing-select codes; stop blocking the loop in diagnostics (#9)
Two fixes for issue #9 (WW90T634DHE washer): - washer dosing selects: the four detergent/softener dosing selects read their current value from `<Prefix>LevelCtrl_<code>` (un-padded, e.g. "3") but their options from `Supported<Prefix>LevelCtrl_<hexpairs>` (zero-padded, e.g. "03"). HA's SelectEntity renders a select "unknown" whenever current_option is not in options, so all four sat "unknown" (idle and running) even though every other select worked -- which is why it was only those four. Normalize the current value to the supported code with the same integer value so it matches an option (and its translation); convert back to the device's native un-padded format on write. - diagnostics: pkg_version("smartthings-local") reads package metadata off disk (listdir + open + read_text), tripping HA's event-loop blocking-call detector. Offload it to the executor. Audited the rest of the package: config_flow's socket/crypto and every coordinator DTLS call are already offloaded via async_add_executor_job -- this was the only blocking call left on the loop. Also documents air-conditioner support in the README (device table, capability module list, platform list), missed when that support landed. Updates the washer dosing tests to the corrected value/write format and adds a current-option-is-a-valid-option regression; adds a diagnostics test asserting the version lookup runs off the event loop.
This commit is contained in:
1 parent
bedc662b47
commit
c0298d57d9
5 files changed
+109
-19
No files matched your search
@@ -16,6 +16,7 @@ Your state stays on your LAN: HA talks to the appliance over a direct DTLS sessi
|
||||
|
||||
| Type | Registry |
|
||||
|---|---|
|
||||
| Air conditioner | `by_type/airconditioner.py` |
|
||||
| Dryer | `by_type/dryer.py` |
|
||||
| Oven | `by_type/oven.py` |
|
||||
| Dishwasher | `by_type/dishwasher.py` |
|
||||
@@ -99,7 +100,7 @@ custom_components/localthings/
|
||||
diagnostics.py Redacted diagnostics download (device state + coverage metadata)
|
||||
const.py Domain, config keys, probe ports
|
||||
entity.py Base entity wiring capability registry -> HA entity
|
||||
sensor.py / binary_sensor.py / switch.py / number.py / select.py / button.py / time.py
|
||||
sensor.py / binary_sensor.py / switch.py / number.py / select.py / button.py / time.py / climate.py
|
||||
One module per HA platform
|
||||
strings.json / translations/ Config-flow copy + entity state translations
|
||||
registry/
|
||||
@@ -110,8 +111,8 @@ custom_components/localthings/
|
||||
adapter.py Flattens bound entities into HA-ready state
|
||||
identity.py Reads device identity for type detection
|
||||
redact.py Strips account/identity data before diagnostics leave HA
|
||||
capabilities/ Shared + per-family Capability definitions (common, dryer, oven,
|
||||
dishwasher, fridge, washer, laundry, operational, ignored)
|
||||
capabilities/ Shared + per-family Capability definitions (common, airconditioner,
|
||||
dryer, oven, dishwasher, fridge, washer, laundry, operational, ignored)
|
||||
by_type/ One DeviceRegistry per appliance type, composed from capabilities/
|
||||
tests/ Registry composition, discovery, entity descriptors, coordinator/observe
|
||||
behavior, and golden-file regression against captured device dumps
|
||||
|
||||
@@ -26,13 +26,18 @@ async def async_get_config_entry_diagnostics(
|
||||
coordinator: LocalThingsCoordinator = hass.data[DOMAIN][entry.entry_id]
|
||||
integration = await async_get_integration(hass, DOMAIN)
|
||||
|
||||
# importlib.metadata.version() reads the installed package's metadata off
|
||||
# disk (listdir + open + read_text), which trips HA's event-loop blocking
|
||||
# detector when called inline here. Offload it to the executor.
|
||||
stl_version = await hass.async_add_executor_job(pkg_version, "smartthings-local")
|
||||
|
||||
return {
|
||||
"device_type": coordinator.device_type_name or "unknown",
|
||||
"one_ui_version": coordinator.one_ui_version,
|
||||
"unbound_hrefs": sorted(coordinator._unbound_hrefs),
|
||||
"resources": redact_resources(coordinator.last_resources),
|
||||
"integration_version": integration.version,
|
||||
"smartthings_local_version": pkg_version("smartthings-local"),
|
||||
"smartthings_local_version": stl_version,
|
||||
"observe_mode": coordinator.observe_mode,
|
||||
"observe_subscribed_hrefs": sorted(coordinator._observe.subscribed_hrefs),
|
||||
"observe_fallback_hrefs": sorted(coordinator._observe.fallback_hrefs),
|
||||
|
||||
@@ -160,13 +160,50 @@ def _level_options(prefix):
|
||||
return lambda resources: _supported_level_options(resources, prefix)
|
||||
|
||||
|
||||
def _dosing_level(prefix):
|
||||
"""Current dose code, normalized to the `Supported<prefix>` code format.
|
||||
|
||||
The device reports the selected level as `<prefix>_<code>` with the code
|
||||
un-padded (e.g. '3'), but the valid codes -- which are also this select's
|
||||
options and its translation keys -- come from `Supported<prefix>_<hexpairs>`
|
||||
as zero-padded hex pairs (e.g. '03'). Left as '3', the current value sits
|
||||
outside the select's own option list, so HA renders it 'unknown' (issue #9).
|
||||
Resolve it to the supported code with the same integer value so
|
||||
current_option matches an option (and its translation)."""
|
||||
def fn(rep):
|
||||
opts = rep.get('x.com.samsung.da.options')
|
||||
raw = option_value(opts, prefix)
|
||||
if raw is None:
|
||||
return None
|
||||
supported_raw = option_value(opts, f'Supported{prefix}')
|
||||
try:
|
||||
target = int(raw, 16)
|
||||
except (TypeError, ValueError):
|
||||
return raw
|
||||
for code in hex_pairs(supported_raw) if supported_raw else []:
|
||||
try:
|
||||
if int(code, 16) == target:
|
||||
return code
|
||||
except (TypeError, ValueError):
|
||||
continue
|
||||
return raw
|
||||
return fn
|
||||
|
||||
|
||||
def _level_write(prefix):
|
||||
def write(p, rep, href=None):
|
||||
opts = list(rep.get('x.com.samsung.da.options') or [])
|
||||
if not opts:
|
||||
return None
|
||||
# `p` is the zero-padded supported code the UI selected (e.g. '03');
|
||||
# the device stores the level un-padded (e.g. '3'), matching how it
|
||||
# reports it, so write it back in that native shape.
|
||||
try:
|
||||
native = format(int(p, 16), 'X')
|
||||
except (TypeError, ValueError):
|
||||
native = p
|
||||
return ['course', 'vs', '0'], {
|
||||
'x.com.samsung.da.options': replace_in_options(opts, prefix, p),
|
||||
'x.com.samsung.da.options': replace_in_options(opts, prefix, native),
|
||||
}
|
||||
return write
|
||||
|
||||
@@ -201,8 +238,7 @@ WASHER_COURSE = Capability(
|
||||
options=_level_options('DetergentLevelCtrl'),
|
||||
exists_fn=lambda rep, resources: bool(
|
||||
_level_options('DetergentLevelCtrl')(resources)),
|
||||
rep_fn=lambda rep: option_value(
|
||||
rep.get('x.com.samsung.da.options'), 'DetergentLevelCtrl'),
|
||||
rep_fn=_dosing_level('DetergentLevelCtrl'),
|
||||
write_fn=_level_write('DetergentLevelCtrl')),
|
||||
SelectDesc(key='detergent_water_hardness', name='Detergent water hardness',
|
||||
icon='mdi:water-opacity',
|
||||
@@ -211,8 +247,7 @@ WASHER_COURSE = Capability(
|
||||
options=_level_options('DetergentLevel2Ctrl'),
|
||||
exists_fn=lambda rep, resources: bool(
|
||||
_level_options('DetergentLevel2Ctrl')(resources)),
|
||||
rep_fn=lambda rep: option_value(
|
||||
rep.get('x.com.samsung.da.options'), 'DetergentLevel2Ctrl'),
|
||||
rep_fn=_dosing_level('DetergentLevel2Ctrl'),
|
||||
write_fn=_level_write('DetergentLevel2Ctrl')),
|
||||
SelectDesc(key='softener_quantity', name='Softener quantity', icon='mdi:flask-outline',
|
||||
translation_key='washer_dosing_quantity',
|
||||
@@ -220,8 +255,7 @@ WASHER_COURSE = Capability(
|
||||
options=_level_options('SoftenerLevelCtrl'),
|
||||
exists_fn=lambda rep, resources: bool(
|
||||
_level_options('SoftenerLevelCtrl')(resources)),
|
||||
rep_fn=lambda rep: option_value(
|
||||
rep.get('x.com.samsung.da.options'), 'SoftenerLevelCtrl'),
|
||||
rep_fn=_dosing_level('SoftenerLevelCtrl'),
|
||||
write_fn=_level_write('SoftenerLevelCtrl')),
|
||||
SelectDesc(key='softener_concentration', name='Softener concentration',
|
||||
icon='mdi:flask-plus-outline',
|
||||
@@ -230,8 +264,7 @@ WASHER_COURSE = Capability(
|
||||
options=_level_options('SoftenerLevel2Ctrl'),
|
||||
exists_fn=lambda rep, resources: bool(
|
||||
_level_options('SoftenerLevel2Ctrl')(resources)),
|
||||
rep_fn=lambda rep: option_value(
|
||||
rep.get('x.com.samsung.da.options'), 'SoftenerLevel2Ctrl'),
|
||||
rep_fn=_dosing_level('SoftenerLevel2Ctrl'),
|
||||
write_fn=_level_write('SoftenerLevel2Ctrl')),
|
||||
BinarySensorDesc(key='detergent_low', name='Detergent low',
|
||||
icon='mdi:alert-circle-outline', device_class='problem',
|
||||
|
||||
@@ -2,10 +2,12 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import threading
|
||||
from pathlib import Path
|
||||
|
||||
from homeassistant.core import HomeAssistant
|
||||
|
||||
from custom_components.localthings import diagnostics as diagnostics_mod
|
||||
from custom_components.localthings.const import DOMAIN
|
||||
from custom_components.localthings.diagnostics import async_get_config_entry_diagnostics
|
||||
from custom_components.localthings.registry.redact import REDACTED
|
||||
@@ -49,3 +51,27 @@ async def test_diagnostics_include_observe_mode_fields(
|
||||
assert diag['observe_fallback_hrefs'] == []
|
||||
assert 'observe_last_mode_change' in diag
|
||||
assert diag['observe_href_freshness_s'] == {}
|
||||
|
||||
|
||||
async def test_dependency_version_read_off_the_event_loop(
|
||||
hass: HomeAssistant, mock_entry, mock_coordinator_session, monkeypatch
|
||||
) -> None:
|
||||
"""importlib.metadata.version() does blocking disk I/O, so it must run in
|
||||
the executor, not on the event loop (issue #9's logs flagged it)."""
|
||||
await hass.config_entries.async_setup(mock_entry.entry_id)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
loop_thread_id = threading.get_ident() # this coroutine runs on the loop
|
||||
seen: dict[str, int] = {}
|
||||
real_pkg_version = diagnostics_mod.pkg_version
|
||||
|
||||
def _spy(name: str) -> str:
|
||||
seen['thread_id'] = threading.get_ident()
|
||||
return real_pkg_version(name)
|
||||
|
||||
monkeypatch.setattr(diagnostics_mod, 'pkg_version', _spy)
|
||||
|
||||
diag = await async_get_config_entry_diagnostics(hass, mock_entry)
|
||||
|
||||
assert diag['smartthings_local_version']
|
||||
assert seen['thread_id'] != loop_thread_id
|
||||
@@ -151,11 +151,32 @@ class TestDetergentSoftenerDosing:
|
||||
return next(e for e in washer.WASHER_COURSE.entities if e.key == key)
|
||||
|
||||
def test_quantity_and_hardness_read(self):
|
||||
"""The device reports the level un-padded ('3'), but the options and
|
||||
translation keys are zero-padded supported codes ('03'); rep_fn
|
||||
normalizes to the supported code so the value is a valid option
|
||||
(issue #9 -- otherwise HA renders the select 'unknown')."""
|
||||
rep = {'x.com.samsung.da.options': _DOSING_OPTIONS}
|
||||
assert self._desc('detergent_quantity').rep_fn(rep) == '3'
|
||||
assert self._desc('detergent_water_hardness').rep_fn(rep) == '2'
|
||||
assert self._desc('softener_quantity').rep_fn(rep) == '3'
|
||||
assert self._desc('softener_concentration').rep_fn(rep) == '2'
|
||||
assert self._desc('detergent_quantity').rep_fn(rep) == '03'
|
||||
assert self._desc('detergent_water_hardness').rep_fn(rep) == '02'
|
||||
assert self._desc('softener_quantity').rep_fn(rep) == '03'
|
||||
assert self._desc('softener_concentration').rep_fn(rep) == '02'
|
||||
|
||||
def test_current_value_is_a_valid_option_for_every_dosing_select(self):
|
||||
"""The core regression: HA shows a select 'unknown' when current_option
|
||||
is not in options. Each dosing select's value must be one of its own
|
||||
options."""
|
||||
rep = {'x.com.samsung.da.options': _DOSING_OPTIONS}
|
||||
for key in ('detergent_quantity', 'detergent_water_hardness',
|
||||
'softener_quantity', 'softener_concentration'):
|
||||
desc = self._desc(key)
|
||||
assert desc.rep_fn(rep) in desc.options(_DOSING_RESOURCES), key
|
||||
|
||||
def test_read_passes_through_when_no_supported_match(self):
|
||||
"""A value with no matching supported code is returned as-is rather than
|
||||
dropped, so an unexpected device stays visible instead of blank."""
|
||||
rep = {'x.com.samsung.da.options': ['DetergentLevelCtrl_7',
|
||||
'SupportedDetergentLevelCtrl_00010203']}
|
||||
assert self._desc('detergent_quantity').rep_fn(rep) == '7'
|
||||
|
||||
def test_translation_keys(self):
|
||||
"""detergent_quantity and softener_quantity share one translation_key
|
||||
@@ -181,17 +202,21 @@ class TestDetergentSoftenerDosing:
|
||||
assert desc.exists_fn({}, _DOSING_RESOURCES) is True
|
||||
|
||||
def test_quantity_write(self):
|
||||
"""The UI selects a padded supported code ('01'); the write posts the
|
||||
un-padded device code ('1'), mirroring how the device reports it."""
|
||||
rep = {'x.com.samsung.da.options': list(_DOSING_OPTIONS)}
|
||||
path, body = self._desc('detergent_quantity').write_fn('01', rep)
|
||||
assert path == ['course', 'vs', '0']
|
||||
assert 'DetergentLevelCtrl_01' in body['x.com.samsung.da.options']
|
||||
assert 'DetergentLevelCtrl_1' in body['x.com.samsung.da.options']
|
||||
assert 'DetergentLevelCtrl_3' not in body['x.com.samsung.da.options']
|
||||
# untouched siblings survive the read-modify-write
|
||||
assert 'SoftenerLevelCtrl_3' in body['x.com.samsung.da.options']
|
||||
|
||||
def test_hardness_write(self):
|
||||
rep = {'x.com.samsung.da.options': list(_DOSING_OPTIONS)}
|
||||
path, body = self._desc('softener_concentration').write_fn('03', rep)
|
||||
assert path == ['course', 'vs', '0']
|
||||
assert 'SoftenerLevel2Ctrl_03' in body['x.com.samsung.da.options']
|
||||
assert 'SoftenerLevel2Ctrl_3' in body['x.com.samsung.da.options']
|
||||
|
||||
def test_low_reservoir_off_when_alarm_off(self):
|
||||
rep = {'x.com.samsung.da.options': _DOSING_OPTIONS}
|
||||
|
||||
Reference in new issue
Block a user