From de75f8223517029abc1fe94af731a15a55e08df1 Mon Sep 17 00:00:00 2001 From: rappenze Date: Mon, 22 Apr 2024 09:29:58 +0200 Subject: [PATCH] Refactor fibaro connect (#106875) * Refactor fibaro connect * Remove obsolete test * Add comment about ignored return value --- homeassistant/components/fibaro/__init__.py | 21 +++++-------- .../components/fibaro/config_flow.py | 11 ++----- tests/components/fibaro/test_config_flow.py | 30 ------------------- 3 files changed, 9 insertions(+), 53 deletions(-) diff --git a/homeassistant/components/fibaro/__init__.py b/homeassistant/components/fibaro/__init__.py index 4c1feb27629..5b7908ddf08 100644 --- a/homeassistant/components/fibaro/__init__.py +++ b/homeassistant/components/fibaro/__init__.py @@ -108,26 +108,21 @@ class FibaroController: # Device infos by fibaro device id self._device_infos: dict[int, DeviceInfo] = {} - def connect(self) -> bool: + def connect(self) -> None: """Start the communication with the Fibaro controller.""" - connected = self._client.connect() + # Return value doesn't need to be checked, + # it is only relevant when connecting without credentials + self._client.connect() info = self._client.read_info() self.hub_serial = info.serial_number self.hub_name = info.hc_name self.hub_model = info.platform self.hub_software_version = info.current_version - if connected is False: - _LOGGER.error( - "Invalid login for Fibaro HC. Please check username and password" - ) - return False - self._room_map = {room.fibaro_id: room for room in self._client.read_rooms()} self._read_devices() self._scenes = self._client.read_scenes() - return True def connect_with_error_handling(self) -> None: """Translate connect errors to easily differentiate auth and connect failures. @@ -135,9 +130,7 @@ class FibaroController: When there is a better error handling in the used library this can be improved. """ try: - connected = self.connect() - if not connected: - raise FibaroConnectFailed("Connect status is false") + self.connect() except HTTPError as http_ex: if http_ex.response.status_code == 403: raise FibaroAuthFailed from http_ex @@ -382,7 +375,7 @@ class FibaroController: pass -def _init_controller(data: Mapping[str, Any]) -> FibaroController: +def init_controller(data: Mapping[str, Any]) -> FibaroController: """Validate the user input allows us to connect to fibaro.""" controller = FibaroController(data) controller.connect_with_error_handling() @@ -395,7 +388,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: The unique id of the config entry is the serial number of the home center. """ try: - controller = await hass.async_add_executor_job(_init_controller, entry.data) + controller = await hass.async_add_executor_job(init_controller, entry.data) except FibaroConnectFailed as connect_ex: raise ConfigEntryNotReady( f"Could not connect to controller at {entry.data[CONF_URL]}" diff --git a/homeassistant/components/fibaro/config_flow.py b/homeassistant/components/fibaro/config_flow.py index 8c2fb502488..9003704348d 100644 --- a/homeassistant/components/fibaro/config_flow.py +++ b/homeassistant/components/fibaro/config_flow.py @@ -13,7 +13,7 @@ from homeassistant.config_entries import ConfigEntry, ConfigFlow, ConfigFlowResu from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME from homeassistant.core import HomeAssistant -from . import FibaroAuthFailed, FibaroConnectFailed, FibaroController +from . import FibaroAuthFailed, FibaroConnectFailed, init_controller from .const import CONF_IMPORT_PLUGINS, DOMAIN _LOGGER = logging.getLogger(__name__) @@ -28,19 +28,12 @@ STEP_USER_DATA_SCHEMA = vol.Schema( ) -def _connect_to_fibaro(data: dict[str, Any]) -> FibaroController: - """Validate the user input allows us to connect to fibaro.""" - controller = FibaroController(data) - controller.connect_with_error_handling() - return controller - - async def _validate_input(hass: HomeAssistant, data: dict[str, Any]) -> dict[str, Any]: """Validate the user input allows us to connect. Data has the keys from STEP_USER_DATA_SCHEMA with values provided by the user. """ - controller = await hass.async_add_executor_job(_connect_to_fibaro, data) + controller = await hass.async_add_executor_job(init_controller, data) _LOGGER.debug( "Successfully connected to fibaro home center %s with name %s", diff --git a/tests/components/fibaro/test_config_flow.py b/tests/components/fibaro/test_config_flow.py index dcf5f12a24a..b6b4e3992cd 100644 --- a/tests/components/fibaro/test_config_flow.py +++ b/tests/components/fibaro/test_config_flow.py @@ -89,36 +89,6 @@ async def test_config_flow_user_initiated_success(hass: HomeAssistant) -> None: } -async def test_config_flow_user_initiated_connect_failure( - hass: HomeAssistant, mock_fibaro_client: Mock -) -> None: - """Connect failure in flow manually initialized by the user.""" - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - - assert result["type"] is FlowResultType.FORM - assert result["step_id"] == "user" - assert result["errors"] == {} - - mock_fibaro_client.connect.return_value = False - - result = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - CONF_URL: TEST_URL, - CONF_USERNAME: TEST_USERNAME, - CONF_PASSWORD: TEST_PASSWORD, - }, - ) - - assert result["type"] is FlowResultType.FORM - assert result["step_id"] == "user" - assert result["errors"] == {"base": "cannot_connect"} - - await _recovery_after_failure_works(hass, mock_fibaro_client, result) - - async def test_config_flow_user_initiated_auth_failure( hass: HomeAssistant, mock_fibaro_client: Mock ) -> None: