From 595e399fc74e62534ab3c606acb4a45f8dda4a03 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 26 Apr 2020 15:28:10 -0700 Subject: [PATCH 1/2] Add unique ID to TRADFRI --- .../components/tradfri/config_flow.py | 28 ++++++------ .../components/tradfri/manifest.json | 1 - homeassistant/generated/zeroconf.py | 3 -- tests/components/tradfri/test_config_flow.py | 43 ++++++++++++++++--- 4 files changed, 52 insertions(+), 23 deletions(-) diff --git a/homeassistant/components/tradfri/config_flow.py b/homeassistant/components/tradfri/config_flow.py index 048541b540254..de18e405d5444 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 @@ async def async_step_auth(self, user_input=None): 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 @@ async def async_step_auth(self, user_input=None): 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["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 + for entry in self._async_current_entries(): + if entry.data[CONF_HOST] != host: + continue - if any(host == flow["context"]["host"] for flow in self._async_in_progress()): - return self.async_abort(reason="already_in_progress") + # Backwards compat, we update old entries + if not entry.unique_id: + self.hass.config_entries.async_update_entry( + entry, unique_id=user_input["id"] + ) - for entry in self._async_current_entries(): - if entry.data[CONF_HOST] == host: - return self.async_abort(reason="already_configured") + 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 ce88766039b24..12457975eee67 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 d6e4965c235d1..fa0c5ad593aa3 100644 --- a/homeassistant/generated/zeroconf.py +++ b/homeassistant/generated/zeroconf.py @@ -10,9 +10,6 @@ "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 2a4a831575abd..2b7441d8c2b98 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", "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", "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", "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", "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", "id": "homekit-id"}, + ) + + assert flow["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert flow["reason"] == "already_configured" + + assert entry.unique_id == "homekit-id" From 311180c2f9dc05797ef9ce88b0a3dc70fcd628c5 Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Mon, 27 Apr 2020 00:57:05 +0200 Subject: [PATCH 2/2] Fix homekit id --- homeassistant/components/tradfri/config_flow.py | 4 ++-- tests/components/tradfri/test_config_flow.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/tradfri/config_flow.py b/homeassistant/components/tradfri/config_flow.py index de18e405d5444..1663ba675a39b 100644 --- a/homeassistant/components/tradfri/config_flow.py +++ b/homeassistant/components/tradfri/config_flow.py @@ -84,7 +84,7 @@ async def async_step_auth(self, user_input=None): async def async_step_homekit(self, user_input): """Handle homekit discovery.""" - await self.async_set_unique_id(user_input["id"]) + 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"] @@ -96,7 +96,7 @@ async def async_step_homekit(self, user_input): # Backwards compat, we update old entries if not entry.unique_id: self.hass.config_entries.async_update_entry( - entry, unique_id=user_input["id"] + entry, unique_id=user_input["properties"]["id"] ) return self.async_abort(reason="already_configured") diff --git a/tests/components/tradfri/test_config_flow.py b/tests/components/tradfri/test_config_flow.py index 2b7441d8c2b98..45e18be83d311 100644 --- a/tests/components/tradfri/test_config_flow.py +++ b/tests/components/tradfri/test_config_flow.py @@ -82,7 +82,7 @@ async def test_discovery_connection(hass, mock_auth, mock_entry_setup): flow = await hass.config_entries.flow.async_init( "tradfri", context={"source": "homekit"}, - data={"host": "123.123.123.123", "id": "homekit-id"}, + data={"host": "123.123.123.123", "properties": {"id": "homekit-id"}}, ) result = await hass.config_entries.flow.async_configure( @@ -230,7 +230,7 @@ async def test_discovery_duplicate_aborted(hass): flow = await hass.config_entries.flow.async_init( "tradfri", context={"source": "homekit"}, - data={"host": "new-host", "id": "homekit-id"}, + data={"host": "new-host", "properties": {"id": "homekit-id"}}, ) assert flow["type"] == data_entry_flow.RESULT_TYPE_ABORT @@ -256,7 +256,7 @@ async def test_duplicate_discovery(hass, mock_auth, mock_entry_setup): result = await hass.config_entries.flow.async_init( "tradfri", context={"source": "homekit"}, - data={"host": "123.123.123.123", "id": "homekit-id"}, + data={"host": "123.123.123.123", "properties": {"id": "homekit-id"}}, ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM @@ -264,7 +264,7 @@ async def test_duplicate_discovery(hass, mock_auth, mock_entry_setup): result2 = await hass.config_entries.flow.async_init( "tradfri", context={"source": "homekit"}, - data={"host": "123.123.123.123", "id": "homekit-id"}, + data={"host": "123.123.123.123", "properties": {"id": "homekit-id"}}, ) assert result2["type"] == data_entry_flow.RESULT_TYPE_ABORT @@ -278,7 +278,7 @@ async def test_discovery_updates_unique_id(hass): flow = await hass.config_entries.flow.async_init( "tradfri", context={"source": "homekit"}, - data={"host": "some-host", "id": "homekit-id"}, + data={"host": "some-host", "properties": {"id": "homekit-id"}}, ) assert flow["type"] == data_entry_flow.RESULT_TYPE_ABORT