From 13c8c2e841280acc0466c5184f308369977e92e3 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 26 Apr 2020 16:57:29 -0700 Subject: [PATCH] Add unique ID to TRADFRI (#34745) --- .../components/tradfri/config_flow.py | 30 +++++++------ .../components/tradfri/manifest.json | 1 - homeassistant/generated/zeroconf.py | 3 -- tests/components/tradfri/test_config_flow.py | 43 ++++++++++++++++--- 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/homeassistant/components/tradfri/config_flow.py b/homeassistant/components/tradfri/config_flow.py index 048541b5402..1663ba675a3 100644 --- a/homeassistant/components/tradfri/config_flow.py +++ b/homeassistant/components/tradfri/config_flow.py @@ -1,6 +1,5 @@ """Config flow for Tradfri.""" import asyncio -from collections import OrderedDict from uuid import uuid4 import async_timeout @@ -70,7 +69,7 @@ class FlowHandler(config_entries.ConfigFlow): else: user_input = {} - fields = OrderedDict() + fields = {} if self._host is None: fields[vol.Required(CONF_HOST, default=user_input.get(CONF_HOST))] = str @@ -83,25 +82,28 @@ class FlowHandler(config_entries.ConfigFlow): step_id="auth", data_schema=vol.Schema(fields), errors=errors ) - async def async_step_zeroconf(self, user_input): - """Handle zeroconf discovery.""" + async def async_step_homekit(self, user_input): + """Handle homekit discovery.""" + await self.async_set_unique_id(user_input["properties"]["id"]) + self._abort_if_unique_id_configured({CONF_HOST: user_input["host"]}) + host = user_input["host"] - # pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 - self.context["host"] = host - - if any(host == flow["context"]["host"] for flow in self._async_in_progress()): - return self.async_abort(reason="already_in_progress") - for entry in self._async_current_entries(): - if entry.data[CONF_HOST] == host: - return self.async_abort(reason="already_configured") + if entry.data[CONF_HOST] != host: + continue + + # Backwards compat, we update old entries + if not entry.unique_id: + self.hass.config_entries.async_update_entry( + entry, unique_id=user_input["properties"]["id"] + ) + + return self.async_abort(reason="already_configured") self._host = host return await self.async_step_auth() - async_step_homekit = async_step_zeroconf - async def async_step_import(self, user_input): """Import a config entry.""" for entry in self._async_current_entries(): diff --git a/homeassistant/components/tradfri/manifest.json b/homeassistant/components/tradfri/manifest.json index ce88766039b..12457975eee 100644 --- a/homeassistant/components/tradfri/manifest.json +++ b/homeassistant/components/tradfri/manifest.json @@ -7,6 +7,5 @@ "homekit": { "models": ["TRADFRI"] }, - "zeroconf": ["_coap._udp.local."], "codeowners": ["@ggravlingen"] } diff --git a/homeassistant/generated/zeroconf.py b/homeassistant/generated/zeroconf.py index d6e4965c235..fa0c5ad593a 100644 --- a/homeassistant/generated/zeroconf.py +++ b/homeassistant/generated/zeroconf.py @@ -10,9 +10,6 @@ ZEROCONF = { "axis", "doorbird" ], - "_coap._udp.local.": [ - "tradfri" - ], "_elg._tcp.local.": [ "elgato" ], diff --git a/tests/components/tradfri/test_config_flow.py b/tests/components/tradfri/test_config_flow.py index 2a4a831575a..45e18be83d3 100644 --- a/tests/components/tradfri/test_config_flow.py +++ b/tests/components/tradfri/test_config_flow.py @@ -80,7 +80,9 @@ async def test_discovery_connection(hass, mock_auth, mock_entry_setup): mock_auth.side_effect = lambda hass, host, code: {"host": host, "gateway_id": "bla"} flow = await hass.config_entries.flow.async_init( - "tradfri", context={"source": "zeroconf"}, data={"host": "123.123.123.123"} + "tradfri", + context={"source": "homekit"}, + data={"host": "123.123.123.123", "properties": {"id": "homekit-id"}}, ) result = await hass.config_entries.flow.async_configure( @@ -90,6 +92,7 @@ async def test_discovery_connection(hass, mock_auth, mock_entry_setup): assert len(mock_entry_setup.mock_calls) == 1 assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["result"].unique_id == "homekit-id" assert result["result"].data == { "host": "123.123.123.123", "gateway_id": "bla", @@ -218,16 +221,23 @@ async def test_import_connection_legacy_no_groups( async def test_discovery_duplicate_aborted(hass): - """Test a duplicate discovery host is ignored.""" - MockConfigEntry(domain="tradfri", data={"host": "some-host"}).add_to_hass(hass) + """Test a duplicate discovery host aborts and updates existing entry.""" + entry = MockConfigEntry( + domain="tradfri", data={"host": "some-host"}, unique_id="homekit-id" + ) + entry.add_to_hass(hass) flow = await hass.config_entries.flow.async_init( - "tradfri", context={"source": "zeroconf"}, data={"host": "some-host"} + "tradfri", + context={"source": "homekit"}, + data={"host": "new-host", "properties": {"id": "homekit-id"}}, ) assert flow["type"] == data_entry_flow.RESULT_TYPE_ABORT assert flow["reason"] == "already_configured" + assert entry.data["host"] == "new-host" + async def test_import_duplicate_aborted(hass): """Test a duplicate import host is ignored.""" @@ -244,13 +254,34 @@ async def test_import_duplicate_aborted(hass): async def test_duplicate_discovery(hass, mock_auth, mock_entry_setup): """Test a duplicate discovery in progress is ignored.""" result = await hass.config_entries.flow.async_init( - "tradfri", context={"source": "zeroconf"}, data={"host": "123.123.123.123"} + "tradfri", + context={"source": "homekit"}, + data={"host": "123.123.123.123", "properties": {"id": "homekit-id"}}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM result2 = await hass.config_entries.flow.async_init( - "tradfri", context={"source": "zeroconf"}, data={"host": "123.123.123.123"} + "tradfri", + context={"source": "homekit"}, + data={"host": "123.123.123.123", "properties": {"id": "homekit-id"}}, ) assert result2["type"] == data_entry_flow.RESULT_TYPE_ABORT + + +async def test_discovery_updates_unique_id(hass): + """Test a duplicate discovery host aborts and updates existing entry.""" + entry = MockConfigEntry(domain="tradfri", data={"host": "some-host"},) + entry.add_to_hass(hass) + + flow = await hass.config_entries.flow.async_init( + "tradfri", + context={"source": "homekit"}, + data={"host": "some-host", "properties": {"id": "homekit-id"}}, + ) + + assert flow["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert flow["reason"] == "already_configured" + + assert entry.unique_id == "homekit-id"