From 427f2a085b904d796833cfd812990013d07de0cb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 15 Oct 2021 06:37:13 -1000 Subject: [PATCH] Reconnect and retry yeelight commands after previous wifi drop out (#57741) --- homeassistant/components/yeelight/__init__.py | 17 ++++--- homeassistant/components/yeelight/light.py | 48 +++++++++++------- .../components/yeelight/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/yeelight/test_init.py | 50 +++++++++++++++++++ 6 files changed, 92 insertions(+), 29 deletions(-) diff --git a/homeassistant/components/yeelight/__init__.py b/homeassistant/components/yeelight/__init__.py index 64fa7b01f28..a1463daed12 100644 --- a/homeassistant/components/yeelight/__init__.py +++ b/homeassistant/components/yeelight/__init__.py @@ -6,7 +6,6 @@ import contextlib from datetime import timedelta from ipaddress import IPv4Address, IPv6Address import logging -import socket from urllib.parse import urlparse from async_upnp_client.search import SsdpSearchListener @@ -163,9 +162,6 @@ UPDATE_REQUEST_PROPERTIES = [ "active_mode", ] -BULB_NETWORK_EXCEPTIONS = (socket.error,) -BULB_EXCEPTIONS = (BulbException, asyncio.TimeoutError, *BULB_NETWORK_EXCEPTIONS) - PLATFORMS = ["binary_sensor", "light"] @@ -270,7 +266,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: try: device = await _async_get_device(hass, entry.data[CONF_HOST], entry) await _async_initialize(hass, entry, device) - except BULB_EXCEPTIONS as ex: + except (asyncio.TimeoutError, OSError, BulbException) as ex: raise ConfigEntryNotReady from ex hass.config_entries.async_setup_platforms(entry, PLATFORMS) @@ -594,13 +590,20 @@ class YeelightDevice: self._available = True if not self._initialized: self._initialized = True - except BULB_NETWORK_EXCEPTIONS as ex: + except OSError as ex: if self._available: # just inform once _LOGGER.error( "Unable to update device %s, %s: %s", self._host, self.name, ex ) self._available = False - except BULB_EXCEPTIONS as ex: + except asyncio.TimeoutError as ex: + _LOGGER.debug( + "timed out while trying to update device %s, %s: %s", + self._host, + self.name, + ex, + ) + except BulbException as ex: _LOGGER.debug( "Unable to update device %s, %s: %s", self._host, self.name, ex ) diff --git a/homeassistant/components/yeelight/light.py b/homeassistant/components/yeelight/light.py index 67c9dc2ba07..e40fd2726b2 100644 --- a/homeassistant/components/yeelight/light.py +++ b/homeassistant/components/yeelight/light.py @@ -1,6 +1,7 @@ """Light platform support for yeelight.""" from __future__ import annotations +import asyncio import logging import math @@ -8,6 +9,7 @@ import voluptuous as vol import yeelight from yeelight import Bulb, Flow, RGBTransition, SleepTransition, flows from yeelight.enums import BulbType, LightType, PowerMode, SceneClass +from yeelight.main import BulbException from homeassistant.components.light import ( ATTR_BRIGHTNESS, @@ -51,8 +53,6 @@ from . import ( ATTR_COUNT, ATTR_MODE_MUSIC, ATTR_TRANSITIONS, - BULB_EXCEPTIONS, - BULB_NETWORK_EXCEPTIONS, CONF_FLOW_PARAMS, CONF_MODE_MUSIC, CONF_NIGHTLIGHT_SWITCH, @@ -243,23 +243,33 @@ def _async_cmd(func): """Define a wrapper to catch exceptions from the bulb.""" async def _async_wrap(self, *args, **kwargs): - try: - _LOGGER.debug("Calling %s with %s %s", func, args, kwargs) - return await func(self, *args, **kwargs) - except BULB_NETWORK_EXCEPTIONS as ex: - # A network error happened, the bulb is likely offline now - self.device.async_mark_unavailable() - self.async_state_changed() - exc_message = str(ex) or type(ex) - raise HomeAssistantError( - f"Error when calling {func.__name__} for bulb {self.device.name} at {self.device.host}: {exc_message}" - ) from ex - except BULB_EXCEPTIONS as ex: - # The bulb likely responded but had an error - exc_message = str(ex) or type(ex) - raise HomeAssistantError( - f"Error when calling {func.__name__} for bulb {self.device.name} at {self.device.host}: {exc_message}" - ) from ex + for attempts in range(2): + try: + _LOGGER.debug("Calling %s with %s %s", func, args, kwargs) + return await func(self, *args, **kwargs) + except asyncio.TimeoutError as ex: + # The wifi likely dropped, so we want to retry once since + # python-yeelight will auto reconnect + exc_message = str(ex) or type(ex) + if attempts == 0: + continue + raise HomeAssistantError( + f"Timed out when calling {func.__name__} for bulb {self.device.name} at {self.device.host}: {exc_message}" + ) from ex + except OSError as ex: + # A network error happened, the bulb is likely offline now + self.device.async_mark_unavailable() + self.async_state_changed() + exc_message = str(ex) or type(ex) + raise HomeAssistantError( + f"Error when calling {func.__name__} for bulb {self.device.name} at {self.device.host}: {exc_message}" + ) from ex + except BulbException as ex: + # The bulb likely responded but had an error + exc_message = str(ex) or type(ex) + raise HomeAssistantError( + f"Error when calling {func.__name__} for bulb {self.device.name} at {self.device.host}: {exc_message}" + ) from ex return _async_wrap diff --git a/homeassistant/components/yeelight/manifest.json b/homeassistant/components/yeelight/manifest.json index 632fdf426f2..4682215092b 100644 --- a/homeassistant/components/yeelight/manifest.json +++ b/homeassistant/components/yeelight/manifest.json @@ -2,7 +2,7 @@ "domain": "yeelight", "name": "Yeelight", "documentation": "https://www.home-assistant.io/integrations/yeelight", - "requirements": ["yeelight==0.7.7", "async-upnp-client==0.22.8"], + "requirements": ["yeelight==0.7.8", "async-upnp-client==0.22.8"], "codeowners": ["@rytilahti", "@zewelor", "@shenxn", "@starkillerOG"], "config_flow": true, "dependencies": ["network"], diff --git a/requirements_all.txt b/requirements_all.txt index 23ac2a8dd71..e855c20caac 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2447,7 +2447,7 @@ yalesmartalarmclient==0.3.4 yalexs==1.1.13 # homeassistant.components.yeelight -yeelight==0.7.7 +yeelight==0.7.8 # homeassistant.components.yeelightsunflower yeelightsunflower==0.0.10 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 05d686cc856..64bb1109159 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1412,7 +1412,7 @@ yalesmartalarmclient==0.3.4 yalexs==1.1.13 # homeassistant.components.yeelight -yeelight==0.7.7 +yeelight==0.7.8 # homeassistant.components.youless youless-api==0.14 diff --git a/tests/components/yeelight/test_init.py b/tests/components/yeelight/test_init.py index 7ddb2845ac8..73d2543f9d4 100644 --- a/tests/components/yeelight/test_init.py +++ b/tests/components/yeelight/test_init.py @@ -1,7 +1,9 @@ """Test Yeelight.""" +import asyncio from datetime import timedelta from unittest.mock import AsyncMock, patch +import pytest from yeelight import BulbException, BulbType from yeelight.aio import KEY_CONNECTED @@ -507,3 +509,51 @@ async def test_connection_dropped_resyncs_properties(hass: HomeAssistant): ) await hass.async_block_till_done() assert len(mocked_bulb.async_get_properties.mock_calls) == 2 + + +async def test_oserror_on_first_update_results_in_unavailable(hass: HomeAssistant): + """Test that an OSError on first update results in unavailable.""" + config_entry = MockConfigEntry( + domain=DOMAIN, + unique_id=ID, + data={CONF_HOST: "127.0.0.1"}, + options={CONF_NAME: "Test name"}, + ) + config_entry.add_to_hass(hass) + mocked_bulb = _mocked_bulb() + mocked_bulb.async_get_properties = AsyncMock(side_effect=OSError) + + with _patch_discovery(), _patch_discovery_timeout(), _patch_discovery_interval(), patch( + f"{MODULE}.AsyncBulb", return_value=mocked_bulb + ): + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert hass.states.get("light.test_name").state == STATE_UNAVAILABLE + + +@pytest.mark.parametrize("exception", [BulbException, asyncio.TimeoutError]) +async def test_non_oserror_exception_on_first_update( + hass: HomeAssistant, exception: Exception +): + """Test that an exceptions other than OSError on first update do not result in unavailable. + + The unavailable state will come as a push update in this case + """ + config_entry = MockConfigEntry( + domain=DOMAIN, + unique_id=ID, + data={CONF_HOST: "127.0.0.1"}, + options={CONF_NAME: "Test name"}, + ) + config_entry.add_to_hass(hass) + mocked_bulb = _mocked_bulb() + mocked_bulb.async_get_properties = AsyncMock(side_effect=exception) + + with _patch_discovery(), _patch_discovery_timeout(), _patch_discovery_interval(), patch( + f"{MODULE}.AsyncBulb", return_value=mocked_bulb + ): + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert hass.states.get("light.test_name").state != STATE_UNAVAILABLE