From 04222c32b53cd5dac15c7730cc64a532a694434c Mon Sep 17 00:00:00 2001 From: Shay Levy Date: Sun, 9 Jun 2024 12:59:40 +0300 Subject: [PATCH] Handle Shelly BLE errors during connect and disconnect (#119174) --- homeassistant/components/shelly/__init__.py | 9 +--- .../components/shelly/coordinator.py | 18 ++++++- tests/components/shelly/test_coordinator.py | 47 +++++++++++++++++++ 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/shelly/__init__.py b/homeassistant/components/shelly/__init__.py index 1bcd9c7c1e4..cc1ea5e81a6 100644 --- a/homeassistant/components/shelly/__init__.py +++ b/homeassistant/components/shelly/__init__.py @@ -2,7 +2,6 @@ from __future__ import annotations -import contextlib from typing import Final from aioshelly.block_device import BlockDevice @@ -301,13 +300,7 @@ async def async_unload_entry(hass: HomeAssistant, entry: ShellyConfigEntry) -> b entry, platforms ): if shelly_entry_data.rpc: - with contextlib.suppress(DeviceConnectionError): - # If the device is restarting or has gone offline before - # the ping/pong timeout happens, the shutdown command - # will fail, but we don't care since we are unloading - # and if we setup again, we will fix anything that is - # in an inconsistent state at that time. - await shelly_entry_data.rpc.shutdown() + await shelly_entry_data.rpc.shutdown() return unload_ok diff --git a/homeassistant/components/shelly/coordinator.py b/homeassistant/components/shelly/coordinator.py index b6ccc1540f1..3415f1b22db 100644 --- a/homeassistant/components/shelly/coordinator.py +++ b/homeassistant/components/shelly/coordinator.py @@ -627,7 +627,13 @@ class ShellyRpcCoordinator(ShellyCoordinatorBase[RpcDevice]): if self.connected: # Already connected return self.connected = True - await self._async_run_connected_events() + try: + await self._async_run_connected_events() + except DeviceConnectionError as err: + LOGGER.error( + "Error running connected events for device %s: %s", self.name, err + ) + self.last_update_success = False async def _async_run_connected_events(self) -> None: """Run connected events. @@ -701,10 +707,18 @@ class ShellyRpcCoordinator(ShellyCoordinatorBase[RpcDevice]): if self.device.connected: try: await async_stop_scanner(self.device) + await super().shutdown() except InvalidAuthError: self.entry.async_start_reauth(self.hass) return - await super().shutdown() + except DeviceConnectionError as err: + # If the device is restarting or has gone offline before + # the ping/pong timeout happens, the shutdown command + # will fail, but we don't care since we are unloading + # and if we setup again, we will fix anything that is + # in an inconsistent state at that time. + LOGGER.debug("Error during shutdown for device %s: %s", self.name, err) + return await self._async_disconnected(False) diff --git a/tests/components/shelly/test_coordinator.py b/tests/components/shelly/test_coordinator.py index 1dc45a98c44..cd750e53f0b 100644 --- a/tests/components/shelly/test_coordinator.py +++ b/tests/components/shelly/test_coordinator.py @@ -15,12 +15,14 @@ from homeassistant.components.shelly.const import ( ATTR_CLICK_TYPE, ATTR_DEVICE, ATTR_GENERATION, + CONF_BLE_SCANNER_MODE, DOMAIN, ENTRY_RELOAD_COOLDOWN, MAX_PUSH_UPDATE_FAILURES, RPC_RECONNECT_INTERVAL, SLEEP_PERIOD_MULTIPLIER, UPDATE_PERIOD_MULTIPLIER, + BLEScannerMode, ) from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState from homeassistant.const import ATTR_DEVICE_ID, STATE_ON, STATE_UNAVAILABLE @@ -485,6 +487,25 @@ async def test_rpc_reload_with_invalid_auth( assert flow["context"].get("entry_id") == entry.entry_id +async def test_rpc_connection_error_during_unload( + hass: HomeAssistant, mock_rpc_device: Mock, caplog: pytest.LogCaptureFixture +) -> None: + """Test RPC DeviceConnectionError suppressed during config entry unload.""" + entry = await init_integration(hass, 2) + + assert entry.state is ConfigEntryState.LOADED + + with patch( + "homeassistant.components.shelly.coordinator.async_stop_scanner", + side_effect=DeviceConnectionError, + ): + await hass.config_entries.async_unload(entry.entry_id) + await hass.async_block_till_done() + + assert "Error during shutdown for device" in caplog.text + assert entry.state is ConfigEntryState.NOT_LOADED + + async def test_rpc_click_event( hass: HomeAssistant, mock_rpc_device: Mock, @@ -713,6 +734,32 @@ async def test_rpc_reconnect_error( assert get_entity_state(hass, "switch.test_switch_0") == STATE_UNAVAILABLE +async def test_rpc_error_running_connected_events( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + mock_rpc_device: Mock, + caplog: pytest.LogCaptureFixture, +) -> None: + """Test RPC error while running connected events.""" + with patch( + "homeassistant.components.shelly.coordinator.async_ensure_ble_enabled", + side_effect=DeviceConnectionError, + ): + await init_integration( + hass, 2, options={CONF_BLE_SCANNER_MODE: BLEScannerMode.ACTIVE} + ) + + assert "Error running connected events for device" in caplog.text + assert get_entity_state(hass, "switch.test_switch_0") == STATE_UNAVAILABLE + + # Move time to generate reconnect without error + freezer.tick(timedelta(seconds=RPC_RECONNECT_INTERVAL)) + async_fire_time_changed(hass) + await hass.async_block_till_done(wait_background_tasks=True) + + assert get_entity_state(hass, "switch.test_switch_0") == STATE_ON + + async def test_rpc_polling_connection_error( hass: HomeAssistant, freezer: FrozenDateTimeFactory,