From 771d9e5debc68ecf0f59d3c0d8e0567cdf62d369 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Sat, 2 Oct 2021 19:50:16 -0700 Subject: [PATCH 1/7] Add wemo options enable_subscription & enable_long_press --- homeassistant/components/wemo/config_flow.py | 44 +++++++- homeassistant/components/wemo/strings.json | 10 ++ homeassistant/components/wemo/wemo_device.py | 103 ++++++++++++++++--- tests/components/wemo/test_wemo_device.py | 24 +++++ 4 files changed, 164 insertions(+), 17 deletions(-) diff --git a/homeassistant/components/wemo/config_flow.py b/homeassistant/components/wemo/config_flow.py index 73f4303cfd6751..f523f023c818ab 100644 --- a/homeassistant/components/wemo/config_flow.py +++ b/homeassistant/components/wemo/config_flow.py @@ -1,11 +1,18 @@ """Config flow for Wemo.""" +from __future__ import annotations + +from typing import Any + import pywemo -from homeassistant.core import HomeAssistant -from homeassistant.helpers import config_entry_flow +from homeassistant.config_entries import ConfigEntry, OptionsFlow +from homeassistant.core import HomeAssistant, callback +from homeassistant.data_entry_flow import FlowResult +from homeassistant.helpers.config_entry_flow import DiscoveryFlowHandler from .const import DOMAIN +from .wemo_device import Options async def _async_has_devices(hass: HomeAssistant) -> bool: @@ -13,4 +20,35 @@ async def _async_has_devices(hass: HomeAssistant) -> bool: return bool(await hass.async_add_executor_job(pywemo.discover_devices)) -config_entry_flow.register_discovery_flow(DOMAIN, "Wemo", _async_has_devices) +class WemoFlow(DiscoveryFlowHandler, domain=DOMAIN): + """Discovery flow with options for Wemo.""" + + def __init__(self) -> None: + """Init discovery flow.""" + super().__init__(DOMAIN, "Wemo", _async_has_devices) + + @staticmethod + @callback + def async_get_options_flow(config_entry: ConfigEntry) -> OptionsFlow: + """Get the options flow for this handler.""" + return WemoOptionsFlow(config_entry) + + +class WemoOptionsFlow(OptionsFlow): + """Options flow for the WeMo component.""" + + def __init__(self, config_entry: ConfigEntry) -> None: + """Initialize options flow.""" + self.config_entry = config_entry + + async def async_step_init( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Manage options for the WeMo component.""" + if user_input is not None: + return self.async_create_entry(title="", data=user_input) + + return self.async_show_form( + step_id="init", + data_schema=Options(**self.config_entry.options).schema(), + ) diff --git a/homeassistant/components/wemo/strings.json b/homeassistant/components/wemo/strings.json index 3419b2cb3d1926..8150b4d55989b4 100644 --- a/homeassistant/components/wemo/strings.json +++ b/homeassistant/components/wemo/strings.json @@ -10,6 +10,16 @@ "no_devices_found": "[%key:common::config_flow::abort::no_devices_found%]" } }, + "options": { + "step": { + "init": { + "data": { + "enable_subscription": "Subscribe to device local push updates", + "enable_long_press": "Register for device long-press events" + } + } + } + }, "device_automation": { "trigger_type": { "long_press": "Wemo button was pressed for 2 seconds" diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index 56ed9b8e1e6773..fa4b0b6abf3fd6 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -1,11 +1,16 @@ """Home Assistant wrapper for a pyWeMo device.""" +from __future__ import annotations + import asyncio +from dataclasses import dataclass, fields from datetime import timedelta import logging +import typing from pywemo import Insight, LongPressMixin, WeMoDevice from pywemo.exceptions import ActionException from pywemo.subscribe import EVENT_TYPE_LONG_PRESS +import voluptuous as vol from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( @@ -30,9 +35,43 @@ _LOGGER = logging.getLogger(__name__) +@dataclass +class Options: + """Configuration options for the DeviceCoordinator class.""" + + # Subscribe to device local push updates. + enable_subscription: bool = True + + # Register for device long-press events. + enable_long_press: bool = True + + def __post_init__(self) -> None: + """Validate parameters.""" + if self.enable_subscription is False: + # Long press support requires subscriptions. + self.enable_long_press = False + + def schema(self) -> vol.Schema: + """Return the Voluptuous schema for the Options instance. + + All values are optional. The default value is set to the current value and + the type hint is set to the value of the field type annotation. + """ + return vol.Schema( + { + vol.Optional( + field.name, default=getattr(self, field.name) + ): typing.get_type_hints(self)[field.name] + for field in fields(self) + } + ) + + class DeviceCoordinator(DataUpdateCoordinator[None]): """Home Assistant wrapper for a pyWeMo device.""" + options: Options | None = None + def __init__(self, hass: HomeAssistant, wemo: WeMoDevice, device_id: str) -> None: """Initialize DeviceCoordinator.""" super().__init__( @@ -45,7 +84,7 @@ def __init__(self, hass: HomeAssistant, wemo: WeMoDevice, device_id: str) -> Non self.wemo = wemo self.device_id = device_id self.device_info = _create_device_info(wemo) - self.supports_long_press = wemo.supports_long_press() + self.supports_long_press = isinstance(wemo, LongPressMixin) self.update_lock = asyncio.Lock() def subscription_callback( @@ -68,6 +107,51 @@ def subscription_callback( updated = self.wemo.subscription_update(event_type, params) self.hass.create_task(self._async_subscription_callback(updated)) + async def _async_set_enable_subscription(self, enable_subscription: bool) -> None: + """Turn on/off push updates from the device.""" + registry = self.hass.data[DOMAIN]["registry"] + if enable_subscription: + registry.on(self.wemo, None, self.subscription_callback) + await self.hass.async_add_executor_job(registry.register, self.wemo) + elif self.options is not None: + await self.hass.async_add_executor_job(registry.unregister, self.wemo) + + async def _async_set_enable_long_press(self, enable_long_press: bool) -> None: + """Turn on/off long-press events from the device.""" + if not (isinstance(self.wemo, LongPressMixin) and self.supports_long_press): + return + try: + if enable_long_press: + await self.hass.async_add_executor_job( + self.wemo.ensure_long_press_virtual_device + ) + elif self.options is not None: + await self.hass.async_add_executor_job( + self.wemo.remove_long_press_virtual_device + ) + # Temporarily handling all exceptions for #52996 & pywemo/pywemo/issues/276 + # Replace this with `except: PyWeMoException` after upstream has been fixed. + except Exception: # pylint: disable=broad-except + _LOGGER.exception( + "Failed to enable long press support for device: %s", self.wemo.name + ) + self.supports_long_press = False + + async def async_set_options( + self, hass: HomeAssistant, config_entry: ConfigEntry + ) -> None: + """Update the configuration options for the device.""" + options = Options(**config_entry.options) + _LOGGER.debug( + "async_set_options old(%s) new(%s)", repr(self.options), repr(options) + ) + for field in fields(options): + new_value = getattr(options, field.name) + if self.options is None or getattr(self.options, field.name) != new_value: + # The value changed, call the _async_set_* method for the option. + await getattr(self, f"_async_set_{field.name}")(new_value) + self.options = options + async def _async_subscription_callback(self, updated: bool) -> None: """Update the state by the Wemo device.""" # If an update is in progress, we don't do anything. @@ -160,20 +244,11 @@ async def async_register_device( device = DeviceCoordinator(hass, wemo, entry.id) hass.data[DOMAIN].setdefault("devices", {})[entry.id] = device - registry = hass.data[DOMAIN]["registry"] - registry.on(wemo, None, device.subscription_callback) - await hass.async_add_executor_job(registry.register, wemo) - if isinstance(wemo, LongPressMixin): - try: - await hass.async_add_executor_job(wemo.ensure_long_press_virtual_device) - # Temporarily handling all exceptions for #52996 & pywemo/pywemo/issues/276 - # Replace this with `except: PyWeMoException` after upstream has been fixed. - except Exception: # pylint: disable=broad-except - _LOGGER.exception( - "Failed to enable long press support for device: %s", wemo.name - ) - device.supports_long_press = False + config_entry.async_on_unload( + config_entry.add_update_listener(device.async_set_options) + ) + await device.async_set_options(hass, config_entry) return device diff --git a/tests/components/wemo/test_wemo_device.py b/tests/components/wemo/test_wemo_device.py index 49c6664f7bb497..84199818be2d13 100644 --- a/tests/components/wemo/test_wemo_device.py +++ b/tests/components/wemo/test_wemo_device.py @@ -1,5 +1,6 @@ """Tests for wemo_device.py.""" import asyncio +from dataclasses import asdict from datetime import timedelta from unittest.mock import call, patch @@ -188,6 +189,29 @@ async def test_dli_device_info( assert device_entries[0].identifiers == {(DOMAIN, "123456789")} +async def test_options_enable_subscription_false( + hass, pywemo_registry, pywemo_device, wemo_entity +): + """Test setting Options.enable_subscription = False.""" + config_entry = hass.config_entries.async_get_entry(wemo_entity.config_entry_id) + assert hass.config_entries.async_update_entry( + config_entry, options=asdict(wemo_device.Options(enable_subscription=False)) + ) + await hass.async_block_till_done() + pywemo_registry.unregister.assert_called_once_with(pywemo_device) + pywemo_device.remove_long_press_virtual_device.assert_called_once_with() + + +async def test_options_enable_long_press_false(hass, pywemo_device, wemo_entity): + """Test setting Options.enable_long_press = False.""" + config_entry = hass.config_entries.async_get_entry(wemo_entity.config_entry_id) + assert hass.config_entries.async_update_entry( + config_entry, options=asdict(wemo_device.Options(enable_long_press=False)) + ) + await hass.async_block_till_done() + pywemo_device.remove_long_press_virtual_device.assert_called_once_with() + + class TestInsight: """Tests specific to the WeMo Insight device.""" From 3ec76fdc86d8a0ce496276b4e4ca1f380de040de Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Sat, 11 Dec 2021 20:22:43 -0800 Subject: [PATCH 2/7] Also add polling_interval_seconds option --- homeassistant/components/wemo/strings.json | 3 ++- homeassistant/components/wemo/wemo_device.py | 9 ++++++- tests/components/wemo/test_wemo_device.py | 27 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/wemo/strings.json b/homeassistant/components/wemo/strings.json index 8150b4d55989b4..6a26c79559cfc0 100644 --- a/homeassistant/components/wemo/strings.json +++ b/homeassistant/components/wemo/strings.json @@ -15,7 +15,8 @@ "init": { "data": { "enable_subscription": "Subscribe to device local push updates", - "enable_long_press": "Register for device long-press events" + "enable_long_press": "Register for device long-press events", + "polling_interval_seconds": "Seconds to wait between polling the device" } } } diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index fa4b0b6abf3fd6..49617f1738344b 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -45,6 +45,9 @@ class Options: # Register for device long-press events. enable_long_press: bool = True + # Polling interval for when subscriptions are not enabled. + polling_interval_seconds: int = 30 + def __post_init__(self) -> None: """Validate parameters.""" if self.enable_subscription is False: @@ -78,7 +81,6 @@ def __init__(self, hass: HomeAssistant, wemo: WeMoDevice, device_id: str) -> Non hass, _LOGGER, name=wemo.name, - update_interval=timedelta(seconds=30), ) self.hass = hass self.wemo = wemo @@ -137,6 +139,11 @@ async def _async_set_enable_long_press(self, enable_long_press: bool) -> None: ) self.supports_long_press = False + async def _async_set_polling_interval_seconds( + self, polling_interval_seconds: int + ) -> None: + self.update_interval = timedelta(seconds=polling_interval_seconds) + async def async_set_options( self, hass: HomeAssistant, config_entry: ConfigEntry ) -> None: diff --git a/tests/components/wemo/test_wemo_device.py b/tests/components/wemo/test_wemo_device.py index 84199818be2d13..ac68e774088019 100644 --- a/tests/components/wemo/test_wemo_device.py +++ b/tests/components/wemo/test_wemo_device.py @@ -212,6 +212,33 @@ async def test_options_enable_long_press_false(hass, pywemo_device, wemo_entity) pywemo_device.remove_long_press_virtual_device.assert_called_once_with() +async def test_options_polling_interval_seconds(hass, pywemo_device, wemo_entity): + """Test setting Options.polling_interval_seconds = 45.""" + config_entry = hass.config_entries.async_get_entry(wemo_entity.config_entry_id) + assert hass.config_entries.async_update_entry( + config_entry, + options=asdict( + wemo_device.Options(enable_subscription=False, polling_interval_seconds=45) + ), + ) + await hass.async_block_till_done() + + # Move time forward to capture the new interval. + async_fire_time_changed(hass, utcnow() + timedelta(seconds=31)) + await hass.async_block_till_done() + pywemo_device.get_state.reset_mock() + + # Make sure no polling occurs before 45 seconds. + async_fire_time_changed(hass, utcnow() + timedelta(seconds=31)) + await hass.async_block_till_done() + pywemo_device.get_state.assert_not_called() + + # Polling occurred after the interval. + async_fire_time_changed(hass, utcnow() + timedelta(seconds=46)) + await hass.async_block_till_done() + pywemo_device.get_state.assert_has_calls([call(True), call()]) + + class TestInsight: """Tests specific to the WeMo Insight device.""" From 1b4e1e32f7198acf4b263f6223520bec6fe66ac4 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Sat, 11 Dec 2021 21:57:33 -0800 Subject: [PATCH 3/7] Give feedback about invalid option combinations --- homeassistant/components/wemo/config_flow.py | 33 ++++++++++++-- homeassistant/components/wemo/strings.json | 4 ++ homeassistant/components/wemo/wemo_device.py | 38 ++++++++-------- tests/components/wemo/test_config_flow.py | 46 +++++++++++++++++++- tests/components/wemo/test_wemo_device.py | 12 +++-- 5 files changed, 105 insertions(+), 28 deletions(-) diff --git a/homeassistant/components/wemo/config_flow.py b/homeassistant/components/wemo/config_flow.py index f523f023c818ab..b4357868c50871 100644 --- a/homeassistant/components/wemo/config_flow.py +++ b/homeassistant/components/wemo/config_flow.py @@ -2,9 +2,11 @@ from __future__ import annotations -from typing import Any +from dataclasses import fields +from typing import Any, get_type_hints import pywemo +import voluptuous as vol from homeassistant.config_entries import ConfigEntry, OptionsFlow from homeassistant.core import HomeAssistant, callback @@ -12,7 +14,7 @@ from homeassistant.helpers.config_entry_flow import DiscoveryFlowHandler from .const import DOMAIN -from .wemo_device import Options +from .wemo_device import Options, OptionsValidationError async def _async_has_devices(hass: HomeAssistant) -> bool: @@ -45,10 +47,33 @@ async def async_step_init( self, user_input: dict[str, Any] | None = None ) -> FlowResult: """Manage options for the WeMo component.""" + errors: dict[str, str] | None = None if user_input is not None: - return self.async_create_entry(title="", data=user_input) + try: + Options(**user_input) + except OptionsValidationError as err: + errors = {err.field: err.error_string} + else: + return self.async_create_entry(title="", data=user_input) return self.async_show_form( step_id="init", - data_schema=Options(**self.config_entry.options).schema(), + data_schema=_schema_for_options(Options(**self.config_entry.options)), + errors=errors, ) + + +def _schema_for_options(options: Options) -> vol.Schema: + """Return the Voluptuous schema for the Options instance. + + All values are optional. The default value is set to the current value and + the type hint is set to the value of the field type annotation. + """ + return vol.Schema( + { + vol.Optional( + field.name, default=getattr(options, field.name) + ): get_type_hints(options)[field.name] + for field in fields(options) + } + ) diff --git a/homeassistant/components/wemo/strings.json b/homeassistant/components/wemo/strings.json index 6a26c79559cfc0..8843baad1df837 100644 --- a/homeassistant/components/wemo/strings.json +++ b/homeassistant/components/wemo/strings.json @@ -19,6 +19,10 @@ "polling_interval_seconds": "Seconds to wait between polling the device" } } + }, + "error": { + "long_press_requires_subscription": "Local push update subscriptions must be enabled to use long-press events", + "unknown": "[%key:common::config_flow::error::unknown%]" } }, "device_automation": { diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index 49617f1738344b..ae9da605260a1a 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -5,12 +5,10 @@ from dataclasses import dataclass, fields from datetime import timedelta import logging -import typing from pywemo import Insight, LongPressMixin, WeMoDevice from pywemo.exceptions import ActionException from pywemo.subscribe import EVENT_TYPE_LONG_PRESS -import voluptuous as vol from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( @@ -35,6 +33,20 @@ _LOGGER = logging.getLogger(__name__) +class OptionsValidationError(Exception): + """Error validating options.""" + + field: str = "base" + error_string: str = "unknown" + + +class LongPressRequiresSubscriptionError(OptionsValidationError): + """subscriptions must be enabled to use long-press events.""" + + field = "enable_subscription" + error_string = "long_press_requires_subscription" + + @dataclass class Options: """Configuration options for the DeviceCoordinator class.""" @@ -50,24 +62,10 @@ class Options: def __post_init__(self) -> None: """Validate parameters.""" - if self.enable_subscription is False: - # Long press support requires subscriptions. - self.enable_long_press = False - - def schema(self) -> vol.Schema: - """Return the Voluptuous schema for the Options instance. - - All values are optional. The default value is set to the current value and - the type hint is set to the value of the field type annotation. - """ - return vol.Schema( - { - vol.Optional( - field.name, default=getattr(self, field.name) - ): typing.get_type_hints(self)[field.name] - for field in fields(self) - } - ) + if not self.enable_subscription and self.enable_long_press: + raise LongPressRequiresSubscriptionError( + "Local push update subscriptions must be enabled to use long-press events", + ) class DeviceCoordinator(DataUpdateCoordinator[None]): diff --git a/tests/components/wemo/test_config_flow.py b/tests/components/wemo/test_config_flow.py index 0ffcb9d7f5effa..58ec65fac0fa09 100644 --- a/tests/components/wemo/test_config_flow.py +++ b/tests/components/wemo/test_config_flow.py @@ -1,11 +1,17 @@ """Tests for Wemo config flow.""" +from dataclasses import asdict + from homeassistant import data_entry_flow from homeassistant.components.wemo.const import DOMAIN +from homeassistant.components.wemo.wemo_device import ( + LongPressRequiresSubscriptionError, + Options, +) from homeassistant.config_entries import SOURCE_USER from homeassistant.core import HomeAssistant -from tests.common import patch +from tests.common import MockConfigEntry, patch async def test_not_discovered(hass: HomeAssistant) -> None: @@ -20,3 +26,41 @@ async def test_not_discovered(hass: HomeAssistant) -> None: result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) assert result["type"] == data_entry_flow.FlowResultType.ABORT assert result["reason"] == "no_devices_found" + + +async def test_options(hass: HomeAssistant) -> None: + """Test updating options.""" + options = Options(enable_subscription=False, enable_long_press=False) + entry = MockConfigEntry(domain=DOMAIN, title="Wemo") + entry.add_to_hass(hass) + + result = await hass.config_entries.options.async_init(entry.entry_id) + + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "init" + + result = await hass.config_entries.options.async_configure( + result["flow_id"], user_input=asdict(options) + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert Options(**result["data"]) == options + + +async def test_invalid_options(hass: HomeAssistant) -> None: + """Test invalid option combinations.""" + entry = MockConfigEntry(domain=DOMAIN, title="Wemo") + entry.add_to_hass(hass) + + result = await hass.config_entries.options.async_init(entry.entry_id) + + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "init" + + result = await hass.config_entries.options.async_configure( + result["flow_id"], user_input={"enable_subscription": False} + ) + + assert result["errors"] == { + LongPressRequiresSubscriptionError.field: LongPressRequiresSubscriptionError.error_string + } diff --git a/tests/components/wemo/test_wemo_device.py b/tests/components/wemo/test_wemo_device.py index ac68e774088019..86401cde5248c4 100644 --- a/tests/components/wemo/test_wemo_device.py +++ b/tests/components/wemo/test_wemo_device.py @@ -195,11 +195,13 @@ async def test_options_enable_subscription_false( """Test setting Options.enable_subscription = False.""" config_entry = hass.config_entries.async_get_entry(wemo_entity.config_entry_id) assert hass.config_entries.async_update_entry( - config_entry, options=asdict(wemo_device.Options(enable_subscription=False)) + config_entry, + options=asdict( + wemo_device.Options(enable_subscription=False, enable_long_press=False) + ), ) await hass.async_block_till_done() pywemo_registry.unregister.assert_called_once_with(pywemo_device) - pywemo_device.remove_long_press_virtual_device.assert_called_once_with() async def test_options_enable_long_press_false(hass, pywemo_device, wemo_entity): @@ -218,7 +220,11 @@ async def test_options_polling_interval_seconds(hass, pywemo_device, wemo_entity assert hass.config_entries.async_update_entry( config_entry, options=asdict( - wemo_device.Options(enable_subscription=False, polling_interval_seconds=45) + wemo_device.Options( + enable_subscription=False, + enable_long_press=False, + polling_interval_seconds=45, + ) ), ) await hass.async_block_till_done() From 34fb800fb20752f53d0272de66f1b25bb5985f21 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Sat, 11 Dec 2021 22:12:12 -0800 Subject: [PATCH 4/7] Use a frozen dataclass for Options --- homeassistant/components/wemo/wemo_device.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index ae9da605260a1a..cf2b081b9c435d 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -47,7 +47,7 @@ class LongPressRequiresSubscriptionError(OptionsValidationError): error_string = "long_press_requires_subscription" -@dataclass +@dataclass(frozen=True) class Options: """Configuration options for the DeviceCoordinator class.""" From d7885b00fccec5f8465c785ccb909e4d9d210659 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Fri, 24 Dec 2021 11:35:26 -0800 Subject: [PATCH 5/7] Validate polling_interval_seconds --- homeassistant/components/wemo/config_flow.py | 2 +- homeassistant/components/wemo/strings.json | 1 + homeassistant/components/wemo/wemo_device.py | 48 ++++++++++++++++---- tests/components/wemo/test_config_flow.py | 15 +++--- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/homeassistant/components/wemo/config_flow.py b/homeassistant/components/wemo/config_flow.py index b4357868c50871..e4626b4deaf517 100644 --- a/homeassistant/components/wemo/config_flow.py +++ b/homeassistant/components/wemo/config_flow.py @@ -52,7 +52,7 @@ async def async_step_init( try: Options(**user_input) except OptionsValidationError as err: - errors = {err.field: err.error_string} + errors = {err.field_key: err.error_key} else: return self.async_create_entry(title="", data=user_input) diff --git a/homeassistant/components/wemo/strings.json b/homeassistant/components/wemo/strings.json index 8843baad1df837..dfe5d94bb8ae38 100644 --- a/homeassistant/components/wemo/strings.json +++ b/homeassistant/components/wemo/strings.json @@ -22,6 +22,7 @@ }, "error": { "long_press_requires_subscription": "Local push update subscriptions must be enabled to use long-press events", + "polling_interval_to_small": "Polling more frequently than 10 seconds is not supported", "unknown": "[%key:common::config_flow::error::unknown%]" } }, diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index cf2b081b9c435d..3d19c0aaab534c 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -5,6 +5,7 @@ from dataclasses import dataclass, fields from datetime import timedelta import logging +from typing import Literal from pywemo import Insight, LongPressMixin, WeMoDevice from pywemo.exceptions import ActionException @@ -32,24 +33,43 @@ _LOGGER = logging.getLogger(__name__) +# Literal values must match options.error keys from strings.json. +ErrorStringKey = Literal[ + "long_press_requires_subscription", "polling_interval_to_small" +] +# Literal values must match options.step.init.data keys from strings.json. +OptionsFieldKey = Literal[ + "enable_subscription", "enable_long_press", "polling_interval_seconds" +] + class OptionsValidationError(Exception): """Error validating options.""" - field: str = "base" - error_string: str = "unknown" - + def __init__( + self, field_key: OptionsFieldKey, error_key: ErrorStringKey, message: str + ) -> None: + """Store field and error_key so the exception handler can used them. -class LongPressRequiresSubscriptionError(OptionsValidationError): - """subscriptions must be enabled to use long-press events.""" + The field_key and error_key strings must be the same as in strings.json. - field = "enable_subscription" - error_string = "long_press_requires_subscription" + Args: + field_key: Name of the options.step.init.data key that corresponds to this error. + field_key must also match one of the field names inside the Options class. + error_key: Name of the options.error key that corresponds to this error. + """ + super().__init__(message) + self.field_key = field_key + self.error_key = error_key @dataclass(frozen=True) class Options: - """Configuration options for the DeviceCoordinator class.""" + """Configuration options for the DeviceCoordinator class. + + Note: The field names must match the keys (OptionsFieldKey) + from options.step.init.data in strings.json. + """ # Subscribe to device local push updates. enable_subscription: bool = True @@ -57,15 +77,23 @@ class Options: # Register for device long-press events. enable_long_press: bool = True - # Polling interval for when subscriptions are not enabled. + # Polling interval for when subscriptions are not enabled or broken. polling_interval_seconds: int = 30 def __post_init__(self) -> None: """Validate parameters.""" if not self.enable_subscription and self.enable_long_press: - raise LongPressRequiresSubscriptionError( + raise OptionsValidationError( + "enable_subscription", + "long_press_requires_subscription", "Local push update subscriptions must be enabled to use long-press events", ) + if self.polling_interval_seconds < 10: + raise OptionsValidationError( + "polling_interval_seconds", + "polling_interval_to_small", + "Polling more frequently than 10 seconds is not supported", + ) class DeviceCoordinator(DataUpdateCoordinator[None]): diff --git a/tests/components/wemo/test_config_flow.py b/tests/components/wemo/test_config_flow.py index 58ec65fac0fa09..e84ae4f0205dba 100644 --- a/tests/components/wemo/test_config_flow.py +++ b/tests/components/wemo/test_config_flow.py @@ -4,10 +4,7 @@ from homeassistant import data_entry_flow from homeassistant.components.wemo.const import DOMAIN -from homeassistant.components.wemo.wemo_device import ( - LongPressRequiresSubscriptionError, - Options, -) +from homeassistant.components.wemo.wemo_device import Options from homeassistant.config_entries import SOURCE_USER from homeassistant.core import HomeAssistant @@ -57,10 +54,16 @@ async def test_invalid_options(hass: HomeAssistant) -> None: assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "init" + # enable_subscription must be True if enable_long_press is True (default). result = await hass.config_entries.options.async_configure( result["flow_id"], user_input={"enable_subscription": False} ) - assert result["errors"] == { - LongPressRequiresSubscriptionError.field: LongPressRequiresSubscriptionError.error_string + "enable_subscription": "long_press_requires_subscription" } + + # polling_interval_seconds must be larger than 10. + result = await hass.config_entries.options.async_configure( + result["flow_id"], user_input={"polling_interval_seconds": 1} + ) + assert result["errors"] == {"polling_interval_seconds": "polling_interval_to_small"} From 65028e05589d52c63202f42667a9ed822d4ec0fe Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Thu, 8 Jun 2023 09:14:47 -0700 Subject: [PATCH 6/7] Describe message arg --- homeassistant/components/wemo/wemo_device.py | 1 + 1 file changed, 1 insertion(+) diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index 3d19c0aaab534c..1438f5f9a2d94b 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -57,6 +57,7 @@ def __init__( field_key: Name of the options.step.init.data key that corresponds to this error. field_key must also match one of the field names inside the Options class. error_key: Name of the options.error key that corresponds to this error. + message: Message for the Exception class. """ super().__init__(message) self.field_key = field_key From 6493772bafec118a72c4ef22b1ef5d338b145316 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Mon, 12 Jun 2023 08:41:07 -0700 Subject: [PATCH 7/7] Replace broad exception with PyWeMoException --- homeassistant/components/wemo/wemo_device.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index 1438f5f9a2d94b..19a78c0fccfe9b 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -8,7 +8,7 @@ from typing import Literal from pywemo import Insight, LongPressMixin, WeMoDevice -from pywemo.exceptions import ActionException +from pywemo.exceptions import ActionException, PyWeMoException from pywemo.subscribe import EVENT_TYPE_LONG_PRESS from homeassistant.config_entries import ConfigEntry @@ -158,9 +158,7 @@ async def _async_set_enable_long_press(self, enable_long_press: bool) -> None: await self.hass.async_add_executor_job( self.wemo.remove_long_press_virtual_device ) - # Temporarily handling all exceptions for #52996 & pywemo/pywemo/issues/276 - # Replace this with `except: PyWeMoException` after upstream has been fixed. - except Exception: # pylint: disable=broad-except + except PyWeMoException: _LOGGER.exception( "Failed to enable long press support for device: %s", self.wemo.name )