From 8459a284894b28442a69c886180341d9061c43ff Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Mon, 17 Jan 2022 01:03:24 -0800 Subject: [PATCH] WeMo state changes are seen by all coordinator entities (#64228) * WeMo push update is seen by all coordinator entities * Rename _wemo_exception_handler -> _wemo_call_wrapper * Test turning off the entity * Test setting light brightness * Improve brightness test * It is unnecessary to setup the platform integration * Use domain names, not platform enum, in service calls --- homeassistant/components/wemo/entity.py | 30 ++++++++-------- homeassistant/components/wemo/fan.py | 16 +++------ homeassistant/components/wemo/light.py | 18 +++------- homeassistant/components/wemo/switch.py | 8 ++--- homeassistant/components/wemo/wemo_device.py | 6 ++++ tests/components/wemo/entity_test_helpers.py | 15 ++++++-- tests/components/wemo/test_fan.py | 10 ++++-- tests/components/wemo/test_light_bridge.py | 11 ++++-- tests/components/wemo/test_light_dimmer.py | 37 ++++++++++++++++++-- tests/components/wemo/test_switch.py | 10 ++++-- 10 files changed, 103 insertions(+), 58 deletions(-) diff --git a/homeassistant/components/wemo/entity.py b/homeassistant/components/wemo/entity.py index 44f6b5a4e63..9884b5b340c 100644 --- a/homeassistant/components/wemo/entity.py +++ b/homeassistant/components/wemo/entity.py @@ -8,7 +8,6 @@ from typing import cast from pywemo.exceptions import ActionException -from homeassistant.core import callback from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.update_coordinator import CoordinatorEntity @@ -20,6 +19,8 @@ _LOGGER = logging.getLogger(__name__) class WemoEntity(CoordinatorEntity): """Common methods for Wemo entities.""" + coordinator: DeviceCoordinator # Override CoordinatorEntity.coordinator type. + # Most pyWeMo devices are associated with a single Home Assistant entity. When # that is not the case, name_suffix & unique_id_suffix can be used to provide # names and unique ids for additional Home Assistant entities. @@ -31,7 +32,6 @@ class WemoEntity(CoordinatorEntity): super().__init__(coordinator) self.wemo = coordinator.wemo self._device_info = coordinator.device_info - self._available = True @property def name_suffix(self) -> str | None: @@ -46,11 +46,6 @@ class WemoEntity(CoordinatorEntity): return f"{wemo_name} {suffix}" return wemo_name - @property - def available(self) -> bool: - """Return true if the device is available.""" - return super().available and self._available - @property def unique_id_suffix(self) -> str | None: """Suffix to append to the WeMo device's unique ID.""" @@ -71,20 +66,23 @@ class WemoEntity(CoordinatorEntity): """Return the device info.""" return self._device_info - @callback - def _handle_coordinator_update(self) -> None: - """Handle updated data from the coordinator.""" - self._available = True - super()._handle_coordinator_update() - @contextlib.contextmanager - def _wemo_exception_handler(self, message: str) -> Generator[None, None, None]: - """Wrap device calls to set `_available` when wemo exceptions happen.""" + def _wemo_call_wrapper(self, message: str) -> Generator[None, None, None]: + """Wrap calls to the device that change its state. + + 1. Takes care of making available=False when communications with the + device fails. + 2. Ensures all entities sharing the same coordinator are aware of + updates to the device state. + """ try: yield except ActionException as err: _LOGGER.warning("Could not %s for %s (%s)", message, self.name, err) - self._available = False + self.coordinator.last_exception = err + self.coordinator.last_update_success = False # Used for self.available. + finally: + self.hass.add_job(self.coordinator.async_update_listeners) class WemoBinaryStateEntity(WemoEntity): diff --git a/homeassistant/components/wemo/fan.py b/homeassistant/components/wemo/fan.py index 03c5964a313..93da8d158a2 100644 --- a/homeassistant/components/wemo/fan.py +++ b/homeassistant/components/wemo/fan.py @@ -160,11 +160,9 @@ class WemoHumidifier(WemoBinaryStateEntity, FanEntity): def turn_off(self, **kwargs: Any) -> None: """Turn the switch off.""" - with self._wemo_exception_handler("turn off"): + with self._wemo_call_wrapper("turn off"): self.wemo.set_state(WEMO_FAN_OFF) - self.schedule_update_ha_state() - def set_percentage(self, percentage: int | None) -> None: """Set the fan_mode of the Humidifier.""" if percentage is None: @@ -174,11 +172,9 @@ class WemoHumidifier(WemoBinaryStateEntity, FanEntity): else: named_speed = math.ceil(percentage_to_ranged_value(SPEED_RANGE, percentage)) - with self._wemo_exception_handler("set speed"): + with self._wemo_call_wrapper("set speed"): self.wemo.set_state(named_speed) - self.schedule_update_ha_state() - def set_humidity(self, target_humidity: float) -> None: """Set the target humidity level for the Humidifier.""" if target_humidity < 50: @@ -192,14 +188,10 @@ class WemoHumidifier(WemoBinaryStateEntity, FanEntity): elif target_humidity >= 100: pywemo_humidity = WEMO_HUMIDITY_100 - with self._wemo_exception_handler("set humidity"): + with self._wemo_call_wrapper("set humidity"): self.wemo.set_humidity(pywemo_humidity) - self.schedule_update_ha_state() - def reset_filter_life(self) -> None: """Reset the filter life to 100%.""" - with self._wemo_exception_handler("reset filter life"): + with self._wemo_call_wrapper("reset filter life"): self.wemo.reset_filter_life() - - self.schedule_update_ha_state() diff --git a/homeassistant/components/wemo/light.py b/homeassistant/components/wemo/light.py index fbb1c84e449..b7c6aefa868 100644 --- a/homeassistant/components/wemo/light.py +++ b/homeassistant/components/wemo/light.py @@ -169,7 +169,7 @@ class WemoLight(WemoEntity, LightEntity): "force_update": False, } - with self._wemo_exception_handler("turn on"): + with self._wemo_call_wrapper("turn on"): if xy_color is not None: self.light.set_color(xy_color, transition=transition_time) @@ -180,17 +180,13 @@ class WemoLight(WemoEntity, LightEntity): self.light.turn_on(**turn_on_kwargs) - self.schedule_update_ha_state() - def turn_off(self, **kwargs: Any) -> None: """Turn the light off.""" transition_time = int(kwargs.get(ATTR_TRANSITION, 0)) - with self._wemo_exception_handler("turn off"): + with self._wemo_call_wrapper("turn off"): self.light.turn_off(transition=transition_time) - self.schedule_update_ha_state() - class WemoDimmer(WemoBinaryStateEntity, LightEntity): """Representation of a WeMo dimmer.""" @@ -213,17 +209,13 @@ class WemoDimmer(WemoBinaryStateEntity, LightEntity): if ATTR_BRIGHTNESS in kwargs: brightness = kwargs[ATTR_BRIGHTNESS] brightness = int((brightness / 255) * 100) - with self._wemo_exception_handler("set brightness"): + with self._wemo_call_wrapper("set brightness"): self.wemo.set_brightness(brightness) else: - with self._wemo_exception_handler("turn on"): + with self._wemo_call_wrapper("turn on"): self.wemo.on() - self.schedule_update_ha_state() - def turn_off(self, **kwargs: Any) -> None: """Turn the dimmer off.""" - with self._wemo_exception_handler("turn off"): + with self._wemo_call_wrapper("turn off"): self.wemo.off() - - self.schedule_update_ha_state() diff --git a/homeassistant/components/wemo/switch.py b/homeassistant/components/wemo/switch.py index 9982274bbc0..8f8e5dcb5e3 100644 --- a/homeassistant/components/wemo/switch.py +++ b/homeassistant/components/wemo/switch.py @@ -154,14 +154,10 @@ class WemoSwitch(WemoBinaryStateEntity, SwitchEntity): def turn_on(self, **kwargs: Any) -> None: """Turn the switch on.""" - with self._wemo_exception_handler("turn on"): + with self._wemo_call_wrapper("turn on"): self.wemo.on() - self.schedule_update_ha_state() - def turn_off(self, **kwargs: Any) -> None: """Turn the switch off.""" - with self._wemo_exception_handler("turn off"): + with self._wemo_call_wrapper("turn off"): self.wemo.off() - - self.schedule_update_ha_state() diff --git a/homeassistant/components/wemo/wemo_device.py b/homeassistant/components/wemo/wemo_device.py index 267ea84e308..3ca47544fd7 100644 --- a/homeassistant/components/wemo/wemo_device.py +++ b/homeassistant/components/wemo/wemo_device.py @@ -123,6 +123,12 @@ class DeviceCoordinator(DataUpdateCoordinator): except ActionException as err: raise UpdateFailed("WeMo update failed") from err + @callback + def async_update_listeners(self) -> None: + """Update all listeners.""" + for update_callback in self._listeners: + update_callback() + def _device_info(wemo: WeMoDevice) -> DeviceInfo: return DeviceInfo( diff --git a/tests/components/wemo/entity_test_helpers.py b/tests/components/wemo/entity_test_helpers.py index 4ca11c11100..61c7cd5bf2e 100644 --- a/tests/components/wemo/entity_test_helpers.py +++ b/tests/components/wemo/entity_test_helpers.py @@ -9,7 +9,9 @@ from homeassistant.components.homeassistant import DOMAIN as HA_DOMAIN from homeassistant.components.wemo import wemo_device from homeassistant.const import ( ATTR_ENTITY_ID, + SERVICE_TURN_OFF, SERVICE_TURN_ON, + STATE_OFF, STATE_ON, STATE_UNAVAILABLE, ) @@ -121,8 +123,6 @@ async def test_avaliable_after_update( ActionException when the SERVICE_TURN_ON method is called and that the state will be On after the update. """ - await async_setup_component(hass, domain, {}) - await hass.services.async_call( domain, SERVICE_TURN_ON, @@ -136,6 +136,17 @@ async def test_avaliable_after_update( assert hass.states.get(wemo_entity.entity_id).state == STATE_ON +async def test_turn_off_state(hass, wemo_entity, domain): + """Test that the device state is updated after turning off.""" + await hass.services.async_call( + domain, + SERVICE_TURN_OFF, + {ATTR_ENTITY_ID: [wemo_entity.entity_id]}, + blocking=True, + ) + assert hass.states.get(wemo_entity.entity_id).state == STATE_OFF + + class EntityTestHelpers: """Common state update helpers.""" diff --git a/tests/components/wemo/test_fan.py b/tests/components/wemo/test_fan.py index a00d5523678..8795b7cdc94 100644 --- a/tests/components/wemo/test_fan.py +++ b/tests/components/wemo/test_fan.py @@ -3,13 +3,14 @@ import pytest from pywemo.exceptions import ActionException +from homeassistant.components.fan import DOMAIN as FAN_DOMAIN from homeassistant.components.homeassistant import ( DOMAIN as HA_DOMAIN, SERVICE_UPDATE_ENTITY, ) from homeassistant.components.wemo import fan from homeassistant.components.wemo.const import DOMAIN -from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON, Platform +from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON from homeassistant.setup import async_setup_component from . import entity_test_helpers @@ -84,10 +85,15 @@ async def test_available_after_update( pywemo_device.set_state.side_effect = ActionException pywemo_device.get_state.return_value = 1 await entity_test_helpers.test_avaliable_after_update( - hass, pywemo_registry, pywemo_device, wemo_entity, Platform.FAN + hass, pywemo_registry, pywemo_device, wemo_entity, FAN_DOMAIN ) +async def test_turn_off_state(hass, wemo_entity): + """Test that the device state is updated after turning off.""" + await entity_test_helpers.test_turn_off_state(hass, wemo_entity, FAN_DOMAIN) + + async def test_fan_reset_filter_service(hass, pywemo_device, wemo_entity): """Verify that SERVICE_RESET_FILTER_LIFE is registered and works.""" assert await hass.services.async_call( diff --git a/tests/components/wemo/test_light_bridge.py b/tests/components/wemo/test_light_bridge.py index 1d316d93c8f..3184335b173 100644 --- a/tests/components/wemo/test_light_bridge.py +++ b/tests/components/wemo/test_light_bridge.py @@ -8,8 +8,8 @@ from homeassistant.components.homeassistant import ( DOMAIN as HA_DOMAIN, SERVICE_UPDATE_ENTITY, ) -from homeassistant.components.light import ATTR_COLOR_TEMP -from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON, Platform +from homeassistant.components.light import ATTR_COLOR_TEMP, DOMAIN as LIGHT_DOMAIN +from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON from homeassistant.setup import async_setup_component from . import entity_test_helpers @@ -76,10 +76,15 @@ async def test_available_after_update( pywemo_bridge_light.turn_on.side_effect = pywemo.exceptions.ActionException pywemo_bridge_light.state["onoff"] = 1 await entity_test_helpers.test_avaliable_after_update( - hass, pywemo_registry, pywemo_device, wemo_entity, Platform.LIGHT + hass, pywemo_registry, pywemo_device, wemo_entity, LIGHT_DOMAIN ) +async def test_turn_off_state(hass, pywemo_bridge_light, wemo_entity): + """Test that the device state is updated after turning off.""" + await entity_test_helpers.test_turn_off_state(hass, wemo_entity, LIGHT_DOMAIN) + + async def test_light_update_entity( hass, pywemo_registry, pywemo_bridge_light, wemo_entity ): diff --git a/tests/components/wemo/test_light_dimmer.py b/tests/components/wemo/test_light_dimmer.py index 0460994aa86..56054fa77e9 100644 --- a/tests/components/wemo/test_light_dimmer.py +++ b/tests/components/wemo/test_light_dimmer.py @@ -7,7 +7,8 @@ from homeassistant.components.homeassistant import ( DOMAIN as HA_DOMAIN, SERVICE_UPDATE_ENTITY, ) -from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON, Platform +from homeassistant.components.light import ATTR_BRIGHTNESS, DOMAIN as LIGHT_DOMAIN +from homeassistant.const import ATTR_ENTITY_ID, SERVICE_TURN_ON, STATE_OFF, STATE_ON from homeassistant.setup import async_setup_component from . import entity_test_helpers @@ -40,10 +41,42 @@ async def test_available_after_update( pywemo_device.on.side_effect = ActionException pywemo_device.get_state.return_value = 1 await entity_test_helpers.test_avaliable_after_update( - hass, pywemo_registry, pywemo_device, wemo_entity, Platform.LIGHT + hass, pywemo_registry, pywemo_device, wemo_entity, LIGHT_DOMAIN ) +async def test_turn_off_state(hass, wemo_entity): + """Test that the device state is updated after turning off.""" + await entity_test_helpers.test_turn_off_state(hass, wemo_entity, LIGHT_DOMAIN) + + +async def test_turn_on_brightness(hass, pywemo_device, wemo_entity): + """Test setting the brightness value of the light.""" + brightness = 0 + state = 0 + + def set_brightness(b): + nonlocal brightness + nonlocal state + brightness, state = (b, int(bool(b))) + + pywemo_device.get_state.side_effect = lambda: state + pywemo_device.get_brightness.side_effect = lambda: brightness + pywemo_device.set_brightness.side_effect = set_brightness + + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: [wemo_entity.entity_id], ATTR_BRIGHTNESS: 204}, + blocking=True, + ) + + pywemo_device.set_brightness.assert_called_once_with(80) + states = hass.states.get(wemo_entity.entity_id) + assert states.state == STATE_ON + assert states.attributes[ATTR_BRIGHTNESS] == 204 + + async def test_light_registry_state_callback( hass, pywemo_registry, pywemo_device, wemo_entity ): diff --git a/tests/components/wemo/test_switch.py b/tests/components/wemo/test_switch.py index 963c662b124..9c1dc804645 100644 --- a/tests/components/wemo/test_switch.py +++ b/tests/components/wemo/test_switch.py @@ -7,7 +7,8 @@ from homeassistant.components.homeassistant import ( DOMAIN as HA_DOMAIN, SERVICE_UPDATE_ENTITY, ) -from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON, Platform +from homeassistant.components.switch import DOMAIN as SWITCH_DOMAIN +from homeassistant.const import ATTR_ENTITY_ID, STATE_OFF, STATE_ON from homeassistant.setup import async_setup_component from . import entity_test_helpers @@ -82,5 +83,10 @@ async def test_available_after_update( pywemo_device.on.side_effect = ActionException pywemo_device.get_state.return_value = 1 await entity_test_helpers.test_avaliable_after_update( - hass, pywemo_registry, pywemo_device, wemo_entity, Platform.SWITCH + hass, pywemo_registry, pywemo_device, wemo_entity, SWITCH_DOMAIN ) + + +async def test_turn_off_state(hass, wemo_entity): + """Test that the device state is updated after turning off.""" + await entity_test_helpers.test_turn_off_state(hass, wemo_entity, SWITCH_DOMAIN)