Refactor fibaro connect (#106875)

* Refactor fibaro connect

* Remove obsolete test

* Add comment about ignored return value

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()
+            self.connect()
-            if not connected:
-                raise FibaroConnectFailed("Connect status is false")
         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]}"

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",

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: