Enhance Dynalite Integration after review

homeassistant/components/dynalite/__init__.py

@@ -8,7 +8,6 @@
 
 # Loading the config flow file will register the flow
 from .bridge import DynaliteBridge
-from .config_flow import configured_hosts
 from .const import CONF_BRIDGES, DATA_CONFIGS, DOMAIN, LOGGER
 
 CONFIG_SCHEMA = vol.Schema(
@@ -37,8 +36,6 @@ async def async_setup(hass, config):
     hass.data[DOMAIN] = {}
     hass.data[DOMAIN][DATA_CONFIGS] = {}
 
-    configured = configured_hosts(hass)
-
     # User has configured bridges
     if CONF_BRIDGES not in conf:
         return True
@@ -52,10 +49,6 @@ async def async_setup(hass, config):
         # Store config in hass.data so the config entry can find it
         hass.data[DOMAIN][DATA_CONFIGS][host] = bridge_conf
 
-        if host in configured:
-            LOGGER.debug("async_setup host=%s already configured", host)
-            continue
-
         hass.async_create_task(
             hass.config_entries.flow.async_init(
                 DOMAIN,
@@ -70,12 +63,6 @@ async def async_setup(hass, config):
 async def async_setup_entry(hass, entry):
     """Set up a bridge from a config entry."""
     LOGGER.debug("__init async_setup_entry %s", entry.data)
-    host = entry.data[CONF_HOST]
-    config = hass.data[DOMAIN][DATA_CONFIGS].get(host)
-
-    if config is None:
-        LOGGER.error("__init async_setup_entry empty config for host %s", host)
-        return False
 
     bridge = DynaliteBridge(hass, entry)
 

homeassistant/components/dynalite/bridge.py

@@ -5,18 +5,9 @@
 
 from homeassistant.const import CONF_HOST
 from homeassistant.core import callback
+from homeassistant.helpers.dispatcher import async_dispatcher_send
 
 from .const import DATA_CONFIGS, DOMAIN, LOGGER
-from .light import DynaliteLight
-
-
-class BridgeError(Exception):
-    """Class to throw exceptions from DynaliteBridge."""
-
-    def __init__(self, message):
-        """Initialize the exception."""
-        super().__init__()
-        self.message = message
 
 
 class DynaliteBridge:
@@ -27,19 +18,15 @@ def __init__(self, hass, config_entry):
         self.config_entry = config_entry
         self.hass = hass
         self.area = {}
-        self.async_add_entities = None
-        self.waiting_entities = []
-        self.all_entities = {}
+        self.async_add_devices = None
+        self.waiting_devices = []
         self.config = None
         self.host = config_entry.data[CONF_HOST]
-        if self.host not in hass.data[DOMAIN][DATA_CONFIGS]:
-            LOGGER.info("invalid host - %s", self.host)
-            raise BridgeError(f"invalid host - {self.host}")
         self.config = hass.data[DOMAIN][DATA_CONFIGS][self.host]
         # Configure the dynalite devices
         self.dynalite_devices = DynaliteDevices(
             config=self.config,
-            newDeviceFunc=self.add_devices,
+            newDeviceFunc=self.add_devices_when_registered,
             updateDeviceFunc=self.update_device,
         )
 
@@ -56,54 +43,42 @@ async def async_setup(self, tries=0):
 
         return True
 
-    @callback
-    def add_devices(self, devices):
-        """Call when devices should be added to home assistant."""
-        added_entities = []
-
-        for device in devices:
-            if device.category == "light":
-                entity = DynaliteLight(device, self)
-            else:
-                LOGGER.debug("Illegal device category %s", device.category)
-                continue
-            added_entities.append(entity)
-            self.all_entities[entity.unique_id] = entity
-
-        if added_entities:
-            self.add_entities_when_registered(added_entities)
+    def update_signal(self, device=None):
+        """Create signal to use to trigger entity update."""
+        if device:
+            signal = f"dynalite-update-{self.host}-{device.unique_id}"
+        else:
+            signal = f"dynalite-update-{self.host}"
+        return signal
 
     @callback
     def update_device(self, device):
         """Call when a device or all devices should be updated."""
         if device == CONF_ALL:
             # This is used to signal connection or disconnection, so all devices may become available or not.
-            if self.dynalite_devices.available:
-                LOGGER.info("Connected to dynalite host")
-            else:
-                LOGGER.info("Disconnected from dynalite host")
-            for uid in self.all_entities:
-                self.all_entities[uid].try_schedule_ha()
+            log_string = (
+                "Connected" if self.dynalite_devices.available else "Disconnected"
+            )
+            LOGGER.info("%s to dynalite host", log_string)
+            async_dispatcher_send(self.hass, self.update_signal())
         else:
-            uid = device.unique_id
-            if uid in self.all_entities:
-                self.all_entities[uid].try_schedule_ha()
+            async_dispatcher_send(self.hass, self.update_signal(device))
 
     @callback
-    def register_add_entities(self, async_add_entities):
+    def register_add_devices(self, async_add_devices):
         """Add an async_add_entities for a category."""
-        self.async_add_entities = async_add_entities
-        if self.waiting_entities:
-            self.async_add_entities(self.waiting_entities)
+        self.async_add_devices = async_add_devices
+        if self.waiting_devices:
+            self.async_add_devices(self.waiting_devices)
 
-    def add_entities_when_registered(self, entities):
-        """Add the entities to HA if async_add_entities was registered, otherwise queue until it is."""
-        if not entities:
+    def add_devices_when_registered(self, devices):
+        """Add the devices to HA if the add devices callback was registered, otherwise queue until it is."""
+        if not devices:
             return
-        if self.async_add_entities:
-            self.async_add_entities(entities)
+        if self.async_add_devices:
+            self.async_add_devices(devices)
         else:  # handle it later when it is registered
-            self.waiting_entities.extend(entities)
+            self.waiting_devices.extend(devices)
 
     async def async_reset(self):
         """Reset this bridge to default state.

homeassistant/components/dynalite/config_flow.py

@@ -1,19 +1,8 @@
 """Config flow to configure Dynalite hub."""
-import asyncio
-
 from homeassistant import config_entries
 from homeassistant.const import CONF_HOST
-from homeassistant.core import callback
-
-from .const import DOMAIN, LOGGER
-
 
-@callback
-def configured_hosts(hass):
-    """Return a set of the configured hosts."""
-    return set(
-        entry.data[CONF_HOST] for entry in hass.config_entries.async_entries(DOMAIN)
-    )
+from .const import DOMAIN, LOGGER  # pylint: disable=unused-import
 
 
 class DynaliteFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
@@ -31,28 +20,13 @@ def __init__(self):
     async def async_step_import(self, import_info):
         """Import a new bridge as a config entry."""
         LOGGER.debug("async_step_import - %s", import_info)
-        host = self.context[CONF_HOST] = import_info[CONF_HOST]
+        host = import_info[CONF_HOST]
+        await self.async_set_unique_id(host)
+        self._abort_if_unique_id_configured()
         return await self._entry_from_bridge(host)
 
     async def _entry_from_bridge(self, host):
         """Return a config entry from an initialized bridge."""
         LOGGER.debug("entry_from_bridge - %s", host)
-        # Remove all other entries of hubs with same ID or host
-
-        same_hub_entries = [
-            entry.entry_id
-            for entry in self.hass.config_entries.async_entries(DOMAIN)
-            if entry.data[CONF_HOST] == host
-        ]
-
-        LOGGER.debug("entry_from_bridge same_hub - %s", same_hub_entries)
-
-        if same_hub_entries:
-            await asyncio.wait(
-                [
-                    self.hass.config_entries.async_remove(entry_id)
-                    for entry_id in same_hub_entries
-                ]
-            )
 
         return self.async_create_entry(title=host, data={CONF_HOST: host})

homeassistant/components/dynalite/light.py

@@ -1,6 +1,7 @@
 """Support for Dynalite channels as lights."""
 from homeassistant.components.light import SUPPORT_BRIGHTNESS, Light
 from homeassistant.core import callback
+from homeassistant.helpers.dispatcher import async_dispatcher_connect
 
 from .const import DOMAIN, LOGGER
 
@@ -9,7 +10,17 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     """Record the async_add_entities function to add them later when received from Dynalite."""
     LOGGER.debug("async_setup_entry light entry = %s", config_entry.data)
     bridge = hass.data[DOMAIN][config_entry.entry_id]
-    bridge.register_add_entities(async_add_entities)
+
+    @callback
+    def async_add_lights(devices):
+        added_lights = []
+        for device in devices:
+            if device.category == "light":
+                added_lights.append(DynaliteLight(device, bridge))
+        if added_lights:
+            async_add_entities(added_lights)
+
+    bridge.register_add_devices(async_add_lights)
 
 
 class DynaliteLight(Light):
@@ -77,8 +88,15 @@ def supported_features(self):
         """Flag supported features."""
         return SUPPORT_BRIGHTNESS
 
-    @callback
-    def try_schedule_ha(self):
-        """Schedule update HA state if configured."""
-        if self.hass:
-            self.schedule_update_ha_state()
+    async def async_added_to_hass(self):
+        """Added to hass so need to register to dispatch."""
+        # register for device specific update
+        async_dispatcher_connect(
+            self.hass,
+            self._bridge.update_signal(self._device),
+            self.async_schedule_update_ha_state,
+        )
+        # register for wide update
+        async_dispatcher_connect(
+            self.hass, self._bridge.update_signal(), self.async_schedule_update_ha_state
+        )

tests/components/dynalite/test_bridge.py

@@ -2,10 +2,9 @@
 from unittest.mock import Mock, call, patch
 
 from dynalite_lib import CONF_ALL
-import pytest
 
 from homeassistant.components.dynalite import DATA_CONFIGS, DOMAIN
-from homeassistant.components.dynalite.bridge import BridgeError, DynaliteBridge
+from homeassistant.components.dynalite.bridge import DynaliteBridge
 
 from tests.common import mock_coro
 
@@ -31,18 +30,30 @@ async def test_bridge_setup():
     assert forward_entries == set(["light"])
 
 
-async def test_invalid_host():
-    """Test without host in hass.data."""
+async def test_update_device():
+    """Test a successful setup."""
     hass = Mock()
     entry = Mock()
     host = "1.2.3.4"
     entry.data = {"host": host}
-    hass.data = {DOMAIN: {DATA_CONFIGS: {}}}
+    hass.data = {DOMAIN: {DATA_CONFIGS: {host: {}}}}
+    dyn_bridge = DynaliteBridge(hass, entry)
+    async_dispatch = Mock()
 
-    dyn_bridge = None
-    with pytest.raises(BridgeError):
-        dyn_bridge = DynaliteBridge(hass, entry)
-    assert dyn_bridge is None
+    with patch(
+        "homeassistant.components.dynalite.bridge.async_dispatcher_send", async_dispatch
+    ):
+        dyn_bridge.update_device(CONF_ALL)
+        async_dispatch.assert_called_once()
+        assert async_dispatch.mock_calls[0] == call(hass, f"dynalite-update-{host}")
+        async_dispatch.reset_mock()
+        device = Mock
+        device.unique_id = "abcdef"
+        dyn_bridge.update_device(device)
+        async_dispatch.assert_called_once()
+        assert async_dispatch.mock_calls[0] == call(
+            hass, f"dynalite-update-{host}-{device.unique_id}"
+        )
 
 
 async def test_add_devices_then_register():
@@ -53,16 +64,19 @@ async def test_add_devices_then_register():
     entry.data = {"host": host}
     hass.data = {DOMAIN: {DATA_CONFIGS: {host: {}}}}
     dyn_bridge = DynaliteBridge(hass, entry)
-
+    # First test empty
+    dyn_bridge.add_devices_when_registered([])
+    assert not dyn_bridge.waiting_devices
+    # Now with devices
     device1 = Mock()
     device1.category = "light"
     device2 = Mock()
     device2.category = "switch"
-    dyn_bridge.add_devices([device1, device2])
+    dyn_bridge.add_devices_when_registered([device1, device2])
     reg_func = Mock()
-    dyn_bridge.register_add_entities(reg_func)
+    dyn_bridge.register_add_devices(reg_func)
     reg_func.assert_called_once()
-    assert reg_func.mock_calls[0][1][0][0].device is device1
+    assert reg_func.mock_calls[0][1][0][0] is device1
 
 
 async def test_register_then_add_devices():
@@ -79,43 +93,10 @@ async def test_register_then_add_devices():
     device2 = Mock()
     device2.category = "switch"
     reg_func = Mock()
-    dyn_bridge.register_add_entities(reg_func)
-    dyn_bridge.add_devices([device1, device2])
+    dyn_bridge.register_add_devices(reg_func)
+    dyn_bridge.add_devices_when_registered([device1, device2])
     reg_func.assert_called_once()
-    assert reg_func.mock_calls[0][1][0][0].device is device1
-
-
-async def test_update_device():
-    """Test the update_device callback."""
-    hass = Mock()
-    entry = Mock()
-    host = "1.2.3.4"
-    entry.data = {"host": host}
-    hass.data = {DOMAIN: {DATA_CONFIGS: {host: {}}}}
-    dyn_bridge = DynaliteBridge(hass, entry)
-    with patch.object(dyn_bridge, "dynalite_devices") as devices_mock:
-        # Single device update
-        device1 = Mock()
-        device1.unique_id = "testing1"
-        device2 = Mock()
-        device2.unique_id = "testing2"
-        dyn_bridge.all_entities = {
-            device1.unique_id: device1,
-            device2.unique_id: device2,
-        }
-        dyn_bridge.update_device(device1)
-        device1.try_schedule_ha.assert_called_once()
-        device2.try_schedule_ha.assert_not_called()
-        # connected to network - all devices update
-        devices_mock.available = True
-        dyn_bridge.update_device(CONF_ALL)
-        assert device1.try_schedule_ha.call_count == 2
-        device2.try_schedule_ha.assert_called_once()
-        # disconnected from network - all devices update
-        devices_mock.available = False
-        dyn_bridge.update_device(CONF_ALL)
-        assert device1.try_schedule_ha.call_count == 3
-        assert device2.try_schedule_ha.call_count == 2
+    assert reg_func.mock_calls[0][1][0][0] is device1
 
 
 async def test_async_reset():