From 9ed5b70d01a5562508880c7a89cc722b9460ac13 Mon Sep 17 00:00:00 2001 From: Robert Svensson Date: Mon, 3 Jun 2019 18:26:01 +0200 Subject: [PATCH] deCONZ migrate to SSDP discovery (#24252) * Migrate deCONZ to use new SSDP discovery Add new discovery info manufacturer URL to be able to separate Hue and deCONZ bridges * Mark deCONZ as migrated in Discovery component * Fix tests * Fix Hue discovery ignore deCONZ bridge * Less snake more badger * Mushroom * Fix indentation * Config flow ignore manufacturer url that is not philips --- .../components/deconz/config_flow.py | 21 +++++++++---- homeassistant/components/deconz/manifest.json | 5 ++++ homeassistant/components/deconz/strings.json | 6 ++-- .../components/discovery/__init__.py | 3 +- homeassistant/components/hue/config_flow.py | 6 ++++ homeassistant/components/hue/strings.json | 3 +- homeassistant/components/ssdp/__init__.py | 2 ++ homeassistant/generated/ssdp.py | 1 + tests/components/deconz/test_config_flow.py | 30 +++++++++++++++---- tests/components/hue/test_config_flow.py | 21 +++++++++++-- 10 files changed, 78 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/deconz/config_flow.py b/homeassistant/components/deconz/config_flow.py index 24eb3dd4d5d..cf172ad7991 100644 --- a/homeassistant/components/deconz/config_flow.py +++ b/homeassistant/components/deconz/config_flow.py @@ -9,12 +9,14 @@ from pydeconz.utils import ( async_discovery, async_get_api_key, async_get_bridgeid) from homeassistant import config_entries +from homeassistant.components.ssdp import ATTR_MANUFACTURERURL, ATTR_SERIAL from homeassistant.const import CONF_API_KEY, CONF_HOST, CONF_PORT from homeassistant.core import callback from homeassistant.helpers import aiohttp_client from .const import CONF_BRIDGEID, DEFAULT_PORT, DOMAIN +DECONZ_MANUFACTURERURL = 'http://www.dresden-elektronik.de' CONF_SERIAL = 'serial' @@ -149,12 +151,12 @@ class DeconzFlowHandler(config_entries.ConfigFlow): entry.data[CONF_HOST] = host self.hass.config_entries.async_update_entry(entry) - async def async_step_discovery(self, discovery_info): - """Prepare configuration for a discovered deCONZ bridge. + async def async_step_ssdp(self, discovery_info): + """Handle a discovered deCONZ bridge.""" + if discovery_info[ATTR_MANUFACTURERURL] != DECONZ_MANUFACTURERURL: + return self.async_abort(reason='not_deconz_bridge') - This flow is triggered by the discovery component. - """ - bridgeid = discovery_info[CONF_SERIAL] + bridgeid = discovery_info[ATTR_SERIAL] gateway_entries = configured_gateways(self.hass) if bridgeid in gateway_entries: @@ -162,10 +164,17 @@ class DeconzFlowHandler(config_entries.ConfigFlow): await self._update_entry(entry, discovery_info[CONF_HOST]) return self.async_abort(reason='updated_instance') + # pylint: disable=unsupported-assignment-operation + self.context[ATTR_SERIAL] = bridgeid + + if any(bridgeid == flow['context'][ATTR_SERIAL] + for flow in self._async_in_progress()): + return self.async_abort(reason='already_in_progress') + deconz_config = { CONF_HOST: discovery_info[CONF_HOST], CONF_PORT: discovery_info[CONF_PORT], - CONF_BRIDGEID: discovery_info[CONF_SERIAL] + CONF_BRIDGEID: bridgeid } return await self.async_step_import(deconz_config) diff --git a/homeassistant/components/deconz/manifest.json b/homeassistant/components/deconz/manifest.json index 08a01cd1379..56ea52b7693 100644 --- a/homeassistant/components/deconz/manifest.json +++ b/homeassistant/components/deconz/manifest.json @@ -6,6 +6,11 @@ "requirements": [ "pydeconz==59" ], + "ssdp": { + "manufacturer": [ + "Royal Philips Electronics" + ] + }, "dependencies": [], "codeowners": [ "@kane610" diff --git a/homeassistant/components/deconz/strings.json b/homeassistant/components/deconz/strings.json index 16177dbd3cc..d1c70793063 100644 --- a/homeassistant/components/deconz/strings.json +++ b/homeassistant/components/deconz/strings.json @@ -34,9 +34,11 @@ }, "abort": { "already_configured": "Bridge is already configured", + "already_in_progress": "Config flow for bridge is already in progress.", "no_bridges": "No deCONZ bridges discovered", - "updated_instance": "Updated deCONZ instance with new host address", - "one_instance_only": "Component only supports one deCONZ instance" + "not_deconz_bridge": "Not a deCONZ bridge", + "one_instance_only": "Component only supports one deCONZ instance", + "updated_instance": "Updated deCONZ instance with new host address" } } } diff --git a/homeassistant/components/discovery/__init__.py b/homeassistant/components/discovery/__init__.py index ee6a8590515..0541b5d223a 100644 --- a/homeassistant/components/discovery/__init__.py +++ b/homeassistant/components/discovery/__init__.py @@ -25,7 +25,6 @@ DOMAIN = 'discovery' SCAN_INTERVAL = timedelta(seconds=300) SERVICE_APPLE_TV = 'apple_tv' SERVICE_DAIKIN = 'daikin' -SERVICE_DECONZ = 'deconz' SERVICE_DLNA_DMR = 'dlna_dmr' SERVICE_ENIGMA2 = 'enigma2' SERVICE_FREEBOX = 'freebox' @@ -48,7 +47,6 @@ SERVICE_XIAOMI_GW = 'xiaomi_gw' CONFIG_ENTRY_HANDLERS = { SERVICE_DAIKIN: 'daikin', - SERVICE_DECONZ: 'deconz', 'google_cast': 'cast', SERVICE_HEOS: 'heos', SERVICE_TELLDUSLIVE: 'tellduslive', @@ -98,6 +96,7 @@ OPTIONAL_SERVICE_HANDLERS = { MIGRATED_SERVICE_HANDLERS = { 'axis': None, + 'deconz': None, 'esphome': None, 'ikea_tradfri': None, 'homekit': None, diff --git a/homeassistant/components/hue/config_flow.py b/homeassistant/components/hue/config_flow.py index 4167027bf89..9c81d144d1c 100644 --- a/homeassistant/components/hue/config_flow.py +++ b/homeassistant/components/hue/config_flow.py @@ -8,6 +8,7 @@ import async_timeout import voluptuous as vol from homeassistant import config_entries +from homeassistant.components.ssdp import ATTR_MANUFACTURERURL from homeassistant.core import callback from homeassistant.helpers import aiohttp_client @@ -15,6 +16,8 @@ from .bridge import get_bridge from .const import DOMAIN, LOGGER from .errors import AuthenticationRequired, CannotConnect +HUE_MANUFACTURERURL = 'http://www.philips.com' + @callback def configured_hosts(hass): @@ -143,6 +146,9 @@ class HueFlowHandler(config_entries.ConfigFlow): This flow is triggered by the SSDP component. It will check if the host is already configured and delegate to the import step if not. """ + if discovery_info[ATTR_MANUFACTURERURL] != HUE_MANUFACTURERURL: + return self.async_abort(reason='not_hue_bridge') + # Filter out emulated Hue if "HASS Bridge" in discovery_info.get('name', ''): return self.async_abort(reason='already_configured') diff --git a/homeassistant/components/hue/strings.json b/homeassistant/components/hue/strings.json index 079ac1a2b8d..78b990d5f42 100644 --- a/homeassistant/components/hue/strings.json +++ b/homeassistant/components/hue/strings.json @@ -24,7 +24,8 @@ "unknown": "Unknown error occurred", "cannot_connect": "Unable to connect to the bridge", "already_configured": "Bridge is already configured", - "already_in_progress": "Config flow for bridge is already in progress." + "already_in_progress": "Config flow for bridge is already in progress.", + "not_hue_bridge": "Not a Hue bridge" } } } diff --git a/homeassistant/components/ssdp/__init__.py b/homeassistant/components/ssdp/__init__.py index aecca614e73..e250b9c16fb 100644 --- a/homeassistant/components/ssdp/__init__.py +++ b/homeassistant/components/ssdp/__init__.py @@ -23,6 +23,7 @@ ATTR_MODEL_NAME = 'model_name' ATTR_MODEL_NUMBER = 'model_number' ATTR_SERIAL = 'serial_number' ATTR_MANUFACTURER = 'manufacturer' +ATTR_MANUFACTURERURL = 'manufacturerURL' ATTR_UDN = 'udn' ATTR_UPNP_DEVICE_TYPE = 'upnp_device_type' @@ -164,6 +165,7 @@ def info_from_entry(entry, device_info): info[ATTR_MODEL_NUMBER] = device_info.get('modelNumber') info[ATTR_SERIAL] = device_info.get('serialNumber') info[ATTR_MANUFACTURER] = device_info.get('manufacturer') + info[ATTR_MANUFACTURERURL] = device_info.get('manufacturerURL') info[ATTR_UDN] = device_info.get('UDN') info[ATTR_UPNP_DEVICE_TYPE] = device_info.get('deviceType') diff --git a/homeassistant/generated/ssdp.py b/homeassistant/generated/ssdp.py index 897f68a6521..cc1d286bf5f 100644 --- a/homeassistant/generated/ssdp.py +++ b/homeassistant/generated/ssdp.py @@ -8,6 +8,7 @@ SSDP = { "device_type": {}, "manufacturer": { "Royal Philips Electronics": [ + "deconz", "hue" ] }, diff --git a/tests/components/deconz/test_config_flow.py b/tests/components/deconz/test_config_flow.py index 46b0084b01b..2b9f2c013b0 100644 --- a/tests/components/deconz/test_config_flow.py +++ b/tests/components/deconz/test_config_flow.py @@ -168,22 +168,38 @@ async def test_link_no_api_key(hass): assert result['errors'] == {'base': 'no_key'} -async def test_bridge_discovery(hass): - """Test a bridge being discovered.""" +async def test_bridge_ssdp_discovery(hass): + """Test a bridge being discovered over ssdp.""" result = await hass.config_entries.flow.async_init( config_flow.DOMAIN, data={ config_flow.CONF_HOST: '1.2.3.4', config_flow.CONF_PORT: 80, - config_flow.CONF_SERIAL: 'id', + config_flow.ATTR_SERIAL: 'id', + config_flow.ATTR_MANUFACTURERURL: + config_flow.DECONZ_MANUFACTURERURL }, - context={'source': 'discovery'} + context={'source': 'ssdp'} ) assert result['type'] == 'form' assert result['step_id'] == 'link' +async def test_bridge_ssdp_discovery_not_deconz_bridge(hass): + """Test a non deconz bridge being discovered over ssdp.""" + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + data={ + config_flow.ATTR_MANUFACTURERURL: 'not deconz bridge' + }, + context={'source': 'ssdp'} + ) + + assert result['type'] == 'abort' + assert result['reason'] == 'not_deconz_bridge' + + async def test_bridge_discovery_update_existing_entry(hass): """Test if a discovered bridge has already been configured.""" entry = MockConfigEntry(domain=config_flow.DOMAIN, data={ @@ -195,9 +211,11 @@ async def test_bridge_discovery_update_existing_entry(hass): config_flow.DOMAIN, data={ config_flow.CONF_HOST: 'mock-deconz', - config_flow.CONF_SERIAL: 'id', + config_flow.ATTR_SERIAL: 'id', + config_flow.ATTR_MANUFACTURERURL: + config_flow.DECONZ_MANUFACTURERURL }, - context={'source': 'discovery'} + context={'source': 'ssdp'} ) assert result['type'] == 'abort' diff --git a/tests/components/hue/test_config_flow.py b/tests/components/hue/test_config_flow.py index 37cece0bbd8..b7736e62390 100644 --- a/tests/components/hue/test_config_flow.py +++ b/tests/components/hue/test_config_flow.py @@ -195,13 +195,26 @@ async def test_bridge_ssdp(hass): side_effect=errors.AuthenticationRequired): result = await flow.async_step_ssdp({ 'host': '0.0.0.0', - 'serial': '1234' + 'serial': '1234', + 'manufacturerURL': config_flow.HUE_MANUFACTURERURL }) assert result['type'] == 'form' assert result['step_id'] == 'link' +async def test_bridge_ssdp_discover_other_bridge(hass): + """Test that discovery ignores other bridges.""" + flow = config_flow.HueFlowHandler() + flow.hass = hass + + result = await flow.async_step_ssdp({ + 'manufacturerURL': 'http://www.notphilips.com' + }) + + assert result['type'] == 'abort' + + async def test_bridge_ssdp_emulated_hue(hass): """Test if discovery info is from an emulated hue instance.""" flow = config_flow.HueFlowHandler() @@ -211,7 +224,8 @@ async def test_bridge_ssdp_emulated_hue(hass): result = await flow.async_step_ssdp({ 'name': 'HASS Bridge', 'host': '0.0.0.0', - 'serial': '1234' + 'serial': '1234', + 'manufacturerURL': config_flow.HUE_MANUFACTURERURL }) assert result['type'] == 'abort' @@ -229,7 +243,8 @@ async def test_bridge_ssdp_already_configured(hass): result = await flow.async_step_ssdp({ 'host': '0.0.0.0', - 'serial': '1234' + 'serial': '1234', + 'manufacturerURL': config_flow.HUE_MANUFACTURERURL }) assert result['type'] == 'abort'