From a547d0fea2825a7f6a9943ade9bda09e72bb1b9b Mon Sep 17 00:00:00 2001 From: Emily Mills Date: Fri, 5 Mar 2021 13:14:03 -0600 Subject: [PATCH] Prevent Zerproc leaving open unnecessary connections (#47401) * Zerproc: Prevent leaving open unnecessary connections * Fix config entry unloading --- homeassistant/components/zerproc/__init__.py | 6 ++++ homeassistant/components/zerproc/light.py | 33 ++++------------- .../components/zerproc/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/zerproc/test_light.py | 35 +++---------------- 6 files changed, 20 insertions(+), 60 deletions(-) diff --git a/homeassistant/components/zerproc/__init__.py b/homeassistant/components/zerproc/__init__.py index f3d2e9daebc..d6faaf7a981 100644 --- a/homeassistant/components/zerproc/__init__.py +++ b/homeassistant/components/zerproc/__init__.py @@ -20,6 +20,11 @@ async def async_setup(hass, config): async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): """Set up Zerproc from a config entry.""" + if DOMAIN not in hass.data: + hass.data[DOMAIN] = {} + if "addresses" not in hass.data[DOMAIN]: + hass.data[DOMAIN]["addresses"] = set() + for platform in PLATFORMS: hass.async_create_task( hass.config_entries.async_forward_entry_setup(entry, platform) @@ -30,6 +35,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry): async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry): """Unload a config entry.""" + hass.data.pop(DOMAIN, None) return all( await asyncio.gather( *[ diff --git a/homeassistant/components/zerproc/light.py b/homeassistant/components/zerproc/light.py index 89f60faf84e..bf9f917f230 100644 --- a/homeassistant/components/zerproc/light.py +++ b/homeassistant/components/zerproc/light.py @@ -1,5 +1,4 @@ """Zerproc light platform.""" -import asyncio from datetime import timedelta import logging from typing import Callable, List, Optional @@ -30,16 +29,6 @@ SUPPORT_ZERPROC = SUPPORT_BRIGHTNESS | SUPPORT_COLOR DISCOVERY_INTERVAL = timedelta(seconds=60) -async def connect_light(light: pyzerproc.Light) -> Optional[pyzerproc.Light]: - """Return the given light if it connects successfully.""" - try: - await light.connect() - except pyzerproc.ZerprocException: - _LOGGER.debug("Unable to connect to '%s'", light.address, exc_info=True) - return None - return light - - async def discover_entities(hass: HomeAssistant) -> List[Entity]: """Attempt to discover new lights.""" lights = await pyzerproc.discover() @@ -50,14 +39,9 @@ async def discover_entities(hass: HomeAssistant) -> List[Entity]: ] entities = [] - connected_lights = filter( - None, await asyncio.gather(*(connect_light(light) for light in new_lights)) - ) - for light in connected_lights: - # Double-check the light hasn't been added in the meantime - if light.address not in hass.data[DOMAIN]["addresses"]: - hass.data[DOMAIN]["addresses"].add(light.address) - entities.append(ZerprocLight(light)) + for light in new_lights: + hass.data[DOMAIN]["addresses"].add(light.address) + entities.append(ZerprocLight(light)) return entities @@ -68,11 +52,6 @@ async def async_setup_entry( async_add_entities: Callable[[List[Entity], bool], None], ) -> None: """Set up Zerproc light devices.""" - if DOMAIN not in hass.data: - hass.data[DOMAIN] = {} - if "addresses" not in hass.data[DOMAIN]: - hass.data[DOMAIN]["addresses"] = set() - warned = False async def discover(*args): @@ -120,7 +99,7 @@ class ZerprocLight(LightEntity): await self._light.disconnect() except pyzerproc.ZerprocException: _LOGGER.debug( - "Exception disconnected from %s", self.entity_id, exc_info=True + "Exception disconnecting from %s", self._light.address, exc_info=True ) @property @@ -198,11 +177,11 @@ class ZerprocLight(LightEntity): state = await self._light.get_state() except pyzerproc.ZerprocException: if self._available: - _LOGGER.warning("Unable to connect to %s", self.entity_id) + _LOGGER.warning("Unable to connect to %s", self._light.address) self._available = False return if self._available is False: - _LOGGER.info("Reconnected to %s", self.entity_id) + _LOGGER.info("Reconnected to %s", self._light.address) self._available = True self._is_on = state.is_on hsv = color_util.color_RGB_to_hsv(*state.color) diff --git a/homeassistant/components/zerproc/manifest.json b/homeassistant/components/zerproc/manifest.json index 54b70d78673..d2d00987ab7 100644 --- a/homeassistant/components/zerproc/manifest.json +++ b/homeassistant/components/zerproc/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/zerproc", "requirements": [ - "pyzerproc==0.4.7" + "pyzerproc==0.4.8" ], "codeowners": [ "@emlove" diff --git a/requirements_all.txt b/requirements_all.txt index f52f9b7e353..9b00a2b7267 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1920,7 +1920,7 @@ pyxeoma==1.4.1 pyzbar==0.1.7 # homeassistant.components.zerproc -pyzerproc==0.4.7 +pyzerproc==0.4.8 # homeassistant.components.qnap qnapstats==0.3.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index b60ce427b00..02bc7304084 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -993,7 +993,7 @@ pywemo==0.6.3 pywilight==0.0.68 # homeassistant.components.zerproc -pyzerproc==0.4.7 +pyzerproc==0.4.8 # homeassistant.components.rachio rachiopy==1.0.3 diff --git a/tests/components/zerproc/test_light.py b/tests/components/zerproc/test_light.py index 1f0c7652bfd..1dc608e18df 100644 --- a/tests/components/zerproc/test_light.py +++ b/tests/components/zerproc/test_light.py @@ -118,6 +118,8 @@ async def test_init(hass, mock_entry): assert mock_light_1.disconnect.called assert mock_light_2.disconnect.called + assert hass.data[DOMAIN]["addresses"] == {"AA:BB:CC:DD:EE:FF", "11:22:33:44:55:66"} + async def test_discovery_exception(hass, mock_entry): """Test platform setup.""" @@ -136,42 +138,15 @@ async def test_discovery_exception(hass, mock_entry): assert len(hass.data[DOMAIN]["addresses"]) == 0 -async def test_connect_exception(hass, mock_entry): - """Test platform setup.""" - await setup.async_setup_component(hass, "persistent_notification", {}) - - mock_entry.add_to_hass(hass) - - mock_light_1 = MagicMock(spec=pyzerproc.Light) - mock_light_1.address = "AA:BB:CC:DD:EE:FF" - mock_light_1.name = "LEDBlue-CCDDEEFF" - mock_light_1.is_connected.return_value = False - - mock_light_2 = MagicMock(spec=pyzerproc.Light) - mock_light_2.address = "11:22:33:44:55:66" - mock_light_2.name = "LEDBlue-33445566" - mock_light_2.is_connected.return_value = False - - with patch( - "homeassistant.components.zerproc.light.pyzerproc.discover", - return_value=[mock_light_1, mock_light_2], - ), patch.object( - mock_light_1, "connect", side_effect=pyzerproc.ZerprocException("TEST") - ): - await hass.config_entries.async_setup(mock_entry.entry_id) - await hass.async_block_till_done() - - # The exception connecting to light 1 should be captured, but light 2 - # should still be added - assert len(hass.data[DOMAIN]["addresses"]) == 1 - - async def test_remove_entry(hass, mock_light, mock_entry): """Test platform setup.""" + assert hass.data[DOMAIN]["addresses"] == {"AA:BB:CC:DD:EE:FF"} + with patch.object(mock_light, "disconnect") as mock_disconnect: await hass.config_entries.async_remove(mock_entry.entry_id) assert mock_disconnect.called + assert DOMAIN not in hass.data async def test_remove_entry_exceptions_caught(hass, mock_light, mock_entry):