From 3794b4e1a18097f924cfd45825dde0546c4e3bce Mon Sep 17 00:00:00 2001 From: David Recordon Date: Tue, 27 Jan 2026 07:28:32 -0800 Subject: [PATCH] Improve Control4 connection error logging (#159979) Co-authored-by: Joostlek --- .../components/control4/config_flow.py | 178 ++++++----- .../components/control4/strings.json | 4 +- tests/components/control4/conftest.py | 39 ++- tests/components/control4/test_config_flow.py | 282 ++++++++++-------- 4 files changed, 273 insertions(+), 230 deletions(-) diff --git a/homeassistant/components/control4/config_flow.py b/homeassistant/components/control4/config_flow.py index 9d5df61b513..1ba8eba47dd 100644 --- a/homeassistant/components/control4/config_flow.py +++ b/homeassistant/components/control4/config_flow.py @@ -3,12 +3,12 @@ from __future__ import annotations import logging -from typing import TYPE_CHECKING, Any +from typing import Any from aiohttp.client_exceptions import ClientError from pyControl4.account import C4Account from pyControl4.director import C4Director -from pyControl4.error_handling import NotFound, Unauthorized +from pyControl4.error_handling import BadCredentials, NotFound, Unauthorized import voluptuous as vol from homeassistant.config_entries import ( @@ -22,8 +22,7 @@ from homeassistant.const import ( CONF_SCAN_INTERVAL, CONF_USERNAME, ) -from homeassistant.core import HomeAssistant, callback -from homeassistant.exceptions import HomeAssistantError +from homeassistant.core import callback from homeassistant.helpers import aiohttp_client, config_validation as cv from homeassistant.helpers.device_registry import format_mac @@ -46,106 +45,107 @@ DATA_SCHEMA = vol.Schema( ) -class Control4Validator: - """Validates that config details can be used to authenticate and communicate with Control4.""" - - def __init__( - self, host: str, username: str, password: str, hass: HomeAssistant - ) -> None: - """Initialize.""" - self.host = host - self.username = username - self.password = password - self.controller_unique_id = None - self.director_bearer_token = None - self.hass = hass - - async def authenticate(self) -> bool: - """Test if we can authenticate with the Control4 account API.""" - try: - account_session = aiohttp_client.async_get_clientsession(self.hass) - account = C4Account(self.username, self.password, account_session) - # Authenticate with Control4 account - await account.getAccountBearerToken() - - # Get controller name - account_controllers = await account.getAccountControllers() - self.controller_unique_id = account_controllers["controllerCommonName"] - - # Get bearer token to communicate with controller locally - self.director_bearer_token = ( - await account.getDirectorBearerToken(self.controller_unique_id) - )["token"] - except (Unauthorized, NotFound): - return False - return True - - async def connect_to_director(self) -> bool: - """Test if we can connect to the local Control4 Director.""" - try: - director_session = aiohttp_client.async_get_clientsession( - self.hass, verify_ssl=False - ) - director = C4Director( - self.host, self.director_bearer_token, director_session - ) - await director.getAllItemInfo() - except (Unauthorized, ClientError, TimeoutError): - _LOGGER.error("Failed to connect to the Control4 controller") - return False - return True - - class Control4ConfigFlow(ConfigFlow, domain=DOMAIN): """Handle a config flow for Control4.""" VERSION = 1 + async def _async_try_connect( + self, user_input: dict[str, Any] + ) -> tuple[dict[str, str], dict[str, Any] | None, dict[str, str]]: + """Try to connect to Control4 and return errors, data, and placeholders.""" + errors: dict[str, str] = {} + description_placeholders: dict[str, str] = {} + data: dict[str, Any] | None = None + + host = user_input[CONF_HOST] + username = user_input[CONF_USERNAME] + password = user_input[CONF_PASSWORD] + + # Step 1: Authenticate with Control4 cloud API + account_session = aiohttp_client.async_get_clientsession(self.hass) + account = C4Account(username, password, account_session) + try: + await account.getAccountBearerToken() + + account_controllers = await account.getAccountControllers() + controller_unique_id = account_controllers["controllerCommonName"] + + director_bearer_token = ( + await account.getDirectorBearerToken(controller_unique_id) + )["token"] + except (BadCredentials, Unauthorized): + errors["base"] = "invalid_auth" + return errors, data, description_placeholders + except NotFound: + errors["base"] = "controller_not_found" + return errors, data, description_placeholders + except Exception: + _LOGGER.exception( + "Unexpected exception during Control4 account authentication" + ) + errors["base"] = "unknown" + return errors, data, description_placeholders + + # Step 2: Connect to local Control4 Director + director_session = aiohttp_client.async_get_clientsession( + self.hass, verify_ssl=False + ) + director = C4Director(host, director_bearer_token, director_session) + try: + await director.getAllItemInfo() + except Unauthorized: + errors["base"] = "director_auth_failed" + return errors, data, description_placeholders + except (ClientError, TimeoutError): + errors["base"] = "cannot_connect" + description_placeholders["host"] = host + return errors, data, description_placeholders + except Exception: + _LOGGER.exception( + "Unexpected exception during Control4 director connection" + ) + errors["base"] = "unknown" + return errors, data, description_placeholders + + # Success - return the data needed for entry creation + data = { + CONF_HOST: host, + CONF_USERNAME: username, + CONF_PASSWORD: password, + CONF_CONTROLLER_UNIQUE_ID: controller_unique_id, + } + + return errors, data, description_placeholders + async def async_step_user( self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Handle the initial step.""" - errors = {} - if user_input is not None: - hub = Control4Validator( - user_input[CONF_HOST], - user_input[CONF_USERNAME], - user_input[CONF_PASSWORD], - self.hass, - ) - try: - if not await hub.authenticate(): - raise InvalidAuth # noqa: TRY301 - if not await hub.connect_to_director(): - raise CannotConnect # noqa: TRY301 - except InvalidAuth: - errors["base"] = "invalid_auth" - except CannotConnect: - errors["base"] = "cannot_connect" - except Exception: - _LOGGER.exception("Unexpected exception") - errors["base"] = "unknown" + errors: dict[str, str] = {} + description_placeholders: dict[str, str] = {} - if not errors: - controller_unique_id = hub.controller_unique_id - if TYPE_CHECKING: - assert hub.controller_unique_id + if user_input is not None: + errors, data, description_placeholders = await self._async_try_connect( + user_input + ) + + if not errors and data is not None: + controller_unique_id = data[CONF_CONTROLLER_UNIQUE_ID] mac = (controller_unique_id.split("_", 3))[2] formatted_mac = format_mac(mac) await self.async_set_unique_id(formatted_mac) self._abort_if_unique_id_configured() return self.async_create_entry( title=controller_unique_id, - data={ - CONF_HOST: user_input[CONF_HOST], - CONF_USERNAME: user_input[CONF_USERNAME], - CONF_PASSWORD: user_input[CONF_PASSWORD], - CONF_CONTROLLER_UNIQUE_ID: controller_unique_id, - }, + data=data, ) return self.async_show_form( - step_id="user", data_schema=DATA_SCHEMA, errors=errors + step_id="user", + data_schema=DATA_SCHEMA, + errors=errors, + description_placeholders=description_placeholders, ) @staticmethod @@ -178,11 +178,3 @@ class OptionsFlowHandler(OptionsFlowWithReload): } ) return self.async_show_form(step_id="init", data_schema=data_schema) - - -class CannotConnect(HomeAssistantError): - """Error to indicate we cannot connect.""" - - -class InvalidAuth(HomeAssistantError): - """Error to indicate there is invalid auth.""" diff --git a/homeassistant/components/control4/strings.json b/homeassistant/components/control4/strings.json index 34331bc18fa..9b5981c2e3c 100644 --- a/homeassistant/components/control4/strings.json +++ b/homeassistant/components/control4/strings.json @@ -4,7 +4,9 @@ "already_configured": "[%key:common::config_flow::abort::already_configured_device%]" }, "error": { - "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]", + "cannot_connect": "Failed to connect to the Control4 director at {host}", + "controller_not_found": "No Control4 controller found on this account", + "director_auth_failed": "The Control4 director rejected the authentication token", "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]", "unknown": "[%key:common::config_flow::error::unknown%]" }, diff --git a/tests/components/control4/conftest.py b/tests/components/control4/conftest.py index c6b54fb8373..d5c0e1d2993 100644 --- a/tests/components/control4/conftest.py +++ b/tests/components/control4/conftest.py @@ -32,16 +32,35 @@ def mock_config_entry() -> MockConfigEntry: ) +@pytest.fixture +def mock_setup_entry() -> Generator[AsyncMock]: + """Mock control4 setup entry.""" + with patch( + "homeassistant.components.control4.async_setup_entry", return_value=True + ) as mock_setup: + yield mock_setup + + @pytest.fixture def mock_c4_account() -> Generator[MagicMock]: """Mock a Control4 Account client.""" - with patch( - "homeassistant.components.control4.C4Account", autospec=True - ) as mock_account_class: + with ( + patch( + "homeassistant.components.control4.C4Account", autospec=True + ) as mock_account_class, + patch( + "homeassistant.components.control4.config_flow.C4Account", + new=mock_account_class, + ), + ): mock_account = mock_account_class.return_value mock_account.getAccountBearerToken = AsyncMock() mock_account.getAccountControllers = AsyncMock( - return_value={"href": "https://example.com"} + return_value={ + "controllerCommonName": "control4_model_00AA00AA00AA", + "href": "https://apis.control4.com/account/v3/rest/accounts/000000", + "name": "Name", + } ) mock_account.getDirectorBearerToken = AsyncMock(return_value={"token": "test"}) mock_account.getControllerOSVersion = AsyncMock(return_value="3.2.0") @@ -51,9 +70,15 @@ def mock_c4_account() -> Generator[MagicMock]: @pytest.fixture def mock_c4_director() -> Generator[MagicMock]: """Mock a Control4 Director client.""" - with patch( - "homeassistant.components.control4.C4Director", autospec=True - ) as mock_director_class: + with ( + patch( + "homeassistant.components.control4.C4Director", autospec=True + ) as mock_director_class, + patch( + "homeassistant.components.control4.config_flow.C4Director", + new=mock_director_class, + ), + ): mock_director = mock_director_class.return_value # Multi-platform setup: media room, climate room, shared devices # Note: The API returns JSON strings, so we load fixtures as strings diff --git a/tests/components/control4/test_config_flow.py b/tests/components/control4/test_config_flow.py index edab8c27164..773f692c2ef 100644 --- a/tests/components/control4/test_config_flow.py +++ b/tests/components/control4/test_config_flow.py @@ -1,13 +1,13 @@ """Test the Control4 config flow.""" -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock -from pyControl4.account import C4Account -from pyControl4.director import C4Director -from pyControl4.error_handling import Unauthorized +from aiohttp.client_exceptions import ClientError +from pyControl4.error_handling import BadCredentials, NotFound, Unauthorized +import pytest -from homeassistant import config_entries from homeassistant.components.control4.const import DEFAULT_SCAN_INTERVAL, DOMAIN +from homeassistant.config_entries import SOURCE_USER from homeassistant.const import ( CONF_HOST, CONF_PASSWORD, @@ -22,155 +22,178 @@ from .conftest import MOCK_HOST, MOCK_PASSWORD, MOCK_USERNAME from tests.common import MockConfigEntry -def _get_mock_c4_account(): - c4_account_mock = AsyncMock(C4Account) - - c4_account_mock.getAccountControllers.return_value = { - "controllerCommonName": "control4_model_00AA00AA00AA", - "href": "https://apis.control4.com/account/v3/rest/accounts/000000", - "name": "Name", - } - - c4_account_mock.getDirectorBearerToken.return_value = {"token": "token"} - - return c4_account_mock - - -def _get_mock_c4_director(): - c4_director_mock = AsyncMock(C4Director) - c4_director_mock.getAllItemInfo.return_value = {} - - return c4_director_mock - - -async def test_form(hass: HomeAssistant) -> None: - """Test we get the form.""" +async def test_full_flow( + hass: HomeAssistant, + mock_c4_account: AsyncMock, + mock_c4_director: AsyncMock, + mock_setup_entry: AsyncMock, +) -> None: + """Test full config flow.""" result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} + DOMAIN, context={"source": SOURCE_USER} ) assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "user" assert result["errors"] == {} - c4_account = _get_mock_c4_account() - c4_director = _get_mock_c4_director() - with ( - patch( - "homeassistant.components.control4.config_flow.C4Account", - return_value=c4_account, - ), - patch( - "homeassistant.components.control4.config_flow.C4Director", - return_value=c4_director, - ), - patch( - "homeassistant.components.control4.async_setup_entry", - return_value=True, - ) as mock_setup_entry, - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_HOST: MOCK_HOST, - CONF_USERNAME: MOCK_USERNAME, - CONF_PASSWORD: MOCK_PASSWORD, - }, - ) - await hass.async_block_till_done() + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_HOST: MOCK_HOST, + CONF_USERNAME: MOCK_USERNAME, + CONF_PASSWORD: MOCK_PASSWORD, + }, + ) - assert result2["type"] is FlowResultType.CREATE_ENTRY - assert result2["title"] == "control4_model_00AA00AA00AA" - assert result2["data"] == { + assert result["type"] is FlowResultType.CREATE_ENTRY + assert result["title"] == "control4_model_00AA00AA00AA" + assert result["data"] == { CONF_HOST: MOCK_HOST, CONF_USERNAME: MOCK_USERNAME, CONF_PASSWORD: MOCK_PASSWORD, "controller_unique_id": "control4_model_00AA00AA00AA", } - assert result2["result"].unique_id == "00:aa:00:aa:00:aa" + assert result["result"].unique_id == "00:aa:00:aa:00:aa" assert len(mock_setup_entry.mock_calls) == 1 -async def test_form_invalid_auth(hass: HomeAssistant) -> None: - """Test we handle invalid auth.""" +@pytest.mark.parametrize( + ("exception", "error"), + [ + (BadCredentials("Invalid username or password"), "invalid_auth"), + (Unauthorized("Permission denied"), "invalid_auth"), + (NotFound("something"), "controller_not_found"), + (Exception("Some other exception"), "unknown"), + ], +) +@pytest.mark.usefixtures("mock_setup_entry") +async def test_user_flow_errors( + hass: HomeAssistant, + mock_c4_account: AsyncMock, + mock_c4_director: AsyncMock, + exception: Exception, + error: str, +) -> None: + """Test we handle errors in the user flow.""" result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} + DOMAIN, context={"source": SOURCE_USER} ) - with patch( - "homeassistant.components.control4.config_flow.C4Account", - side_effect=Unauthorized("message"), - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_HOST: MOCK_HOST, - CONF_USERNAME: MOCK_USERNAME, - CONF_PASSWORD: MOCK_PASSWORD, - }, - ) + mock_c4_account.getAccountBearerToken.side_effect = exception - assert result2["type"] is FlowResultType.FORM - assert result2["errors"] == {"base": "invalid_auth"} - - -async def test_form_unexpected_exception(hass: HomeAssistant) -> None: - """Test we handle an unexpected exception.""" - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_HOST: MOCK_HOST, + CONF_USERNAME: MOCK_USERNAME, + CONF_PASSWORD: MOCK_PASSWORD, + }, ) - with patch( - "homeassistant.components.control4.config_flow.C4Account", - side_effect=ValueError("message"), - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_HOST: MOCK_HOST, - CONF_USERNAME: MOCK_USERNAME, - CONF_PASSWORD: MOCK_PASSWORD, - }, - ) + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": error} - assert result2["type"] is FlowResultType.FORM - assert result2["errors"] == {"base": "unknown"} + mock_c4_account.getAccountBearerToken.side_effect = None - -async def test_form_cannot_connect(hass: HomeAssistant) -> None: - """Test we handle cannot connect error.""" - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_HOST: MOCK_HOST, + CONF_USERNAME: MOCK_USERNAME, + CONF_PASSWORD: MOCK_PASSWORD, + }, ) - with ( - patch( - "homeassistant.components.control4.config_flow.Control4Validator.authenticate", - return_value=True, - ), - patch( - "homeassistant.components.control4.config_flow.C4Director", - side_effect=Unauthorized("message"), - ), - ): - result2 = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_HOST: MOCK_HOST, - CONF_USERNAME: MOCK_USERNAME, - CONF_PASSWORD: MOCK_PASSWORD, - }, - ) - - assert result2["type"] is FlowResultType.FORM - assert result2["errors"] == {"base": "cannot_connect"} + assert result["type"] is FlowResultType.CREATE_ENTRY -async def test_option_flow(hass: HomeAssistant) -> None: +@pytest.mark.parametrize( + ("exception", "error"), + [ + (Unauthorized("Permission denied"), "director_auth_failed"), + (ClientError, "cannot_connect"), + (TimeoutError, "cannot_connect"), + (Exception("Some other exception"), "unknown"), + ], +) +@pytest.mark.usefixtures("mock_setup_entry") +async def test_user_flow_director_errors( + hass: HomeAssistant, + mock_c4_account: AsyncMock, + mock_c4_director: AsyncMock, + exception: Exception, + error: str, +) -> None: + """Test we handle director auth failure.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER} + ) + + mock_c4_director.getAllItemInfo.side_effect = exception + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_HOST: MOCK_HOST, + CONF_USERNAME: MOCK_USERNAME, + CONF_PASSWORD: MOCK_PASSWORD, + }, + ) + + assert result["type"] is FlowResultType.FORM + assert result["errors"] == {"base": error} + + mock_c4_director.getAllItemInfo.side_effect = None + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_HOST: MOCK_HOST, + CONF_USERNAME: MOCK_USERNAME, + CONF_PASSWORD: MOCK_PASSWORD, + }, + ) + + assert result["type"] is FlowResultType.CREATE_ENTRY + + +async def test_duplicate_entry( + hass: HomeAssistant, + mock_c4_account: AsyncMock, + mock_c4_director: AsyncMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test that duplicate entries are not created.""" + mock_config_entry.add_to_hass(hass) + + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER} + ) + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "user" + assert result["errors"] == {} + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + { + CONF_HOST: MOCK_HOST, + CONF_USERNAME: MOCK_USERNAME, + CONF_PASSWORD: MOCK_PASSWORD, + }, + ) + + assert result["type"] is FlowResultType.ABORT + assert result["reason"] == "already_configured" + + +async def test_option_flow( + hass: HomeAssistant, mock_config_entry: MockConfigEntry +) -> None: """Test config flow options.""" - entry = MockConfigEntry(domain=DOMAIN, data={}, options=None) - entry.add_to_hass(hass) + mock_config_entry.add_to_hass(hass) - result = await hass.config_entries.options.async_init(entry.entry_id) + result = await hass.config_entries.options.async_init(mock_config_entry.entry_id) assert result["type"] is FlowResultType.FORM assert result["step_id"] == "init" @@ -185,12 +208,13 @@ async def test_option_flow(hass: HomeAssistant) -> None: } -async def test_option_flow_defaults(hass: HomeAssistant) -> None: +async def test_option_flow_defaults( + hass: HomeAssistant, mock_config_entry: MockConfigEntry +) -> None: """Test config flow options.""" - entry = MockConfigEntry(domain=DOMAIN, data={}, options=None) - entry.add_to_hass(hass) + mock_config_entry.add_to_hass(hass) - result = await hass.config_entries.options.async_init(entry.entry_id) + result = await hass.config_entries.options.async_init(mock_config_entry.entry_id) assert result["type"] is FlowResultType.FORM assert result["step_id"] == "init"