Enhance Dynalite Integration after review

homeassistant/components/dynalite/__init__.py

@@ -1,24 +1,75 @@
 """Support for the Dynalite networks."""
-from dynalite_devices_lib import BRIDGE_CONFIG_SCHEMA
 import voluptuous as vol
 
 from homeassistant import config_entries
 from homeassistant.const import CONF_HOST
+from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers import config_validation as cv
 
 # 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
+from .const import (
+    CONF_ACTIVE,
+    CONF_AREA,
+    CONF_AUTO_DISCOVER,
+    CONF_BRIDGES,
+    CONF_CHANNEL,
+    CONF_DEFAULT,
+    CONF_FADE,
+    CONF_NAME,
+    CONF_POLLTIMER,
+    CONF_PORT,
+    DEFAULT_NAME,
+    DEFAULT_PORT,
+    DOMAIN,
+    LOGGER,
+)
+
+
+def num_string(value):
+    """Test if value is a string of digits, aka an integer."""
+    new_value = str(value)
+    if new_value.isdigit():
+        return new_value
+    raise vol.Invalid("Not a string with numbers")
+
+
+CHANNEL_DATA_SCHEMA = vol.Schema(
+    {vol.Optional(CONF_NAME): cv.string, vol.Optional(CONF_FADE): vol.Coerce(float)}
+)
+
+CHANNEL_SCHEMA = vol.Schema({num_string: CHANNEL_DATA_SCHEMA})
+
+AREA_DATA_SCHEMA = vol.Schema(
+    {
+        vol.Required(CONF_NAME): cv.string,
+        vol.Optional(CONF_FADE): vol.Coerce(float),
+        vol.Optional(CONF_CHANNEL): CHANNEL_SCHEMA,
+    },
+)
+
+AREA_SCHEMA = vol.Schema({num_string: vol.Any(AREA_DATA_SCHEMA, None)})
+
+PLATFORM_DEFAULTS_SCHEMA = vol.Schema({vol.Optional(CONF_FADE): vol.Coerce(float)})
+
+
+BRIDGE_SCHEMA = vol.Schema(
+    {
+        vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
+        vol.Required(CONF_HOST): cv.string,
+        vol.Optional(CONF_PORT, default=DEFAULT_PORT): int,
+        vol.Optional(CONF_AUTO_DISCOVER, default=False): vol.Coerce(bool),
+        vol.Optional(CONF_POLLTIMER, default=1.0): vol.Coerce(float),
+        vol.Optional(CONF_AREA): AREA_SCHEMA,
+        vol.Optional(CONF_DEFAULT): PLATFORM_DEFAULTS_SCHEMA,
+        vol.Optional(CONF_ACTIVE, default=False): vol.Coerce(bool),
+    }
+)
 
 CONFIG_SCHEMA = vol.Schema(
     {
         DOMAIN: vol.Schema(
-            {
-                vol.Optional(CONF_BRIDGES): vol.All(
-                    cv.ensure_list, [BRIDGE_CONFIG_SCHEMA]
-                )
-            }
+            {vol.Optional(CONF_BRIDGES): vol.All(cv.ensure_list, [BRIDGE_SCHEMA])}
         )
     },
     extra=vol.ALLOW_EXTRA,
@@ -35,9 +86,6 @@ async def async_setup(hass, config):
         conf = {}
 
     hass.data[DOMAIN] = {}
-    hass.data[DOMAIN][DATA_CONFIGS] = {}
-
-    configured = configured_hosts(hass)
 
     # User has configured bridges
     if CONF_BRIDGES not in conf:
@@ -47,20 +95,13 @@ async def async_setup(hass, config):
 
     for bridge_conf in bridges:
         host = bridge_conf[CONF_HOST]
-        LOGGER.debug("async_setup host=%s conf=%s", host, bridge_conf)
-
-        # 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
+        LOGGER.debug("Starting config entry flow host=%s conf=%s", host, bridge_conf)
 
         hass.async_create_task(
             hass.config_entries.flow.async_init(
                 DOMAIN,
                 context={"source": config_entries.SOURCE_IMPORT},
-                data={CONF_HOST: bridge_conf[CONF_HOST]},
+                data=bridge_conf,
             )
         )
 
@@ -69,25 +110,29 @@ 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
+    LOGGER.debug("Setting up entry %s", entry.data)
 
-    bridge = DynaliteBridge(hass, entry)
+    bridge = DynaliteBridge(hass, entry.data)
 
     if not await bridge.async_setup():
-        LOGGER.error("bridge.async_setup failed")
+        LOGGER.error("Could not set up bridge for entry %s", entry.data)
         return False
+
+    if not await bridge.try_connection():
+        LOGGER.errot("Could not connect with entry %s", entry)
+        raise ConfigEntryNotReady
+
     hass.data[DOMAIN][entry.entry_id] = bridge
+
+    hass.async_create_task(
+        hass.config_entries.async_forward_entry_setup(entry, "light")
+    )
     return True
 
 
 async def async_unload_entry(hass, entry):
     """Unload a config entry."""
-    LOGGER.error("async_unload_entry %s", entry.data)
-    bridge = hass.data[DOMAIN].pop(entry.entry_id)
-    return await bridge.async_reset()
+    LOGGER.debug("Unloading entry %s", entry.data)
+    hass.data[DOMAIN].pop(entry.entry_id)
+    result = await hass.config_entries.async_forward_entry_unload(entry, "light")
+    return result

homeassistant/components/dynalite/bridge.py

@@ -1,118 +1,82 @@
 """Code to handle a Dynalite bridge."""
 
+import asyncio
+
 from dynalite_devices_lib import DynaliteDevices
-from dynalite_lib import CONF_ALL
 
-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."""
+from .const import CONF_ALL, CONF_HOST, LOGGER
 
-    def __init__(self, message):
-        """Initialize the exception."""
-        super().__init__()
-        self.message = message
+CONNECT_TIMEOUT = 30
+CONNECT_INTERVAL = 1
 
 
 class DynaliteBridge:
     """Manages a single Dynalite bridge."""
 
-    def __init__(self, hass, config_entry):
+    def __init__(self, hass, config):
         """Initialize the system based on host parameter."""
-        self.config_entry = config_entry
         self.hass = hass
         self.area = {}
-        self.async_add_entities = None
-        self.waiting_entities = []
-        self.all_entities = {}
-        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]
+        self.async_add_devices = None
+        self.waiting_devices = []
+        self.host = config[CONF_HOST]
         # Configure the dynalite devices
         self.dynalite_devices = DynaliteDevices(
-            config=self.config,
-            newDeviceFunc=self.add_devices,
+            config=config,
+            newDeviceFunc=self.add_devices_when_registered,
             updateDeviceFunc=self.update_device,
         )
 
-    async def async_setup(self, tries=0):
+    async def async_setup(self):
         """Set up a Dynalite bridge."""
         # Configure the dynalite devices
-        await self.dynalite_devices.async_setup()
-
-        self.hass.async_create_task(
-            self.hass.config_entries.async_forward_entry_setup(
-                self.config_entry, "light"
-            )
-        )
-
-        return True
-
-    @callback
-    def add_devices(self, devices):
-        """Call when devices should be added to home assistant."""
-        added_entities = []
+        return await self.dynalite_devices.async_setup()
 
-        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))
+
+    async def try_connection(self):
+        """Try to connect to dynalite with timeout."""
+        # Currently by polling. Future - will need to change the library to be proactive
+        for _ in range(0, CONNECT_TIMEOUT):
+            if self.dynalite_devices.available:
+                return True
+            await asyncio.sleep(CONNECT_INTERVAL)
+        return False
 
     @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)
-
-    async def async_reset(self):
-        """Reset this bridge to default state.
-
-        Will cancel any scheduled setup retry and will unload
-        the config entry.
-        """
-        result = await self.hass.config_entries.async_forward_entry_unload(
-            self.config_entry, "light"
-        )
-        # None and True are OK
-        return result
+            self.waiting_devices.extend(devices)

homeassistant/components/dynalite/config_flow.py

@@ -1,19 +1,9 @@
 """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 .bridge import DynaliteBridge
+from .const import DOMAIN, LOGGER  # pylint: disable=unused-import
 
 
 class DynaliteFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
@@ -30,29 +20,16 @@ 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]
-        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})
+        LOGGER.debug("Starting async_step_import - %s", import_info)
+        host = import_info[CONF_HOST]
+        await self.async_set_unique_id(host)
+        self._abort_if_unique_id_configured(import_info)
+        # New entry
+        bridge = DynaliteBridge(self.hass, import_info)
+        if not await bridge.async_setup():
+            LOGGER.error("Unable to setup bridge - import info=%s", import_info)
+            return self.async_abort(reason="bridge_setup_failed")
+        if not await bridge.try_connection():
+            return self.async_abort(reason="no_connection")
+        LOGGER.debug("Creating entry for the bridge - %s", import_info)
+        return self.async_create_entry(title=host, data=import_info)