From f4f945e65e4ddb0c5985856f605ce1cbf15ef6b9 Mon Sep 17 00:00:00 2001 From: h2zero <32826625+h2zero@users.noreply.github.com> Date: Wed, 24 Nov 2021 03:35:00 -0700 Subject: [PATCH] Fix Konnected multiple discovery of panels (#59953) * Konnected - Fix multiple discovery of panels. This resolves an issue which creates multiple discoveries of a Konnected panel if it is restarted and fails to connect to home assistant. See #57467. * Revert changes to user step, add handling to ssdp step. * Add abort reason string to strings.json * Abort ssdp discovery if device is already known. * Add test for multiple discovery fix. * Remove unrelated file change. * Add ssdp discovery abort tests. * Add missing abort reason check. * Add "already_configured" to strings. * Use "cannot_connect" abort reason. --- .../components/konnected/config_flow.py | 22 +++++- .../components/konnected/strings.json | 3 +- .../components/konnected/test_config_flow.py | 77 +++++++++++++++++++ 3 files changed, 99 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/konnected/config_flow.py b/homeassistant/components/konnected/config_flow.py index 4cc53c9069a..82a720bfff3 100644 --- a/homeassistant/components/konnected/config_flow.py +++ b/homeassistant/components/konnected/config_flow.py @@ -267,10 +267,28 @@ class KonnectedFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): else: # extract host/port from ssdp_location netloc = urlparse(discovery_info["ssdp_location"]).netloc.split(":") - return await self.async_step_user( - user_input={CONF_HOST: netloc[0], CONF_PORT: int(netloc[1])} + self._async_abort_entries_match( + {CONF_HOST: netloc[0], CONF_PORT: int(netloc[1])} ) + try: + status = await get_status(self.hass, netloc[0], int(netloc[1])) + except CannotConnect: + return self.async_abort(reason="cannot_connect") + else: + self.data[CONF_HOST] = netloc[0] + self.data[CONF_PORT] = int(netloc[1]) + self.data[CONF_ID] = status.get( + "chipId", status["mac"].replace(":", "") + ) + self.data[CONF_MODEL] = status.get("model", KONN_MODEL) + + KonnectedFlowHandler.discovered_hosts[self.data[CONF_ID]] = { + CONF_HOST: self.data[CONF_HOST], + CONF_PORT: self.data[CONF_PORT], + } + return await self.async_step_confirm() + return self.async_abort(reason="unknown") async def async_step_user(self, user_input=None): diff --git a/homeassistant/components/konnected/strings.json b/homeassistant/components/konnected/strings.json index e53838ad0d7..905597035d5 100644 --- a/homeassistant/components/konnected/strings.json +++ b/homeassistant/components/konnected/strings.json @@ -24,7 +24,8 @@ "unknown": "[%key:common::config_flow::error::unknown%]", "already_configured": "[%key:common::config_flow::abort::already_configured_device%]", "already_in_progress": "[%key:common::config_flow::abort::already_in_progress%]", - "not_konn_panel": "Not a recognized Konnected.io device" + "not_konn_panel": "Not a recognized Konnected.io device", + "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]" } }, "options": { diff --git a/tests/components/konnected/test_config_flow.py b/tests/components/konnected/test_config_flow.py index 36f582fcb57..466359c3dc3 100644 --- a/tests/components/konnected/test_config_flow.py +++ b/tests/components/konnected/test_config_flow.py @@ -109,6 +109,7 @@ async def test_ssdp(hass, mock_panel): "model": "Konnected", } + # Test success result = await hass.config_entries.flow.async_init( config_flow.DOMAIN, context={"source": config_entries.SOURCE_SSDP}, @@ -128,6 +129,82 @@ async def test_ssdp(hass, mock_panel): "port": 1234, } + # Test abort if connection failed + mock_panel.get_status.side_effect = config_flow.CannotConnect + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data={ + "ssdp_location": "http://1.2.3.4:1234/Device.xml", + "manufacturer": config_flow.KONN_MANUFACTURER, + "modelName": config_flow.KONN_MODEL, + }, + ) + + assert result["type"] == "abort" + assert result["reason"] == "cannot_connect" + + # Test abort if invalid data + mock_panel.get_status.side_effect = KeyError + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data={ + "ssdp_location": "http://1.2.3.4:1234/Device.xml", + }, + ) + + assert result["type"] == "abort" + assert result["reason"] == "unknown" + + # Test abort if invalid manufacturer + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data={ + "ssdp_location": "http://1.2.3.4:1234/Device.xml", + "manufacturer": "SHOULD_FAIL", + "modelName": config_flow.KONN_MODEL, + }, + ) + + assert result["type"] == "abort" + assert result["reason"] == "not_konn_panel" + + # Test abort if invalid model + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data={ + "ssdp_location": "http://1.2.3.4:1234/Device.xml", + "manufacturer": config_flow.KONN_MANUFACTURER, + "modelName": "SHOULD_FAIL", + }, + ) + + assert result["type"] == "abort" + assert result["reason"] == "not_konn_panel" + + # Test abort if already configured + config_entry = MockConfigEntry( + domain=config_flow.DOMAIN, + data={config_flow.CONF_HOST: "1.2.3.4", config_flow.CONF_PORT: 1234}, + ) + config_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + config_flow.DOMAIN, + context={"source": config_entries.SOURCE_SSDP}, + data={ + "ssdp_location": "http://1.2.3.4:1234/Device.xml", + "manufacturer": config_flow.KONN_MANUFACTURER, + "modelName": config_flow.KONN_MODEL, + }, + ) + + assert result["type"] == "abort" + assert result["reason"] == "already_configured" + async def test_import_no_host_user_finish(hass, mock_panel): """Test importing a panel with no host info."""