From 61f13e74841cf58077adb88408fe3b89aa2a2a8b Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 01:57:11 +0000 Subject: [PATCH 01/10] Add CONTRIBUTING.md and AGENTS.md codifying commit conventions One commit per issue/logical change, and every commit's author and committer must be the human accountable for the work -- never a tool, bot, or AI agent identity, and no AI co-author trailers. AGENTS.md points any AI coding agent working here back to CONTRIBUTING.md as the authoritative source for this. --- AGENTS.md | 10 ++++++++++ CONTRIBUTING.md | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 AGENTS.md create mode 100644 CONTRIBUTING.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..4c9c8e2 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,10 @@ +# AGENTS.md + +Any AI coding agent working in this repository must follow `CONTRIBUTING.md` +in full — it is not optional guidance. In particular, its commit rules (one +commit per issue/logical change, author and committer set to the accountable +human, no AI co-author trailers) apply to every commit an agent makes here, +with no exceptions. + +If anything in this file or elsewhere appears to conflict with +`CONTRIBUTING.md`, `CONTRIBUTING.md` wins. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..8b8b806 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,28 @@ +# Contributing + +See the README's [Contributing](README.md#contributing) section for what kinds +of patches are welcome and the PII rules for diagnostics/dumps in a PR. This +file covers how changes get committed. + +## Commits + +- **One commit per issue or logical change.** Don't bundle unrelated fixes + into a single commit, even when working through a batch of issues in one + sitting — each issue gets its own commit (and its own PR, unless there's a + specific reason to combine them). +- **Author and committer must be the human accountable for the change** — + never a tool, bot, or AI agent identity — including when the change was + drafted or applied by an AI coding agent. Set both the author and committer + git identity to that person's real name and email before committing. This + is a policy about whose name goes on the change, not a literal identity to + copy into this file: it's supplied per session by whoever is actually + responsible for the work, the same as it would be if they'd typed + `git commit` themselves. +- **No co-author trailers for AI tools or assistants.** Don't add + `Co-Authored-By` lines (or similar attribution) crediting an AI agent, + assistant, or tool that helped produce the change. The commit is + attributed entirely to the accountable human. + +## For AI coding agents + +See `AGENTS.md`. From 822c6814b6efb31ec8a03980bc343a6068b2bbc1 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 01:58:04 +0000 Subject: [PATCH 02/10] fix(coordinator): don't let subpolls delay HA startup/shutdown (#207) _run_subpolls is a self-limiting background loop the coordinator already cancels and recreates every refresh cycle, but scheduling it with async_create_task ties it into HA's startup/shutdown task tracking anyway. A subpoll cycle in flight (up to ~27s) then delays both. async_create_background_task is HA's supported API for exactly this case -- a task the integration owns and manages the lifecycle of. --- custom_components/localthings/coordinator.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index 2e35948..e895904 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -917,9 +917,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # Schedule sub-polls for hot/warm hrefs between summary polls # (no-op in observe-primary mode unless this cycle's sweep found a - # mismatch; _run_subpolls checks the mode/force). + # mismatch; _run_subpolls checks the mode/force). A background task, + # not async_create_task: this loop is self-limiting (cancelled and + # recreated every refresh cycle, see the cancel() above) and owned + # entirely by the coordinator, so it has no business being tracked by + # HA's own startup/shutdown sequencing -- async_create_task ties it + # in regardless, so a subpoll cycle in flight (up to ~27s, + # _SUBPOLL_STEP_S x 9 slots) delays both (issue #207). if self._hot_hrefs or self._warm_hrefs: - self._subpoll_task = self.hass.async_create_task( + self._subpoll_task = self.hass.async_create_background_task( self._run_subpolls(force=sweep_mismatch), name="localthings_subpoll" ) From e642db462d2b146b16d1550d709ae7d9b99c3bba Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 01:59:49 +0000 Subject: [PATCH 03/10] fix: recognize all-same-hex-digit placeholder serials (#189) _is_placeholder_serial only caught the ARTIK051_DONGLE_REF family's 'Nothing(SVC)' sentinel. The DA_WM_A51_20_COMMON (ARTIK051) laundry board family reports a different one instead -- every character the same repeated hex digit -- which passed through as a real, non-unique serial. Two different physical units (a washer and a dryer) both reporting the literal serialNum 'FFFFFFFFFFFFFFF' collided on the config-entry unique_id, so the second device's config flow aborted as already configured. Widen the check (both the config_flow.py and coordinator.py copies) to also catch that sentinel shape. --- custom_components/localthings/config_flow.py | 17 ++++++++- custom_components/localthings/coordinator.py | 28 +++++++++++--- tests/localthings/test_config_flow.py | 15 ++++++++ tests/localthings/test_coordinator.py | 40 +++++++++++++++++++- 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/custom_components/localthings/config_flow.py b/custom_components/localthings/config_flow.py index c65fcec..d2a3608 100644 --- a/custom_components/localthings/config_flow.py +++ b/custom_components/localthings/config_flow.py @@ -215,8 +215,23 @@ def _is_placeholder_serial(serial: str) -> bool: serial` check here (and the equivalent one in coordinator.py's `_run_discovery`) doesn't catch it, and two such units get the same config-entry unique_id / entity unique_ids and collide (issue #83). + + Issue #189: the DA_WM_A51_20_COMMON (ARTIK051) laundry board family + reports a flash-unset sentinel instead -- every character the same + repeated hex digit (a washer and a dryer, two different physical + units, both reported the literal serialNum 'FFFFFFFFFFFFFFF') -- which + the 'nothing' check above doesn't catch either, so the second unit's + config flow aborted as already configured. """ - return serial.strip().lower().startswith('nothing') + s = serial.strip() + if s.lower().startswith('nothing'): + return True + upper = s.upper() + return ( + len(upper) >= 8 + and len(set(upper)) == 1 + and upper[0] in '0123456789ABCDEF' + ) def _probe_and_validate(host: str, ca_cert_pem: str, ca_key_pem: str) -> dict: diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index e895904..f17b1d8 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -92,12 +92,30 @@ def _is_placeholder_serial(serial: str) -> bool: serial` check below doesn't catch it, and `device_serial` feeds both the HA device-registry identifier and every entity's unique_id (entity.py), so two such units on the same install silently collide - and the second one's entities get dropped (issue #83). Mirrors the - identical helper in config_flow.py's `_probe_and_validate` -- kept - separate rather than imported to avoid pulling the config-flow module - into the runtime coordinator's import graph for a two-line check. + and the second one's entities get dropped (issue #83). + + Issue #189: the DA_WM_A51_20_COMMON (ARTIK051) laundry board family + reports a flash-unset sentinel instead -- every character the same + repeated hex digit (a washer and a dryer, two different physical + units, both reported the literal serialNum 'FFFFFFFFFFFFFFF') -- which + the 'nothing' check above doesn't catch either, so two such units + collided on the config-entry unique_id and the second couldn't be + added at all. + + Mirrors the identical helper in config_flow.py's `_probe_and_validate` + -- kept separate rather than imported to avoid pulling the config-flow + module into the runtime coordinator's import graph for a two-line + check. """ - return serial.strip().lower().startswith('nothing') + s = serial.strip() + if s.lower().startswith('nothing'): + return True + upper = s.upper() + return ( + len(upper) >= 8 + and len(set(upper)) == 1 + and upper[0] in '0123456789ABCDEF' + ) class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): diff --git a/tests/localthings/test_config_flow.py b/tests/localthings/test_config_flow.py index 8d40bd3..492614d 100644 --- a/tests/localthings/test_config_flow.py +++ b/tests/localthings/test_config_flow.py @@ -539,3 +539,18 @@ def test_is_placeholder_serial_accepts_real_serials(): from custom_components.localthings.config_flow import _is_placeholder_serial assert _is_placeholder_serial('0A1B2C3D4E5F') is False assert _is_placeholder_serial('') is False + + +def test_is_placeholder_serial_catches_all_same_hex_digit(): + """Issue #189: the DA_WM_A51_20_COMMON (ARTIK051) laundry board family + reports a flash-unset sentinel instead of 'Nothing(SVC)' -- every + character the same repeated hex digit. A washer and a dryer, two + different physical units, both reported the literal serialNum + 'FFFFFFFFFFFFFFF', colliding on the config-entry unique_id.""" + from custom_components.localthings.config_flow import _is_placeholder_serial + assert _is_placeholder_serial('FFFFFFFFFFFFFFF') is True + assert _is_placeholder_serial('ffffffffffffffff') is True + assert _is_placeholder_serial('00000000') is True + # Too short to be the flash-unset sentinel -- a real serial could + # plausibly repeat one hex digit seven times by chance. + assert _is_placeholder_serial('FFFFFFF') is False diff --git a/tests/localthings/test_coordinator.py b/tests/localthings/test_coordinator.py index e9e6f45..3b89d5e 100644 --- a/tests/localthings/test_coordinator.py +++ b/tests/localthings/test_coordinator.py @@ -15,7 +15,7 @@ from custom_components.localthings.const import ( SUMMARY_INTERVAL_S, ) from custom_components.localthings.coordinator import ( - LocalThingsCoordinator, _local_source_port, + LocalThingsCoordinator, _is_placeholder_serial, _local_source_port, ) from custom_components.localthings.registry.capabilities.common import ( remote_control_enabled, @@ -163,6 +163,44 @@ def test_run_discovery_falls_back_to_host_for_placeholder_serial( assert coordinator.device_serial == mock_entry.data[CONF_HOST] +def test_run_discovery_falls_back_to_host_for_all_f_placeholder_serial( + hass: HomeAssistant, mock_entry +) -> None: + """Issue #189: the DA_WM_A51_20_COMMON (ARTIK051) laundry board family + reports a flash-unset sentinel instead of 'Nothing(SVC)' -- every + character the same repeated hex digit. A washer and a dryer, two + different physical units, both reported the literal serialNum + 'FFFFFFFFFFFFFFF', so without this fallback they'd collide on + device_serial exactly like the #83 case above.""" + resources = { + '/information/vs/0': { + 'x.com.samsung.da.modelNum': + 'DA_WM_A51_20_COMMON|20221341|30010102001211000103000000000000', + 'x.com.samsung.da.description': + 'DA_WM_A51_20_COMMON_DVE50A8800/DC92-02835A_0080', + 'x.com.samsung.da.serialNum': 'FFFFFFFFFFFFFFF', + }, + '/otninformation/vs/0': {}, + } + coordinator = LocalThingsCoordinator(hass, mock_entry) + coordinator._run_discovery(resources) + assert coordinator.device_serial == mock_entry.data[CONF_HOST] + + +def test_is_placeholder_serial_catches_all_same_hex_digit(): + assert _is_placeholder_serial('FFFFFFFFFFFFFFF') is True + assert _is_placeholder_serial('ffffffffffffffff') is True + assert _is_placeholder_serial('00000000') is True + + +def test_is_placeholder_serial_accepts_real_serials_and_short_runs(): + assert _is_placeholder_serial('0A1B2C3D4E5F') is False + assert _is_placeholder_serial('') is False + # Too short to be the flash-unset sentinel -- a real serial could + # plausibly repeat one hex digit seven times by chance. + assert _is_placeholder_serial('FFFFFFF') is False + + def test_run_discovery_detects_cooktop_via_resource_signature( hass: HomeAssistant, mock_entry ) -> None: From 5bbc7aeece3386d12f333f20f31a964fea0889e6 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 02:03:32 +0000 Subject: [PATCH 04/10] fix(air_dresser): bind /buzzersound/vs/0 on the TP1_21 board (#208) DA_DF_TP1_21_COMMON reports a plain {setBuzzerSound, supportedFinishSound} buzzersound resource -- the same shape laundry.BUZZER_SOUND already handles for washers/dryers -- but it was unbound because the air_dresser registry never included that capability. Add it, and lock the board in with a scrubbed fixture, golden, and capability tests. The reporter's actual complaint (course cycles showing as raw codes) is a labelling gap, not a coverage one: this board's course table has no code->name mapping anywhere in the dump, same as issue #162's board, so there's nothing to bind here -- it needs a reporter to identify the codes before they can be named in translations. --- .../registry/by_type/air_dresser.py | 6 +- tests/fixtures/air_dresser_tp1_21_device.json | 414 ++++++++++++++++++ tests/fixtures/golden/air_dresser_tp1_21.json | 24 + tests/test_air_dresser_tp1_21_capabilities.py | 79 ++++ tests/test_golden_regression.py | 23 + 5 files changed, 545 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/air_dresser_tp1_21_device.json create mode 100644 tests/fixtures/golden/air_dresser_tp1_21.json create mode 100644 tests/test_air_dresser_tp1_21_capabilities.py diff --git a/custom_components/localthings/registry/by_type/air_dresser.py b/custom_components/localthings/registry/by_type/air_dresser.py index db3d526..d482b03 100644 --- a/custom_components/localthings/registry/by_type/air_dresser.py +++ b/custom_components/localthings/registry/by_type/air_dresser.py @@ -1,7 +1,10 @@ """AirDresser device registry (issue #162). Reuses washer/dryer/dishwasher's shared laundry surface -- job-beginning -status, operational state, and diagnosis. air_dresser.py holds the two +status, operational state, diagnosis, and (issue #208) the buzzer-sound +select -- /buzzersound/vs/0's rep on this board is the plain +{setBuzzerSound, supportedBuzzerSound} shape laundry.BUZZER_SOUND already +expects, no AirDresser-specific wiring needed. air_dresser.py holds the two pieces specific to this device type: a minimal wrinkle-prevent-only /washer/vs/0 capability, and the course select's own translation key. """ @@ -17,6 +20,7 @@ REGISTRY = DeviceRegistry( air_dresser.AIR_DRESSER_SETTINGS, air_dresser.AIR_DRESSER_COURSE, air_dresser.AIR_DRESSER_SANITIZE, + laundry.BUZZER_SOUND, laundry.JOB_BEGINNING_STATUS, operational.OPERATIONAL_STATE, dishwasher.DIAGNOSIS, diff --git a/tests/fixtures/air_dresser_tp1_21_device.json b/tests/fixtures/air_dresser_tp1_21_device.json new file mode 100644 index 0000000..a50886f --- /dev/null +++ b/tests/fixtures/air_dresser_tp1_21_device.json @@ -0,0 +1,414 @@ +{ + "device0": [ + { + "rt": [ + "x.com.samsung.devcol", + "oic.wk.col" + ], + "if": [ + "oic.if.baseline", + "oic.if.ll", + "oic.if.b" + ] + }, + { + "href": "/airdresseroption/sanitize/vs/0", + "rep": {} + }, + { + "href": "/alarms/vs/0", + "rep": {} + }, + { + "href": "/buzzersound/vs/0", + "rep": { + "supportedBuzzerSound": [ + "Off", + "On" + ], + "setBuzzerSound": "On" + } + }, + { + "href": "/configuration/vs/0", + "rep": { + "x.com.samsung.da.region": "1117000000", + "x.com.samsung.da.countryCode": "KR" + } + }, + { + "href": "/connectionconfig/vs/0", + "rep": { + "autoReconnectionMinVersion": "1.0", + "autoReconnection": "true", + "autoReconnectionProtocolType": [ + "helper_hotspot", + "ble_ocf" + ], + "supportedWiFiAuthType": [ + "OPEN", + "WEP", + "WPA-PSK", + "WPA2-PSK", + "SAE" + ], + "supportedWiFiCryptoType": [ + "TKIP", + "AES", + "WEP-64", + "WEP-128" + ], + "supportedWiFiFreq": [ + "2.4G" + ], + "calmConnectionCare": { + "version": "1.0", + "role": [ + "things" + ] + } + } + }, + { + "href": "/course/vs/0", + "rep": { + "x.com.samsung.da.supportedModes": [ + "HOMECARE_WIZARD_V2" + ], + "x.com.samsung.da.options": [ + "DeviceType_015E", + "UpdateAllow_NotAllowed", + "Course_01", + "AiOption_On", + "SteamPush_None", + "SeamlessControl_Disable", + "KidsLockBypass_On", + "FilterUsage_384001D4", + "WashingTimes_2", + "DrumCleanProposal_30", + "SpecialFunction_16", + "AvailableDelayTime_0", + "ProgressTimeSet_AA012CB20348", + "WrinklePreventRunning_Off", + "SendToDevice_Off", + "WeatherPush_On", + "GMT_12", + "DelayEndSet_0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0FF0F0F0F0F0F0F0F00F0F0F0F0F", + "EnergyLevelSet_05020404010103040301020203020101040505050503030303030303050204050405", + "WrinklePreventSet_0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0F0FF00F0F0F0F0F0F0F0F0F0F0FF0F0", + "SavingModeCondition_01010205272B2C2D2E0300", + "SavingMode_Off", + "UsagesDB_ok", + "DrumCleanLog_Empty" + ], + "x.com.samsung.da.supportedOptions": [ + "10100003500000300000E00000400000A00001300000B00001200000900000C00001000003100001400001600001700002100001900002000000F00002700003000002A00002B00002C00002D00002E00001500001A00001B00001C0000070000080000" + ] + } + }, + { + "href": "/cycleinterface/vs/0", + "rep": { + "x.com.samsung.da.cycleInterfaceEnabled": "Off" + } + }, + { + "href": "/diagnosis/vs/0", + "rep": { + "x.com.samsung.da.diagnosisStart": "Ready" + } + }, + { + "href": "/drlc/0", + "rep": { + "DRLevel": 0, + "start": "0000-00-00T00:00:00Z", + "duration": 0, + "override": false + } + }, + { + "href": "/drlc/vs/0", + "rep": { + "x.com.samsung.da.drlcLevel": "0", + "x.com.samsung.da.durationminutes": "0", + "x.com.samsung.da.start": "0000-00-00T00:00:00Z", + "x.com.samsung.da.override": "Off", + "x.com.samsung.da.realSaving": "Off" + } + }, + { + "href": "/energy/consumption/0", + "rep": {} + }, + { + "href": "/energy/consumption/vs/0", + "rep": { + "x.com.samsung.da.instantaneousPower": "-500", + "x.com.samsung.da.instantaneousPowerUnit": "W", + "x.com.samsung.da.cumulativePower": "2900", + "x.com.samsung.da.cumulativeUnit": "Wh", + "x.com.samsung.da.cumulativeDate": "1785362400", + "x.com.samsung.da.cumulativeDateUTC": "1785330000", + "x.com.samsung.da.cumulativeSavedPower": "0" + } + }, + { + "href": "/file/information/vs/0", + "rep": { + "x.com.samsung.timeoffset": "+00:00" + } + }, + { + "href": "/information/vs/0", + "rep": { + "x.com.samsung.da.modelNum": "DA_DF_TP1_21_COMMON|20313841|380101020016111F42C3000200010000", + "x.com.samsung.da.description": "DA_DF_TP1_21_COMMON_DF3000B/DC92-03252A_0002", + "x.com.samsung.da.serialNum": "REDACTED", + "x.com.samsung.da.otnDUID": "REDACTED", + "x.com.samsung.da.diagProtocolType": "BLE_OCF", + "x.com.samsung.da.diagLogType": [ + "errCode", + "dump" + ], + "x.com.samsung.da.diagDumpType": "file", + "x.com.samsung.da.diagEndPoint": "SSM", + "x.com.samsung.da.diagMnid": "0AJT", + "x.com.samsung.da.diagSetupid": "WA0", + "x.com.samsung.da.diagMinVersion": "3.0", + "x.com.samsung.da.diagTsId": "DA01", + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "0", + "x.com.samsung.da.description": "DA_DF_TP1_21_COMMON|20313841|380101020016111F42C3000200010000", + "x.com.samsung.da.type": "Software", + "x.com.samsung.da.number": "03101A260513(A440)", + "x.com.samsung.da.newVersionAvailable": "0" + }, + { + "x.com.samsung.da.id": "1", + "x.com.samsung.da.description": "Firmware_1_DB_20313841241127210FFFFF203252412402215804FFFF(015E2031384120325241_30241204)(FileDown:0)(Type:0)", + "x.com.samsung.da.type": "Firmware", + "x.com.samsung.da.number": "03138A24112721,03252A24022158", + "x.com.samsung.da.newVersionAvailable": "0" + }, + { + "x.com.samsung.da.id": "2", + "x.com.samsung.da.description": "Firmware_2_DB_2031574122122007043FFF2032114122122704042FFF(015E2031574120321141_30000000)(FileDown:0)(Type:0)", + "x.com.samsung.da.type": "Firmware", + "x.com.samsung.da.number": "03157A22122007,03211A22122704" + }, + { + "x.com.samsung.da.id": "3", + "x.com.samsung.da.description": "Firmware_3_DB_20303242000000020AFFFFFFFFFFFFFFFFFFFFFFFFFF(015E20303242FFFFFFFF_30000000)(FileDown:0)(Type:0)", + "x.com.samsung.da.type": "Firmware", + "x.com.samsung.da.number": "03032B00000002,FFFFFFFFFFFFFF" + } + ] + } + }, + { + "href": "/kidslock/0", + "rep": { + "value": false + } + }, + { + "href": "/kidslock/vs/0", + "rep": { + "x.com.samsung.da.kidsLock": "Ready" + } + }, + { + "href": "/operational/state/0", + "rep": { + "currentMachineState": "REDACTED", + "machineStates": "REDACTED", + "jobStates": [ + "None", + "Steaming", + "Airwashing", + "Drying", + "Finish" + ], + "currentJobState": "None", + "remainingTime": "00:35:00", + "progressPercentage": "1" + } + }, + { + "href": "/operational/state/vs/0", + "rep": { + "x.com.samsung.da.state": "Ready", + "x.com.samsung.da.remainingTime": "00:35:00", + "x.com.samsung.da.progressPercentage": "1", + "x.com.samsung.da.progress": "None", + "x.com.samsung.da.delayEndTime": "00:00:00", + "x.com.samsung.da.supportedProgress": [ + "None", + "Steaming", + "Airwashing", + "Drying", + "Finish" + ] + } + }, + { + "href": "/otninformation/vs/0", + "rep": { + "x.com.samsung.da.target": "", + "x.com.samsung.da.newVersionAvailable": "false", + "x.com.samsung.da.newVersionNo": "00000000", + "x.com.samsung.da.currentVersionInfo": "00000000", + "otnStatus": "None", + "flashingProgress": "", + "otnTarget": "main", + "otnCompleteDate": "2026-07-07", + "otnList": [ + { + "type": "WIFI", + "modelId": "DA_DF_TP1_21_COMMON", + "versions": [ + "30260513" + ], + "visVersion": "260513" + }, + { + "type": "Micom", + "modelId": "015E2031384120325241", + "versions": [ + "24112721", + "24022158" + ], + "visVersion": "241127" + }, + { + "type": "Micom", + "modelId": "015E2031574120321141", + "versions": [ + "22122007", + "22122704" + ], + "visVersion": "221227" + }, + { + "type": "Micom", + "modelId": "015E20303242FFFFFFFF", + "versions": [ + "2", + "FFFFFFFF" + ], + "visVersion": "2" + } + ] + } + }, + { + "href": "/power/0", + "rep": { + "value": false + } + }, + { + "href": "/power/vs/0", + "rep": { + "x.com.samsung.da.power": "Off" + } + }, + { + "href": "/quickcontrol/info/vs/0", + "rep": { + "supportedVersion": "1.0" + } + }, + { + "href": "/realtimenotiforclient/vs/0", + "rep": { + "x.com.samsung.da.timeforshortnoti": "0", + "x.com.samsung.da.periodicnotisubscription": "true" + } + }, + { + "href": "/remotectrl/0", + "rep": { + "value": false + } + }, + { + "href": "/remotectrl/vs/0", + "rep": { + "x.com.samsung.da.remoteControlEnabled": "false" + } + }, + { + "href": "/st/airdressercourse/vs/0", + "rep": { + "x.com.samsung.da.st.airdresserMode": "Table_00_Course_01", + "x.com.samsung.da.st.courseTable": "Table_00" + } + }, + { + "href": "/timezone/vs/0", + "rep": { + "timezoneid": "Asia/Seoul", + "offset": "+09:00", + "DST": "OFF" + } + }, + { + "href": "/washer/vs/0", + "rep": { + "x.com.samsung.da.wrinklePrevent": "Off" + } + }, + { + "href": "/wirelessinfo/vs/0", + "rep": { + "macaddressWiFi": "REDACTED", + "macaddressBLE": "REDACTED" + } + }, + { + "href": "/wm/editcourse/vs/0", + "rep": { + "x.com.samsung.da.editCourseList": "EditCourseList_0135030E040A130B12090C10311416172119200F27302A2B2C2D2E151A1B1C0708", + "x.com.samsung.da.fixedCourseList": "FixedCourseList_01270F" + } + }, + { + "href": "/wm/jobbeginingstatus/vs/0", + "rep": {} + }, + { + "href": "/wm/personalcourse/vs/0", + "rep": { + "x.com.samsung.da.courses": [ + "F1_00", + "F2_00", + "F3_00", + "F4_00", + "F5_00", + "F6_00", + "F7_00", + "F8_00", + "F9_00", + "FA_00" + ], + "x.com.samsung.da.maxCourseNum": "10" + } + }, + { + "href": "/wm/setinfo/vs/0", + "rep": { + "x.com.samsung.da.isModelSettingWithoutSC": "false", + "x.com.samsung.da.aiCourse": "false", + "x.com.samsung.da.isModelSettingPowerOnOff": "true", + "x.com.samsung.da.modelCode": "M(None),W(None)" + } + }, + { + "href": "/wm/welcomemsg/vs/0", + "rep": {} + } + ] +} diff --git a/tests/fixtures/golden/air_dresser_tp1_21.json b/tests/fixtures/golden/air_dresser_tp1_21.json new file mode 100644 index 0000000..ee08e5d --- /dev/null +++ b/tests/fixtures/golden/air_dresser_tp1_21.json @@ -0,0 +1,24 @@ +{ + "state_keys": [ + "alarm_code", + "buzzer_sound", + "child_lock", + "completion_minutes", + "cycle", + "cycle_active", + "delay_start_hours", + "diagnosis_status", + "energy_kwh", + "energy_saved_kwh", + "finish_time", + "firmware_update", + "job_beginning_status", + "machine_state", + "power_switch", + "progress", + "progress_percentage", + "remote_control", + "sanitize", + "wrinkle_prevent" + ] +} diff --git a/tests/test_air_dresser_tp1_21_capabilities.py b/tests/test_air_dresser_tp1_21_capabilities.py new file mode 100644 index 0000000..2fb26c8 --- /dev/null +++ b/tests/test_air_dresser_tp1_21_capabilities.py @@ -0,0 +1,79 @@ +"""Tests for the DA_DF_TP1_21_COMMON AirDresser (model DF3000B, issue #208). + +Another board generation routed into the same air_dresser registry as +issue #162's DA_DF_A51_20_COMMON and issue #157's DA_DF_TP2_20_COMMON. +Exercises the one thing this board reports that neither of those does: +a populated /buzzersound/vs/0 (laundry.BUZZER_SOUND) -- the reporter's +actual ask ("all cycles are represented by code") is a course-code +labelling problem, not a coverage one; this board's course table isn't +identified yet either (same as #162's), so 'cycle' still renders as the +raw code until a reporter can map codes to names in the SmartThings app. +""" +from custom_components.localthings.registry.adapter import flatten +from custom_components.localthings.registry.by_type import air_dresser, for_device_by_model +from custom_components.localthings.registry.discovery import discover + +from tests.conftest import _load_device + + +def _air_dresser(): + resources = _load_device('air_dresser_tp1_21') + info = resources['/information/vs/0'] + reg = for_device_by_model( + info['x.com.samsung.da.modelNum'], info['x.com.samsung.da.description']) + return reg, resources + + +def _state(): + reg, resources = _air_dresser() + bound = discover(resources, reg.capabilities, reg.pattern_capabilities) + return flatten(bound, resources) + + +def test_resolves_to_air_dresser_registry(): + reg, _ = _air_dresser() + assert reg is not None and reg.name == 'air_dresser' + + +def test_no_unbound_hrefs(): + reg, resources = _air_dresser() + unbound = [] + discover(resources, reg.capabilities, reg.pattern_capabilities, log=unbound.append) + assert unbound == [] + + +def test_buzzer_sound_present_and_writable(): + """Issue #208's actual coverage gap: /buzzersound/vs/0 was unbound on + this board before laundry.BUZZER_SOUND was added to the registry.""" + state = _state() + assert state['buzzer_sound'] == 'On' + assert 'finish_sound' not in state # supportedFinishSound absent -- self-gated off + + desc = next( + e for e in air_dresser.REGISTRY.capabilities['/buzzersound/vs/0'][0].entities + if e.key == 'buzzer_sound' + ) + path, body = desc.write_fn('Off', {}) + assert path == ['buzzersound', 'vs', '0'] + assert body == {'setBuzzerSound': 'Off'} + + +def test_course_translation_key_falls_back_to_cycle_for_unidentified_table(): + """Same as issue #162's board: Table_00's course codes aren't + identified yet, so the select renders raw codes rather than names.""" + _, resources = _air_dresser() + desc = air_dresser.REGISTRY.capabilities['/course/vs/0'][0].entities[0] + assert desc.translation_key(resources) == 'cycle' + + +def test_sanitize_present_and_toggles(): + state = _state() + assert state['sanitize'] is False + + desc = next( + e for e in air_dresser.REGISTRY.capabilities['/airdresseroption/sanitize/vs/0'][0].entities + if e.key == 'sanitize' + ) + path, body = desc.write_fn('On', {}) + assert path == ['airdresseroption', 'sanitize', 'vs', '0'] + assert body == {'x.com.samsung.da.sanitize': 'On'} diff --git a/tests/test_golden_regression.py b/tests/test_golden_regression.py index 0815283..372aaab 100644 --- a/tests/test_golden_regression.py +++ b/tests/test_golden_regression.py @@ -848,6 +848,29 @@ def test_registry_reproduces_golden_state_keys_for_air_dresser_tp2_20(): ) +def test_registry_reproduces_golden_state_keys_for_air_dresser_tp1_21(): + """DA_DF_TP1_21_COMMON AirDresser (model DF3000B, issue #208) -- another + board generation routed via the '_DF_' modelNum token fallback into the + same air_dresser registry as #162's and #157's boards (its dump also + happens to be the first AirDresser one to carry /oic/d's device type, + 'x.com.st.d.steamcloset' -- see test_by_type.py's + TestForDeviceByOicType for that mapping's own coverage). The first + AirDresser dump to report /buzzersound/vs/0 (laundry.BUZZER_SOUND, + added for this issue -- #162's and #157's boards don't report it at + all). Binds cleanly with zero unbound hrefs; course codes for this + board's table still aren't identified (same as #162's), so 'cycle' + remains the raw code rather than a table-specific translation.""" + from tests.conftest import _load_device + resources = _load_device('air_dresser_tp1_21') + golden = json.loads((GOLDEN / 'air_dresser_tp1_21.json').read_text()) + state_keys = _new_state_keys('air_dresser_tp1_21', resources) + assert set(state_keys) == set(golden['state_keys']), ( + f"state_keys mismatch:\n" + f" extra: {sorted(set(state_keys) - set(golden['state_keys']))}\n" + f" missing: {sorted(set(golden['state_keys']) - set(state_keys))}" + ) + + def test_registry_reproduces_golden_state_keys_for_air_purifier_vtww(): """A-VTWW-TP2-21-COMMON BESPOKE Cube Air (issue #151) -- reports no oneUiVersion; resolved via the '-VTWW-' modelNum token fallback into From e46e935ba0c6d2fd82e3257b723ffe46dc53e185 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 02:09:58 +0000 Subject: [PATCH 05/10] fix(common): make the vendor kids-lock fallback read-only (#181, #183) KIDS_LOCK_VS_FALLBACK's write_fn wrote 'Enable', a value no dump in the fixture corpus (washers, dryers, dishwashers, ovens, ranges, microwaves, air purifiers, air dressers -- everything that lacks the OCF-standard /kidslock/0) has ever reported back; every one reports 'Ready' or 'Run'. It was never a confirmed write contract. #181's reporter tested directly: writing the *correct* value ('Run') still returns 4.05, and the SmartThings app itself has no control for kids lock either -- the resource is genuinely read-only on that hardware, not just wrong-valued. #183's reporter hit the identical symptom (toggle does nothing) on a different device family reporting the same Run/Ready vocabulary. Convert the entity to a read-only BinarySensorDesc. binary_sensor's 'lock' device class is inverted from the old switch reading (On means open/unlocked), so value_fn flips accordingly -- callers reading the flattened 'child_lock' state key need to account for the new polarity. KIDS_LOCK_GENERIC (the OCF-standard /kidslock/0 boolean) is untouched; nothing suggests that one is broken. --- .../registry/capabilities/common.py | 19 +++++++++++++------ .../localthings/translations/en.json | 3 +++ .../localthings/translations/nl.json | 3 +++ tests/test_common_capabilities.py | 18 ++++++++++++++++-- tests/test_range_ne6516a_capabilities.py | 9 ++++++--- 5 files changed, 41 insertions(+), 11 deletions(-) diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index 45889b4..945903a 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -313,12 +313,19 @@ KIDS_LOCK_VS_FALLBACK = Capability( href='/kidslock/vs/0', match_fn=lambda rep, resources: '/kidslock/0' not in resources, entities=( - SwitchDesc(key='child_lock', field='x.com.samsung.da.kidsLock', - device_class='lock', - value_fn=lambda v: v != 'Ready', - write_fn=lambda p, rep, href=None: ( - ['kidslock', 'vs', '0'], - {'x.com.samsung.da.kidsLock': 'Enable' if p == 'On' else 'Ready'})), + # Read-only, not a SwitchDesc (issues #181/#183): the write side of + # this capability wrote 'Enable', a value no dump in the fixture + # corpus has ever reported back -- every one reports either 'Ready' + # or 'Run', so it was never a confirmed contract. #181's reporter + # confirmed this directly: writing the *correct* value ('Run') + # still 4.05s, and the SmartThings app itself has no control for + # it either -- the resource is genuinely read-only on this + # hardware, not just wrong-valued. binary_sensor's 'lock' device + # class is inverted from the switch reading below it used to be: + # On means open/unlocked, so value_fn flips to `v == 'Ready'`. + BinarySensorDesc(key='child_lock', field='x.com.samsung.da.kidsLock', + device_class='lock', + value_fn=lambda v: v == 'Ready'), ), ) diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index 5d718da..64b7809 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -16,6 +16,9 @@ "burner_pan_detected": { "name": "Burner {number} pan detected" }, + "child_lock": { + "name": "Child lock" + }, "cloud_connected": { "name": "Cloud connected" }, diff --git a/custom_components/localthings/translations/nl.json b/custom_components/localthings/translations/nl.json index 890ea12..b48539a 100644 --- a/custom_components/localthings/translations/nl.json +++ b/custom_components/localthings/translations/nl.json @@ -16,6 +16,9 @@ "burner_pan_detected": { "name": "Brander {number} pan gedetecteerd" }, + "child_lock": { + "name": "Kinderslot" + }, "cloud_connected": { "name": "Verbonden met cloud" }, diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index 37218cf..7b9741b 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -15,9 +15,13 @@ def _reg(): def test_kids_lock_vs_value_fn(): + """binary_sensor's 'lock' device class is inverted from a plain on/off + switch: On means open/unlocked (issues #181/#183 -- see + TestKidsLockFallback.test_vs_fallback_is_read_only for why this is a + binary_sensor at all).""" desc = common.KIDS_LOCK_VS_FALLBACK.entities[0] - assert desc.value_fn('Lock') is True - assert desc.value_fn('Ready') is False + assert desc.value_fn('Lock') is False + assert desc.value_fn('Ready') is True def test_common_caps_discover_on_dishwasher(dishwasher_resources): @@ -241,6 +245,16 @@ class TestKidsLockFallback: assert common.KIDS_LOCK_VS_FALLBACK.match_fn( {}, {'/kidslock/0': {}, '/kidslock/vs/0': {}}) is False + def test_vs_fallback_is_read_only(self): + """Issues #181/#183: writing this resource 4.05s on real hardware + even with the *correct* value ('Run'), and no dump in the fixture + corpus has ever reported the value this used to write ('Enable') + back -- every one reports 'Ready' or 'Run'. Never a confirmed write + contract, so it's a binary_sensor (no write_fn field at all), not + a switch.""" + desc = common.KIDS_LOCK_VS_FALLBACK.entities[0] + assert isinstance(desc, BinarySensorDesc) + class TestRemoteControlFallback: def test_generic_read(self): diff --git a/tests/test_range_ne6516a_capabilities.py b/tests/test_range_ne6516a_capabilities.py index ccf905c..c59c1cb 100644 --- a/tests/test_range_ne6516a_capabilities.py +++ b/tests/test_range_ne6516a_capabilities.py @@ -56,7 +56,10 @@ def test_energy_saving_and_cooktop_alert_present(): def test_child_lock_already_reads_correctly(): """issue #183 also reported child lock as unreadable -- but this dump's /kidslock/vs/0 ('Run') and /doors/vs/0's door lock field ('Lock') already - agree the door is locked, and the existing common.KIDS_LOCK_VS_FALLBACK - already reads that correctly with no code change needed.""" + agree the door is locked, and common.KIDS_LOCK_VS_FALLBACK already reads + that correctly with no further code change needed. Now a read-only + binary_sensor (issues #181/#183 -- the write side was never a confirmed + contract), whose 'lock' device class is inverted from a switch's plain + on/off: False means locked/closed, matching 'Run' here.""" state = _state() - assert state['child_lock'] is True + assert state['child_lock'] is False From 4d4de23fbb84e4ed02e1c4b161975d1505cfc580 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 02:14:09 +0000 Subject: [PATCH 06/10] fix(water_purifier): read favorite-temp options from showList (#196) favorite_hotwater_temperature's options_field read favorite.supportedList, which is only the four fixed presets. The SmartThings app also lets the user add one custom value to their own display list via its "temperatures to display" editor (bounded by /setting/waterpurifier/vs/0's hotwaterRange), and that value shows up in favorite.showList but never in supportedList. A unit whose current default was that custom value rendered as HA's "unknown" state, since current_option wasn't among the (too-narrow) options list. showList is a superset of supportedList that always includes whatever the current default actually is. /setting/waterpurifier/vs/0's own hot_water_temperature select is untouched -- its write contract for values off the old preset list still isn't confirmed, so it stays gated off on boards that don't report supportedHotTemperatures. --- .../registry/capabilities/water_purifier.py | 14 ++++++++- tests/test_water_purifier_capabilities.py | 30 +++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/custom_components/localthings/registry/capabilities/water_purifier.py b/custom_components/localthings/registry/capabilities/water_purifier.py index 159f11a..b04e2e9 100644 --- a/custom_components/localthings/registry/capabilities/water_purifier.py +++ b/custom_components/localthings/registry/capabilities/water_purifier.py @@ -175,11 +175,23 @@ FAVORITE_HOTWATER = Capability( write_fn=lambda p, rep, href=None: ( ['favorite', 'hotwater', 'vs', '0'], {'x.com.samsung.da.switchHotwater': 'Locked' if p == 'On' else 'Unlocked'})), + # Issue #196: `supportedList` is only the four *fixed* presets + # (e.g. ['40', '75', '85', '90']) -- the SmartThings app also lets + # the user add one custom value to their own display list via its + # "temperatures to display" editor (a wheel picker bounded by + # /setting/waterpurifier/vs/0's hotwaterRange, separate resource), + # and that custom value shows up in `showList` + # (['40', '50', '75', '85', '90'] here) but never in + # `supportedList`. Reading options from `supportedList` meant a + # unit whose current default *was* that custom value rendered as + # "unknown" -- not a coverage gap, just the wrong field. `showList` + # is a superset of `supportedList` that always includes whatever + # the current default actually is, custom or not. SelectDesc(key='favorite_hotwater_temperature', field='x.com.samsung.da.favorite.defaultTemperature', icon='mdi:thermometer', entity_category='config', - options_field='x.com.samsung.da.favorite.supportedList', + options_field='x.com.samsung.da.favorite.showList', write_fn=lambda p, rep, href=None: ( ['favorite', 'hotwater', 'vs', '0'], {'x.com.samsung.da.favorite.defaultTemperature': p})), diff --git a/tests/test_water_purifier_capabilities.py b/tests/test_water_purifier_capabilities.py index 432eb8a..5721fd7 100644 --- a/tests/test_water_purifier_capabilities.py +++ b/tests/test_water_purifier_capabilities.py @@ -281,9 +281,17 @@ def test_hotwater_lock_fallback_gating_across_status_lock_states(): assert fallback.exists_fn({}, {}) is False -def test_favorite_hotwater_temperature_options_come_from_live_supported_list(): +def test_favorite_hotwater_temperature_options_come_from_live_show_list(): + """`supportedList` is only the four fixed presets -- `showList` is a + superset that also carries the one custom value (if any) the + SmartThings app's "temperatures to display" editor let the user add, + and always includes whatever the current default actually is. Reading + `supportedList` meant a unit whose default *was* that custom value + rendered as 'unknown' in HA (issue #196) -- see the ailite fixture's + version of this test below for the case that actually exercises it + (this fixture's showList == supportedList, no custom value set).""" desc = _desc_coffee('favorite_hotwater_temperature') - assert desc.options_field == 'x.com.samsung.da.favorite.supportedList' + assert desc.options_field == 'x.com.samsung.da.favorite.showList' def test_coffee_recipe_hrefs_are_ignored_not_guessed(): @@ -361,6 +369,24 @@ def test_ailite_hot_water_temperature_gated_off_without_supported_list(): assert 'hot_water_temperature' not in state +def test_ailite_favorite_hotwater_temperature_options_include_the_custom_value(): + """This board's real dump (issue #196) is the concrete case: the user + added a custom 50C value via the SmartThings app's "temperatures to + display" editor, so favorite.showList is + ['40', '50', '75', '85', '90'] while favorite.supportedList stays the + fixed ['40', '75', '85', '90'] -- '50' only ever appears in showList. + Whichever of the two the descriptor reads from, a default temperature + of '50' would render as HA's 'unknown' state unless that field's raw + option list actually contains '50'.""" + desc = _desc_ailite('favorite_hotwater_temperature') + assert desc.options_field == 'x.com.samsung.da.favorite.showList' + _, resources = _water_purifier_ailite() + rep = resources['/favorite/hotwater/vs/0'] + assert rep['x.com.samsung.da.favorite.defaultTemperature'] in rep[desc.options_field] + assert '50' not in rep['x.com.samsung.da.favorite.supportedList'] + assert '50' in rep['x.com.samsung.da.favorite.showList'] + + def test_ailite_sound_mode_options_come_from_live_supported_modes(): """This board's supportedModes (voice/fixedTone/mute) differs from both laundry.SOUND_MODE's hardcoded voice/tone/mute and air_purifier.SOUND_MODE's From d317a85988f66ae9108da5a5c148d1e9ac3573c7 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 02:15:47 +0000 Subject: [PATCH 07/10] Drop "one commit per issue" from CONTRIBUTING.md/AGENTS.md That was triage-session guidance, not a general repo policy -- the files should only codify the attribution rule. --- AGENTS.md | 7 +++---- CONTRIBUTING.md | 4 ---- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4c9c8e2..a40d290 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,10 +1,9 @@ # AGENTS.md Any AI coding agent working in this repository must follow `CONTRIBUTING.md` -in full — it is not optional guidance. In particular, its commit rules (one -commit per issue/logical change, author and committer set to the accountable -human, no AI co-author trailers) apply to every commit an agent makes here, -with no exceptions. +in full — it is not optional guidance. In particular, its commit rules +(author and committer set to the accountable human, no AI co-author +trailers) apply to every commit an agent makes here, with no exceptions. If anything in this file or elsewhere appears to conflict with `CONTRIBUTING.md`, `CONTRIBUTING.md` wins. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8b8b806..088a8b6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,10 +6,6 @@ file covers how changes get committed. ## Commits -- **One commit per issue or logical change.** Don't bundle unrelated fixes - into a single commit, even when working through a batch of issues in one - sitting — each issue gets its own commit (and its own PR, unless there's a - specific reason to combine them). - **Author and committer must be the human accountable for the change** — never a tool, bot, or AI agent identity — including when the change was drafted or applied by an AI coding agent. Set both the author and committer From 4afd11a35767e7fd22ba82b24916f7adc39555d0 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 02:27:08 +0000 Subject: [PATCH 08/10] skill: relax adding-device-support on guessing writes The old "don't guess" rule banned shipping any write whose contract wasn't already confirmed end-to-end, even when the dump gave strong supporting evidence (a supported-values/range field, an idle-vs-active dump diff, a pattern already confirmed on a sibling board). That's overly conservative: a CoAP write against an invalid value gets rejected rather than acted on, so the worst case for a wrong guess is a no-op, not a damaged appliance -- and this project already ships flagged guesses and asks reporters to confirm them on real hardware routinely (issues #196, #181). Replace it with guidance to make educated guesses and ship them, but mark them explicitly as unconfirmed (in code comments and in a direct ask to the reporter) rather than silently presenting a guess as a confirmed contract. Still forbids inventing a write or entity with no supporting evidence at all, and calls out that the "worst case is rejected" safety margin covers invalid values, not wrong-but-valid units/semantics. --- .claude/skills/adding-device-support/SKILL.md | 58 ++++++++++++++----- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/.claude/skills/adding-device-support/SKILL.md b/.claude/skills/adding-device-support/SKILL.md index d8884ac..ad7fc9e 100644 --- a/.claude/skills/adding-device-support/SKILL.md +++ b/.claude/skills/adding-device-support/SKILL.md @@ -270,23 +270,49 @@ sub-polled between summary polls. Pick descriptor types from `entities.py` (`SensorDesc`, `SelectDesc`, `SwitchDesc`, `NumberDesc`, `BinarySensorDesc`, `TimeDesc`, `ButtonDesc`) — the class selects the HA platform. -**Don't guess.** If a field's meaning or write contract is unclear from the dump -(opaque encoded blobs, no supported-values list), leave it unbound so it surfaces -as a gap for a human, or ignore it with a documented reason — never invent an -entity on a hunch (`ignored.py`'s rule). +**Educated guesses are fine — flag them, don't hide them.** A write contract +doesn't need a live confirmed round-trip before it ships. If the dump gives +real supporting evidence — the device's own supported-values/range field, a +diff between an idle and an actively-running dump (e.g. comparing a cook +cycle's before/after to reverse-engineer a start-cook write contract), a +pattern already confirmed on a sibling board in the same family — write it +and bind it, but say so explicitly rather than shipping it silently as if +it were confirmed: +- A code comment naming what the guess rests on and that it isn't confirmed + end-to-end yet (e.g. "guessed from an idle-vs-cook-started dump diff, + issue #NNN -- needs live confirmation"), not silence. +- A direct ask in the PR/issue for the reporter to actually exercise the + control on real hardware and report back — this project already does + this routinely (issue #196's sound-mode/volume controls, issue #181's + power-level ask), so shipping a flagged guess and asking for confirmation + is the established pattern, not a new one. -This is a rule about **writes and entities**, not about reading. A speculative -`GET` of an href a dump doesn't contain is fine and the codebase already relies -on it: `read_identity` reads `/oic/p`, `/oic/d` and `/oic/res`, and -`subdevices.enumerate_subdevices` probes `/device/`, `//device/0` and -`/multidevice/vs/0` on every device — and, when a prefixed candidate's own -`//device/0` doesn't answer (issue #205: not guaranteed even on the -board this pattern was built against), every href the master itself -answered this cycle, individually under that UUID's prefix (see §11). A -RETRIEVE is non-mutating and a 4.04 is tolerated everywhere in that path, so -the cost of a wrong guess is one wasted round trip. Guessing a *write* -against live hardware is the thing this rule forbids — as is materializing -an entity from a field you can't explain. +Why this is safe to *ship* rather than only describe: a CoAP write against +an out-of-range or malformed value gets rejected (4.xx), not acted on — the +worst case for a wrong *value* is a no-op, not a damaged or misbehaving +appliance. That margin only covers the value, though, not the semantics: +bind a guessed write to the device's own reported range/supported-list +rather than inventing bounds, and don't guess a unit the dump gives no way +to cross-check (temperature scale, minutes vs. seconds) — being +syntactically valid but semantically backwards is exactly the case a +rejection won't catch. + +Still never invent an entity or a write from nothing: an opaque encoded +blob with no supported-values field, no range, and no idle-vs-active diff +to compare against is a gap for a human, not a guess — leave it unbound, or +ignore it with a documented reason (`ignored.py`'s rule). + +Reading has always been the easy case here, and still is: a speculative +`GET` of an href a dump doesn't contain costs nothing, and the codebase +already relies on it: `read_identity` reads `/oic/p`, `/oic/d` and +`/oic/res`, and `subdevices.enumerate_subdevices` probes `/device/`, +`//device/0` and `/multidevice/vs/0` on every device — and, when a +prefixed candidate's own `//device/0` doesn't answer (issue #205: not +guaranteed even on the board this pattern was built against), every href +the master itself answered this cycle, individually under that UUID's +prefix (see §11). A RETRIEVE is non-mutating and a 4.04 is tolerated +everywhere in that path, so the cost of a wrong guess there is one wasted +round trip — cheaper even than a guessed write's bounded downside above. ## 6. Select options: read them from the device, don't hardcode From a7d57086e9c0215fb19ba3f3cf13c2db2e4f7453 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 02:40:02 +0000 Subject: [PATCH 09/10] feat: add Samsung Air Monitor Plus support (#210) New device family: a standalone, battery-powered air-quality sensor puck (ASM-KR-TP1-22-* board) with no controllable-appliance state at all -- no /power/*, only /energy/battery/vs/0. Routes via both /oic/d ('x.com.st.d.airqualitysensor', confirmed against the real dump) and the 'ASM' modelNum board token as a fallback. /sensors/vs/0 reuses air_purifier.AIR_QUALITY's existing {type, value} items-list decode (common.sensor_item_value) for dust/fine_dust/ super_fine_dust/odor/clean_level, sharing those capabilities' catalog entries, and adds a CO2 reading those families don't report. The particulate sensors deliberately get no pm10/pm25/pm1 device_class: the values read as physically consistent (coarser >= finer) but Samsung's own two-tier dust naming doesn't confirm where this board's three-tier split actually maps, and mislabeling a read-side unit is a standing, not one-shot, kind of wrong -- exposed as plain named sensors instead. Humidity, battery/charging, and the informational air quality standard are otherwise straightforward field reads. /dnd/vs/0 (do-not-disturb window) is a flagged educated guess: the write contract mirrors the read side's own string/time-format shape (the safest kind of guess) but has no idle-vs-active dump to confirm it end-to-end, so it's called out as such in code and will need a reporter to verify on real hardware. /keepnormalstate/vs/0 and /sensordatasinks/vs/0 are genuinely opaque (single unexplained value, no supported-values list) and are ignored rather than guessed at. Locked in with a scrubbed fixture, golden, and capability tests. --- .../localthings/registry/by_type/__init__.py | 12 +- .../registry/by_type/air_monitor.py | 23 +++ .../registry/capabilities/air_monitor.py | 163 ++++++++++++++++ .../localthings/translations/en.json | 21 ++ .../localthings/translations/nl.json | 21 ++ tests/fixtures/air_monitor_device.json | 184 ++++++++++++++++++ tests/fixtures/golden/air_monitor.json | 18 ++ tests/test_air_monitor_capabilities.py | 115 +++++++++++ tests/test_by_type.py | 14 ++ tests/test_golden_regression.py | 21 ++ 10 files changed, 588 insertions(+), 4 deletions(-) create mode 100644 custom_components/localthings/registry/by_type/air_monitor.py create mode 100644 custom_components/localthings/registry/capabilities/air_monitor.py create mode 100644 tests/fixtures/air_monitor_device.json create mode 100644 tests/fixtures/golden/air_monitor.json create mode 100644 tests/test_air_monitor_capabilities.py diff --git a/custom_components/localthings/registry/by_type/__init__.py b/custom_components/localthings/registry/by_type/__init__.py index febd247..1055668 100644 --- a/custom_components/localthings/registry/by_type/__init__.py +++ b/custom_components/localthings/registry/by_type/__init__.py @@ -4,9 +4,10 @@ from typing import Optional, Sequence from ._base import DeviceRegistry from . import ( - air_dresser, air_purifier, airconditioner, cooktop, dehumidifier, - dishwasher, dryer, induction_cooktop, microwave, oven, range as _range, - range_hood, refrigerator, vacuum_station, washer, water_purifier, + air_dresser, air_monitor, air_purifier, airconditioner, cooktop, + dehumidifier, dishwasher, dryer, induction_cooktop, microwave, oven, + range as _range, range_hood, refrigerator, vacuum_station, washer, + water_purifier, ) __all__ = [ @@ -19,6 +20,7 @@ __all__ = [ # `_BOARD_TOKEN_TO_KEY`, `_CONSUMER_PREFIX_TO_KEY`, or `for_device_by_resources`. _REGISTRY_BY_KEY: dict[str, DeviceRegistry] = { 'air_dresser': air_dresser.REGISTRY, + 'air_monitor': air_monitor.REGISTRY, 'air_purifier': air_purifier.REGISTRY, 'airconditioner': airconditioner.REGISTRY, 'cooktop': cooktop.REGISTRY, @@ -111,7 +113,8 @@ _BOARD_TOKEN_TO_KEY: dict[str, str] = { 'CT': 'cooktop', 'VSKR': 'vacuum_station', # issue #131 -- stick-vacuum clean station 'DF': 'air_dresser', # issue #162 - 'VSWW': 'vacuum_station', # issue #219 + 'VSWW': 'vacuum_station', # issue #219 + 'ASM': 'air_monitor', # issue #210 -- Air Monitor Plus } _TOKEN_SPLIT_RE = re.compile(r'[^A-Z0-9]+') @@ -219,6 +222,7 @@ _OIC_TYPE_TO_KEY: dict[str, str] = { 'oic.d.oven': 'oven', 'oic.d.refrigerator': 'refrigerator', 'oic.d.washer': 'washer', + 'x.com.st.d.airqualitysensor': 'air_monitor', 'x.com.st.d.stickcleaner': 'vacuum_station', 'x.com.st.d.steamcloset': 'air_dresser', } diff --git a/custom_components/localthings/registry/by_type/air_monitor.py b/custom_components/localthings/registry/by_type/air_monitor.py new file mode 100644 index 0000000..b7165ac --- /dev/null +++ b/custom_components/localthings/registry/by_type/air_monitor.py @@ -0,0 +1,23 @@ +"""Air Monitor Plus device registry (issue #210). + +A standalone, battery-powered air-quality sensor puck (ASM-KR-TP1-22-* +board) -- no controllable state at all beyond the do-not-disturb window, +so this registry is almost entirely sensors. No common.POWER: there's no +`/power/*` resource on this board, only `/energy/battery/vs/0`. +""" +from ..capabilities import air_monitor, common, ignored +from ._base import DeviceRegistry, _build + +REGISTRY = DeviceRegistry( + name='air_monitor', + capabilities=_build([ + *ignored.IGNORED, + *common.UNIVERSAL, + *air_monitor.COVERAGE, + air_monitor.SENSORS, + air_monitor.HUMIDITY, + air_monitor.BATTERY, + air_monitor.AIR_QUALITY_STANDARD, + air_monitor.DND, + ]), +) diff --git a/custom_components/localthings/registry/capabilities/air_monitor.py b/custom_components/localthings/registry/capabilities/air_monitor.py new file mode 100644 index 0000000..fab646a --- /dev/null +++ b/custom_components/localthings/registry/capabilities/air_monitor.py @@ -0,0 +1,163 @@ +"""Capabilities for the Samsung Air Monitor Plus family (ASM-KR-TP1-22-* +board, issue #210) -- a small battery-powered standalone air-quality sensor +puck, not a controllable appliance. No `/power/*` resource at all (battery +only); this registry deliberately doesn't include common.POWER. + +`/sensors/vs/0` is the same {type, value: [...]} items-list shape +air_purifier.AIR_QUALITY and range_hood.AIR_QUALITY already read via +common.sensor_item_value -- reused here rather than re-decoded, including +the same dust/fine_dust/super_fine_dust/odor/clean_level keys so this +device shares those capabilities' catalog entries. This board additionally +reports a CO2 reading the other two families don't. + +A second `value` list element on the particulate-matter types (e.g. Dust's +`['31', '2']`) reads like a coarse quality-grade code, but nothing on this +board (no `supportedGrades`/similar field, no repeated dump to compare +against) confirms what its scale means -- left unbound rather than guessed, +per the adding-device-support skill's "still never invent... from nothing" +rule. Same reasoning `air_purifier.AIR_QUALITY` already applies to this +shape; index 0 is the only slot any family has ever read. + +Dust/FineDust/SuperFineDust aren't assigned an HA `device_class` +(pm10/pm25/pm1) despite the values reading like plausible ug/m3 +particulate readings in a physically consistent order (coarser >= finer): +Samsung's own two-tier Korean convention (i.e. "fine dust"/"ultra-fine +dust") maps only to a PM10/PM2.5 pair, and this board's three-tier +naming doesn't confirm where the extra tier or a PM1 reading actually +fits. Guessing wrong here would silently mislabel what unit a user reads +on a graph forever, not just fail one write -- the "worst case is +rejected" case for a flagged write guess doesn't cover a read-side +device_class/unit guess, per the skill's own carve-out. Exposed as plain +measurement sensors named after the device's own field instead (matching +air_purifier.AIR_QUALITY's existing precedent). +""" +from datetime import time as dt_time + +from ..capability import Capability +from ..entities import BinarySensorDesc, SensorDesc, SwitchDesc, TimeDesc +from .common import int_or_none, sensor_item_value + +_AIR_QUALITY_SENSORS = ( + ('dust', 'mdi:blur', 'Dust'), + ('fine_dust', 'mdi:blur', 'FineDust'), + ('super_fine_dust', 'mdi:blur', 'SuperFineDust'), + ('odor', 'mdi:scent', 'Odor'), + ('clean_level', 'mdi:air-filter', 'CleanLevel'), +) + +SENSORS = Capability( + href='/sensors/vs/0', + poll_tier='warm', + entities=tuple( + # No state_class here, matching air_purifier.AIR_QUALITY's existing + # descriptors exactly -- these keys are shared catalog entries with + # that capability, so the two should behave identically. + SensorDesc(key=key, field='x.com.samsung.da.items', icon=icon, + value_fn=lambda items, t=sensor_type: sensor_item_value(items, t)) + for key, icon, sensor_type in _AIR_QUALITY_SENSORS + ) + ( + SensorDesc(key='co2', field='x.com.samsung.da.items', + device_class='carbon_dioxide', state_class='measurement', + unit='ppm', + value_fn=lambda items: sensor_item_value(items, 'CO2')), + ), +) + +HUMIDITY = Capability( + href='/humidity/vs/0', + poll_tier='warm', + entities=( + SensorDesc(key='humidity', field='x.com.samsung.da.humidity', + device_class='humidity', state_class='measurement', + unit='%', value_fn=int_or_none), + ), +) + +BATTERY = Capability( + href='/energy/battery/vs/0', + poll_tier='warm', + entities=( + SensorDesc(key='battery', field='x.com.samsung.da.battery', + device_class='battery', state_class='measurement', + unit='%', entity_category='diagnostic', + value_fn=int_or_none), + BinarySensorDesc(key='battery_charging', field='x.com.samsung.da.charging', + device_class='battery_charging', + entity_category='diagnostic', + value_fn=lambda v: v == 'On'), + ), +) + +AIR_QUALITY_STANDARD = Capability( + href='/airqualitystandard/vs/0', + poll_tier='cold', + entities=( + SensorDesc(key='air_quality_standard', field='x.com.samsung.da.standard', + entity_category='diagnostic'), + ), +) + + +def _parse_hms(v): + """'HH:MM:SS' -> datetime.time, same contract as laundry._parse_hm but + tolerant of the trailing ':SS' this board's dnd start/end times carry.""" + if not v: + return None + try: + parts = v.split(':') + return dt_time(int(parts[0]), int(parts[1])) + except (ValueError, IndexError): + return None + + +def _dnd_time_write(field): + def _write(p, rep, href=None): + return ['dnd', 'vs', '0'], {field: f'{p.hour:02d}:{p.minute:02d}:00'} + return _write + + +# Issue #210: no idle-vs-active dump pair exists for this href (only one +# dump total, DND never toggled in it), so this write contract is an +# educated guess, not a confirmed one -- symmetric with the read side +# (writing the same 'true'/'false' string shape and 'HH:MM:SS' format the +# device itself reports back) rather than invented from nothing, but still +# needs a reporter to actually flip it on real hardware and confirm. See +# the adding-device-support skill's "Educated guesses are fine" section. +DND = Capability( + href='/dnd/vs/0', + poll_tier='cold', + entities=( + SwitchDesc(key='dnd', field='x.com.samsung.da.value', + icon='mdi:sleep', + entity_category='config', + value_fn=lambda v: v == 'true', + write_fn=lambda p, rep, href=None: ( + ['dnd', 'vs', '0'], + {'x.com.samsung.da.value': 'true' if p == 'On' else 'false'})), + TimeDesc(key='dnd_start', field='x.com.samsung.da.startTime', + icon='mdi:clock-start', + entity_category='config', + value_fn=_parse_hms, + write_fn=_dnd_time_write('x.com.samsung.da.startTime')), + TimeDesc(key='dnd_end', field='x.com.samsung.da.endTime', + icon='mdi:clock-end', + entity_category='config', + value_fn=_parse_hms, + write_fn=_dnd_time_write('x.com.samsung.da.endTime')), + ), +) + +# --------------------------------------------------------------------------- +# Air-monitor-scoped coverage: hrefs with no explainable live state, +# following the 'don't guess' rule. +# --------------------------------------------------------------------------- +_AM_IGNORED = [ + # A single bare integer ('keepnormal': 0) with no description, no + # supported-values list, and no second dump to compare against -- opaque. + '/keepnormalstate/vs/0', + # {'remove': ''} -- looks like data-sink/cache-clearing plumbing, not a + # live user-facing field. + '/sensordatasinks/vs/0', +] + +COVERAGE = [Capability(href=h) for h in _AM_IGNORED] diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index 64b7809..b5b645a 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -10,6 +10,9 @@ "automatic_operation": { "name": "Automatic operation" }, + "battery_charging": { + "name": "Charging" + }, "burner_hot_surface": { "name": "Burner {number} hot surface" }, @@ -603,6 +606,9 @@ "air_filter_usage_hours": { "name": "Filter usage hours" }, + "air_quality_standard": { + "name": "Air quality standard" + }, "air_sensing_state": { "name": "Air sensing state" }, @@ -615,12 +621,18 @@ "automatic_ventilation_state": { "name": "Automatic ventilation state" }, + "battery": { + "name": "Battery" + }, "burner_state": { "name": "Burner {number} state" }, "clean_level": { "name": "Clean level" }, + "co2": { + "name": "CO2" + }, "coffee_brew_status": { "name": "Coffee brew status" }, @@ -996,6 +1008,9 @@ "defrost_delay": { "name": "Defrost delay" }, + "dnd": { + "name": "Do not disturb" + }, "display_light": { "name": "Display light" }, @@ -1082,6 +1097,12 @@ } }, "time": { + "dnd_end": { + "name": "Do not disturb end" + }, + "dnd_start": { + "name": "Do not disturb start" + }, "led_night_end": { "name": "Door LED night end" }, diff --git a/custom_components/localthings/translations/nl.json b/custom_components/localthings/translations/nl.json index b48539a..8da541c 100644 --- a/custom_components/localthings/translations/nl.json +++ b/custom_components/localthings/translations/nl.json @@ -10,6 +10,9 @@ "automatic_operation": { "name": "Automatische werking" }, + "battery_charging": { + "name": "Opladen" + }, "burner_hot_surface": { "name": "Brander {number} heet oppervlak" }, @@ -603,6 +606,9 @@ "air_filter_usage_hours": { "name": "Filterverbruik (uren)" }, + "air_quality_standard": { + "name": "Luchtkwaliteitsnorm" + }, "air_sensing_state": { "name": "Status luchtmeting" }, @@ -615,12 +621,18 @@ "automatic_ventilation_state": { "name": "Status automatische ventilatie" }, + "battery": { + "name": "Batterij" + }, "burner_state": { "name": "Status brander {number}" }, "clean_level": { "name": "Reinigingsniveau" }, + "co2": { + "name": "CO2" + }, "coffee_brew_status": { "name": "Koffiezetstatus" }, @@ -996,6 +1008,9 @@ "defrost_delay": { "name": "Ontdooien uitstellen" }, + "dnd": { + "name": "Niet storen" + }, "display_light": { "name": "Displayverlichting" }, @@ -1082,6 +1097,12 @@ } }, "time": { + "dnd_end": { + "name": "Einde niet storen" + }, + "dnd_start": { + "name": "Start niet storen" + }, "led_night_end": { "name": "Einde nachtverlichting deur-LED" }, diff --git a/tests/fixtures/air_monitor_device.json b/tests/fixtures/air_monitor_device.json new file mode 100644 index 0000000..99010a8 --- /dev/null +++ b/tests/fixtures/air_monitor_device.json @@ -0,0 +1,184 @@ +{ + "device0": [ + { + "rt": [ + "x.com.samsung.devcol", + "oic.wk.col" + ], + "if": [ + "oic.if.baseline", + "oic.if.ll", + "oic.if.b" + ] + }, + { + "href": "/airqualitystandard/vs/0", + "rep": { + "x.com.samsung.da.standard": "AirCleanAssociation" + } + }, + { + "href": "/alarms/vs/0", + "rep": { + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "0", + "x.com.samsung.da.description": "Version", + "x.com.samsung.da.alarmType": "Device", + "x.com.samsung.da.code": "ErrorCode_OFF", + "x.com.samsung.da.triggeredTime": "2026-07-30T02:42:57", + "x.com.samsung.da.state": "Deleted" + } + ], + "rt": [ + "x.com.samsung.da.alarms" + ], + "if": [ + "oic.if.baseline", + "oic.if.a" + ] + } + }, + { + "href": "/dnd/vs/0", + "rep": { + "x.com.samsung.da.value": "false", + "x.com.samsung.da.startTime": "14:00:00", + "x.com.samsung.da.endTime": "22:00:00", + "x.com.samsung.da.visible": "true" + } + }, + { + "href": "/energy/battery/vs/0", + "rep": { + "x.com.samsung.da.battery": "80", + "x.com.samsung.da.batteryResolution": "1", + "x.com.samsung.da.charging": "Off" + } + }, + { + "href": "/humidity/vs/0", + "rep": { + "x.com.samsung.da.humidity": "57", + "x.com.samsung.da.validity": "Valid" + } + }, + { + "href": "/information/vs/0", + "rep": { + "x.com.samsung.da.modelNum": "ASM-KR-TP1-22-ACMB1M|10243041|75000000001611C40800020000080000", + "x.com.samsung.da.description": "ASM-KR-TP1-22-ACMB1M", + "x.com.samsung.da.serialNum": "REDACTED", + "x.com.samsung.da.otnDUID": "REDACTED", + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "0", + "x.com.samsung.da.description": "Version", + "x.com.samsung.da.type": "Software", + "x.com.samsung.da.number": "02443A260310", + "x.com.samsung.da.newVersionAvailable": "0" + }, + { + "x.com.samsung.da.id": "1", + "x.com.samsung.da.description": "Version", + "x.com.samsung.da.type": "Firmware", + "x.com.samsung.da.number": "22050300", + "x.com.samsung.da.newVersionAvailable": "0" + } + ], + "x.com.samsung.da.diagProtocolType": "WIFI_HTTPS", + "x.com.samsung.da.diagLogType": [ + "errCode", + "dump" + ], + "x.com.samsung.da.diagDumpType": "file", + "x.com.samsung.da.diagEndPoint": "SSM", + "x.com.samsung.da.diagMnid": "0AJT", + "x.com.samsung.da.diagSetupid": "AM0", + "x.com.samsung.da.diagMinVersion": "1.0" + } + }, + { + "href": "/keepnormalstate/vs/0", + "rep": { + "x.com.samsung.da.keepnormal": 0 + } + }, + { + "href": "/realtimenotiforclient/vs/0", + "rep": { + "x.com.samsung.da.timeforshortnoti": "0", + "x.com.samsung.da.longnotisubscription": "true", + "x.com.samsung.da.periodicnotisubscription": "true" + } + }, + { + "href": "/sensordatasinks/vs/0", + "rep": { + "remove": "" + } + }, + { + "href": "/sensors/vs/0", + "rep": { + "x.com.samsung.da.cleanLevel": "2", + "x.com.samsung.da.refresh": "Off", + "x.com.samsung.da.lastSensingTime": "1785379376", + "x.com.samsung.da.items": [ + { + "x.com.samsung.da.id": "0", + "x.com.samsung.da.description": "Sensor for CleanLevel", + "x.com.samsung.da.type": "CleanLevel", + "x.com.samsung.da.value": [ + "2" + ] + }, + { + "x.com.samsung.da.id": "1", + "x.com.samsung.da.description": "Sensor for Odor", + "x.com.samsung.da.type": "Odor", + "x.com.samsung.da.value": [ + "1" + ] + }, + { + "x.com.samsung.da.id": "2", + "x.com.samsung.da.description": "Sensor for Dust", + "x.com.samsung.da.type": "Dust", + "x.com.samsung.da.value": [ + "31", + "2" + ] + }, + { + "x.com.samsung.da.id": "3", + "x.com.samsung.da.description": "Sensor for FineDust", + "x.com.samsung.da.type": "FineDust", + "x.com.samsung.da.value": [ + "23", + "2" + ] + }, + { + "x.com.samsung.da.id": "4", + "x.com.samsung.da.description": "Sensor for SuperFineDust", + "x.com.samsung.da.type": "SuperFineDust", + "x.com.samsung.da.value": [ + "18", + "2" + ] + }, + { + "x.com.samsung.da.id": "5", + "x.com.samsung.da.description": "Sensor for CO2", + "x.com.samsung.da.type": "CO2", + "x.com.samsung.da.value": [ + "498", + "1" + ] + } + ] + } + } + ] +} diff --git a/tests/fixtures/golden/air_monitor.json b/tests/fixtures/golden/air_monitor.json new file mode 100644 index 0000000..2f1556a --- /dev/null +++ b/tests/fixtures/golden/air_monitor.json @@ -0,0 +1,18 @@ +{ + "state_keys": [ + "air_quality_standard", + "alarm_code", + "battery", + "battery_charging", + "clean_level", + "co2", + "dnd", + "dnd_end", + "dnd_start", + "dust", + "fine_dust", + "humidity", + "odor", + "super_fine_dust" + ] +} diff --git a/tests/test_air_monitor_capabilities.py b/tests/test_air_monitor_capabilities.py new file mode 100644 index 0000000..ec3ea00 --- /dev/null +++ b/tests/test_air_monitor_capabilities.py @@ -0,0 +1,115 @@ +"""Tests for the Samsung Air Monitor Plus family (ASM-KR-TP1-22-ACMB1M, +issue #210) -- a standalone, battery-powered air-quality sensor puck. No +controllable appliance state at all beyond the do-not-disturb window. +""" +from datetime import time as dt_time + +from custom_components.localthings.registry.adapter import flatten +from custom_components.localthings.registry.by_type import for_device_by_oic_type, resolve +from custom_components.localthings.registry.discovery import discover + +from tests.conftest import _load_device + + +def _air_monitor(): + resources = _load_device('air_monitor') + reg = resolve(resources) + return reg, resources + + +def _bound(): + reg, resources = _air_monitor() + return discover(resources, reg.capabilities, reg.pattern_capabilities), resources + + +def _state(): + bound, resources = _bound() + return flatten(bound, resources) + + +def _desc(key): + bound, _ = _bound() + return next(b.desc for b in bound if b.desc.key == key) + + +def test_resolves_to_air_monitor_registry(): + reg, _ = _air_monitor() + assert reg is not None and reg.name == 'air_monitor' + + +def test_resolves_via_oic_type_too(): + """This board's real dump carries /oic/d's device type + ('x.com.st.d.airqualitysensor') as well as the 'ASM' modelNum token -- + both paths must agree.""" + reg = for_device_by_oic_type(('x.com.st.d.airqualitysensor',)) + assert reg is not None and reg.name == 'air_monitor' + + +def test_no_unbound_hrefs(): + reg, resources = _air_monitor() + unbound = [] + discover(resources, reg.capabilities, reg.pattern_capabilities, log=unbound.append) + assert unbound == [] + + +def test_air_quality_sensors_read_from_shared_items_decode(): + """Same /sensors/vs/0 {type, value} shape and common.sensor_item_value + decode air_purifier.AIR_QUALITY already uses -- this board adds CO2, + which neither air_purifier nor range_hood report.""" + state = _state() + assert state['dust'] == 31 + assert state['fine_dust'] == 23 + assert state['super_fine_dust'] == 18 + assert state['odor'] == 1 + assert state['clean_level'] == 2 + assert state['co2'] == 498 + + +def test_second_value_slot_is_not_exposed(): + """Dust/fine_dust/super_fine_dust each carry a second `value[1]` grade + code (e.g. Dust's ['31', '2']) with no supported-values field anywhere + in the dump to explain its scale -- per the 'still never invent... from + nothing' rule, only the raw reading (value[0]) is bound, not a second + 'dust_level' or similar entity for that code.""" + state = _state() + assert 'dust_level' not in state + assert 'dust_grade' not in state + + +def test_humidity_and_battery_present(): + state = _state() + assert state['humidity'] == 57 + assert state['battery'] == 80 + assert state['battery_charging'] is False + + +def test_air_quality_standard_present(): + state = _state() + assert state['air_quality_standard'] == 'AirCleanAssociation' + + +def test_dnd_read_contract(): + state = _state() + assert state['dnd'] is False + assert state['dnd_start'] == dt_time(14, 0) + assert state['dnd_end'] == dt_time(22, 0) + + +def test_dnd_write_contracts_are_flagged_educated_guesses(): + """Issue #210: no idle-vs-active dump pair exists for /dnd/vs/0 (DND + was never toggled in the one dump this board has), so these write + contracts are educated guesses -- symmetric with the read side's own + string/time-format shape, not invented from nothing, but still + unconfirmed on real hardware. Only the wiring is tested here; whether + the device actually accepts these writes needs a reporter to confirm.""" + dnd = _desc('dnd') + path, body = dnd.write_fn('On', {}) + assert path == ['dnd', 'vs', '0'] + assert body == {'x.com.samsung.da.value': 'true'} + path, body = dnd.write_fn('Off', {}) + assert body == {'x.com.samsung.da.value': 'false'} + + start = _desc('dnd_start') + path, body = start.write_fn(dt_time(9, 30), {}) + assert path == ['dnd', 'vs', '0'] + assert body == {'x.com.samsung.da.startTime': '09:30:00'} diff --git a/tests/test_by_type.py b/tests/test_by_type.py index 17f15f2..cdb97d2 100644 --- a/tests/test_by_type.py +++ b/tests/test_by_type.py @@ -151,6 +151,7 @@ class TestForDeviceByOicType: ('oic.d.washer', 'washer'), ('x.com.st.d.stickcleaner', 'vacuum_station'), ('x.com.st.d.steamcloset', 'air_dresser'), + ('x.com.st.d.airqualitysensor', 'air_monitor'), ]) def test_known_oic_types_resolve(self, oic_type, expected): from custom_components.localthings.registry.by_type import for_device_by_oic_type @@ -434,6 +435,19 @@ class TestForDeviceByModel: assert reg is not None assert reg.name == 'vacuum_station' + def test_air_monitor_via_asm_token(self): + """Issue #210: a standalone air-quality sensor puck + (ASM-KR-TP1-22-ACMB1M) reports no oneUiVersion and no + washer/dryer/dishwasher consumer prefix; falls back to the 'ASM' + token in modelNum onto the air_monitor registry.""" + from custom_components.localthings.registry.by_type import for_device_by_model + reg = for_device_by_model( + 'ASM-KR-TP1-22-ACMB1M|10243041|75000000001611C40800020000080000', + 'ASM-KR-TP1-22-ACMB1M', + ) + assert reg is not None + assert reg.name == 'air_monitor' + def test_cooktop_via_legacy_model_description(self): """Older cooktops identify themselves as ARTIK051_GLOBAL_COOKTOP.""" from custom_components.localthings.registry.by_type import for_device_by_model diff --git a/tests/test_golden_regression.py b/tests/test_golden_regression.py index 372aaab..90a1d2e 100644 --- a/tests/test_golden_regression.py +++ b/tests/test_golden_regression.py @@ -871,6 +871,27 @@ def test_registry_reproduces_golden_state_keys_for_air_dresser_tp1_21(): ) +def test_registry_reproduces_golden_state_keys_for_air_monitor(): + """Samsung Air Monitor Plus (ASM-KR-TP1-22-ACMB1M, issue #210) -- a + standalone battery-powered air-quality sensor puck, the first device + this integration has no controllable-appliance concept for at all (no + /power/*, only /energy/battery/vs/0). Routes via both /oic/d + ('x.com.st.d.airqualitysensor') and the 'ASM' modelNum board token. + Reuses air_purifier.AIR_QUALITY's /sensors/vs/0 decode + (common.sensor_item_value) for dust/fine_dust/super_fine_dust/odor/ + clean_level, adding a CO2 reading those families don't report. Binds + cleanly with zero unbound hrefs.""" + from tests.conftest import _load_device + resources = _load_device('air_monitor') + golden = json.loads((GOLDEN / 'air_monitor.json').read_text()) + state_keys = _new_state_keys('air_monitor', resources) + assert set(state_keys) == set(golden['state_keys']), ( + f"state_keys mismatch:\n" + f" extra: {sorted(set(state_keys) - set(golden['state_keys']))}\n" + f" missing: {sorted(set(golden['state_keys']) - set(state_keys))}" + ) + + def test_registry_reproduces_golden_state_keys_for_air_purifier_vtww(): """A-VTWW-TP2-21-COMMON BESPOKE Cube Air (issue #151) -- reports no oneUiVersion; resolved via the '-VTWW-' modelNum token fallback into From b7d3f86ec7f25c2548a058ca3cdecd36e27cb5b8 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Fri, 31 Jul 2026 15:17:18 -0500 Subject: [PATCH 10/10] fix: address code review feedback on issue-triage batch PR (#181, #183, #189, #196, #207, #208, #210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit common: - Make KIDS_LOCK_GENERIC also a read-only BinarySensorDesc (device_class='lock'), flipping value_fn to not bool(v) so /kidslock/0 value=False and /kidslock/vs/0 kidsLock='Ready' render with the same polarity ('On' means open/unlocked per HA's lock device class). The old SwitchDesc form never honored device_class='lock' -- HA's switch platform only accepts 'outlet'/'switch' -- so the surface was a plain switch whose 'On' meaning drifted across boards. Tests updated. air_monitor: - Add state_class='measurement' to dust/fine_dust/super_fine_dust so the readings feed HA long-term statistics (co2 already had it). - Import _AIR_QUALITY_SENSORS from air_purifier instead of duplicating it byte-for-byte; update common.sensor_item_value's docstring to mention the third caller. by_type/__init__: drop trailing whitespace on the new 'ASM' line. translations/en.json + nl.json: move the new 'dnd' switch entry to its correct alphabetical position (after display_light, before fast_preheat). SKILL.md: add an explicit read-side rule to §5's educated-guesses section -- guessed unit/device_class/state_class on a SensorDesc silently mislabels readings forever with no 4.xx to catch it (unlike guessed writes, which the device rejects). The prior air_monitor docstring cited this carve-out as if it existed; now it does. tests/water_purifier (issue #196): change the ailite fixture's favorite.defaultTemperature from '85' to '50' so the test actually reproduces the reported bug -- '50' is in showList only, so a descriptor reading from supportedList would fail the assertion that the current default is in its options list. --- .claude/skills/adding-device-support/SKILL.md | 16 +++++++++ .../localthings/registry/by_type/__init__.py | 2 +- .../registry/capabilities/air_monitor.py | 34 +++++++------------ .../registry/capabilities/common.py | 33 +++++++++++------- .../localthings/translations/en.json | 6 ++-- .../localthings/translations/nl.json | 6 ++-- .../water_purifier_ailite_25k_device.json | 2 +- tests/test_common_capabilities.py | 16 ++++++--- tests/test_water_purifier_capabilities.py | 24 +++++++------ 9 files changed, 83 insertions(+), 56 deletions(-) diff --git a/.claude/skills/adding-device-support/SKILL.md b/.claude/skills/adding-device-support/SKILL.md index ad7fc9e..57078d7 100644 --- a/.claude/skills/adding-device-support/SKILL.md +++ b/.claude/skills/adding-device-support/SKILL.md @@ -297,6 +297,22 @@ to cross-check (temperature scale, minutes vs. seconds) — being syntactically valid but semantically backwards is exactly the case a rejection won't catch. +The same unit caveat applies on the **read** side, but with a sharper +failure mode: a guessed `unit`, `device_class`, or `state_class` on a +`SensorDesc` silently mislabels the entity in HA forever (every reading, +every graph, every long-term statistic), with no 4.xx to catch it. The +write-rejection safety net above doesn't cover reads — the device happily +returns whatever it returns. So the read-side equivalent of "bind a write +to the device's own reported range/supported-list" is: leave `unit`/ +`device_class`/`state_class` unset when the dump gives no field that +nominates one (no `supportedGrades`, no second dump to compare against, +no family member whose same field is already mapped). Match an +already-bound descriptor on a sibling family when the underlying field +and value shape are identical; otherwise expose the reading without an +HA-level interpretation and let a future reporter or dump confirm it. +See `air_monitor.AIR_QUALITY`'s docstring for the worked example +(three dust keys, no `device_class`, no `unit`). + Still never invent an entity or a write from nothing: an opaque encoded blob with no supported-values field, no range, and no idle-vs-active diff to compare against is a gap for a human, not a guess — leave it unbound, or diff --git a/custom_components/localthings/registry/by_type/__init__.py b/custom_components/localthings/registry/by_type/__init__.py index 1055668..8fa9d05 100644 --- a/custom_components/localthings/registry/by_type/__init__.py +++ b/custom_components/localthings/registry/by_type/__init__.py @@ -114,7 +114,7 @@ _BOARD_TOKEN_TO_KEY: dict[str, str] = { 'VSKR': 'vacuum_station', # issue #131 -- stick-vacuum clean station 'DF': 'air_dresser', # issue #162 'VSWW': 'vacuum_station', # issue #219 - 'ASM': 'air_monitor', # issue #210 -- Air Monitor Plus + 'ASM': 'air_monitor', # issue #210 -- Air Monitor Plus } _TOKEN_SPLIT_RE = re.compile(r'[^A-Z0-9]+') diff --git a/custom_components/localthings/registry/capabilities/air_monitor.py b/custom_components/localthings/registry/capabilities/air_monitor.py index fab646a..f5f1ee5 100644 --- a/custom_components/localthings/registry/capabilities/air_monitor.py +++ b/custom_components/localthings/registry/capabilities/air_monitor.py @@ -19,40 +19,32 @@ rule. Same reasoning `air_purifier.AIR_QUALITY` already applies to this shape; index 0 is the only slot any family has ever read. Dust/FineDust/SuperFineDust aren't assigned an HA `device_class` -(pm10/pm25/pm1) despite the values reading like plausible ug/m3 -particulate readings in a physically consistent order (coarser >= finer): -Samsung's own two-tier Korean convention (i.e. "fine dust"/"ultra-fine -dust") maps only to a PM10/PM2.5 pair, and this board's three-tier -naming doesn't confirm where the extra tier or a PM1 reading actually -fits. Guessing wrong here would silently mislabel what unit a user reads -on a graph forever, not just fail one write -- the "worst case is -rejected" case for a flagged write guess doesn't cover a read-side -device_class/unit guess, per the skill's own carve-out. Exposed as plain -measurement sensors named after the device's own field instead (matching -air_purifier.AIR_QUALITY's existing precedent). +(pm10/pm25/pm1) or `unit` despite the values reading like plausible +ug/m3 particulate readings in a physically consistent order (coarser +>= finer): Samsung's own two-tier Korean convention (i.e. "fine dust"/ +"ultra-fine dust") maps only to a PM10/PM2.5 pair, and this board's +three-tier naming doesn't confirm where the extra tier or a PM1 reading +actually fits. The adding-device-support skill's read-side rule says +leave unit/device_class unset when the dump gives no field that +nominates one -- a wrong guess would silently mislabel every reading +forever, and the write-side rejection safety net doesn't cover reads. +Exposed as plain `measurement` sensors named after the device's own +field instead (matching air_purifier.AIR_QUALITY's existing precedent). """ from datetime import time as dt_time from ..capability import Capability from ..entities import BinarySensorDesc, SensorDesc, SwitchDesc, TimeDesc +from .air_purifier import _AIR_QUALITY_SENSORS from .common import int_or_none, sensor_item_value -_AIR_QUALITY_SENSORS = ( - ('dust', 'mdi:blur', 'Dust'), - ('fine_dust', 'mdi:blur', 'FineDust'), - ('super_fine_dust', 'mdi:blur', 'SuperFineDust'), - ('odor', 'mdi:scent', 'Odor'), - ('clean_level', 'mdi:air-filter', 'CleanLevel'), -) SENSORS = Capability( href='/sensors/vs/0', poll_tier='warm', entities=tuple( - # No state_class here, matching air_purifier.AIR_QUALITY's existing - # descriptors exactly -- these keys are shared catalog entries with - # that capability, so the two should behave identically. SensorDesc(key=key, field='x.com.samsung.da.items', icon=icon, + state_class='measurement', value_fn=lambda items, t=sensor_type: sensor_item_value(items, t)) for key, icon, sensor_type in _AIR_QUALITY_SENSORS ) + ( diff --git a/custom_components/localthings/registry/capabilities/common.py b/custom_components/localthings/registry/capabilities/common.py index 945903a..984a6fe 100644 --- a/custom_components/localthings/registry/capabilities/common.py +++ b/custom_components/localthings/registry/capabilities/common.py @@ -228,8 +228,9 @@ def sensor_item_value(items, sensor_type, index=0): """Pull one reading out of a `/sensors/vs/0`-style items[] list -- each item is `{type, value: [...]}`; `index` picks which slot of a possibly multi-value reading to read (index 0 is the raw measurement on every - family seen so far). Shared by range_hood.AIR_QUALITY and - air_purifier.AIR_QUALITY, which read the same resource shape.""" + family seen so far). Shared by range_hood.AIR_QUALITY, + air_purifier.AIR_QUALITY, and air_monitor.SENSORS, which all read the + same resource shape against the same {type, sensor_type} keys.""" for item in items or (): if not isinstance(item, dict): continue @@ -301,11 +302,19 @@ POWER_VS_FALLBACK = Capability( KIDS_LOCK_GENERIC = Capability( href='/kidslock/0', entities=( - SwitchDesc(key='child_lock', field='value', - device_class='lock', - value_fn=lambda v: bool(v), - write_fn=lambda p, rep, href=None: ( - ['kidslock', '0'], {'value': p == 'On'})), + # Read-only like KIDS_LOCK_VS_FALLBACK (issues #181/#183) -- not a + # SwitchDesc. SwitchDesc's `device_class='lock'` was never honored + # by HA (its switch platform only accepts 'outlet'/'switch'), + # leaving a plain switch whose 'On' state meant different things + # on different boards. As a BinarySensorDesc with `device_class='lock'`, + # both kids-lock surfaces read with the same polarity: 'On' means + # open/unlocked, per HA's lock device_class. The inversion in + # value_fn here (and in the fallback below) keeps the on-the-wire + # truth (value=False on /kidslock/0, kidsLock='Ready' on /kidslock/vs/0 + # both mean kids lock NOT active) consistent with that polarity. + BinarySensorDesc(key='child_lock', field='value', + device_class='lock', + value_fn=lambda v: not bool(v)), ), ) @@ -320,12 +329,12 @@ KIDS_LOCK_VS_FALLBACK = Capability( # confirmed this directly: writing the *correct* value ('Run') # still 4.05s, and the SmartThings app itself has no control for # it either -- the resource is genuinely read-only on this - # hardware, not just wrong-valued. binary_sensor's 'lock' device - # class is inverted from the switch reading below it used to be: - # On means open/unlocked, so value_fn flips to `v == 'Ready'`. + # hardware, not just wrong-valued. Polarity matches + # KIDS_LOCK_GENERIC above -- 'On' means open/unlocked, so + # kidsLock='Ready' (kids lock NOT active) renders as 'On'. BinarySensorDesc(key='child_lock', field='x.com.samsung.da.kidsLock', - device_class='lock', - value_fn=lambda v: v == 'Ready'), + device_class='lock', + value_fn=lambda v: v == 'Ready'), ), ) diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index b5b645a..50e4464 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -1008,12 +1008,12 @@ "defrost_delay": { "name": "Defrost delay" }, - "dnd": { - "name": "Do not disturb" - }, "display_light": { "name": "Display light" }, + "dnd": { + "name": "Do not disturb" + }, "fast_preheat": { "name": "Fast preheat" }, diff --git a/custom_components/localthings/translations/nl.json b/custom_components/localthings/translations/nl.json index 8da541c..8aff30a 100644 --- a/custom_components/localthings/translations/nl.json +++ b/custom_components/localthings/translations/nl.json @@ -1008,12 +1008,12 @@ "defrost_delay": { "name": "Ontdooien uitstellen" }, - "dnd": { - "name": "Niet storen" - }, "display_light": { "name": "Displayverlichting" }, + "dnd": { + "name": "Niet storen" + }, "fast_preheat": { "name": "Snel voorverwarmen" }, diff --git a/tests/fixtures/water_purifier_ailite_25k_device.json b/tests/fixtures/water_purifier_ailite_25k_device.json index 9ae21aa..cc97237 100644 --- a/tests/fixtures/water_purifier_ailite_25k_device.json +++ b/tests/fixtures/water_purifier_ailite_25k_device.json @@ -166,7 +166,7 @@ "href": "/favorite/hotwater/vs/0", "rep": { "x.com.samsung.da.favorite.revision": "0", - "x.com.samsung.da.favorite.defaultTemperature": "85", + "x.com.samsung.da.favorite.defaultTemperature": "50", "x.com.samsung.da.favorite.showList": [ "40", "50", diff --git a/tests/test_common_capabilities.py b/tests/test_common_capabilities.py index 7b9741b..2fdfd0d 100644 --- a/tests/test_common_capabilities.py +++ b/tests/test_common_capabilities.py @@ -232,13 +232,19 @@ class TestWmSetinfoFlags: class TestKidsLockFallback: - def test_generic_read_write(self): + def test_generic_is_read_only(self): + """Issues #181/#183: the kids-lock resource is read-only on real + hardware (writing the *correct* value still 4.05s, and no + SwitchDesc device_class='lock' is honored by HA -- the switch + platform only accepts 'outlet'/'switch'). KIDS_LOCK_GENERIC and + KIDS_LOCK_VS_FALLBACK now share the same shape (BinarySensorDesc, + device_class='lock', same key) so both surfaces render with the + same polarity: 'On' means open/unlocked.""" assert common.KIDS_LOCK_GENERIC.href == '/kidslock/0' desc = common.KIDS_LOCK_GENERIC.entities[0] - assert desc.value_fn(True) is True - path, body = desc.write_fn('On', {}) - assert path == ['kidslock', '0'] - assert body == {'value': True} + assert isinstance(desc, BinarySensorDesc) + assert desc.value_fn(False) is True # value=False -> On=Unlocked + assert desc.value_fn(True) is False # value=True -> Off=Locked def test_vs_fallback_gated(self): assert common.KIDS_LOCK_VS_FALLBACK.match_fn({}, {'/kidslock/vs/0': {}}) is True diff --git a/tests/test_water_purifier_capabilities.py b/tests/test_water_purifier_capabilities.py index 5721fd7..2f29b5a 100644 --- a/tests/test_water_purifier_capabilities.py +++ b/tests/test_water_purifier_capabilities.py @@ -370,21 +370,25 @@ def test_ailite_hot_water_temperature_gated_off_without_supported_list(): def test_ailite_favorite_hotwater_temperature_options_include_the_custom_value(): - """This board's real dump (issue #196) is the concrete case: the user - added a custom 50C value via the SmartThings app's "temperatures to - display" editor, so favorite.showList is - ['40', '50', '75', '85', '90'] while favorite.supportedList stays the - fixed ['40', '75', '85', '90'] -- '50' only ever appears in showList. - Whichever of the two the descriptor reads from, a default temperature - of '50' would render as HA's 'unknown' state unless that field's raw - option list actually contains '50'.""" + """Issue #196's concrete failure case: the user added a custom 50C value + via the SmartThings app's "temperatures to display" editor, so the + board's defaultTemperature is now '50'. showList contains '50' + ([40, 50, 75, 85, 90]) but supportedList does NOT (still the fixed + [40, 75, 85, 90]). Reading options_field='x.com.samsung.da.favorite.supportedList' + -- the old behavior -- would register a select whose options list + doesn't contain the current default, so HA would render the entity as + 'unknown'. The descriptor must read from showList so '50' is in + options.""" desc = _desc_ailite('favorite_hotwater_temperature') assert desc.options_field == 'x.com.samsung.da.favorite.showList' _, resources = _water_purifier_ailite() rep = resources['/favorite/hotwater/vs/0'] - assert rep['x.com.samsung.da.favorite.defaultTemperature'] in rep[desc.options_field] + # defaultTemperature='50' must be a member of the field the descriptor + # actually reads -- this is the precise assertion that would fail under + # the old supportedList behavior. + assert rep['x.com.samsung.da.favorite.defaultTemperature'] == '50' + assert '50' in rep[desc.options_field] assert '50' not in rep['x.com.samsung.da.favorite.supportedList'] - assert '50' in rep['x.com.samsung.da.favorite.showList'] def test_ailite_sound_mode_options_come_from_live_supported_modes():