refactor: adopt cleaner write pattern for options arrays

tests/test_air_purifier_capabilities.py

@@ -73,9 +73,10 @@ def test_diagnosis_reuses_dishwasher_capability():
 
 
 def test_light_switch_write_contract():
-    """The display-light switch RMW-replaces only the 'Light_*' entry in the
-    packed /mode/vs/0 options list (via laundry.replace_in_options), leaving
-    the other flags and the list order untouched."""
+    """The display-light switch writes only the changed 'Light_*' token
+    (via laundry.option_write) -- confirmed on real hardware (issue #54)
+    that the device merges by prefix itself, so no read-modify-write of the
+    whole packed /mode/vs/0 options list is needed."""
     desc = next(e for e in air_purifier.MODE.entities if e.key == 'display_light')
     rep = {'x.com.samsung.da.options': [
         'Comode_Off', 'Blooming_0', 'Light_On', 'OptionCode_60282',
@@ -83,9 +84,7 @@ def test_light_switch_write_contract():
     assert desc.rep_fn(rep) is True
     assert desc.write_fn('Off', rep) == (
         ['mode', 'vs', '0'],
-        {'x.com.samsung.da.options': [
-            'Comode_Off', 'Blooming_0', 'Light_Off', 'OptionCode_60282',
-        ]},
+        {'x.com.samsung.da.options': ['Light_Off']},
     )
 
 

tests/test_common_capabilities.py

@@ -26,6 +26,37 @@ def test_common_caps_discover_on_dishwasher(dishwasher_resources):
     assert 'power_switch' in keys
 
 
+class TestMergeOptionsField:
+    """merge_options_field() is the read side of issue #54's finding: a
+    write only needs to carry the changed token, so the coordinator uses
+    this to keep its optimistic cache entry complete (every sibling option
+    still present) without waiting on a real poll."""
+
+    def test_replaces_matching_prefix(self):
+        cached = ['DeviceType_0167', 'Course_16', 'GMT_04']
+        assert common.merge_options_field(cached, ['Course_1D']) == [
+            'DeviceType_0167', 'Course_1D', 'GMT_04',
+        ]
+
+    def test_appends_when_prefix_absent(self):
+        cached = ['DeviceType_0167']
+        assert common.merge_options_field(cached, ['NaturalSteam_On']) == [
+            'DeviceType_0167', 'NaturalSteam_On',
+        ]
+
+    def test_merges_multiple_tokens_independently(self):
+        cached = ['DetergentLevelCtrl_1', 'SoftenerLevelCtrl_0', 'GMT_04']
+        merged = common.merge_options_field(cached, ['SoftenerLevelCtrl_2'])
+        assert merged == ['DetergentLevelCtrl_1', 'SoftenerLevelCtrl_2', 'GMT_04']
+
+    def test_handles_missing_cache(self):
+        assert common.merge_options_field(None, ['Course_1D']) == ['Course_1D']
+
+    def test_ignores_malformed_tokens(self):
+        cached = ['Course_16']
+        assert common.merge_options_field(cached, ['nounderscore']) == ['Course_16']
+
+
 # ---------------------------------------------------------------------------
 # OCF-native / vendor '-vs' fallback pairs (power, kids-lock, remote control).
 # ---------------------------------------------------------------------------

tests/test_coordinator_send_command.py

@@ -0,0 +1,94 @@
+"""Tests for LocalThingsCoordinator.async_send_command's optimistic-apply
+step, specifically for x.com.samsung.da.options[] writes (issue #54).
+
+The write itself only needs to carry the single changed token -- confirmed
+on real hardware, the device merges by prefix and evicts the stale token
+itself. But observe.ObserveManager.apply() does a shallow {**cached, **rep}
+field merge, so handing it that same minimal single-token body would
+overwrite the *whole* cached options[] field, wiping every sibling option
+(other courses/levels/toggles packed into the same array) until the next
+real poll lands. async_send_command must pre-merge the token into the
+cached array itself before applying it optimistically, while still POSTing
+only the minimal body over the wire.
+"""
+from __future__ import annotations
+
+from unittest.mock import AsyncMock
+
+import cbor2
+import pytest
+from homeassistant.core import HomeAssistant
+from pytest_homeassistant_custom_component.common import MockConfigEntry
+
+from custom_components.localthings.const import (
+    CONF_HOST, CONF_LEAF_CERT_PEM, CONF_LEAF_KEY_PEM, CONF_PORT, DOMAIN,
+)
+from custom_components.localthings.coordinator import LocalThingsCoordinator
+from custom_components.localthings.registry.capabilities import laundry
+from custom_components.localthings.registry.discovery import BoundEntity
+
+ENTRY_DATA = {
+    CONF_HOST: '10.0.0.199',
+    CONF_PORT: 49154,
+    CONF_LEAF_CERT_PEM: '-----BEGIN CERTIFICATE-----\nTEST-LEAF\n-----END CERTIFICATE-----',
+    CONF_LEAF_KEY_PEM: '-----BEGIN PRIVATE KEY-----\nTEST-LEAF-KEY\n-----END PRIVATE KEY-----',
+}
+
+
+class _FakeSendSession:
+    def __init__(self):
+        self.post_calls: list[tuple[list[str], bytes]] = []
+
+    def post(self, path_segs, payload, timeout=None):
+        self.post_calls.append((list(path_segs), payload))
+        return 0x44, b''
+
+    def pace(self):
+        pass
+
+
+@pytest.fixture
+def coordinator(hass: HomeAssistant) -> LocalThingsCoordinator:
+    entry = MockConfigEntry(
+        domain=DOMAIN, data=ENTRY_DATA, unique_id='localthings_SENDCMD-TEST',
+    )
+    entry.add_to_hass(hass)
+    coord = LocalThingsCoordinator(hass, entry)
+    coord.async_request_refresh = AsyncMock()
+    coord._session = _FakeSendSession()
+    return coord
+
+
+async def test_options_write_posts_only_the_changed_token(coordinator) -> None:
+    href = '/course/vs/0'
+    coordinator._observe.apply(href, {
+        'x.com.samsung.da.options': ['DeviceType_0167', 'Course_16', 'GMT_04'],
+    }, source='poll')
+
+    desc = laundry.cycle_select(translation_key='dryer_cycle', icon='x')
+    bound = BoundEntity(href=href, capability=None, desc=desc)
+
+    await coordinator.async_send_command(bound, '1D')
+
+    posted_path, posted_bytes = coordinator._session.post_calls[0]
+    assert posted_path == ['course', 'vs', '0']
+    assert cbor2.loads(posted_bytes) == {'x.com.samsung.da.options': ['Course_1D']}
+
+
+async def test_options_write_optimistic_cache_keeps_sibling_tokens(coordinator) -> None:
+    """The regression this guards: applying the minimal wire body straight
+    to the cache would wipe DeviceType_0167/GMT_04 until the next poll."""
+    href = '/course/vs/0'
+    coordinator._observe.apply(href, {
+        'x.com.samsung.da.options': ['DeviceType_0167', 'Course_16', 'GMT_04'],
+    }, source='poll')
+
+    desc = laundry.cycle_select(translation_key='dryer_cycle', icon='x')
+    bound = BoundEntity(href=href, capability=None, desc=desc)
+
+    await coordinator.async_send_command(bound, '1D')
+
+    cached = coordinator._cache.get(href)
+    assert cached['x.com.samsung.da.options'] == [
+        'DeviceType_0167', 'Course_1D', 'GMT_04',
+    ]

tests/test_dishwasher_capabilities.py

@@ -28,9 +28,7 @@ class TestCycleOptions:
         rep = {'x.com.samsung.da.options': ['DeviceType_0001', 'Course_0E', 'GMT_04']}
         path, body = desc.write_fn('90', rep)
         assert path == ['course', 'vs', '0']
-        assert body == {
-            'x.com.samsung.da.options': ['DeviceType_0001', 'Course_90', 'GMT_04'],
-        }
+        assert body == {'x.com.samsung.da.options': ['Course_90']}
 
 
 class TestDishwasherOptions:

tests/test_laundry_capabilities.py

@@ -153,14 +153,15 @@ class TestCycleSelect:
         live = {'/wm/editcourse/vs/0': {'x.com.samsung.da.editCourseList': 'EditCourseList_16'}}
         assert desc.exists_fn({}, live) is True
 
-    def test_cycle_write_rmw_on_options(self):
+    def test_cycle_write_is_single_token(self):
+        """Confirmed on real hardware (issue #54): the device merges a
+        single-token options[] write by prefix itself, so the write only
+        needs to carry the changed token, not the whole rewritten array."""
         desc = laundry.cycle_select(translation_key='dryer_cycle', icon='x')
         rep = {'x.com.samsung.da.options': ['DeviceType_0167', 'Course_16', 'GMT_04']}
         path, body = desc.write_fn('1D', rep)
         assert path == ['course', 'vs', '0']
-        assert body == {
-            'x.com.samsung.da.options': ['DeviceType_0167', 'Course_1D', 'GMT_04'],
-        }
+        assert body == {'x.com.samsung.da.options': ['Course_1D']}
 
     def test_cycle_write_noop_without_options(self):
         desc = laundry.cycle_select(translation_key='dryer_cycle', icon='x')

tests/test_oven_capabilities.py

@@ -137,7 +137,10 @@ def test_oven_mode_rejects_unknown():
 
 
 # ---------------------------------------------------------------------------
-# OVEN_MODE options-array RMW (lamp, sound, fast_preheat, natural_steam)
+# OVEN_MODE options-array writes (lamp, sound, fast_preheat, natural_steam).
+# Confirmed on real hardware (issue #54): a write only needs to carry the
+# changed token -- the device matches by prefix, evicts the stale token, and
+# merges the result into the array itself. No read-modify-write needed.
 # ---------------------------------------------------------------------------
 
 def _mode_rep(*extra_opts):
@@ -146,12 +149,11 @@ def _mode_rep(*extra_opts):
     ]}
 
 
-def test_lamp_write_replaces_slot():
+def test_lamp_write_is_single_token():
     desc = next(e for e in oven.OVEN_MODE.entities if e.key == 'lamp')
     path, body = desc.write_fn('On', _mode_rep())
-    opts = body['x.com.samsung.da.options']
-    assert 'UpperLamp_On' in opts
-    assert 'UpperLamp_Off' not in opts
+    assert path == ['mode', 'vs', '0']
+    assert body == {'x.com.samsung.da.options': ['UpperLamp_On']}
 
 
 def test_lamp_write_requires_existing_options():
@@ -159,20 +161,19 @@ def test_lamp_write_requires_existing_options():
     assert desc.write_fn('On', {}) is None
 
 
-def test_sound_write_preserves_other_options():
+def test_sound_write_is_single_token():
     desc = next(e for e in oven.OVEN_MODE.entities if e.key == 'sound')
     path, body = desc.write_fn('Off', _mode_rep())
-    opts = body['x.com.samsung.da.options']
-    assert 'Sound_Off' in opts
-    assert 'UpperLamp_Off' in opts     # other slot unchanged
+    assert body == {'x.com.samsung.da.options': ['Sound_Off']}
 
 
-def test_natural_steam_appended_if_absent():
-    """NaturalSteam slot is absent until first write — write_fn must append it."""
+def test_natural_steam_write_is_single_token():
+    """NaturalSteam's slot may be absent from the live rep until first
+    write -- the single-token write covers both the insert and replace case
+    identically, since the device merges by prefix either way."""
     desc = next(e for e in oven.OVEN_MODE.entities if e.key == 'natural_steam')
     path, body = desc.write_fn('On', _mode_rep())   # no NaturalSteam_* in rep
-    opts = body['x.com.samsung.da.options']
-    assert any(o.startswith('NaturalSteam_') for o in opts)
+    assert body == {'x.com.samsung.da.options': ['NaturalSteam_On']}
 
 
 # ---------------------------------------------------------------------------

tests/test_washer_capabilities.py

@@ -102,13 +102,14 @@ class TestWasherCourse:
         assert desc.exists_fn({}, live) is True
 
     def test_cycle_write(self):
+        """Confirmed on real hardware (issue #54): the write only needs to
+        carry the changed token -- the device matches by prefix, evicts the
+        stale token, and merges the result into the array itself."""
         desc = next(e for e in washer.WASHER_COURSE.entities if e.key == 'cycle')
         rep = {'x.com.samsung.da.options': ['DeviceType_0167', 'Course_1C', 'GMT_04']}
         path, body = desc.write_fn('1D', rep)
         assert path == ['course', 'vs', '0']
-        assert body == {
-            'x.com.samsung.da.options': ['DeviceType_0167', 'Course_1D', 'GMT_04'],
-        }
+        assert body == {'x.com.samsung.da.options': ['Course_1D']}
 
 
 class TestDrumClean:
@@ -230,20 +231,22 @@ class TestDetergentSoftenerDosing:
 
     def test_quantity_write(self):
         """The UI selects a padded supported code ('01'); the write posts the
-        un-padded device code ('1'), mirroring how the device reports it."""
+        un-padded device code ('1'), mirroring how the device reports it.
+
+        Confirmed on real hardware (issue #54): the write only needs to carry
+        the changed token -- the device matches by prefix, evicts the stale
+        token, and merges the result into the array itself. No need to read
+        the current array back and rewrite it whole."""
         rep = {'x.com.samsung.da.options': list(_DOSING_OPTIONS)}
         path, body = self._desc('detergent_quantity').write_fn('01', rep)
         assert path == ['course', 'vs', '0']
-        assert 'DetergentLevelCtrl_1' in body['x.com.samsung.da.options']
-        assert 'DetergentLevelCtrl_3' not in body['x.com.samsung.da.options']
-        # untouched siblings survive the read-modify-write
-        assert 'SoftenerLevelCtrl_3' in body['x.com.samsung.da.options']
+        assert body == {'x.com.samsung.da.options': ['DetergentLevelCtrl_1']}
 
     def test_hardness_write(self):
         rep = {'x.com.samsung.da.options': list(_DOSING_OPTIONS)}
         path, body = self._desc('softener_concentration').write_fn('03', rep)
         assert path == ['course', 'vs', '0']
-        assert 'SoftenerLevel2Ctrl_3' in body['x.com.samsung.da.options']
+        assert body == {'x.com.samsung.da.options': ['SoftenerLevel2Ctrl_3']}
 
     def test_low_reservoir_off_when_alarm_off(self):
         rep = {'x.com.samsung.da.options': _DOSING_OPTIONS}
@@ -305,18 +308,20 @@ class TestWashOptionToggles:
         """write_fn receives the same 'On'/'Off' string switch.py sends
         (not a bool) -- covers a bug where an earlier `'On' if p else 'Off'`
         implementation always wrote 'On', since any non-empty string
-        (including 'Off') is truthy."""
+        (including 'Off') is truthy.
+
+        The write body carries only the changed token (issue #54: confirmed
+        the device merges by prefix itself), not the whole options array."""
         for key in self._keys():
             prefix = self._prefix(key)
             rep = {'x.com.samsung.da.options': [f'{prefix}_Off', 'GMT_02']}
             path, body = self._desc(key).write_fn('On', rep)
             assert path == ['course', 'vs', '0']
-            assert f'{prefix}_On' in body['x.com.samsung.da.options']
-            assert 'GMT_02' in body['x.com.samsung.da.options']
+            assert body == {'x.com.samsung.da.options': [f'{prefix}_On']}
 
             rep = {'x.com.samsung.da.options': [f'{prefix}_On']}
             path, body = self._desc(key).write_fn('Off', rep)
-            assert f'{prefix}_Off' in body['x.com.samsung.da.options']
+            assert body == {'x.com.samsung.da.options': [f'{prefix}_Off']}
             assert f'{prefix}_On' not in body['x.com.samsung.da.options']
 
     def test_write_rejects_non_on_off_payload(self):