From 7ff77936adde7e60feb9d516e8a4d2a260cedafa Mon Sep 17 00:00:00 2001 From: Jc2k Date: Thu, 2 May 2019 00:02:29 +0100 Subject: [PATCH] Add and improve Homekit controller pairing messages and errors (#23532) * Be clear about pairing code format * Handle more homekit errors while pairing * Update en translation * Fix log message feedback --- .../homekit_controller/.translations/en.json | 8 +- .../homekit_controller/config_flow.py | 13 +++ .../homekit_controller/strings.json | 11 ++- .../homekit_controller/test_config_flow.py | 87 +++++++++++++++++++ 4 files changed, 115 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/homekit_controller/.translations/en.json b/homeassistant/components/homekit_controller/.translations/en.json index 591e035ed18..059f0f7afe7 100644 --- a/homeassistant/components/homekit_controller/.translations/en.json +++ b/homeassistant/components/homekit_controller/.translations/en.json @@ -1,6 +1,7 @@ { "config": { "abort": { + "accessory_not_found_error": "Cannot add pairing as device can no longer be found.", "already_configured": "Accessory is already configured with this controller.", "already_paired": "This accessory is already paired to another device. Please reset the accessory and try again.", "ignored_model": "HomeKit support for this model is blocked as a more feature complete native integration is available.", @@ -9,15 +10,20 @@ }, "error": { "authentication_error": "Incorrect HomeKit code. Please check it and try again.", + "busy_error": "Device refused to add pairing as it is already pairing with another controller.", + "max_peers_error": "Device refused to add pairing as it has no free pairing storage.", + "max_tries_error": "Device refused to add pairing as it has received more than 100 unsuccessful authentication attempts.", + "pairing_failed": "An unhandled error occured while attempting to pair with this device. This may be a temporary failure or your device may not be supported currently.", "unable_to_pair": "Unable to pair, please try again.", "unknown_error": "Device reported an unknown error. Pairing failed." }, + "flow_title": "HomeKit Accessory: {name}", "step": { "pair": { "data": { "pairing_code": "Pairing Code" }, - "description": "Enter your HomeKit pairing code to use this accessory", + "description": "Enter your HomeKit pairing code (in the format XXX-XX-XXX) to use this accessory", "title": "Pair with HomeKit Accessory" }, "user": { diff --git a/homeassistant/components/homekit_controller/config_flow.py b/homeassistant/components/homekit_controller/config_flow.py index 310f187556d..197d15116b1 100644 --- a/homeassistant/components/homekit_controller/config_flow.py +++ b/homeassistant/components/homekit_controller/config_flow.py @@ -237,8 +237,21 @@ class HomekitControllerFlowHandler(config_entries.ConfigFlow): errors['pairing_code'] = 'authentication_error' except homekit.UnknownError: errors['pairing_code'] = 'unknown_error' + except homekit.MaxTriesError: + errors['pairing_code'] = 'max_tries_error' + except homekit.BusyError: + errors['pairing_code'] = 'busy_error' + except homekit.MaxPeersError: + errors['pairing_code'] = 'max_peers_error' + except homekit.AccessoryNotFoundError: + return self.async_abort(reason='accessory_not_found_error') except homekit.UnavailableError: return self.async_abort(reason='already_paired') + except Exception: # pylint: disable=broad-except + _LOGGER.exception( + "Pairing attempt failed with an unhandled exception" + ) + errors['pairing_code'] = 'pairing_failed' return self.async_show_form( step_id='pair', diff --git a/homeassistant/components/homekit_controller/strings.json b/homeassistant/components/homekit_controller/strings.json index 075bf6ca6cd..eceaa624b0f 100644 --- a/homeassistant/components/homekit_controller/strings.json +++ b/homeassistant/components/homekit_controller/strings.json @@ -12,7 +12,7 @@ }, "pair": { "title": "Pair with HomeKit Accessory", - "description": "Enter your HomeKit pairing code to use this accessory", + "description": "Enter your HomeKit pairing code (in the format XXX-XX-XXX) to use this accessory", "data": { "pairing_code": "Pairing Code" } @@ -21,14 +21,19 @@ "error": { "unable_to_pair": "Unable to pair, please try again.", "unknown_error": "Device reported an unknown error. Pairing failed.", - "authentication_error": "Incorrect HomeKit code. Please check it and try again." + "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 occured while attempting to pair with this device. This may be a temporary failure or your device may not be supported currently." }, "abort": { "no_devices": "No unpaired devices could be found", "already_paired": "This accessory is already paired to another device. Please reset the accessory and try again.", "ignored_model": "HomeKit support for this model is blocked as a more feature complete native integration is available.", "already_configured": "Accessory is already configured with this controller.", - "invalid_config_entry": "This device is showing as ready to pair but there is already a conflicting config entry for it in Home Assistant that must first be removed." + "invalid_config_entry": "This device is showing as ready to pair but there is already a conflicting config entry for it in Home Assistant that must first be removed.", + "accessory_not_found_error": "Cannot add pairing as device can no longer be found." } } } diff --git a/tests/components/homekit_controller/test_config_flow.py b/tests/components/homekit_controller/test_config_flow.py index 2dd42737477..c8b81a88478 100644 --- a/tests/components/homekit_controller/test_config_flow.py +++ b/tests/components/homekit_controller/test_config_flow.py @@ -3,6 +3,7 @@ import json from unittest import mock import homekit +import pytest from homeassistant.components.homekit_controller import config_flow from homeassistant.components.homekit_controller.const import KNOWN_DEVICES @@ -12,6 +13,18 @@ from tests.components.homekit_controller.common import ( ) +ERROR_MAPPING_FORM_FIXTURE = [ + (homekit.MaxPeersError, 'max_peers_error'), + (homekit.BusyError, 'busy_error'), + (homekit.MaxTriesError, 'max_tries_error'), + (KeyError, 'pairing_failed'), +] + +ERROR_MAPPING_ABORT_FIXTURE = [ + (homekit.AccessoryNotFoundError, 'accessory_not_found_error'), +] + + def _setup_flow_handler(hass): flow = config_flow.HomekitControllerFlowHandler() flow.hass = hass @@ -347,6 +360,80 @@ async def test_pair_unable_to_pair(hass): assert result['errors']['pairing_code'] == 'unable_to_pair' +@pytest.mark.parametrize("exception,expected", ERROR_MAPPING_ABORT_FIXTURE) +async def test_pair_abort_errors(hass, exception, expected): + """Test various pairing errors.""" + discovery_info = { + 'name': 'TestDevice', + 'host': '127.0.0.1', + 'port': 8080, + 'properties': { + 'md': 'TestDevice', + 'id': '00:00:00:00:00:00', + 'c#': 1, + 'sf': 1, + } + } + + flow = _setup_flow_handler(hass) + + result = await flow.async_step_discovery(discovery_info) + assert result['type'] == 'form' + assert result['step_id'] == 'pair' + assert flow.context == {'title_placeholders': {'name': 'TestDevice'}} + + controller = mock.Mock() + controller.pairings = {} + + with mock.patch('homekit.Controller') as controller_cls: + controller_cls.return_value = controller + controller.perform_pairing.side_effect = exception('error') + result = await flow.async_step_pair({ + 'pairing_code': '111-22-33', + }) + + assert result['type'] == 'abort' + assert result['reason'] == expected + assert flow.context == {'title_placeholders': {'name': 'TestDevice'}} + + +@pytest.mark.parametrize("exception,expected", ERROR_MAPPING_FORM_FIXTURE) +async def test_pair_form_errors(hass, exception, expected): + """Test various pairing errors.""" + discovery_info = { + 'name': 'TestDevice', + 'host': '127.0.0.1', + 'port': 8080, + 'properties': { + 'md': 'TestDevice', + 'id': '00:00:00:00:00:00', + 'c#': 1, + 'sf': 1, + } + } + + flow = _setup_flow_handler(hass) + + result = await flow.async_step_discovery(discovery_info) + assert result['type'] == 'form' + assert result['step_id'] == 'pair' + assert flow.context == {'title_placeholders': {'name': 'TestDevice'}} + + controller = mock.Mock() + controller.pairings = {} + + with mock.patch('homekit.Controller') as controller_cls: + controller_cls.return_value = controller + controller.perform_pairing.side_effect = exception('error') + result = await flow.async_step_pair({ + 'pairing_code': '111-22-33', + }) + + assert result['type'] == 'form' + assert result['errors']['pairing_code'] == expected + assert flow.context == {'title_placeholders': {'name': 'TestDevice'}} + + async def test_pair_authentication_error(hass): """Pairing code is incorrect.""" discovery_info = {