From c7d46bc71971f0072e7aaa93e2b2ef219cebd92c Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Tue, 16 Aug 2022 08:30:07 -0400 Subject: [PATCH] Improve Awair config flow (#76838) --- homeassistant/components/awair/config_flow.py | 79 +++++++++++++++---- homeassistant/components/awair/manifest.json | 5 ++ homeassistant/components/awair/strings.json | 7 +- homeassistant/generated/dhcp.py | 1 + homeassistant/strings.json | 1 + tests/components/awair/test_config_flow.py | 56 ++++++++++++- 6 files changed, 130 insertions(+), 19 deletions(-) diff --git a/homeassistant/components/awair/config_flow.py b/homeassistant/components/awair/config_flow.py index 3fb822ab4fe..418413b690f 100644 --- a/homeassistant/components/awair/config_flow.py +++ b/homeassistant/components/awair/config_flow.py @@ -4,14 +4,15 @@ from __future__ import annotations from collections.abc import Mapping from typing import Any -from aiohttp.client_exceptions import ClientConnectorError +from aiohttp.client_exceptions import ClientError from python_awair import Awair, AwairLocal, AwairLocalDevice from python_awair.exceptions import AuthError, AwairError import voluptuous as vol from homeassistant.components import zeroconf -from homeassistant.config_entries import ConfigFlow -from homeassistant.const import CONF_ACCESS_TOKEN, CONF_HOST +from homeassistant.config_entries import SOURCE_ZEROCONF, ConfigFlow +from homeassistant.const import CONF_ACCESS_TOKEN, CONF_DEVICE, CONF_HOST +from homeassistant.core import callback from homeassistant.data_entry_flow import FlowResult from homeassistant.helpers.aiohttp_client import async_get_clientsession @@ -40,10 +41,11 @@ class AwairFlowHandler(ConfigFlow, domain=DOMAIN): self._abort_if_unique_id_configured(error="already_configured_device") self.context.update( { + "host": host, "title_placeholders": { "model": self._device.model, "device_id": self._device.device_id, - } + }, } ) else: @@ -109,31 +111,76 @@ class AwairFlowHandler(ConfigFlow, domain=DOMAIN): errors=errors, ) - async def async_step_local(self, user_input: Mapping[str, Any]) -> FlowResult: + @callback + def _get_discovered_entries(self) -> dict[str, str]: + """Get discovered entries.""" + entries: dict[str, str] = {} + for flow in self._async_in_progress(): + if flow["context"]["source"] == SOURCE_ZEROCONF: + info = flow["context"]["title_placeholders"] + entries[ + flow["context"]["host"] + ] = f"{info['model']} ({info['device_id']})" + return entries + + async def async_step_local( + self, user_input: Mapping[str, Any] | None = None + ) -> FlowResult: + """Show how to enable local API.""" + if user_input is not None: + return await self.async_step_local_pick() + + return self.async_show_form( + step_id="local", + description_placeholders={ + "url": "https://support.getawair.com/hc/en-us/articles/360049221014-Awair-Element-Local-API-Feature#h_01F40FBBW5323GBPV7D6XMG4J8" + }, + ) + + async def async_step_local_pick( + self, user_input: Mapping[str, Any] | None = None + ) -> FlowResult: """Handle collecting and verifying Awair Local API hosts.""" errors = {} - if user_input is not None: + # User input is either: + # 1. None if first time on this step + # 2. {device: manual} if picked manual entry option + # 3. {device: } if picked a device + # 4. {host: } if manually entered a host + # + # Option 1 and 2 will show the form again. + if user_input and user_input.get(CONF_DEVICE) != "manual": + if CONF_DEVICE in user_input: + user_input = {CONF_HOST: user_input[CONF_DEVICE]} + self._device, error = await self._check_local_connection( - user_input[CONF_HOST] + user_input.get(CONF_DEVICE) or user_input[CONF_HOST] ) if self._device is not None: - await self.async_set_unique_id(self._device.mac_address) - self._abort_if_unique_id_configured(error="already_configured_device") + await self.async_set_unique_id( + self._device.mac_address, raise_on_progress=False + ) title = f"{self._device.model} ({self._device.device_id})" return self.async_create_entry(title=title, data=user_input) if error is not None: - errors = {CONF_HOST: error} + errors = {"base": error} + + discovered = self._get_discovered_entries() + + if not discovered or (user_input and user_input.get(CONF_DEVICE) == "manual"): + data_schema = vol.Schema({vol.Required(CONF_HOST): str}) + + elif discovered: + discovered["manual"] = "Manual" + data_schema = vol.Schema({vol.Required(CONF_DEVICE): vol.In(discovered)}) return self.async_show_form( - step_id="local", - data_schema=vol.Schema({vol.Required(CONF_HOST): str}), - description_placeholders={ - "url": "https://support.getawair.com/hc/en-us/articles/360049221014-Awair-Element-Local-API-Feature" - }, + step_id="local_pick", + data_schema=data_schema, errors=errors, ) @@ -177,7 +224,7 @@ class AwairFlowHandler(ConfigFlow, domain=DOMAIN): devices = await awair.devices() return (devices[0], None) - except ClientConnectorError as err: + except ClientError as err: LOGGER.error("Unable to connect error: %s", err) return (None, "unreachable") diff --git a/homeassistant/components/awair/manifest.json b/homeassistant/components/awair/manifest.json index cea5d01bfab..131a955a6eb 100644 --- a/homeassistant/components/awair/manifest.json +++ b/homeassistant/components/awair/manifest.json @@ -12,5 +12,10 @@ "type": "_http._tcp.local.", "name": "awair*" } + ], + "dhcp": [ + { + "macaddress": "70886B1*" + } ] } diff --git a/homeassistant/components/awair/strings.json b/homeassistant/components/awair/strings.json index fc95fc861f1..5b16f359403 100644 --- a/homeassistant/components/awair/strings.json +++ b/homeassistant/components/awair/strings.json @@ -9,10 +9,13 @@ } }, "local": { + "description": "Follow [these instructions]({url}) on how to enable the Awair Local API.\n\nClick submit when done." + }, + "local_pick": { "data": { + "device": "[%key:common::config_flow::data::device%]", "host": "[%key:common::config_flow::data::ip%]" - }, - "description": "Awair Local API must be enabled following these steps: {url}" + } }, "reauth_confirm": { "description": "Please re-enter your Awair developer access token.", diff --git a/homeassistant/generated/dhcp.py b/homeassistant/generated/dhcp.py index fb8000f8393..9179a314215 100644 --- a/homeassistant/generated/dhcp.py +++ b/homeassistant/generated/dhcp.py @@ -11,6 +11,7 @@ DHCP: list[dict[str, str | bool]] = [ {'domain': 'august', 'hostname': 'connect', 'macaddress': 'B8B7F1*'}, {'domain': 'august', 'hostname': 'connect', 'macaddress': '2C9FFB*'}, {'domain': 'august', 'hostname': 'august*', 'macaddress': 'E076D0*'}, + {'domain': 'awair', 'macaddress': '70886B1*'}, {'domain': 'axis', 'registered_devices': True}, {'domain': 'axis', 'hostname': 'axis-00408c*', 'macaddress': '00408C*'}, {'domain': 'axis', 'hostname': 'axis-accc8e*', 'macaddress': 'ACCC8E*'}, diff --git a/homeassistant/strings.json b/homeassistant/strings.json index 9ae30becaee..86155be7b4d 100644 --- a/homeassistant/strings.json +++ b/homeassistant/strings.json @@ -26,6 +26,7 @@ "confirm_setup": "Do you want to start set up?" }, "data": { + "device": "Device", "name": "Name", "email": "Email", "username": "Username", diff --git a/tests/components/awair/test_config_flow.py b/tests/components/awair/test_config_flow.py index f6513321dfb..3fdf84f8260 100644 --- a/tests/components/awair/test_config_flow.py +++ b/tests/components/awair/test_config_flow.py @@ -240,6 +240,12 @@ async def test_create_local_entry(hass: HomeAssistant, local_devices): {"next_step_id": "local"}, ) + # We're being shown the local instructions + form_step = await hass.config_entries.flow.async_configure( + form_step["flow_id"], + {}, + ) + result = await hass.config_entries.flow.async_configure( form_step["flow_id"], LOCAL_CONFIG, @@ -251,6 +257,48 @@ async def test_create_local_entry(hass: HomeAssistant, local_devices): assert result["result"].unique_id == LOCAL_UNIQUE_ID +async def test_create_local_entry_from_discovery(hass: HomeAssistant, local_devices): + """Test local API when device discovered after instructions shown.""" + + menu_step = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER}, data=LOCAL_CONFIG + ) + + form_step = await hass.config_entries.flow.async_configure( + menu_step["flow_id"], + {"next_step_id": "local"}, + ) + + # Create discovered entry in progress + with patch("python_awair.AwairClient.query", side_effect=[local_devices]): + await hass.config_entries.flow.async_init( + DOMAIN, + data=Mock(host=LOCAL_CONFIG[CONF_HOST]), + context={"source": SOURCE_ZEROCONF}, + ) + + # We're being shown the local instructions + form_step = await hass.config_entries.flow.async_configure( + form_step["flow_id"], + {}, + ) + + with patch("python_awair.AwairClient.query", side_effect=[local_devices]), patch( + "homeassistant.components.awair.async_setup_entry", + return_value=True, + ): + result = await hass.config_entries.flow.async_configure( + form_step["flow_id"], + {"device": LOCAL_CONFIG[CONF_HOST]}, + ) + + print(result) + assert result["type"] == data_entry_flow.FlowResultType.CREATE_ENTRY + assert result["title"] == "Awair Element (24947)" + assert result["data"][CONF_HOST] == LOCAL_CONFIG[CONF_HOST] + assert result["result"].unique_id == LOCAL_UNIQUE_ID + + async def test_create_local_entry_awair_error(hass: HomeAssistant): """Test overall flow when using local API and device is returns error.""" @@ -267,6 +315,12 @@ async def test_create_local_entry_awair_error(hass: HomeAssistant): {"next_step_id": "local"}, ) + # We're being shown the local instructions + form_step = await hass.config_entries.flow.async_configure( + form_step["flow_id"], + {}, + ) + result = await hass.config_entries.flow.async_configure( form_step["flow_id"], LOCAL_CONFIG, @@ -274,7 +328,7 @@ async def test_create_local_entry_awair_error(hass: HomeAssistant): # User is returned to form to try again assert result["type"] == data_entry_flow.FlowResultType.FORM - assert result["step_id"] == "local" + assert result["step_id"] == "local_pick" async def test_create_zeroconf_entry(hass: HomeAssistant, local_devices):