Merge pull request #61 from mbillow/claude/issue-9-optimistic-write-debug-k4fjdb
feat: add options flow to bypass the remote-control write block per device (issue #54)
This commit is contained in:
@@ -12,6 +12,7 @@ from typing import Any
|
||||
|
||||
import voluptuous as vol
|
||||
from homeassistant import config_entries
|
||||
from homeassistant.core import callback
|
||||
from homeassistant.data_entry_flow import FlowResult
|
||||
from homeassistant.helpers.selector import (
|
||||
TextSelector,
|
||||
@@ -24,6 +25,7 @@ from .const import (
|
||||
CONF_HOST, CONF_PORT,
|
||||
CONF_CA_CERT_PEM, CONF_CA_KEY_PEM,
|
||||
CONF_LEAF_CERT_PEM, CONF_LEAF_KEY_PEM,
|
||||
CONF_BYPASS_REMOTE_CONTROL,
|
||||
PROBE_PORT_RANGE, PREFERRED_PROBE_PORTS, LIVENESS_PROBE_TIMEOUT_S,
|
||||
PROBE_GET_TIMEOUT_S,
|
||||
)
|
||||
@@ -300,6 +302,13 @@ class LocalThingsConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||
self._ca_key_pem: str = ""
|
||||
self._pending_info: dict | None = None
|
||||
|
||||
@staticmethod
|
||||
@callback
|
||||
def async_get_options_flow(
|
||||
config_entry: config_entries.ConfigEntry,
|
||||
) -> LocalThingsOptionsFlow:
|
||||
return LocalThingsOptionsFlow()
|
||||
|
||||
def _create_entry(self, info: dict) -> FlowResult:
|
||||
return self.async_create_entry(
|
||||
title=f"Samsung Appliance ({self._host})",
|
||||
@@ -384,3 +393,31 @@ class LocalThingsConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||
"one_ui_version": self._pending_info["one_ui_version"] or "(none reported)",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
class LocalThingsOptionsFlow(config_entries.OptionsFlow):
|
||||
"""Per-device override of the remote-control-off write block (issue
|
||||
#54). The block exists because most devices reject writes outright
|
||||
while remote control is off and a clear error beats a silent
|
||||
device-side rejection -- but not every model actually enforces that,
|
||||
so this lets a user who's confirmed their device accepts writes anyway
|
||||
turn the block off for just that device rather than it being
|
||||
hardcoded on for everyone."""
|
||||
|
||||
async def async_step_init(
|
||||
self, user_input: dict[str, Any] | None = None
|
||||
) -> FlowResult:
|
||||
if user_input is not None:
|
||||
return self.async_create_entry(data=user_input)
|
||||
|
||||
return self.async_show_form(
|
||||
step_id="init",
|
||||
data_schema=vol.Schema({
|
||||
vol.Required(
|
||||
CONF_BYPASS_REMOTE_CONTROL,
|
||||
default=self.config_entry.options.get(
|
||||
CONF_BYPASS_REMOTE_CONTROL, False
|
||||
),
|
||||
): bool,
|
||||
}),
|
||||
)
|
||||
|
||||
@@ -12,6 +12,16 @@ CONF_CA_KEY_PEM = "ca_key_pem"
|
||||
CONF_LEAF_CERT_PEM = "leaf_cert_pem"
|
||||
CONF_LEAF_KEY_PEM = "leaf_key_pem"
|
||||
|
||||
# Options-flow key (entry.options, not entry.data): lets a user override the
|
||||
# device-wide remote-control-off write block for a specific device (issue
|
||||
# #54). Some devices report remote control off yet still accept certain
|
||||
# writes (e.g. default detergent/softener dosing on a washer, applied even
|
||||
# to the built-in programs) -- the block exists to give a clear error
|
||||
# instead of a silent device-side rejection, but that assumption doesn't
|
||||
# hold for every model. Defaults to False (block stays on) everywhere it's
|
||||
# read, so devices this doesn't apply to see no behavior change.
|
||||
CONF_BYPASS_REMOTE_CONTROL = "bypass_remote_control_lock"
|
||||
|
||||
# The DTLS/CoAP local API binds somewhere in this ephemeral range; which port
|
||||
# depends on firmware. Newer builds answer on 49154/49155, but older ones have
|
||||
# been seen as low as 49153, so we sweep the whole range for a live UDP port
|
||||
|
||||
@@ -31,7 +31,7 @@ from .observe import ObserveManager, MODE_OBSERVE, MODE_POLL, GRACE_PERIOD_S
|
||||
|
||||
from .const import (
|
||||
DOMAIN, CONF_HOST, CONF_PORT, CONF_LEAF_CERT_PEM, CONF_LEAF_KEY_PEM,
|
||||
DEVICE_SUPPORT_ISSUE_URL, SUMMARY_INTERVAL_S,
|
||||
CONF_BYPASS_REMOTE_CONTROL, DEVICE_SUPPORT_ISSUE_URL, SUMMARY_INTERVAL_S,
|
||||
)
|
||||
|
||||
_LOGGER = logging.getLogger(__name__)
|
||||
@@ -534,7 +534,11 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
user-facing message -- as opposed to write_fn's silent no-op below
|
||||
-- is available to every platform for free. The remote-control
|
||||
check runs first and applies to every platform unconditionally,
|
||||
ahead of any description-specific validate_fn."""
|
||||
ahead of any description-specific validate_fn -- unless the user has
|
||||
opted this device out of it via CONF_BYPASS_REMOTE_CONTROL (issue
|
||||
#54: some devices accept certain writes, e.g. a washer's default
|
||||
dosing levels, even while reporting remote control off, so the
|
||||
block's assumption doesn't hold for every model)."""
|
||||
desc = bound_entity.desc
|
||||
write_fn = getattr(desc, 'write_fn', None)
|
||||
if write_fn is None:
|
||||
@@ -542,7 +546,8 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
href = bound_entity.href
|
||||
rep = self._cache.get(href or '') or {}
|
||||
resources = self._cache.snapshot()
|
||||
if not remote_control_enabled(resources):
|
||||
bypass_remote_control = self._entry.options.get(CONF_BYPASS_REMOTE_CONTROL, False)
|
||||
if not bypass_remote_control and not remote_control_enabled(resources):
|
||||
raise ServiceValidationError(_REMOTE_CONTROL_DISABLED_MESSAGE)
|
||||
validate_fn = getattr(desc, 'validate_fn', None)
|
||||
if validate_fn is not None:
|
||||
|
||||
@@ -246,6 +246,17 @@
|
||||
"already_configured": "This device is already configured."
|
||||
}
|
||||
},
|
||||
"options": {
|
||||
"step": {
|
||||
"init": {
|
||||
"title": "Local Things Options",
|
||||
"description": "Some devices accept certain writes (e.g. default detergent/softener dosing on a washer) even while reporting remote control off. By default, LocalThings blocks every write with a clear error whenever a device reports remote control off, rather than letting the device silently reject it. Only enable this if you've confirmed writes actually work on this device with remote control off -- otherwise you'll trade that clear error for a silent failure.",
|
||||
"data": {
|
||||
"bypass_remote_control_lock": "Allow writes even when remote control is reported off"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"issues": {
|
||||
"device_gap": {
|
||||
"title": "Incomplete capability coverage for {device_name}",
|
||||
|
||||
@@ -246,6 +246,17 @@
|
||||
"already_configured": "This device is already configured."
|
||||
}
|
||||
},
|
||||
"options": {
|
||||
"step": {
|
||||
"init": {
|
||||
"title": "Local Things Options",
|
||||
"description": "Some devices accept certain writes (e.g. default detergent/softener dosing on a washer) even while reporting remote control off. By default, LocalThings blocks every write with a clear error whenever a device reports remote control off, rather than letting the device silently reject it. Only enable this if you've confirmed writes actually work on this device with remote control off -- otherwise you'll trade that clear error for a silent failure.",
|
||||
"data": {
|
||||
"bypass_remote_control_lock": "Allow writes even when remote control is reported off"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"issues": {
|
||||
"device_gap": {
|
||||
"title": "Incomplete capability coverage for {device_name}",
|
||||
|
||||
@@ -9,7 +9,8 @@ from homeassistant.data_entry_flow import FlowResultType
|
||||
from pytest_homeassistant_custom_component.common import MockConfigEntry
|
||||
|
||||
from custom_components.localthings.const import (
|
||||
CONF_CA_CERT_PEM, CONF_CA_KEY_PEM, CONF_HOST, CONF_PORT, DOMAIN,
|
||||
CONF_BYPASS_REMOTE_CONTROL, CONF_CA_CERT_PEM, CONF_CA_KEY_PEM,
|
||||
CONF_HOST, CONF_PORT, DOMAIN,
|
||||
)
|
||||
|
||||
from .conftest import (
|
||||
@@ -276,3 +277,45 @@ def test_probe_marks_washer_as_recognized(monkeypatch):
|
||||
) is not None
|
||||
)
|
||||
assert recognized is True
|
||||
|
||||
|
||||
async def test_options_flow_default_is_off(hass: HomeAssistant) -> None:
|
||||
"""The bypass defaults to False, so devices that never touch this
|
||||
option see no change in the remote-control write block."""
|
||||
entry = MockConfigEntry(domain=DOMAIN, data=ENTRY_DATA, unique_id=f'localthings_{MOCK_SERIAL}')
|
||||
entry.add_to_hass(hass)
|
||||
|
||||
result = await hass.config_entries.options.async_init(entry.entry_id)
|
||||
|
||||
assert result['type'] == FlowResultType.FORM
|
||||
assert result['step_id'] == 'init'
|
||||
assert result['data_schema']({})[CONF_BYPASS_REMOTE_CONTROL] is False
|
||||
|
||||
|
||||
async def test_options_flow_can_enable_bypass(hass: HomeAssistant) -> None:
|
||||
"""Submitting the form with the toggle on stores it in entry.options,
|
||||
where coordinator.async_send_command reads it (issue #54)."""
|
||||
entry = MockConfigEntry(domain=DOMAIN, data=ENTRY_DATA, unique_id=f'localthings_{MOCK_SERIAL}')
|
||||
entry.add_to_hass(hass)
|
||||
|
||||
result = await hass.config_entries.options.async_init(entry.entry_id)
|
||||
result = await hass.config_entries.options.async_configure(
|
||||
result['flow_id'], user_input={CONF_BYPASS_REMOTE_CONTROL: True}
|
||||
)
|
||||
|
||||
assert result['type'] == FlowResultType.CREATE_ENTRY
|
||||
assert entry.options[CONF_BYPASS_REMOTE_CONTROL] is True
|
||||
|
||||
|
||||
async def test_options_flow_reflects_previously_saved_value(hass: HomeAssistant) -> None:
|
||||
"""Reopening the form shows the currently-saved choice as the default,
|
||||
not always False."""
|
||||
entry = MockConfigEntry(
|
||||
domain=DOMAIN, data=ENTRY_DATA, unique_id=f'localthings_{MOCK_SERIAL}',
|
||||
options={CONF_BYPASS_REMOTE_CONTROL: True},
|
||||
)
|
||||
entry.add_to_hass(hass)
|
||||
|
||||
result = await hass.config_entries.options.async_init(entry.entry_id)
|
||||
|
||||
assert result['data_schema']({})[CONF_BYPASS_REMOTE_CONTROL] is True
|
||||
|
||||
@@ -11,7 +11,7 @@ from homeassistant.exceptions import ServiceValidationError
|
||||
from homeassistant.helpers import issue_registry as ir
|
||||
|
||||
from custom_components.localthings.const import (
|
||||
CONF_HOST, DOMAIN, SUMMARY_INTERVAL_S,
|
||||
CONF_BYPASS_REMOTE_CONTROL, CONF_HOST, DOMAIN, SUMMARY_INTERVAL_S,
|
||||
)
|
||||
from custom_components.localthings.coordinator import LocalThingsCoordinator
|
||||
from custom_components.localthings.registry.capabilities.common import (
|
||||
@@ -979,3 +979,44 @@ async def test_send_command_remote_control_check_precedes_validate_fn(
|
||||
|
||||
with pytest.raises(ServiceValidationError, match="Remote control is turned off"):
|
||||
await coordinator.async_send_command(bound, 'On')
|
||||
|
||||
|
||||
async def test_send_command_bypasses_remote_control_when_option_enabled(
|
||||
hass: HomeAssistant, mock_coordinator_observe_session
|
||||
) -> None:
|
||||
"""Issue #54: some devices accept certain writes even while reporting
|
||||
remote control off, so a user who's confirmed that for their device can
|
||||
opt it out of the block entirely via the options flow
|
||||
(CONF_BYPASS_REMOTE_CONTROL, entry.options -- not entry.data)."""
|
||||
from pytest_homeassistant_custom_component.common import MockConfigEntry
|
||||
|
||||
from custom_components.localthings.registry.discovery import BoundEntity
|
||||
from custom_components.localthings.registry.entities import NumberDesc
|
||||
|
||||
entry = MockConfigEntry(
|
||||
domain=DOMAIN, data=ENTRY_DATA, unique_id=f'localthings_{MOCK_SERIAL}',
|
||||
options={CONF_BYPASS_REMOTE_CONTROL: True},
|
||||
)
|
||||
entry.add_to_hass(hass)
|
||||
|
||||
fake = mock_coordinator_observe_session
|
||||
await hass.config_entries.async_setup(entry.entry_id)
|
||||
await hass.async_block_till_done()
|
||||
coordinator: LocalThingsCoordinator = hass.data[DOMAIN][entry.entry_id]
|
||||
coordinator._cache.apply_rep(
|
||||
'/remotectrl/vs/0',
|
||||
{'x.com.samsung.da.remoteControlEnabled': 'false'},
|
||||
source='test',
|
||||
)
|
||||
|
||||
def _write_fn(payload, rep, href):
|
||||
return (['some', 'path'], {'value': payload})
|
||||
|
||||
desc = NumberDesc(key='test', field='value', write_fn=_write_fn)
|
||||
bound = BoundEntity(href='/test/vs/0', capability=coordinator.bound[0].capability, desc=desc)
|
||||
|
||||
with patch.object(fake, 'subscribe'):
|
||||
fake.post = lambda *a, **k: (0x44, b'')
|
||||
await coordinator.async_send_command(bound, 5)
|
||||
|
||||
assert coordinator._cache.get('/some/path') == {'value': 5}
|
||||
|
||||
Reference in New Issue
Block a user