From 7c08592b5af11132c4d75664262041f2946ffecf Mon Sep 17 00:00:00 2001 From: Emily Mills Date: Fri, 5 Mar 2021 14:24:55 -0600 Subject: [PATCH] Convert kulersky to use new async backend (#47403) --- .../components/kulersky/config_flow.py | 4 +- homeassistant/components/kulersky/light.py | 103 +++---- .../components/kulersky/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/kulersky/test_config_flow.py | 18 +- tests/components/kulersky/test_light.py | 258 ++++++------------ 7 files changed, 130 insertions(+), 259 deletions(-) diff --git a/homeassistant/components/kulersky/config_flow.py b/homeassistant/components/kulersky/config_flow.py index 04f7719b8e6..2a11a3c2e17 100644 --- a/homeassistant/components/kulersky/config_flow.py +++ b/homeassistant/components/kulersky/config_flow.py @@ -15,9 +15,7 @@ async def _async_has_devices(hass) -> bool: """Return if there are devices that can be discovered.""" # Check if there are any devices that can be discovered in the network. try: - devices = await hass.async_add_executor_job( - pykulersky.discover_bluetooth_devices - ) + devices = await pykulersky.discover() except pykulersky.PykulerskyException as exc: _LOGGER.error("Unable to discover nearby Kuler Sky devices: %s", exc) return False diff --git a/homeassistant/components/kulersky/light.py b/homeassistant/components/kulersky/light.py index 71dd4a158ca..9098975d500 100644 --- a/homeassistant/components/kulersky/light.py +++ b/homeassistant/components/kulersky/light.py @@ -1,5 +1,4 @@ """Kuler Sky light platform.""" -import asyncio from datetime import timedelta import logging from typing import Callable, List @@ -17,6 +16,7 @@ from homeassistant.components.light import ( ) from homeassistant.config_entries import ConfigEntry from homeassistant.const import EVENT_HOMEASSISTANT_STOP +from homeassistant.core import HomeAssistant from homeassistant.helpers.entity import Entity from homeassistant.helpers.event import async_track_time_interval from homeassistant.helpers.typing import HomeAssistantType @@ -30,14 +30,6 @@ SUPPORT_KULERSKY = SUPPORT_BRIGHTNESS | SUPPORT_COLOR | SUPPORT_WHITE_VALUE DISCOVERY_INTERVAL = timedelta(seconds=60) -PARALLEL_UPDATES = 0 - - -def check_light(light: pykulersky.Light): - """Attempt to connect to this light and read the color.""" - light.connect() - light.get_color() - async def async_setup_entry( hass: HomeAssistantType, @@ -47,45 +39,26 @@ async def async_setup_entry( """Set up Kuler sky light devices.""" if DOMAIN not in hass.data: hass.data[DOMAIN] = {} - if "devices" not in hass.data[DOMAIN]: - hass.data[DOMAIN]["devices"] = set() - if "discovery" not in hass.data[DOMAIN]: - hass.data[DOMAIN]["discovery"] = asyncio.Lock() + if "addresses" not in hass.data[DOMAIN]: + hass.data[DOMAIN]["addresses"] = set() async def discover(*args): """Attempt to discover new lights.""" - # Since discovery needs to connect to all discovered bluetooth devices, and - # only rules out devices after a timeout, it can potentially take a long - # time. If there's already a discovery running, just skip this poll. - if hass.data[DOMAIN]["discovery"].locked(): - return + lights = await pykulersky.discover() - async with hass.data[DOMAIN]["discovery"]: - bluetooth_devices = await hass.async_add_executor_job( - pykulersky.discover_bluetooth_devices - ) + # Filter out already discovered lights + new_lights = [ + light + for light in lights + if light.address not in hass.data[DOMAIN]["addresses"] + ] - # Filter out already connected lights - new_devices = [ - device - for device in bluetooth_devices - if device["address"] not in hass.data[DOMAIN]["devices"] - ] + new_entities = [] + for light in new_lights: + hass.data[DOMAIN]["addresses"].add(light.address) + new_entities.append(KulerskyLight(light)) - for device in new_devices: - light = pykulersky.Light(device["address"], device["name"]) - try: - # If the connection fails, either this is not a Kuler Sky - # light, or it's bluetooth connection is currently locked - # by another device. If the vendor's app is connected to - # the light when home assistant tries to connect, this - # connection will fail. - await hass.async_add_executor_job(check_light, light) - except pykulersky.PykulerskyException: - continue - # The light has successfully connected - hass.data[DOMAIN]["devices"].add(device["address"]) - async_add_entities([KulerskyLight(light)], update_before_add=True) + async_add_entities(new_entities, update_before_add=True) # Start initial discovery hass.async_create_task(discover()) @@ -94,6 +67,11 @@ async def async_setup_entry( async_track_time_interval(hass, discover, DISCOVERY_INTERVAL) +async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry): + """Cleanup the Kuler sky integration.""" + hass.data.pop(DOMAIN, None) + + class KulerskyLight(LightEntity): """Representation of an Kuler Sky Light.""" @@ -103,21 +81,24 @@ class KulerskyLight(LightEntity): self._hs_color = None self._brightness = None self._white_value = None - self._available = True + self._available = None async def async_added_to_hass(self) -> None: """Run when entity about to be added to hass.""" self.async_on_remove( - self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, self.disconnect) + self.hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_STOP, self.async_will_remove_from_hass + ) ) - async def async_will_remove_from_hass(self) -> None: + async def async_will_remove_from_hass(self, *args) -> None: """Run when entity will be removed from hass.""" - await self.hass.async_add_executor_job(self.disconnect) - - def disconnect(self, *args) -> None: - """Disconnect the underlying device.""" - self._light.disconnect() + try: + await self._light.disconnect() + except pykulersky.PykulerskyException: + _LOGGER.debug( + "Exception disconnected from %s", self._light.address, exc_info=True + ) @property def name(self): @@ -168,7 +149,7 @@ class KulerskyLight(LightEntity): """Return True if entity is available.""" return self._available - def turn_on(self, **kwargs): + async def async_turn_on(self, **kwargs): """Instruct the light to turn on.""" default_hs = (0, 0) if self._hs_color is None else self._hs_color hue_sat = kwargs.get(ATTR_HS_COLOR, default_hs) @@ -187,28 +168,28 @@ class KulerskyLight(LightEntity): rgb = color_util.color_hsv_to_RGB(*hue_sat, brightness / 255 * 100) - self._light.set_color(*rgb, white_value) + await self._light.set_color(*rgb, white_value) - def turn_off(self, **kwargs): + async def async_turn_off(self, **kwargs): """Instruct the light to turn off.""" - self._light.set_color(0, 0, 0, 0) + await self._light.set_color(0, 0, 0, 0) - def update(self): + async def async_update(self): """Fetch new state data for this light.""" try: - if not self._light.connected: - self._light.connect() + if not self._available: + await self._light.connect() # pylint: disable=invalid-name - r, g, b, w = self._light.get_color() + r, g, b, w = await self._light.get_color() except pykulersky.PykulerskyException as exc: if self._available: _LOGGER.warning("Unable to connect to %s: %s", self._light.address, exc) self._available = False return - if not self._available: - _LOGGER.info("Reconnected to %s", self.entity_id) - self._available = True + if self._available is False: + _LOGGER.info("Reconnected to %s", self._light.address) + self._available = True hsv = color_util.color_RGB_to_hsv(r, g, b) self._hs_color = hsv[:2] self._brightness = int(round((hsv[2] / 100) * 255)) diff --git a/homeassistant/components/kulersky/manifest.json b/homeassistant/components/kulersky/manifest.json index 4f445e4fc18..b690d94e8d4 100644 --- a/homeassistant/components/kulersky/manifest.json +++ b/homeassistant/components/kulersky/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/kulersky", "requirements": [ - "pykulersky==0.4.0" + "pykulersky==0.5.2" ], "codeowners": [ "@emlove" diff --git a/requirements_all.txt b/requirements_all.txt index 64071253d2f..5253f58a9ee 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1477,7 +1477,7 @@ pykmtronic==0.0.3 pykodi==0.2.1 # homeassistant.components.kulersky -pykulersky==0.4.0 +pykulersky==0.5.2 # homeassistant.components.kwb pykwb==0.0.8 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 9d50753b944..45c06066e20 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -779,7 +779,7 @@ pykmtronic==0.0.3 pykodi==0.2.1 # homeassistant.components.kulersky -pykulersky==0.4.0 +pykulersky==0.5.2 # homeassistant.components.lastfm pylast==4.1.0 diff --git a/tests/components/kulersky/test_config_flow.py b/tests/components/kulersky/test_config_flow.py index 24f3f9a010e..c6933a01d3a 100644 --- a/tests/components/kulersky/test_config_flow.py +++ b/tests/components/kulersky/test_config_flow.py @@ -1,5 +1,5 @@ """Test the Kuler Sky config flow.""" -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pykulersky @@ -16,14 +16,12 @@ async def test_flow_success(hass): assert result["type"] == "form" assert result["errors"] is None + light = MagicMock(spec=pykulersky.Light) + light.address = "AA:BB:CC:11:22:33" + light.name = "Bedroom" with patch( - "homeassistant.components.kulersky.config_flow.pykulersky.discover_bluetooth_devices", - return_value=[ - { - "address": "AA:BB:CC:11:22:33", - "name": "Bedroom", - } - ], + "homeassistant.components.kulersky.config_flow.pykulersky.discover", + return_value=[light], ), patch( "homeassistant.components.kulersky.async_setup", return_value=True ) as mock_setup, patch( @@ -54,7 +52,7 @@ async def test_flow_no_devices_found(hass): assert result["errors"] is None with patch( - "homeassistant.components.kulersky.config_flow.pykulersky.discover_bluetooth_devices", + "homeassistant.components.kulersky.config_flow.pykulersky.discover", return_value=[], ), patch( "homeassistant.components.kulersky.async_setup", return_value=True @@ -84,7 +82,7 @@ async def test_flow_exceptions_caught(hass): assert result["errors"] is None with patch( - "homeassistant.components.kulersky.config_flow.pykulersky.discover_bluetooth_devices", + "homeassistant.components.kulersky.config_flow.pykulersky.discover", side_effect=pykulersky.PykulerskyException("TEST"), ), patch( "homeassistant.components.kulersky.async_setup", return_value=True diff --git a/tests/components/kulersky/test_light.py b/tests/components/kulersky/test_light.py index fd5db92908b..9dd13fad18b 100644 --- a/tests/components/kulersky/test_light.py +++ b/tests/components/kulersky/test_light.py @@ -1,5 +1,4 @@ """Test the Kuler Sky lights.""" -import asyncio from unittest.mock import MagicMock, patch import pykulersky @@ -45,28 +44,17 @@ async def mock_light(hass, mock_entry): light = MagicMock(spec=pykulersky.Light) light.address = "AA:BB:CC:11:22:33" light.name = "Bedroom" - light.connected = False + light.connect.return_value = True + light.get_color.return_value = (0, 0, 0, 0) with patch( - "homeassistant.components.kulersky.light.pykulersky.discover_bluetooth_devices", - return_value=[ - { - "address": "AA:BB:CC:11:22:33", - "name": "Bedroom", - } - ], + "homeassistant.components.kulersky.light.pykulersky.discover", + return_value=[light], ): - with patch( - "homeassistant.components.kulersky.light.pykulersky.Light", - return_value=light, - ), patch.object(light, "connect") as mock_connect, patch.object( - light, "get_color", return_value=(0, 0, 0, 0) - ): - mock_entry.add_to_hass(hass) - await hass.config_entries.async_setup(mock_entry.entry_id) - await hass.async_block_till_done() + mock_entry.add_to_hass(hass) + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() - assert mock_connect.called - light.connected = True + assert light.connect.called yield light @@ -82,113 +70,34 @@ async def test_init(hass, mock_light): | SUPPORT_WHITE_VALUE, } - with patch.object(hass.loop, "stop"), patch.object( - mock_light, "disconnect" - ) as mock_disconnect: + with patch.object(hass.loop, "stop"): await hass.async_stop() await hass.async_block_till_done() - assert mock_disconnect.called - - -async def test_discovery_lock(hass, mock_entry): - """Test discovery lock.""" - await setup.async_setup_component(hass, "persistent_notification", {}) - - discovery_finished = None - first_discovery_started = asyncio.Event() - - async def mock_discovery(*args): - """Block to simulate multiple discovery calls while one still running.""" - nonlocal discovery_finished - if discovery_finished: - first_discovery_started.set() - await discovery_finished.wait() - return [] - - with patch( - "homeassistant.components.kulersky.light.pykulersky.discover_bluetooth_devices", - return_value=[], - ), patch( - "homeassistant.components.kulersky.light.async_track_time_interval", - ) as mock_track_time_interval: - mock_entry.add_to_hass(hass) - await hass.config_entries.async_setup(mock_entry.entry_id) - await hass.async_block_till_done() - - with patch.object( - hass, "async_add_executor_job", side_effect=mock_discovery - ) as mock_run_discovery: - discovery_coroutine = mock_track_time_interval.call_args[0][1] - - discovery_finished = asyncio.Event() - - # Schedule multiple discoveries - hass.async_create_task(discovery_coroutine()) - hass.async_create_task(discovery_coroutine()) - hass.async_create_task(discovery_coroutine()) - - # Wait until the first discovery call is blocked - await first_discovery_started.wait() - - # Unblock the first discovery - discovery_finished.set() - - # Flush the remaining jobs - await hass.async_block_till_done() - - # The discovery method should only have been called once - mock_run_discovery.assert_called_once() - - -async def test_discovery_connection_error(hass, mock_entry): - """Test that invalid devices are skipped.""" - await setup.async_setup_component(hass, "persistent_notification", {}) - - light = MagicMock(spec=pykulersky.Light) - light.address = "AA:BB:CC:11:22:33" - light.name = "Bedroom" - light.connected = False - with patch( - "homeassistant.components.kulersky.light.pykulersky.discover_bluetooth_devices", - return_value=[ - { - "address": "AA:BB:CC:11:22:33", - "name": "Bedroom", - } - ], - ): - with patch( - "homeassistant.components.kulersky.light.pykulersky.Light" - ) as mockdevice, patch.object( - light, "connect", side_effect=pykulersky.PykulerskyException - ): - mockdevice.return_value = light - mock_entry.add_to_hass(hass) - await hass.config_entries.async_setup(mock_entry.entry_id) - await hass.async_block_till_done() - - # Assert entity was not added - state = hass.states.get("light.bedroom") - assert state is None + assert mock_light.disconnect.called async def test_remove_entry(hass, mock_light, mock_entry): """Test platform setup.""" - with patch.object(mock_light, "disconnect") as mock_disconnect: - await hass.config_entries.async_remove(mock_entry.entry_id) + await hass.config_entries.async_remove(mock_entry.entry_id) - assert mock_disconnect.called + assert mock_light.disconnect.called + + +async def test_remove_entry_exceptions_caught(hass, mock_light, mock_entry): + """Assert that disconnect exceptions are caught.""" + mock_light.disconnect.side_effect = pykulersky.PykulerskyException("Mock error") + await hass.config_entries.async_remove(mock_entry.entry_id) + + assert mock_light.disconnect.called async def test_update_exception(hass, mock_light): """Test platform setup.""" await setup.async_setup_component(hass, "persistent_notification", {}) - with patch.object( - mock_light, "get_color", side_effect=pykulersky.PykulerskyException - ): - await hass.helpers.entity_component.async_update_entity("light.bedroom") + mock_light.get_color.side_effect = pykulersky.PykulerskyException + await hass.helpers.entity_component.async_update_entity("light.bedroom") state = hass.states.get("light.bedroom") assert state is not None assert state.state == STATE_UNAVAILABLE @@ -196,69 +105,59 @@ async def test_update_exception(hass, mock_light): async def test_light_turn_on(hass, mock_light): """Test KulerSkyLight turn_on.""" - with patch.object(mock_light, "set_color") as mock_set_color, patch.object( - mock_light, "get_color", return_value=(255, 255, 255, 255) - ): - await hass.services.async_call( - "light", - "turn_on", - {ATTR_ENTITY_ID: "light.bedroom"}, - blocking=True, - ) - await hass.async_block_till_done() - mock_set_color.assert_called_with(255, 255, 255, 255) + mock_light.get_color.return_value = (255, 255, 255, 255) + await hass.services.async_call( + "light", + "turn_on", + {ATTR_ENTITY_ID: "light.bedroom"}, + blocking=True, + ) + await hass.async_block_till_done() + mock_light.set_color.assert_called_with(255, 255, 255, 255) - with patch.object(mock_light, "set_color") as mock_set_color, patch.object( - mock_light, "get_color", return_value=(50, 50, 50, 255) - ): - await hass.services.async_call( - "light", - "turn_on", - {ATTR_ENTITY_ID: "light.bedroom", ATTR_BRIGHTNESS: 50}, - blocking=True, - ) - await hass.async_block_till_done() - mock_set_color.assert_called_with(50, 50, 50, 255) + mock_light.get_color.return_value = (50, 50, 50, 255) + await hass.services.async_call( + "light", + "turn_on", + {ATTR_ENTITY_ID: "light.bedroom", ATTR_BRIGHTNESS: 50}, + blocking=True, + ) + await hass.async_block_till_done() + mock_light.set_color.assert_called_with(50, 50, 50, 255) - with patch.object(mock_light, "set_color") as mock_set_color, patch.object( - mock_light, "get_color", return_value=(50, 45, 25, 255) - ): - await hass.services.async_call( - "light", - "turn_on", - {ATTR_ENTITY_ID: "light.bedroom", ATTR_HS_COLOR: (50, 50)}, - blocking=True, - ) - await hass.async_block_till_done() + mock_light.get_color.return_value = (50, 45, 25, 255) + await hass.services.async_call( + "light", + "turn_on", + {ATTR_ENTITY_ID: "light.bedroom", ATTR_HS_COLOR: (50, 50)}, + blocking=True, + ) + await hass.async_block_till_done() - mock_set_color.assert_called_with(50, 45, 25, 255) + mock_light.set_color.assert_called_with(50, 45, 25, 255) - with patch.object(mock_light, "set_color") as mock_set_color, patch.object( - mock_light, "get_color", return_value=(220, 201, 110, 180) - ): - await hass.services.async_call( - "light", - "turn_on", - {ATTR_ENTITY_ID: "light.bedroom", ATTR_WHITE_VALUE: 180}, - blocking=True, - ) - await hass.async_block_till_done() - mock_set_color.assert_called_with(50, 45, 25, 180) + mock_light.get_color.return_value = (220, 201, 110, 180) + await hass.services.async_call( + "light", + "turn_on", + {ATTR_ENTITY_ID: "light.bedroom", ATTR_WHITE_VALUE: 180}, + blocking=True, + ) + await hass.async_block_till_done() + mock_light.set_color.assert_called_with(50, 45, 25, 180) async def test_light_turn_off(hass, mock_light): """Test KulerSkyLight turn_on.""" - with patch.object(mock_light, "set_color") as mock_set_color, patch.object( - mock_light, "get_color", return_value=(0, 0, 0, 0) - ): - await hass.services.async_call( - "light", - "turn_off", - {ATTR_ENTITY_ID: "light.bedroom"}, - blocking=True, - ) - await hass.async_block_till_done() - mock_set_color.assert_called_with(0, 0, 0, 0) + mock_light.get_color.return_value = (0, 0, 0, 0) + await hass.services.async_call( + "light", + "turn_off", + {ATTR_ENTITY_ID: "light.bedroom"}, + blocking=True, + ) + await hass.async_block_till_done() + mock_light.set_color.assert_called_with(0, 0, 0, 0) async def test_light_update(hass, mock_light): @@ -275,12 +174,10 @@ async def test_light_update(hass, mock_light): } # Test an exception during discovery - with patch.object( - mock_light, "get_color", side_effect=pykulersky.PykulerskyException("TEST") - ): - utcnow = utcnow + SCAN_INTERVAL - async_fire_time_changed(hass, utcnow) - await hass.async_block_till_done() + mock_light.get_color.side_effect = pykulersky.PykulerskyException("TEST") + utcnow = utcnow + SCAN_INTERVAL + async_fire_time_changed(hass, utcnow) + await hass.async_block_till_done() state = hass.states.get("light.bedroom") assert state.state == STATE_UNAVAILABLE @@ -291,14 +188,11 @@ async def test_light_update(hass, mock_light): | SUPPORT_WHITE_VALUE, } - with patch.object( - mock_light, - "get_color", - return_value=(80, 160, 200, 240), - ): - utcnow = utcnow + SCAN_INTERVAL - async_fire_time_changed(hass, utcnow) - await hass.async_block_till_done() + mock_light.get_color.side_effect = None + mock_light.get_color.return_value = (80, 160, 200, 240) + utcnow = utcnow + SCAN_INTERVAL + async_fire_time_changed(hass, utcnow) + await hass.async_block_till_done() state = hass.states.get("light.bedroom") assert state.state == STATE_ON