Handle connection issues after websocket reconnected in homematicip_cloud (home-assistant#147731)

Author: hahn-th

homeassistant/components/homematicip_cloud/hap.py

@@ -113,9 +113,7 @@ def __init__(
 
         self._ws_close_requested = False
         self._ws_connection_closed = asyncio.Event()
-        self._retry_task: asyncio.Task | None = None
-        self._tries = 0
-        self._accesspoint_connected = True
+        self._get_state_task: asyncio.Task | None = None
         self.hmip_device_by_entity_id: dict[str, Any] = {}
         self.reset_connection_listener: Callable | None = None
 
@@ -161,17 +159,8 @@ def async_update(self, *args, **kwargs) -> None:
         """
         if not self.home.connected:
             _LOGGER.error("HMIP access point has lost connection with the cloud")
-            self._accesspoint_connected = False
+            self._ws_connection_closed.set()
             self.set_all_to_unavailable()
-        elif not self._accesspoint_connected:
-            # Now the HOME_CHANGED event has fired indicating the access
-            # point has reconnected to the cloud again.
-            # Explicitly getting an update as entity states might have
-            # changed during access point disconnect."""
-
-            job = self.hass.async_create_task(self.get_state())
-            job.add_done_callback(self.get_state_finished)
-            self._accesspoint_connected = True
 
     @callback
     def async_create_entity(self, *args, **kwargs) -> None:
@@ -185,20 +174,43 @@ async def async_create_entity_lazy(self, is_device=True) -> None:
             await asyncio.sleep(30)
         await self.hass.config_entries.async_reload(self.config_entry.entry_id)
 
+    async def _try_get_state(self) -> None:
+        """Call get_state in a loop until no error occurs, using exponential backoff on error."""
+
+        # Wait until WebSocket connection is established.
+        while not self.home.websocket_is_connected():
+            await asyncio.sleep(2)
+
+        delay = 8
+        max_delay = 1500
+        while True:
+            try:
+                await self.get_state()
+                break
+            except HmipConnectionError as err:
+                _LOGGER.warning(
+                    "Get_state failed, retrying in %s seconds: %s", delay, err
+                )
+                await asyncio.sleep(delay)
+                delay = min(delay * 2, max_delay)
+
     async def get_state(self) -> None:
         """Update HMIP state and tell Home Assistant."""
         await self.home.get_current_state_async()
         self.update_all()
 
     def get_state_finished(self, future) -> None:
-        """Execute when get_state coroutine has finished."""
+        """Execute when try_get_state coroutine has finished."""
         try:
             future.result()
-        except HmipConnectionError:
-            # Somehow connection could not recover. Will disconnect and
-            # so reconnect loop is taking over.
-            _LOGGER.error("Updating state after HMIP access point reconnect failed")
-            self.hass.async_create_task(self.home.disable_events())
+        except Exception as err:  # noqa: BLE001
+            _LOGGER.error(
+                "Error updating state after HMIP access point reconnect: %s", err
+            )
+        else:
+            _LOGGER.info(
+                "Updating state after HMIP access point reconnect finished successfully",
+            )
 
     def set_all_to_unavailable(self) -> None:
         """Set all devices to unavailable and tell Home Assistant."""
@@ -222,8 +234,8 @@ async def async_connect(self, home: AsyncHome) -> None:
     async def async_reset(self) -> bool:
         """Close the websocket connection."""
         self._ws_close_requested = True
-        if self._retry_task is not None:
-            self._retry_task.cancel()
+        if self._get_state_task is not None:
+            self._get_state_task.cancel()
         await self.home.disable_events_async()
         _LOGGER.debug("Closed connection to HomematicIP cloud server")
         await self.hass.config_entries.async_unload_platforms(
@@ -247,7 +259,9 @@ async def ws_connected_handler(self) -> None:
         """Handle websocket connected."""
         _LOGGER.info("Websocket connection to HomematicIP Cloud established")
         if self._ws_connection_closed.is_set():
-            await self.get_state()
+            self._get_state_task = self.hass.async_create_task(self._try_get_state())
+            self._get_state_task.add_done_callback(self.get_state_finished)
+
             self._ws_connection_closed.clear()
 
     async def ws_disconnected_handler(self) -> None:
@@ -256,11 +270,12 @@ async def ws_disconnected_handler(self) -> None:
         self._ws_connection_closed.set()
 
     async def ws_reconnected_handler(self, reason: str) -> None:
-        """Handle websocket reconnection."""
+        """Handle websocket reconnection. Is called when Websocket tries to reconnect."""
         _LOGGER.info(
-            "Websocket connection to HomematicIP Cloud re-established due to reason: %s",
+            "Websocket connection to HomematicIP Cloud trying to reconnect due to reason: %s",
             reason,
         )
+
         self._ws_connection_closed.set()
 
     async def get_hap(

homeassistant/components/homematicip_cloud/manifest.json

@@ -6,5 +6,5 @@
   "documentation": "https://www.home-assistant.io/integrations/homematicip_cloud",
   "iot_class": "cloud_push",
   "loggers": ["homematicip"],
-  "requirements": ["homematicip==2.0.6"]
+  "requirements": ["homematicip==2.0.7"]
 }

requirements_all.txt

@@ -195,9 +195,14 @@ async def test_hap_reconnected(
     ha_state = hass.states.get(entity_id)
     assert ha_state.state == STATE_UNAVAILABLE
 
-    mock_hap._accesspoint_connected = False
-    await async_manipulate_test_data(hass, mock_hap.home, "connected", True)
-    await hass.async_block_till_done()
+    with patch(
+        "homeassistant.components.homematicip_cloud.hap.AsyncHome.websocket_is_connected",
+        return_value=True,
+    ):
+        await async_manipulate_test_data(hass, mock_hap.home, "connected", True)
+        await mock_hap.ws_connected_handler()
+        await hass.async_block_till_done()
+
     ha_state = hass.states.get(entity_id)
     assert ha_state.state == STATE_ON
 

requirements_test_all.txt

@@ -1,6 +1,6 @@
 """Test HomematicIP Cloud accesspoint."""
 
-from unittest.mock import AsyncMock, Mock, patch
+from unittest.mock import AsyncMock, MagicMock, Mock, patch
 
 from homematicip.auth import Auth
 from homematicip.connection.connection_context import ConnectionContext
@@ -242,16 +242,69 @@ async def test_get_state_after_disconnect(
     hap = HomematicipHAP(hass, hmip_config_entry)
     assert hap
 
-    with patch.object(hap, "get_state") as mock_get_state:
+    simple_mock_home = AsyncMock(spec=AsyncHome, autospec=True)
+    hap.home = simple_mock_home
+    hap.home.websocket_is_connected = Mock(side_effect=[False, True])
+
+    with (
+        patch("asyncio.sleep", new=AsyncMock()) as mock_sleep,
+        patch.object(hap, "get_state") as mock_get_state,
+    ):
         assert not hap._ws_connection_closed.is_set()
 
         await hap.ws_connected_handler()
         mock_get_state.assert_not_called()
 
         await hap.ws_disconnected_handler()
         assert hap._ws_connection_closed.is_set()
-        await hap.ws_connected_handler()
-        mock_get_state.assert_called_once()
+        with patch(
+            "homeassistant.components.homematicip_cloud.hap.AsyncHome.websocket_is_connected",
+            return_value=True,
+        ):
+            await hap.ws_connected_handler()
+            mock_get_state.assert_called_once()
+
+    assert not hap._ws_connection_closed.is_set()
+    hap.home.websocket_is_connected.assert_called()
+    mock_sleep.assert_awaited_with(2)
+
+
+async def test_try_get_state_exponential_backoff() -> None:
+    """Test _try_get_state waits for websocket connection."""
+
+    # Arrange: Create instance and mock home
+    hap = HomematicipHAP(MagicMock(), MagicMock())
+    hap.home = MagicMock()
+    hap.home.websocket_is_connected = Mock(return_value=True)
+
+    hap.get_state = AsyncMock(
+        side_effect=[HmipConnectionError, HmipConnectionError, True]
+    )
+
+    with patch("asyncio.sleep", new=AsyncMock()) as mock_sleep:
+        await hap._try_get_state()
+
+    assert mock_sleep.mock_calls[0].args[0] == 8
+    assert mock_sleep.mock_calls[1].args[0] == 16
+    assert hap.get_state.call_count == 3
+
+
+async def test_try_get_state_handle_exception() -> None:
+    """Test _try_get_state handles exceptions."""
+    # Arrange: Create instance and mock home
+    hap = HomematicipHAP(MagicMock(), MagicMock())
+    hap.home = MagicMock()
+
+    expected_exception = Exception("Connection error")
+    future = AsyncMock()
+    future.result = Mock(side_effect=expected_exception)
+
+    with patch("homeassistant.components.homematicip_cloud.hap._LOGGER") as mock_logger:
+        hap.get_state_finished(future)
+
+    mock_logger.error.assert_called_once_with(
+        "Error updating state after HMIP access point reconnect: %s", expected_exception
+    )
 
 
 async def test_async_connect(