From 23cb579f9fbe14bb4bc8eefa6176313a1339181f Mon Sep 17 00:00:00 2001 From: Robert Svensson Date: Mon, 15 Apr 2019 06:50:01 +0200 Subject: [PATCH] Support updating deCONZ host address (#22784) * Update config flow to support updating host address Improve tests * Update gateway to handle new address signal * Improve description why whe need to keep step_init --- .../components/deconz/config_flow.py | 45 ++- homeassistant/components/deconz/gateway.py | 14 + homeassistant/components/deconz/strings.json | 1 + tests/components/deconz/test_config_flow.py | 273 +++++++++++------- 4 files changed, 217 insertions(+), 116 deletions(-) diff --git a/homeassistant/components/deconz/config_flow.py b/homeassistant/components/deconz/config_flow.py index 1ecfee7ada5..d9065ad2727 100644 --- a/homeassistant/components/deconz/config_flow.py +++ b/homeassistant/components/deconz/config_flow.py @@ -11,17 +11,19 @@ from homeassistant.helpers import aiohttp_client from .const import CONF_BRIDGEID, DEFAULT_PORT, DOMAIN +CONF_SERIAL = 'serial' + @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)) +def configured_gateways(hass): + """Return a set of all configured gateways.""" + return {entry.data[CONF_BRIDGEID]: entry for entry + in hass.config_entries.async_entries(DOMAIN)} @callback def get_master_gateway(hass): - """Return a bool telling if this is the master gateway.""" + """Return the gateway which is marked as master.""" for gateway in hass.data[DOMAIN].values(): if gateway.master: return gateway @@ -41,6 +43,10 @@ class DeconzFlowHandler(config_entries.ConfigFlow): self.bridges = [] self.deconz_config = {} + async def async_step_init(self, user_input=None): + """Needed in order to not require re-translation of strings.""" + return await self.async_step_user(user_input) + async def async_step_user(self, user_input=None): """Handle a deCONZ config flow start. @@ -140,20 +146,30 @@ class DeconzFlowHandler(config_entries.ConfigFlow): data=self.deconz_config ) + async def _update_entry(self, entry, host): + """Update existing entry.""" + 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. This flow is triggered by the discovery component. """ + bridgeid = discovery_info[CONF_SERIAL] + gateway_entries = configured_gateways(self.hass) + + if bridgeid in gateway_entries: + entry = gateway_entries[bridgeid] + await self._update_entry(entry, discovery_info[CONF_HOST]) + return self.async_abort(reason='updated_instance') + deconz_config = { CONF_HOST: discovery_info[CONF_HOST], CONF_PORT: discovery_info[CONF_PORT], - CONF_BRIDGEID: discovery_info['serial'] + CONF_BRIDGEID: discovery_info[CONF_SERIAL] } - if deconz_config[CONF_HOST] in configured_hosts(self.hass): - return self.async_abort(reason='one_instance_only') - return await self.async_step_import(deconz_config) async def async_step_import(self, import_config): @@ -180,8 +196,13 @@ class DeconzFlowHandler(config_entries.ConfigFlow): This flow is triggered by the discovery component. """ - if configured_hosts(self.hass): - return self.async_abort(reason='one_instance_only') + bridgeid = user_input[CONF_SERIAL] + gateway_entries = configured_gateways(self.hass) + + if bridgeid in gateway_entries: + entry = gateway_entries[bridgeid] + await self._update_entry(entry, user_input[CONF_HOST]) + return self.async_abort(reason='updated_instance') self._hassio_discovery = user_input @@ -193,7 +214,7 @@ class DeconzFlowHandler(config_entries.ConfigFlow): self.deconz_config = { CONF_HOST: self._hassio_discovery[CONF_HOST], CONF_PORT: self._hassio_discovery[CONF_PORT], - CONF_BRIDGEID: self._hassio_discovery['serial'], + CONF_BRIDGEID: self._hassio_discovery[CONF_SERIAL], CONF_API_KEY: self._hassio_discovery[CONF_API_KEY] } diff --git a/homeassistant/components/deconz/gateway.py b/homeassistant/components/deconz/gateway.py index 4d9e1503902..46078ea6648 100644 --- a/homeassistant/components/deconz/gateway.py +++ b/homeassistant/components/deconz/gateway.py @@ -101,8 +101,22 @@ class DeconzGateway: self.api.start() + self.config_entry.add_update_listener(self.async_new_address_callback) + return True + @staticmethod + async def async_new_address_callback(hass, entry): + """Handle signals of gateway getting new address. + + This is a static method because a class method (bound method), + can not be used with weak references. + """ + gateway = hass.data[DOMAIN][entry.data[CONF_BRIDGEID]] + gateway.api.close() + gateway.api.host = entry.data[CONF_HOST] + gateway.api.start() + @property def event_reachable(self): """Gateway specific event to signal a change in connection status.""" diff --git a/homeassistant/components/deconz/strings.json b/homeassistant/components/deconz/strings.json index d0ae34e7c7a..16177dbd3cc 100644 --- a/homeassistant/components/deconz/strings.json +++ b/homeassistant/components/deconz/strings.json @@ -35,6 +35,7 @@ "abort": { "already_configured": "Bridge is already configured", "no_bridges": "No deCONZ bridges discovered", + "updated_instance": "Updated deCONZ instance with new host address", "one_instance_only": "Component only supports one deCONZ instance" } } diff --git a/tests/components/deconz/test_config_flow.py b/tests/components/deconz/test_config_flow.py index 09510b136c4..ada506be428 100644 --- a/tests/components/deconz/test_config_flow.py +++ b/tests/components/deconz/test_config_flow.py @@ -18,49 +18,57 @@ async def test_flow_works(hass, aioclient_mock): {"success": {"username": "1234567890ABCDEF"}} ], headers={'content-type': 'application/json'}) - flow = config_flow.DeconzFlowHandler() - flow.hass = hass + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={'source': 'user'} + ) - await flow.async_step_user() - result = await flow.async_step_link(user_input={}) + assert result['type'] == 'form' + assert result['step_id'] == 'link' + + result = await hass.config_entries.flow.async_configure( + result['flow_id'], + user_input={} + ) assert result['type'] == 'create_entry' assert result['title'] == 'deCONZ-id' assert result['data'] == { - 'bridgeid': 'id', - 'host': '1.2.3.4', - 'port': 80, - 'api_key': '1234567890ABCDEF' + config_flow.CONF_BRIDGEID: 'id', + config_flow.CONF_HOST: '1.2.3.4', + config_flow.CONF_PORT: 80, + config_flow.CONF_API_KEY: '1234567890ABCDEF' } -async def test_flow_bridge_discovery_fails(hass, aioclient_mock): +async def test_user_step_bridge_discovery_fails(hass, aioclient_mock): """Test config flow works when discovery fails.""" - flow = config_flow.DeconzFlowHandler() - flow.hass = hass - with patch('pydeconz.utils.async_discovery', side_effect=asyncio.TimeoutError): - result = await flow.async_step_user() + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={'source': 'user'} + ) assert result['type'] == 'form' assert result['step_id'] == 'init' -async def test_flow_no_discovered_bridges(hass, aioclient_mock): +async def test_user_step_no_discovered_bridges(hass, aioclient_mock): """Test config flow discovers no bridges.""" aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[], headers={'content-type': 'application/json'}) - flow = config_flow.DeconzFlowHandler() - flow.hass = hass + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={'source': 'user'} + ) - result = await flow.async_step_user() assert result['type'] == 'form' assert result['step_id'] == 'init' -async def test_flow_one_bridge_discovered(hass, aioclient_mock): +async def test_user_step_one_bridge_discovered(hass, aioclient_mock): """Test config flow discovers one bridge.""" aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[ {'id': 'id', 'internalipaddress': '1.2.3.4', 'internalport': 80} @@ -70,61 +78,85 @@ async def test_flow_one_bridge_discovered(hass, aioclient_mock): flow.hass = hass result = await flow.async_step_user() + assert result['type'] == 'form' assert result['step_id'] == 'link' - assert flow.deconz_config['host'] == '1.2.3.4' + assert flow.deconz_config[config_flow.CONF_HOST] == '1.2.3.4' -async def test_flow_two_bridges_discovered(hass, aioclient_mock): +async def test_user_step_two_bridges_discovered(hass, aioclient_mock): """Test config flow discovers two bridges.""" aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[ {'id': 'id1', 'internalipaddress': '1.2.3.4', 'internalport': 80}, {'id': 'id2', 'internalipaddress': '5.6.7.8', 'internalport': 80} ], headers={'content-type': 'application/json'}) - flow = config_flow.DeconzFlowHandler() - flow.hass = hass + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={'source': 'user'} + ) - result = await flow.async_step_user() - assert result['data_schema']({'host': '1.2.3.4'}) - assert result['data_schema']({'host': '5.6.7.8'}) + assert result['data_schema']({config_flow.CONF_HOST: '1.2.3.4'}) + assert result['data_schema']({config_flow.CONF_HOST: '5.6.7.8'}) -async def test_flow_two_bridges_selection(hass, aioclient_mock): +async def test_user_step_two_bridges_selection(hass, aioclient_mock): """Test config flow selection of one of two bridges.""" flow = config_flow.DeconzFlowHandler() flow.hass = hass flow.bridges = [ - {'bridgeid': 'id1', 'host': '1.2.3.4', 'port': 80}, - {'bridgeid': 'id2', 'host': '5.6.7.8', 'port': 80} + { + config_flow.CONF_BRIDGEID: 'id1', + config_flow.CONF_HOST: '1.2.3.4', + config_flow.CONF_PORT: 80 + }, + { + config_flow.CONF_BRIDGEID: 'id2', + config_flow.CONF_HOST: '5.6.7.8', + config_flow.CONF_PORT: 80 + } ] - result = await flow.async_step_user(user_input={'host': '1.2.3.4'}) + result = await flow.async_step_user( + user_input={config_flow.CONF_HOST: '1.2.3.4'}) assert result['type'] == 'form' assert result['step_id'] == 'link' - assert flow.deconz_config['host'] == '1.2.3.4' + assert flow.deconz_config[config_flow.CONF_HOST] == '1.2.3.4' -async def test_flow_manual_configuration(hass, aioclient_mock): +async def test_user_step_manual_configuration(hass, aioclient_mock): """Test config flow with manual input.""" - aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[]) + aioclient_mock.get(pydeconz.utils.URL_DISCOVER, json=[], + headers={'content-type': 'application/json'}) - flow = config_flow.DeconzFlowHandler() - flow.hass = hass + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={'source': 'user'} + ) - user_input = {'host': '1.2.3.4', 'port': 80} + assert result['type'] == 'form' + assert result['step_id'] == 'init' + + result = await hass.config_entries.flow.async_configure( + result['flow_id'], + user_input={ + config_flow.CONF_HOST: '1.2.3.4', + config_flow.CONF_PORT: 80 + } + ) - result = await flow.async_step_user(user_input) assert result['type'] == 'form' assert result['step_id'] == 'link' - assert flow.deconz_config == user_input async def test_link_no_api_key(hass): """Test config flow should abort if no API key was possible to retrieve.""" flow = config_flow.DeconzFlowHandler() flow.hass = hass - flow.deconz_config = {'host': '1.2.3.4', 'port': 80} + flow.deconz_config = { + config_flow.CONF_HOST: '1.2.3.4', + config_flow.CONF_PORT: 80 + } with patch('pydeconz.utils.async_get_api_key', side_effect=pydeconz.errors.ResponseError): @@ -137,45 +169,48 @@ async def test_link_no_api_key(hass): async def test_bridge_discovery(hass): """Test a bridge being discovered.""" - flow = config_flow.DeconzFlowHandler() - flow.hass = hass - - result = await flow.async_step_discovery({ - 'host': '1.2.3.4', - 'port': 80, - 'serial': 'id' - }) + 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', + }, + context={'source': 'discovery'} + ) assert result['type'] == 'form' assert result['step_id'] == 'link' -async def test_bridge_discovery_already_configured(hass): +async def test_bridge_discovery_update_existing_entry(hass): """Test if a discovered bridge has already been configured.""" - MockConfigEntry(domain='deconz', data={ - 'host': '1.2.3.4' - }).add_to_hass(hass) - - flow = config_flow.DeconzFlowHandler() - flow.hass = hass - - result = await flow.async_step_discovery({ - 'host': '1.2.3.4', - 'port': 80, - 'serial': 'id' + entry = MockConfigEntry(domain=config_flow.DOMAIN, data={ + config_flow.CONF_HOST: '1.2.3.4', config_flow.CONF_BRIDGEID: 'id' }) + entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + data={ + config_flow.CONF_HOST: 'mock-deconz', + config_flow.CONF_SERIAL: 'id', + }, + context={'source': 'discovery'} + ) assert result['type'] == 'abort' + assert result['reason'] == 'updated_instance' + assert entry.data[config_flow.CONF_HOST] == 'mock-deconz' async def test_import_without_api_key(hass): """Test importing a host without an API key.""" - flow = config_flow.DeconzFlowHandler() - flow.hass = hass - - result = await flow.async_step_import({ - 'host': '1.2.3.4', - }) + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + data={config_flow.CONF_HOST: '1.2.3.4'}, + context={'source': 'import'} + ) assert result['type'] == 'form' assert result['step_id'] == 'link' @@ -183,23 +218,24 @@ async def test_import_without_api_key(hass): async def test_import_with_api_key(hass): """Test importing a host with an API key.""" - flow = config_flow.DeconzFlowHandler() - flow.hass = hass - - result = await flow.async_step_import({ - 'bridgeid': 'id', - 'host': '1.2.3.4', - 'port': 80, - 'api_key': '1234567890ABCDEF' - }) + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + data={ + config_flow.CONF_BRIDGEID: 'id', + config_flow.CONF_HOST: 'mock-deconz', + config_flow.CONF_PORT: 80, + config_flow.CONF_API_KEY: '1234567890ABCDEF' + }, + context={'source': 'import'} + ) assert result['type'] == 'create_entry' assert result['title'] == 'deCONZ-id' assert result['data'] == { - 'bridgeid': 'id', - 'host': '1.2.3.4', - 'port': 80, - 'api_key': '1234567890ABCDEF' + config_flow.CONF_BRIDGEID: 'id', + config_flow.CONF_HOST: 'mock-deconz', + config_flow.CONF_PORT: 80, + config_flow.CONF_API_KEY: '1234567890ABCDEF' } @@ -211,61 +247,90 @@ async def test_create_entry(hass, aioclient_mock): flow = config_flow.DeconzFlowHandler() flow.hass = hass - flow.deconz_config = {'host': '1.2.3.4', - 'port': 80, - 'api_key': '1234567890ABCDEF'} + flow.deconz_config = { + config_flow.CONF_HOST: '1.2.3.4', + config_flow.CONF_PORT: 80, + config_flow.CONF_API_KEY: '1234567890ABCDEF' + } result = await flow._create_entry() assert result['type'] == 'create_entry' assert result['title'] == 'deCONZ-id' assert result['data'] == { - 'bridgeid': 'id', - 'host': '1.2.3.4', - 'port': 80, - 'api_key': '1234567890ABCDEF' + config_flow.CONF_BRIDGEID: 'id', + config_flow.CONF_HOST: '1.2.3.4', + config_flow.CONF_PORT: 80, + config_flow.CONF_API_KEY: '1234567890ABCDEF' } -async def test_hassio_single_instance(hass): - """Test we only allow a single config flow.""" - MockConfigEntry(domain='deconz', data={ - 'host': '1.2.3.4' - }).add_to_hass(hass) +async def test_create_entry_timeout(hass, aioclient_mock): + """Test that _create_entry handles a timeout.""" + flow = config_flow.DeconzFlowHandler() + flow.hass = hass + flow.deconz_config = { + config_flow.CONF_HOST: '1.2.3.4', + config_flow.CONF_PORT: 80, + config_flow.CONF_API_KEY: '1234567890ABCDEF' + } + + with patch('pydeconz.utils.async_get_bridgeid', + side_effect=asyncio.TimeoutError): + result = await flow._create_entry() + + assert result['type'] == 'abort' + assert result['reason'] == 'no_bridges' + + +async def test_hassio_update_instance(hass): + """Test we can update an existing config entry.""" + entry = MockConfigEntry(domain=config_flow.DOMAIN, data={ + config_flow.CONF_BRIDGEID: 'id', + config_flow.CONF_HOST: '1.2.3.4' + }) + entry.add_to_hass(hass) result = await hass.config_entries.flow.async_init( - 'deconz', context={'source': 'hassio'}) + config_flow.DOMAIN, + data={ + config_flow.CONF_HOST: 'mock-deconz', + config_flow.CONF_SERIAL: 'id' + }, + context={'source': 'hassio'} + ) + assert result['type'] == 'abort' - assert result['reason'] == 'one_instance_only' + assert result['reason'] == 'updated_instance' + assert entry.data[config_flow.CONF_HOST] == 'mock-deconz' async def test_hassio_confirm(hass): """Test we can finish a config flow.""" result = await hass.config_entries.flow.async_init( - 'deconz', + config_flow.DOMAIN, data={ 'addon': 'Mock Addon', - 'host': 'mock-deconz', - 'port': 80, - 'serial': 'id', - 'api_key': '1234567890ABCDEF', + config_flow.CONF_HOST: 'mock-deconz', + config_flow.CONF_PORT: 80, + config_flow.CONF_SERIAL: 'id', + config_flow.CONF_API_KEY: '1234567890ABCDEF', }, context={'source': 'hassio'} ) assert result['type'] == 'form' assert result['step_id'] == 'hassio_confirm' - assert result['description_placeholders'] == { - 'addon': 'Mock Addon', - } + assert result['description_placeholders'] == {'addon': 'Mock Addon'} result = await hass.config_entries.flow.async_configure( - result['flow_id'], user_input={} + result['flow_id'], + user_input={} ) assert result['type'] == 'create_entry' assert result['result'].data == { - 'host': 'mock-deconz', - 'port': 80, - 'bridgeid': 'id', - 'api_key': '1234567890ABCDEF' + config_flow.CONF_HOST: 'mock-deconz', + config_flow.CONF_PORT: 80, + config_flow.CONF_BRIDGEID: 'id', + config_flow.CONF_API_KEY: '1234567890ABCDEF' }