Standardize yeelight exception handling (#56362)

homeassistant/components/yeelight/__init__.py

@@ -6,6 +6,7 @@ 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,7 +164,9 @@ UPDATE_REQUEST_PROPERTIES = [
     "active_mode",
 ]
 
-BULB_EXCEPTIONS = (BulbException, asyncio.TimeoutError)
+BULB_NETWORK_EXCEPTIONS = (socket.error, asyncio.TimeoutError)
+BULB_EXCEPTIONS = (BulbException, *BULB_NETWORK_EXCEPTIONS)
+
 
 PLATFORMS = ["binary_sensor", "light"]
 
@@ -582,6 +585,11 @@ class YeelightDevice:
         """Return true is device is available."""
         return self._available
 
+    @callback
+    def async_mark_unavailable(self):
+        """Set unavailable on api call failure due to a network issue."""
+        self._available = False
+
     @property
     def model(self):
         """Return configured/autodetected device model."""
@@ -642,26 +650,6 @@ class YeelightDevice:
 
         return self._device_type
 
-    async def async_turn_on(
-        self, duration=DEFAULT_TRANSITION, light_type=None, power_mode=None
-    ):
-        """Turn on device."""
-        try:
-            await self.bulb.async_turn_on(
-                duration=duration, light_type=light_type, power_mode=power_mode
-            )
-        except BULB_EXCEPTIONS as ex:
-            _LOGGER.error("Unable to turn the bulb on: %s", ex)
-
-    async def async_turn_off(self, duration=DEFAULT_TRANSITION, light_type=None):
-        """Turn off device."""
-        try:
-            await self.bulb.async_turn_off(duration=duration, light_type=light_type)
-        except BULB_EXCEPTIONS as ex:
-            _LOGGER.error(
-                "Unable to turn the bulb off: %s, %s: %s", self._host, self.name, ex
-            )
-
     async def _async_update_properties(self):
         """Read new properties from the device."""
         if not self.bulb:

homeassistant/components/yeelight/light.py

@@ -6,7 +6,7 @@ import math
 
 import voluptuous as vol
 import yeelight
-from yeelight import Bulb, Flow, RGBTransition, SleepTransition, flows
+from yeelight import Bulb, BulbException, Flow, RGBTransition, SleepTransition, flows
 from yeelight.enums import BulbType, LightType, PowerMode, SceneClass
 
 from homeassistant.components.light import (
@@ -34,6 +34,7 @@ from homeassistant.components.light import (
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import ATTR_ENTITY_ID, ATTR_MODE, CONF_NAME
 from homeassistant.core import HomeAssistant, callback
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import entity_platform
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.dispatcher import async_dispatcher_connect
@@ -49,7 +50,7 @@ from . import (
     ATTR_COUNT,
     ATTR_MODE_MUSIC,
     ATTR_TRANSITIONS,
-    BULB_EXCEPTIONS,
+    BULB_NETWORK_EXCEPTIONS,
     CONF_FLOW_PARAMS,
     CONF_MODE_MUSIC,
     CONF_NIGHTLIGHT_SWITCH,
@@ -242,8 +243,18 @@ def _async_cmd(func):
         try:
             _LOGGER.debug("Calling %s with %s %s", func, args, kwargs)
             return await func(self, *args, **kwargs)
-        except BULB_EXCEPTIONS as ex:
+        except BULB_NETWORK_EXCEPTIONS as ex:
-            _LOGGER.error("Error when calling %s: %s", func, ex)
+            # A network error happened, the bulb is likely offline now
+            self.device.async_mark_unavailable()
+            self.async_write_ha_state()
+            raise HomeAssistantError(
+                f"Error when calling {func.__name__} for bulb {self.device.name} at {self.device.host}: {ex}"
+            ) from ex
+        except BulbException as ex:
+            # The bulb likely responded but had an error
+            raise HomeAssistantError(
+                f"Error when calling {func.__name__} for bulb {self.device.name} at {self.device.host}: {ex}"
+            ) from ex
 
     return _async_wrap
 
@@ -375,7 +386,7 @@ def _async_setup_services(hass: HomeAssistant):
         _async_set_auto_delay_off_scene,
     )
     platform.async_register_entity_service(
-        SERVICE_SET_MUSIC_MODE, SERVICE_SCHEMA_SET_MUSIC_MODE, "set_music_mode"
+        SERVICE_SET_MUSIC_MODE, SERVICE_SCHEMA_SET_MUSIC_MODE, "async_set_music_mode"
     )
 
 
@@ -509,9 +520,7 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
     @property
     def effect(self):
         """Return the current effect."""
-        if not self.device.is_color_flow_enabled:
+        return self._effect if self.device.is_color_flow_enabled else None
-            return None
-        return self._effect
 
     @property
     def _bulb(self) -> Bulb:
@@ -519,9 +528,7 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
 
     @property
     def _properties(self) -> dict:
-        if self._bulb is None:
+        return self._bulb.last_properties if self._bulb else {}
-            return {}
-        return self._bulb.last_properties
 
     def _get_property(self, prop, default=None):
         return self._properties.get(prop, default)
@@ -564,20 +571,25 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
         """Update light properties."""
         await self.device.async_update()
 
-    def set_music_mode(self, music_mode) -> None:
+    async def async_set_music_mode(self, music_mode) -> None:
         """Set the music mode on or off."""
-        if music_mode:
         try:
-                self._bulb.start_music()
+            await self._async_set_music_mode(music_mode)
         except AssertionError as ex:
-                _LOGGER.error(ex)
+            _LOGGER.error("Unable to turn on music mode, consider disabling it: %s", ex)
-        else:
+
-            self._bulb.stop_music()
+    @_async_cmd
+    async def _async_set_music_mode(self, music_mode) -> None:
+        """Set the music mode on or off wrapped with _async_cmd."""
+        bulb = self._bulb
+        method = bulb.stop_music if not music_mode else bulb.start_music
+        await self.hass.async_add_executor_job(method)
 
     @_async_cmd
     async def async_set_brightness(self, brightness, duration) -> None:
         """Set bulb brightness."""
-        if brightness:
+        if not brightness:
+            return
         if math.floor(self.brightness) == math.floor(brightness):
             _LOGGER.debug("brightness already set to: %s", brightness)
             # Already set, and since we get pushed updates
@@ -593,7 +605,8 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
     @_async_cmd
     async def async_set_hs(self, hs_color, duration) -> None:
         """Set bulb's color."""
-        if hs_color and COLOR_MODE_HS in self.supported_color_modes:
+        if not hs_color or COLOR_MODE_HS not in self.supported_color_modes:
+            return
         if self.color_mode == COLOR_MODE_HS and self.hs_color == hs_color:
             _LOGGER.debug("HS already set to: %s", hs_color)
             # Already set, and since we get pushed updates
@@ -609,7 +622,8 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
     @_async_cmd
     async def async_set_rgb(self, rgb, duration) -> None:
         """Set bulb's color."""
-        if rgb and COLOR_MODE_RGB in self.supported_color_modes:
+        if not rgb or COLOR_MODE_RGB not in self.supported_color_modes:
+            return
         if self.color_mode == COLOR_MODE_RGB and self.rgb_color == rgb:
             _LOGGER.debug("RGB already set to: %s", rgb)
             # Already set, and since we get pushed updates
@@ -625,13 +639,11 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
     @_async_cmd
     async def async_set_colortemp(self, colortemp, duration) -> None:
         """Set bulb's color temperature."""
-        if colortemp and COLOR_MODE_COLOR_TEMP in self.supported_color_modes:
+        if not colortemp or COLOR_MODE_COLOR_TEMP not in self.supported_color_modes:
+            return
         temp_in_k = mired_to_kelvin(colortemp)
 
-            if (
+        if self.color_mode == COLOR_MODE_COLOR_TEMP and self.color_temp == colortemp:
-                self.color_mode == COLOR_MODE_COLOR_TEMP
-                and self.color_temp == colortemp
-            ):
             _LOGGER.debug("Color temp already set to: %s", temp_in_k)
             # Already set, and since we get pushed updates
             # we avoid setting it again to ensure we do not
@@ -650,7 +662,8 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
     @_async_cmd
     async def async_set_flash(self, flash) -> None:
         """Activate flash."""
-        if flash:
+        if not flash:
+            return
         if int(self._bulb.last_properties["color_mode"]) != 1:
             _LOGGER.error("Flash supported currently only in RGB mode")
             return
@@ -666,9 +679,7 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
         red, green, blue = color_util.color_hs_to_RGB(*self.hs_color)
 
         transitions = []
-            transitions.append(
+        transitions.append(RGBTransition(255, 0, 0, brightness=10, duration=duration))
-                RGBTransition(255, 0, 0, brightness=10, duration=duration)
-            )
         transitions.append(SleepTransition(duration=transition))
         transitions.append(
             RGBTransition(
@@ -677,10 +688,7 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
         )
 
         flow = Flow(count=count, transitions=transitions)
-            try:
         await self._bulb.async_start_flow(flow, light_type=self.light_type)
-            except BULB_EXCEPTIONS as ex:
-                _LOGGER.error("Unable to set flash: %s", ex)
 
     @_async_cmd
     async def async_set_effect(self, effect) -> None:
@@ -707,11 +715,17 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
         else:
             return
 
-        try:
         await self._bulb.async_start_flow(flow, light_type=self.light_type)
         self._effect = effect
-        except BULB_EXCEPTIONS as ex:
+
-            _LOGGER.error("Unable to set effect: %s", ex)
+    @_async_cmd
+    async def _async_turn_on(self, duration) -> None:
+        """Turn on the bulb for with a transition duration wrapped with _async_cmd."""
+        await self._bulb.async_turn_on(
+            duration=duration,
+            light_type=self.light_type,
+            power_mode=self._turn_on_power_mode,
+        )
 
     async def async_turn_on(self, **kwargs) -> None:
         """Turn the bulb on."""
@@ -727,46 +741,31 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
             duration = int(kwargs.get(ATTR_TRANSITION) * 1000)  # kwarg in s
 
         if not self.is_on:
-            await self.device.async_turn_on(
+            await self._async_turn_on(duration)
-                duration=duration,
-                light_type=self.light_type,
-                power_mode=self._turn_on_power_mode,
-            )
 
         if self.config[CONF_MODE_MUSIC] and not self._bulb.music_mode:
-            try:
+            await self.async_set_music_mode(True)
-                await self.hass.async_add_executor_job(
-                    self.set_music_mode, self.config[CONF_MODE_MUSIC]
-                )
-            except BULB_EXCEPTIONS as ex:
-                _LOGGER.error(
-                    "Unable to turn on music mode, consider disabling it: %s", ex
-                )
 
-        try:
-            # values checked for none in methods
         await self.async_set_hs(hs_color, duration)
         await self.async_set_rgb(rgb, duration)
         await self.async_set_colortemp(colortemp, duration)
         await self.async_set_brightness(brightness, duration)
         await self.async_set_flash(flash)
         await self.async_set_effect(effect)
-        except BULB_EXCEPTIONS as ex:
-            _LOGGER.error("Unable to set bulb properties: %s", ex)
-            return
 
         # save the current state if we had a manual change.
         if self.config[CONF_SAVE_ON_CHANGE] and (brightness or colortemp or rgb):
-            try:
             await self.async_set_default()
-            except BULB_EXCEPTIONS as ex:
-                _LOGGER.error("Unable to set the defaults: %s", ex)
-                return
 
         # Some devices (mainly nightlights) will not send back the on state so we need to force a refresh
         if not self.is_on:
             await self.device.async_update(True)
 
+    @_async_cmd
+    async def _async_turn_off(self, duration) -> None:
+        """Turn off with a given transition duration wrapped with _async_cmd."""
+        await self._bulb.async_turn_off(duration=duration, light_type=self.light_type)
+
     async def async_turn_off(self, **kwargs) -> None:
         """Turn off."""
         if not self.is_on:
@@ -776,39 +775,30 @@ class YeelightGenericLight(YeelightEntity, LightEntity):
         if ATTR_TRANSITION in kwargs:  # passed kwarg overrides config
             duration = int(kwargs.get(ATTR_TRANSITION) * 1000)  # kwarg in s
 
-        await self.device.async_turn_off(duration=duration, light_type=self.light_type)
+        await self._async_turn_off(duration)
         # Some devices will not send back the off state so we need to force a refresh
         if self.is_on:
             await self.device.async_update(True)
 
+    @_async_cmd
     async def async_set_mode(self, mode: str):
         """Set a power mode."""
-        try:
         await self._bulb.async_set_power_mode(PowerMode[mode.upper()])
-        except BULB_EXCEPTIONS as ex:
-            _LOGGER.error("Unable to set the power mode: %s", ex)
 
+    @_async_cmd
     async def async_start_flow(self, transitions, count=0, action=ACTION_RECOVER):
         """Start flow."""
-        try:
+        flow = Flow(count=count, action=Flow.actions[action], transitions=transitions)
-            flow = Flow(
-                count=count, action=Flow.actions[action], transitions=transitions
-            )
-
         await self._bulb.async_start_flow(flow, light_type=self.light_type)
-        except BULB_EXCEPTIONS as ex:
-            _LOGGER.error("Unable to set effect: %s", ex)
 
+    @_async_cmd
     async def async_set_scene(self, scene_class, *args):
         """
         Set the light directly to the specified state.
 
         If the light is off, it will first be turned on.
         """
-        try:
         await self._bulb.async_set_scene(scene_class, *args)
-        except BULB_EXCEPTIONS as ex:
-            _LOGGER.error("Unable to set scene: %s", ex)
 
 
 class YeelightColorLightSupport(YeelightGenericLight):

tests/components/yeelight/__init__.py

@@ -125,6 +125,7 @@ def _mocked_bulb(cannot_connect=False):
     )
     type(bulb).get_model_specs = MagicMock(return_value=_MODEL_SPECS[MODEL])
     bulb.capabilities = CAPABILITIES.copy()
+    bulb.available = True
     bulb.last_properties = PROPERTIES.copy()
     bulb.music_mode = False
     bulb.async_get_properties = AsyncMock()

tests/components/yeelight/test_light.py

@@ -1,7 +1,9 @@
 """Test the Yeelight light."""
+import asyncio
 import logging
 from unittest.mock import ANY, AsyncMock, MagicMock, call, patch
 
+import pytest
 from yeelight import (
     BulbException,
     BulbType,
@@ -28,6 +30,7 @@ from homeassistant.components.light import (
     ATTR_RGB_COLOR,
     ATTR_TRANSITION,
     FLASH_LONG,
+    FLASH_SHORT,
     SERVICE_TURN_OFF,
     SERVICE_TURN_ON,
 )
@@ -82,8 +85,16 @@ from homeassistant.components.yeelight.light import (
     YEELIGHT_MONO_EFFECT_LIST,
     YEELIGHT_TEMP_ONLY_EFFECT_LIST,
 )
-from homeassistant.const import ATTR_ENTITY_ID, CONF_HOST, CONF_NAME
+from homeassistant.const import (
+    ATTR_ENTITY_ID,
+    CONF_HOST,
+    CONF_NAME,
+    STATE_OFF,
+    STATE_ON,
+    STATE_UNAVAILABLE,
+)
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import entity_registry as er
 from homeassistant.setup import async_setup_component
 from homeassistant.util.color import (
@@ -122,6 +133,7 @@ CONFIG_ENTRY_DATA = {
 
 async def test_services(hass: HomeAssistant, caplog):
     """Test Yeelight services."""
+    assert await async_setup_component(hass, "homeassistant", {})
     config_entry = MockConfigEntry(
         domain=DOMAIN,
         data={
@@ -140,13 +152,16 @@ async def test_services(hass: HomeAssistant, caplog):
         assert await hass.config_entries.async_setup(config_entry.entry_id)
         await hass.async_block_till_done()
 
+    assert hass.states.get(ENTITY_LIGHT).state == STATE_ON
+    assert hass.states.get(ENTITY_NIGHTLIGHT).state == STATE_OFF
+
     async def _async_test_service(
         service,
         data,
         method,
         payload=None,
         domain=DOMAIN,
-        failure_side_effect=BulbException,
+        failure_side_effect=HomeAssistantError,
     ):
         err_count = len([x for x in caplog.records if x.levelno == logging.ERROR])
 
@@ -174,11 +189,8 @@ async def test_services(hass: HomeAssistant, caplog):
             else:
                 mocked_method = MagicMock(side_effect=failure_side_effect)
             setattr(mocked_bulb, method, mocked_method)
+            with pytest.raises(failure_side_effect):
                 await hass.services.async_call(domain, service, data, blocking=True)
-            assert (
-                len([x for x in caplog.records if x.levelno == logging.ERROR])
-                == err_count + 1
-            )
 
     # turn_on rgb_color
     brightness = 100
@@ -303,7 +315,50 @@ async def test_services(hass: HomeAssistant, caplog):
     mocked_bulb.async_start_flow.assert_called_once()  # flash
     mocked_bulb.async_stop_flow.assert_called_once_with(light_type=LightType.Main)
 
+    # turn_on color_temp - flash short
+    brightness = 100
+    color_temp = 200
+    transition = 1
+    mocked_bulb.start_music.reset_mock()
+    mocked_bulb.async_set_brightness.reset_mock()
+    mocked_bulb.async_set_color_temp.reset_mock()
+    mocked_bulb.async_start_flow.reset_mock()
+    mocked_bulb.async_stop_flow.reset_mock()
+
     mocked_bulb.last_properties["power"] = "off"
+    await hass.services.async_call(
+        "light",
+        SERVICE_TURN_ON,
+        {
+            ATTR_ENTITY_ID: ENTITY_LIGHT,
+            ATTR_BRIGHTNESS: brightness,
+            ATTR_COLOR_TEMP: color_temp,
+            ATTR_FLASH: FLASH_SHORT,
+            ATTR_EFFECT: EFFECT_STOP,
+            ATTR_TRANSITION: transition,
+        },
+        blocking=True,
+    )
+    mocked_bulb.async_turn_on.assert_called_once_with(
+        duration=transition * 1000,
+        light_type=LightType.Main,
+        power_mode=PowerMode.NORMAL,
+    )
+    mocked_bulb.async_turn_on.reset_mock()
+    mocked_bulb.start_music.assert_called_once()
+    mocked_bulb.async_set_brightness.assert_called_once_with(
+        brightness / 255 * 100, duration=transition * 1000, light_type=LightType.Main
+    )
+    mocked_bulb.async_set_color_temp.assert_called_once_with(
+        color_temperature_mired_to_kelvin(color_temp),
+        duration=transition * 1000,
+        light_type=LightType.Main,
+    )
+    mocked_bulb.async_set_hsv.assert_not_called()
+    mocked_bulb.async_set_rgb.assert_not_called()
+    mocked_bulb.async_start_flow.assert_called_once()  # flash
+    mocked_bulb.async_stop_flow.assert_called_once_with(light_type=LightType.Main)
+
     # turn_on nightlight
     await _async_test_service(
         SERVICE_TURN_ON,
@@ -318,6 +373,7 @@ async def test_services(hass: HomeAssistant, caplog):
     )
 
     mocked_bulb.last_properties["power"] = "on"
+    assert hass.states.get(ENTITY_LIGHT).state != STATE_UNAVAILABLE
     # turn_off
     await _async_test_service(
         SERVICE_TURN_OFF,
@@ -393,12 +449,16 @@ async def test_services(hass: HomeAssistant, caplog):
     )
 
     # set_music_mode failure enable
-    await _async_test_service(
+    mocked_bulb.start_music = MagicMock(side_effect=AssertionError)
+    assert "Unable to turn on music mode, consider disabling it" not in caplog.text
+    await hass.services.async_call(
+        DOMAIN,
         SERVICE_SET_MUSIC_MODE,
         {ATTR_ENTITY_ID: ENTITY_LIGHT, ATTR_MODE_MUSIC: "true"},
-        "start_music",
+        blocking=True,
-        failure_side_effect=AssertionError,
     )
+    assert mocked_bulb.start_music.mock_calls == [call()]
+    assert "Unable to turn on music mode, consider disabling it" in caplog.text
 
     # set_music_mode disable
     await _async_test_service(
@@ -417,18 +477,35 @@ async def test_services(hass: HomeAssistant, caplog):
     )
     # test _cmd wrapper error handler
     mocked_bulb.last_properties["power"] = "off"
-    err_count = len([x for x in caplog.records if x.levelno == logging.ERROR])
+    mocked_bulb.available = True
-    type(mocked_bulb).turn_on = MagicMock()
+    await hass.services.async_call(
-    type(mocked_bulb).set_brightness = MagicMock(side_effect=BulbException)
+        "homeassistant",
+        "update_entity",
+        {ATTR_ENTITY_ID: ENTITY_LIGHT},
+        blocking=True,
+    )
+    assert hass.states.get(ENTITY_LIGHT).state == STATE_OFF
+
+    mocked_bulb.async_turn_on = AsyncMock()
+    mocked_bulb.async_set_brightness = AsyncMock(side_effect=BulbException)
+    with pytest.raises(HomeAssistantError):
         await hass.services.async_call(
             "light",
             SERVICE_TURN_ON,
             {ATTR_ENTITY_ID: ENTITY_LIGHT, ATTR_BRIGHTNESS: 50},
             blocking=True,
         )
-    assert (
+    assert hass.states.get(ENTITY_LIGHT).state == STATE_OFF
-        len([x for x in caplog.records if x.levelno == logging.ERROR]) == err_count + 1
+
+    mocked_bulb.async_set_brightness = AsyncMock(side_effect=asyncio.TimeoutError)
+    with pytest.raises(HomeAssistantError):
+        await hass.services.async_call(
+            "light",
+            SERVICE_TURN_ON,
+            {ATTR_ENTITY_ID: ENTITY_LIGHT, ATTR_BRIGHTNESS: 55},
+            blocking=True,
         )
+    assert hass.states.get(ENTITY_LIGHT).state == STATE_UNAVAILABLE
 
 
 async def test_state_already_set_avoid_ratelimit(hass: HomeAssistant):