From 483d09b9c1d824ff3df4065843ff234a4560606d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 14 Aug 2020 10:34:52 -0500 Subject: [PATCH] Adjust homekit controller pairing to have a new step for each potentially recoverable error (#38742) * Adjust homekit controller pairing errors back to a single step * adjust test * Revert "Adjust homekit controller pairing errors back to a single step" This reverts commit e5ed89bbbb053bb814219052ae99697a5a7b4e46. * Revert "adjust test" This reverts commit c2e9f21a8401c144315260f6fdf71ea8060f8ca3. * adjust * prune * prune * merge * Update tests * remove debug * adjust * Multiple steps * adjust the test --- .../homekit_controller/config_flow.py | 86 +++++++++++-------- .../homekit_controller/strings.json | 19 ++-- .../homekit_controller/translations/en.json | 19 ++-- .../homekit_controller/test_config_flow.py | 18 +++- 4 files changed, 88 insertions(+), 54 deletions(-) diff --git a/homeassistant/components/homekit_controller/config_flow.py b/homeassistant/components/homekit_controller/config_flow.py index 1cd4a0b4c50..2c69512db9d 100644 --- a/homeassistant/components/homekit_controller/config_flow.py +++ b/homeassistant/components/homekit_controller/config_flow.py @@ -242,37 +242,6 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow): if self.controller is None: await self._async_setup_controller() - if not self.finish_pairing: - # Its possible that the first try may have been busy so - # we always check to see if self.finish_paring has been - # set. - discovery = await self.controller.find_ip_by_device_id(self.hkid) - - try: - self.finish_pairing = await discovery.start_pairing(self.hkid) - - except aiohomekit.BusyError: - # Already performing a pair setup operation with a different - # controller - errors["base"] = "busy_error" - except aiohomekit.MaxTriesError: - # The accessory has received more than 100 unsuccessful auth - # attempts. - errors["base"] = "max_tries_error" - except aiohomekit.UnavailableError: - # The accessory is already paired - cannot try to pair again. - return self.async_abort(reason="already_paired") - except aiohomekit.AccessoryNotFoundError: - # Can no longer find the device on the network - return self.async_abort(reason="accessory_not_found_error") - except IndexError: - # TLV error, usually not in pairing mode - _LOGGER.exception("Pairing communication failed") - errors["base"] = "protocol_error" - except Exception: # pylint: disable=broad-except - _LOGGER.exception("Pairing attempt failed with an unhandled exception") - errors["pairing_code"] = "pairing_failed" - if pair_info and self.finish_pairing: code = pair_info["pairing_code"] try: @@ -307,14 +276,59 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow): self.finish_pairing = None errors["pairing_code"] = "pairing_failed" - if errors and "base" in errors: - return self.async_show_form(step_id="try_pair_later", errors=errors) + if not self.finish_pairing: + # Its possible that the first try may have been busy so + # we always check to see if self.finish_paring has been + # set. + discovery = await self.controller.find_ip_by_device_id(self.hkid) + + try: + self.finish_pairing = await discovery.start_pairing(self.hkid) + + except aiohomekit.BusyError: + # Already performing a pair setup operation with a different + # controller + return await self.async_step_busy_error() + except aiohomekit.MaxTriesError: + # The accessory has received more than 100 unsuccessful auth + # attempts. + return await self.async_step_max_tries_error() + except aiohomekit.UnavailableError: + # The accessory is already paired - cannot try to pair again. + return self.async_abort(reason="already_paired") + except aiohomekit.AccessoryNotFoundError: + # Can no longer find the device on the network + return self.async_abort(reason="accessory_not_found_error") + except IndexError: + # TLV error, usually not in pairing mode + _LOGGER.exception("Pairing communication failed") + return await self.async_step_protocol_error() + except Exception: # pylint: disable=broad-except + _LOGGER.exception("Pairing attempt failed with an unhandled exception") + errors["pairing_code"] = "pairing_failed" return self._async_step_pair_show_form(errors) - async def async_step_try_pair_later(self, pair_info=None): - """Retry pairing after the accessory is busy or unavailable.""" - return await self.async_step_pair(pair_info) + async def async_step_busy_error(self, user_input=None): + """Retry pairing after the accessory is busy.""" + if user_input is not None: + return await self.async_step_pair() + + return self.async_show_form(step_id="busy_error") + + async def async_step_max_tries_error(self, user_input=None): + """Retry pairing after the accessory has reached max tries.""" + if user_input is not None: + return await self.async_step_pair() + + return self.async_show_form(step_id="max_tries_error") + + async def async_step_protocol_error(self, user_input=None): + """Retry pairing after the accessory has a protocol error.""" + if user_input is not None: + return await self.async_step_pair() + + return self.async_show_form(step_id="protocol_error") @callback def _async_step_pair_show_form(self, errors=None): diff --git a/homeassistant/components/homekit_controller/strings.json b/homeassistant/components/homekit_controller/strings.json index 6be751e63c9..bc07b71fa75 100644 --- a/homeassistant/components/homekit_controller/strings.json +++ b/homeassistant/components/homekit_controller/strings.json @@ -17,19 +17,24 @@ "pairing_code": "Pairing Code" } }, - "try_pair_later": { - "title": "Pairing Unavailable", - "description": "Ensure the device is in pairing mode or try restarting the device, then continue to re-start pairing." - } + "protocol_error": { + "title": "Error communicating with the accessory", + "description": "The device may not be in pairing mode and may require a physical or virtual button press. Ensure the device is in pairing mode or try restarting the device, then continue to resume pairing." + }, + "busy_error": { + "title": "The device is already pairing with another controller", + "description": "Abort pairing on all controllers, or try restarting the device, then continue to resume pairing." + }, + "max_tries_error": { + "title": "Maximum authentication attempts exceeded", + "description": "The device has received more than 100 unsuccessful authentication attempts. Try restarting the device, then continue to resume pairing." + } }, "error": { "unable_to_pair": "Unable to pair, please try again.", "unknown_error": "Device reported an unknown error. Pairing failed.", - "protocol_error": "Error communicating with the accessory. Device may not be in pairing mode and may require a physical or virtual button press.", "authentication_error": "Incorrect HomeKit code. Please check it and try again.", "max_peers_error": "Device refused to add pairing as it has no free pairing storage.", - "busy_error": "Device refused to add pairing as it is already pairing with another controller.", - "max_tries_error": "Device refused to add pairing as it has received more than 100 unsuccessful authentication attempts.", "pairing_failed": "An unhandled error occurred while attempting to pair with this device. This may be a temporary failure or your device may not be supported currently." }, "abort": { diff --git a/homeassistant/components/homekit_controller/translations/en.json b/homeassistant/components/homekit_controller/translations/en.json index 6be751e63c9..bc07b71fa75 100644 --- a/homeassistant/components/homekit_controller/translations/en.json +++ b/homeassistant/components/homekit_controller/translations/en.json @@ -17,19 +17,24 @@ "pairing_code": "Pairing Code" } }, - "try_pair_later": { - "title": "Pairing Unavailable", - "description": "Ensure the device is in pairing mode or try restarting the device, then continue to re-start pairing." - } + "protocol_error": { + "title": "Error communicating with the accessory", + "description": "The device may not be in pairing mode and may require a physical or virtual button press. Ensure the device is in pairing mode or try restarting the device, then continue to resume pairing." + }, + "busy_error": { + "title": "The device is already pairing with another controller", + "description": "Abort pairing on all controllers, or try restarting the device, then continue to resume pairing." + }, + "max_tries_error": { + "title": "Maximum authentication attempts exceeded", + "description": "The device has received more than 100 unsuccessful authentication attempts. Try restarting the device, then continue to resume pairing." + } }, "error": { "unable_to_pair": "Unable to pair, please try again.", "unknown_error": "Device reported an unknown error. Pairing failed.", - "protocol_error": "Error communicating with the accessory. Device may not be in pairing mode and may require a physical or virtual button press.", "authentication_error": "Incorrect HomeKit code. Please check it and try again.", "max_peers_error": "Device refused to add pairing as it has no free pairing storage.", - "busy_error": "Device refused to add pairing as it is already pairing with another controller.", - "max_tries_error": "Device refused to add pairing as it has received more than 100 unsuccessful authentication attempts.", "pairing_failed": "An unhandled error occurred while attempting to pair with this device. This may be a temporary failure or your device may not be supported currently." }, "abort": { diff --git a/tests/components/homekit_controller/test_config_flow.py b/tests/components/homekit_controller/test_config_flow.py index caab127aa87..09c823ff498 100644 --- a/tests/components/homekit_controller/test_config_flow.py +++ b/tests/components/homekit_controller/test_config_flow.py @@ -334,19 +334,21 @@ async def test_pair_try_later_errors_on_start(hass, controller, exception, expec test_exc = exception("error") with patch.object(device, "start_pairing", side_effect=test_exc): result2 = await hass.config_entries.flow.async_configure(result["flow_id"]) - assert result2["step_id"] == "try_pair_later" + assert result2["step_id"] == expected assert result2["type"] == "form" - assert result2["errors"]["base"] == expected # Device is rebooted or placed into pairing mode as they have been instructed # We start pairing again - result3 = await hass.config_entries.flow.async_configure(result2["flow_id"]) + result3 = await hass.config_entries.flow.async_configure( + result2["flow_id"], user_input={"any": "key"} + ) # .. and successfully complete pair result4 = await hass.config_entries.flow.async_configure( result3["flow_id"], user_input={"pairing_code": "111-22-333"} ) + assert result4["type"] == "create_entry" assert result4["title"] == "Koogeek-LS1-20833F" @@ -373,7 +375,9 @@ async def test_pair_form_errors_on_start(hass, controller, exception, expected): # User initiates pairing - device refuses to enter pairing mode test_exc = exception("error") with patch.object(device, "start_pairing", side_effect=test_exc): - result = await hass.config_entries.flow.async_configure(result["flow_id"]) + result = await hass.config_entries.flow.async_configure( + result["flow_id"], user_input={"pairing_code": "111-22-333"} + ) assert result["type"] == "form" assert result["errors"]["pairing_code"] == expected @@ -384,10 +388,16 @@ async def test_pair_form_errors_on_start(hass, controller, exception, expected): "source": "zeroconf", } + # User gets back the form + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == "form" + assert result["errors"] == {} + # User re-tries entering pairing code result = await hass.config_entries.flow.async_configure( result["flow_id"], user_input={"pairing_code": "111-22-333"} ) + assert result["type"] == "create_entry" assert result["title"] == "Koogeek-LS1-20833F"