Handle connection issues after websocket reconnected in homematicip_cloud #147731
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors how the HomematicIP HAP handles state updates after websocket reconnection by replacing the old _accesspoint_connected logic with a new retry loop and explicit task handling.
- Remove
_retry_task,_tries, and_accesspoint_connectedin favor of a single_get_state_task - Introduce
try_get_stateto retryget_statewith exponential backoff onHmipConnectionError - Update websocket handlers (
ws_connected_handler,ws_disconnected_handler,ws_reconnected_handler) to use the new event and retry task - Adjust tests to use an
AsyncMockhome and patchAsyncHome.websocket_is_connected
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/components/homematicip_cloud/test_hap.py | Added simple_mock_home, patched websocket_is_connected, and verified get_state is called |
| tests/components/homematicip_cloud/test_device.py | Patched websocket_is_connected and invoked ws_connected_handler to simulate reconnection |
| homeassistant/components/homematicip_cloud/hap.py | Refactored reconnection logic: added try_get_state, replaced flags with asyncio.Event |
Comments suppressed due to low confidence (2)
homeassistant/components/homematicip_cloud/hap.py:267
- [nitpick] The docstring could be clearer: specify that this handler sets the
ws_connection_closedevent when a reconnect attempt occurs.
"""Handle websocket reconnection. Is called when Websocket tries to reconnect."""
homeassistant/components/homematicip_cloud/hap.py:177
- Add tests to verify the retry logic in
try_get_state, including exponential backoff behavior onHmipConnectionError.
async def try_get_state(self) -> None:
| future.result() | ||
| _LOGGER.info( | ||
| "Updating state after HMIP access point reconnect finished successfully", | ||
| ) |
There was a problem hiding this comment.
Wrap future.result() in a try/except to catch exceptions and prevent unhandled exceptions in the callback, handling errors gracefully.
Copilot uses AI. Check for mistakes.
| @@ -196,8 +196,14 @@ async def test_hap_reconnected( | |||
| assert ha_state.state == STATE_UNAVAILABLE | |||
|
|
|||
| mock_hap._accesspoint_connected = False | |||
There was a problem hiding this comment.
The _accesspoint_connected attribute was removed from HomematicipHAP; update the test to manipulate hap._ws_connection_closed or hap.home.connected instead.
| mock_hap._accesspoint_connected = False | |
| mock_hap._ws_connection_closed = True |
Copilot uses AI. Check for mistakes.
| @@ -185,20 +174,37 @@ 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: | |||
There was a problem hiding this comment.
[nitpick] Consider renaming try_get_state to _try_get_state to reflect its internal use and align with private method conventions.
| async def try_get_state(self) -> None: | |
| async def _try_get_state(self) -> None: |
Copilot uses AI. Check for mistakes.
Proposed change
Fix an issue where the current state of entities is not updated when the WebSocket connection reconnects.
Bump homematicip to 2.0.7
Changes:
Changelog: https://github.com/hahn-th/homematicip-rest-api/blob/master/CHANGELOG.md
Full diff: https://github.com/hahn-th/homematicip-rest-api/compare/2.0.6..2.0.7
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: