Prevent Zerproc leaving open unnecessary connections (#47401)

* Zerproc: Prevent leaving open unnecessary connections

* Fix config entry unloading

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(
             *[

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(
+    for light in new_lights:
-        None, await asyncio.gather(*(connect_light(light) for light in new_lights))
+        hass.data[DOMAIN]["addresses"].add(light.address)
-    )
+        entities.append(ZerprocLight(light))
-    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))
 
     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)

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"

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

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

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):