always use async_show_progress_done to leave a step that uses async_show_progress

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

homeassistant/components/otbr/config_flow.py

@@ -2,9 +2,10 @@
 
 from __future__ import annotations
 
+import asyncio
 from contextlib import suppress
 import logging
-from typing import TYPE_CHECKING, cast
+from typing import TYPE_CHECKING, Any, cast
 
 import aiohttp
 import python_otbr_api
@@ -40,6 +41,9 @@ if TYPE_CHECKING:
 
 _LOGGER = logging.getLogger(__name__)
 
+OTBR_CONNECTION_TIMEOUT = 20.0
+OTBR_RETRY_BACKOFF = 0.5
+
 
 class AlreadyConfigured(HomeAssistantError):
     """Raised when the router is already configured."""
@@ -83,6 +87,15 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN):
 
     VERSION = 1
 
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        """Instantiate otbr config flow."""
+        super().__init__(*args, **kwargs)
+
+        self._url: str | None = None
+        self._title = "Open Thread Border Router"
+        self._discovery_info: HassioServiceInfo | None = None
+        self._addon_connect_task: asyncio.Task[bytes] | None = None
+
     async def _set_dataset(self, api: python_otbr_api.OTBR, otbr_url: str) -> None:
         """Connect to the OTBR and create or apply a dataset if it doesn't have one."""
         if await api.get_active_dataset_tlvs() is None:
@@ -153,6 +166,41 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN):
 
         return border_agent_id
 
+    async def _connect_with_retry(self, url: str) -> bytes:
+        """Connect to OTBR with retry logic for up to OTBR_CONNECTION_TIMEOUT seconds."""
+        start_time = self.hass.loop.time()
+
+        while True:
+            elapsed_time = self.hass.loop.time() - start_time
+
+            if elapsed_time >= OTBR_CONNECTION_TIMEOUT:
+                raise HomeAssistantError(
+                    f"Failed to connect to OTBR after {OTBR_CONNECTION_TIMEOUT} seconds"
+                )
+
+            try:
+                return await self._connect_and_configure_router(url)
+            except aiohttp.ClientConnectionError as exc:
+                _LOGGER.debug(
+                    "ClientConnectorError after %.2f seconds, retrying in %.1fs: %s",
+                    elapsed_time,
+                    OTBR_RETRY_BACKOFF,
+                    exc,
+                )
+                await asyncio.sleep(OTBR_RETRY_BACKOFF)
+
+    async def _connect_addon_and_configure(self) -> bytes:
+        """Connect to the addon and configure it."""
+        assert self._url is not None
+        assert self._discovery_info is not None
+
+        border_agent_id = await self._connect_with_retry(self._url)
+
+        await self.async_set_unique_id(self._discovery_info.uuid)
+        self._title = await _title(self.hass, self._discovery_info)
+
+        return border_agent_id
+
     async def async_step_user(
         self, user_input: dict[str, str] | None = None
     ) -> ConfigFlowResult:
@@ -160,9 +208,9 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN):
         errors = {}
 
         if user_input is not None:
-            url = user_input[CONF_URL].rstrip("/")
+            self._url = user_input[CONF_URL].rstrip("/")
             try:
-                border_agent_id = await self._connect_and_configure_router(url)
+                border_agent_id = await self._connect_and_configure_router(self._url)
             except AlreadyConfigured:
                 errors["base"] = "already_configured"
             except (
@@ -170,13 +218,13 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN):
                 aiohttp.ClientError,
                 TimeoutError,
             ) as exc:
-                _LOGGER.debug("Failed to communicate with OTBR@%s: %s", url, exc)
+                _LOGGER.debug("Failed to communicate with OTBR@%s: %s", self._url, exc)
                 errors["base"] = "cannot_connect"
             else:
                 await self.async_set_unique_id(border_agent_id.hex())
                 return self.async_create_entry(
-                    title="Open Thread Border Router",
-                    data={CONF_URL: url},
+                    title=self._title,
+                    data={CONF_URL: self._url},
                 )
 
         data_schema = vol.Schema({CONF_URL: str})
@@ -188,11 +236,12 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN):
         self, discovery_info: HassioServiceInfo
     ) -> ConfigFlowResult:
         """Handle hassio discovery."""
+        self._discovery_info = discovery_info
         config = discovery_info.config
-        url = f"http://{config['host']}:{config['port']}"
-        config_entry_data = {"url": url}
+        self._url = f"http://{config['host']}:{config['port']}"
 
         if current_entries := self._async_current_entries():
+            config_entry_data = {"url": self._url}
             for current_entry in current_entries:
                 if current_entry.source != SOURCE_HASSIO:
                     continue
@@ -228,20 +277,56 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN):
                 )
                 return self.async_abort(reason="already_configured")
 
+        return await self.async_step_connect_addon()
+
+    async def async_step_connect_addon(
+        self, user_input: dict[str, str] | None = None
+    ) -> ConfigFlowResult:
+        """Connect to OTBR with retry logic."""
+        assert self._url
+
+        if not self._addon_connect_task:
+            self._addon_connect_task = self.hass.async_create_task(
+                self._connect_addon_and_configure()
+            )
+
+        if not self._addon_connect_task.done():
+            return self.async_show_progress(
+                step_id="connect_addon",
+                progress_action="connect_addon",
+                description_placeholders={},
+                progress_task=self._addon_connect_task,
+            )
+
         try:
-            await self._connect_and_configure_router(url)
+            await self._addon_connect_task
         except AlreadyConfigured:
-            return self.async_abort(reason="already_configured")
+            return self.async_show_progress_done(next_step_id="already_configured")
         except (
             python_otbr_api.OTBRError,
             aiohttp.ClientError,
             TimeoutError,
+            HomeAssistantError,
         ) as exc:
-            _LOGGER.warning("Failed to communicate with OTBR@%s: %s", url, exc)
+            _LOGGER.warning("Failed to communicate with OTBR@%s: %s", self._url, exc)
             return self.async_abort(reason="unknown")
+        finally:
+            self._addon_connect_task = None
 
-        await self.async_set_unique_id(discovery_info.uuid)
+        return self.async_show_progress_done(next_step_id="create_entry")
+
+    async def async_step_already_configured(
+        self, user_input: dict[str, str] | None = None
+    ) -> ConfigFlowResult:
+        """Already configured."""
+        return self.async_abort(reason="already_configured")
+
+    async def async_step_create_entry(
+        self, user_input: dict[str, str] | None = None
+    ) -> ConfigFlowResult:
+        """Create an entry for the OTBR."""
+        assert self._url
         return self.async_create_entry(
-            title=await _title(self.hass, discovery_info),
-            data=config_entry_data,
+            title=self._title,
+            data={"url": self._url},
         )

homeassistant/components/otbr/strings.json

@@ -8,6 +8,9 @@
         "description": "Provide URL for the OpenThread Border Router's REST API"
       }
     },
+    "progress": {
+      "connect_addon": "Connecting to Open Thread Border Router"
+    },
     "error": {
       "already_configured": "The Thread border router is already configured",
       "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]"

tests/components/otbr/test_config_flow.py

@@ -18,8 +18,14 @@ from homeassistant.components.homeassistant_hardware.util import (
     FirmwareInfo,
     OwningAddon,
 )
+from homeassistant.components.otbr.config_flow import (
+    OTBR_CONNECTION_TIMEOUT,
+    OTBR_RETRY_BACKOFF,
+    OTBRConfigFlow,
+)
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.service_info.hassio import HassioServiceInfo
 from homeassistant.setup import async_setup_component
 
@@ -394,21 +400,26 @@ async def test_hassio_discovery_flow(
             otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
         )
 
-    expected_data = {
-        "url": f"http://{HASSIO_DATA.config['host']}:{HASSIO_DATA.config['port']}",
-    }
+        assert result["type"] is FlowResultType.SHOW_PROGRESS_DONE
+        assert result["step_id"] == "create_entry"
 
-    assert result["type"] is FlowResultType.CREATE_ENTRY
-    assert result["title"] == "Silicon Labs Multiprotocol"
-    assert result["data"] == expected_data
-    assert result["options"] == {}
-    assert len(mock_setup_entry.mock_calls) == 1
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
-    config_entry = hass.config_entries.async_entries(otbr.DOMAIN)[0]
-    assert config_entry.data == expected_data
-    assert config_entry.options == {}
-    assert config_entry.title == "Silicon Labs Multiprotocol"
-    assert config_entry.unique_id == HASSIO_DATA.uuid
+        expected_data = {
+            "url": f"http://{HASSIO_DATA.config['host']}:{HASSIO_DATA.config['port']}",
+        }
+
+        assert result["type"] is FlowResultType.CREATE_ENTRY
+        assert result["title"] == "Silicon Labs Multiprotocol"
+        assert result["data"] == expected_data
+        assert result["options"] == {}
+        assert len(mock_setup_entry.mock_calls) == 1
+
+        config_entry = hass.config_entries.async_entries(otbr.DOMAIN)[0]
+        assert config_entry.data == expected_data
+        assert config_entry.options == {}
+        assert config_entry.title == "Silicon Labs Multiprotocol"
+        assert config_entry.unique_id == HASSIO_DATA.uuid
 
 
 @pytest.mark.usefixtures("get_border_agent_id")
@@ -433,21 +444,28 @@ async def test_hassio_discovery_flow_yellow(
             otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
         )
 
-    expected_data = {
-        "url": f"http://{HASSIO_DATA.config['host']}:{HASSIO_DATA.config['port']}",
-    }
+        assert result["type"] is FlowResultType.SHOW_PROGRESS_DONE
+        assert result["step_id"] == "create_entry"
 
-    assert result["type"] is FlowResultType.CREATE_ENTRY
-    assert result["title"] == "Home Assistant Yellow (Silicon Labs Multiprotocol)"
-    assert result["data"] == expected_data
-    assert result["options"] == {}
-    assert len(mock_setup_entry.mock_calls) == 1
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
-    config_entry = hass.config_entries.async_entries(otbr.DOMAIN)[0]
-    assert config_entry.data == expected_data
-    assert config_entry.options == {}
-    assert config_entry.title == "Home Assistant Yellow (Silicon Labs Multiprotocol)"
-    assert config_entry.unique_id == HASSIO_DATA.uuid
+        expected_data = {
+            "url": f"http://{HASSIO_DATA.config['host']}:{HASSIO_DATA.config['port']}",
+        }
+
+        assert result["type"] is FlowResultType.CREATE_ENTRY
+        assert result["title"] == "Home Assistant Yellow (Silicon Labs Multiprotocol)"
+        assert result["data"] == expected_data
+        assert result["options"] == {}
+        assert len(mock_setup_entry.mock_calls) == 1
+
+        config_entry = hass.config_entries.async_entries(otbr.DOMAIN)[0]
+        assert config_entry.data == expected_data
+        assert config_entry.options == {}
+        assert (
+            config_entry.title == "Home Assistant Yellow (Silicon Labs Multiprotocol)"
+        )
+        assert config_entry.unique_id == HASSIO_DATA.uuid
 
 
 @pytest.mark.parametrize(
@@ -486,21 +504,26 @@ async def test_hassio_discovery_flow_sky_connect(
             otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
         )
 
-    expected_data = {
-        "url": f"http://{HASSIO_DATA.config['host']}:{HASSIO_DATA.config['port']}",
-    }
+        assert result["type"] is FlowResultType.SHOW_PROGRESS_DONE
+        assert result["step_id"] == "create_entry"
 
-    assert result["type"] is FlowResultType.CREATE_ENTRY
-    assert result["title"] == title
-    assert result["data"] == expected_data
-    assert result["options"] == {}
-    assert len(mock_setup_entry.mock_calls) == 1
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
-    config_entry = hass.config_entries.async_entries(otbr.DOMAIN)[0]
-    assert config_entry.data == expected_data
-    assert config_entry.options == {}
-    assert config_entry.title == title
-    assert config_entry.unique_id == HASSIO_DATA.uuid
+        expected_data = {
+            "url": f"http://{HASSIO_DATA.config['host']}:{HASSIO_DATA.config['port']}",
+        }
+
+        assert result["type"] is FlowResultType.CREATE_ENTRY
+        assert result["title"] == title
+        assert result["data"] == expected_data
+        assert result["options"] == {}
+        assert len(mock_setup_entry.mock_calls) == 1
+
+        config_entry = hass.config_entries.async_entries(otbr.DOMAIN)[0]
+        assert config_entry.data == expected_data
+        assert config_entry.options == {}
+        assert config_entry.title == title
+        assert config_entry.unique_id == HASSIO_DATA.uuid
 
 
 @pytest.mark.usefixtures("get_active_dataset_tlvs", "get_extended_address")
@@ -541,9 +564,14 @@ async def test_hassio_discovery_flow_2x_addons(
     result1 = await hass.config_entries.flow.async_init(
         otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
     )
+    assert result1["type"] is FlowResultType.SHOW_PROGRESS
     result2 = await hass.config_entries.flow.async_init(
         otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA_2
     )
+    assert result2["type"] is FlowResultType.SHOW_PROGRESS
+    await hass.async_block_till_done()
+    result1 = await hass.config_entries.flow.async_configure(result1["flow_id"])
+    result2 = await hass.config_entries.flow.async_configure(result2["flow_id"])
 
     results = [result1, result2]
 
@@ -633,9 +661,13 @@ async def test_hassio_discovery_flow_2x_addons_same_ext_address(
     result1 = await hass.config_entries.flow.async_init(
         otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
     )
+    await hass.async_block_till_done()
+    result1 = await hass.config_entries.flow.async_configure(result1["flow_id"])
     result2 = await hass.config_entries.flow.async_init(
         otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA_2
     )
+    assert result2["type"] is FlowResultType.SHOW_PROGRESS_DONE
+    result2 = await hass.config_entries.flow.async_configure(result2["flow_id"])
 
     results = [result1, result2]
 
@@ -690,6 +722,8 @@ async def test_hassio_discovery_flow_router_not_setup(
         result = await hass.config_entries.flow.async_init(
             otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
         )
+        await hass.async_block_till_done()
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
     # Check we create a dataset and enable the router
     assert aioclient_mock.mock_calls[-2][0] == "PUT"
@@ -748,6 +782,8 @@ async def test_hassio_discovery_flow_router_not_setup_has_preferred(
         result = await hass.config_entries.flow.async_init(
             otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
         )
+        await hass.async_block_till_done()
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
     # Check we create a dataset and enable the router
     assert aioclient_mock.mock_calls[-2][0] == "PUT"
@@ -807,6 +843,8 @@ async def test_hassio_discovery_flow_router_not_setup_has_preferred_2(
         result = await hass.config_entries.flow.async_init(
             otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
         )
+        await hass.async_block_till_done()
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
     # Check we create a dataset and enable the router
     assert aioclient_mock.mock_calls[-2][0] == "PUT"
@@ -948,6 +986,8 @@ async def test_hassio_discovery_flow_new_port_other_addon(hass: HomeAssistant) -
     result = await hass.config_entries.flow.async_init(
         otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA
     )
+    await hass.async_block_till_done()
+    result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
     # Another entry will be created
     assert result["type"] is FlowResultType.CREATE_ENTRY
@@ -995,6 +1035,8 @@ async def test_config_flow_additional_entry(
         result = await hass.config_entries.flow.async_init(
             otbr.DOMAIN, context={"source": source}, data=data
         )
+        await hass.async_block_till_done()
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
     assert result["type"] is expected_result
 
@@ -1033,9 +1075,11 @@ async def test_hassio_discovery_reload(
             ),
         ),
     ):
-        await hass.config_entries.flow.async_init(
+        result = await hass.config_entries.flow.async_init(
             otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA_OTBR
         )
+        await hass.async_block_till_done()
+        result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
         # OTBR is set up and calls the firmware info notification callback
         assert len(callback.mock_calls) == 1
@@ -1049,3 +1093,38 @@ async def test_hassio_discovery_reload(
 
         assert len(callback.mock_calls) == 2
         assert len(hass.config_entries.async_entries(otbr.DOMAIN)) == 1
+
+
+async def test_connect_with_retry_timeout(
+    hass: HomeAssistant, aioclient_mock: AiohttpClientMocker
+) -> None:
+    """Test _connect_with_retry times out after OTBR_CONNECTION_TIMEOUT seconds."""
+    # Create a config flow instance
+    flow = OTBRConfigFlow()
+    flow.hass = hass
+
+    url = "http://test-url:8080"
+
+    aioclient_mock.get(
+        f"{url}/node/ba-id", exc=aiohttp.ClientConnectionError("Connection failed")
+    )
+
+    # Mock the loop.time() to simulate time passing
+    # Sequence: start_time (0.0), first elapsed check (0.1), second elapsed check (OTBR_CONNECTION_TIMEOUT+0.1)
+    with (
+        patch.object(
+            flow.hass.loop,
+            "time",
+            side_effect=[0.0, 0.1, OTBR_CONNECTION_TIMEOUT + 0.1],
+        ),
+        patch("asyncio.sleep") as mock_sleep,
+    ):
+        with pytest.raises(HomeAssistantError) as exc_info:
+            await flow._connect_with_retry(url)
+
+        assert (
+            f"Failed to connect to OTBR after {OTBR_CONNECTION_TIMEOUT} seconds"
+            in str(exc_info.value)
+        )
+
+        mock_sleep.assert_called_once_with(OTBR_RETRY_BACKOFF)