Support older twinkly devices without effects (#83145)

fixes undefined

homeassistant/components/twinkly/const.py

@@ -9,6 +9,7 @@ CONF_NAME = "name"
 
 # Strongly named HA attributes keys
 ATTR_HOST = "host"
+ATTR_VERSION = "version"
 
 # Keys of attributes read from the get_device_info
 DEV_ID = "uuid"
@@ -27,3 +28,6 @@ HIDDEN_DEV_VALUES = (
     "copyright",  # We should not display a copyright "LEDWORKS 2018" in the Home-Assistant UI
     "mac",  # Does not report the actual device mac address
 )
+
+# Minimum version required to support effects
+MIN_EFFECT_VERSION = "2.7.1"

homeassistant/components/twinkly/light.py

@@ -7,6 +7,7 @@ import logging
 from typing import Any
 
 from aiohttp import ClientError
+from packaging import version
 from ttls.client import Twinkly
 
 from homeassistant.components.light import (
@@ -25,6 +26,7 @@ from homeassistant.helpers.entity import DeviceInfo
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 
 from .const import (
+    ATTR_VERSION,
     CONF_HOST,
     CONF_ID,
     CONF_NAME,
@@ -37,6 +39,7 @@ from .const import (
     DEV_PROFILE_RGBW,
     DOMAIN,
     HIDDEN_DEV_VALUES,
+    MIN_EFFECT_VERSION,
 )
 
 _LOGGER = logging.getLogger(__name__)
@@ -96,6 +99,9 @@ class TwinklyLight(LightEntity):
         self._attributes: dict[Any, Any] = {}
         self._current_movie: dict[Any, Any] = {}
         self._movies: list[Any] = []
+        self._software_version = ""
+        # We guess that most devices are "new" and support effects
+        self._attr_supported_features = LightEntityFeature.EFFECT
 
     @property
     def available(self) -> bool:
@@ -130,13 +136,9 @@ class TwinklyLight(LightEntity):
             manufacturer="LEDWORKS",
             model=self.model,
             name=self.name,
+            sw_version=self._software_version,
         )
 
-    @property
-    def supported_features(self) -> LightEntityFeature:
-        """Return supported features."""
-        return LightEntityFeature.EFFECT
-
     @property
     def is_on(self) -> bool:
         """Return true if light is on."""
@@ -165,6 +167,19 @@ class TwinklyLight(LightEntity):
             effect_list.append(f"{movie['id']} {movie['name']}")
         return effect_list
 
+    async def async_added_to_hass(self) -> None:
+        """Device is added to hass."""
+        software_version = await self._client.get_firmware_version()
+        if ATTR_VERSION in software_version:
+            self._software_version = software_version[ATTR_VERSION]
+
+            if version.parse(self._software_version) < version.parse(
+                MIN_EFFECT_VERSION
+            ):
+                self._attr_supported_features = (
+                    self.supported_features & ~LightEntityFeature.EFFECT
+                )
+
     async def async_turn_on(self, **kwargs: Any) -> None:
         """Turn device on."""
         if ATTR_BRIGHTNESS in kwargs:
@@ -178,36 +193,54 @@ class TwinklyLight(LightEntity):
 
             await self._client.set_brightness(brightness)
 
-        if ATTR_RGBW_COLOR in kwargs:
+        if (
-            if kwargs[ATTR_RGBW_COLOR] != self._attr_rgbw_color:
+            ATTR_RGBW_COLOR in kwargs
-                self._attr_rgbw_color = kwargs[ATTR_RGBW_COLOR]
+            and kwargs[ATTR_RGBW_COLOR] != self._attr_rgbw_color
+        ):
 
-                if isinstance(self._attr_rgbw_color, tuple):
+            await self._client.interview()
-
+            if LightEntityFeature.EFFECT & self.supported_features:
-                    await self._client.interview()
+                # Static color only supports rgb
-                    # Static color only supports rgb
+                await self._client.set_static_colour(
-                    await self._client.set_static_colour(
+                    (
-                        (
+                        kwargs[ATTR_RGBW_COLOR][0],
-                            self._attr_rgbw_color[0],
+                        kwargs[ATTR_RGBW_COLOR][1],
-                            self._attr_rgbw_color[1],
+                        kwargs[ATTR_RGBW_COLOR][2],
-                            self._attr_rgbw_color[2],
-                        )
                     )
-                    await self._client.set_mode("color")
+                )
-                    self._client.default_mode = "color"
+                await self._client.set_mode("color")
+                self._client.default_mode = "color"
+            else:
+                await self._client.set_cycle_colours(
+                    (
+                        kwargs[ATTR_RGBW_COLOR][3],
+                        kwargs[ATTR_RGBW_COLOR][0],
+                        kwargs[ATTR_RGBW_COLOR][1],
+                        kwargs[ATTR_RGBW_COLOR][2],
+                    )
+                )
+                await self._client.set_mode("movie")
+                self._client.default_mode = "movie"
+            self._attr_rgbw_color = kwargs[ATTR_RGBW_COLOR]
 
-        if ATTR_RGB_COLOR in kwargs:
+        if ATTR_RGB_COLOR in kwargs and kwargs[ATTR_RGB_COLOR] != self._attr_rgb_color:
-            if kwargs[ATTR_RGB_COLOR] != self._attr_rgb_color:
-                self._attr_rgb_color = kwargs[ATTR_RGB_COLOR]
 
-                if isinstance(self._attr_rgb_color, tuple):
+            await self._client.interview()
+            if LightEntityFeature.EFFECT & self.supported_features:
+                await self._client.set_static_colour(kwargs[ATTR_RGB_COLOR])
+                await self._client.set_mode("color")
+                self._client.default_mode = "color"
+            else:
+                await self._client.set_cycle_colours(kwargs[ATTR_RGB_COLOR])
+                await self._client.set_mode("movie")
+                self._client.default_mode = "movie"
 
-                    await self._client.interview()
+            self._attr_rgb_color = kwargs[ATTR_RGB_COLOR]
-                    await self._client.set_static_colour(self._attr_rgb_color)
-                    await self._client.set_mode("color")
-                    self._client.default_mode = "color"
 
-        if ATTR_EFFECT in kwargs:
+        if (
+            ATTR_EFFECT in kwargs
+            and LightEntityFeature.EFFECT & self.supported_features
+        ):
             movie_id = kwargs[ATTR_EFFECT].split(" ")[0]
             if "id" not in self._current_movie or int(movie_id) != int(
                 self._current_movie["id"]
@@ -268,8 +301,9 @@ class TwinklyLight(LightEntity):
                 if key not in HIDDEN_DEV_VALUES:
                     self._attributes[key] = value
 
-            await self.async_update_movies()
+            if LightEntityFeature.EFFECT & self.supported_features:
-            await self.async_update_current_movie()
+                await self.async_update_movies()
+                await self.async_update_current_movie()
 
             if not self._is_available:
                 _LOGGER.info("Twinkly '%s' is now available", self._client.host)

tests/components/twinkly/__init__.py

@@ -25,6 +25,8 @@ class ClientMock:
         self.movies = [{"id": 1, "name": "Rainbow"}, {"id": 2, "name": "Flare"}]
         self.current_movie = {}
         self.default_mode = "movie"
+        self.mode = None
+        self.version = "2.8.10"
 
         self.id = str(uuid4())
         self.device_info = {
@@ -55,6 +57,7 @@ class ClientMock:
         if self.is_offline:
             raise ClientConnectionError()
         self.state = True
+        self.mode = self.default_mode
 
     async def turn_off(self) -> None:
         """Set the mocked off state."""
@@ -81,6 +84,12 @@ class ClientMock:
     async def set_static_colour(self, colour) -> None:
         """Set static color."""
         self.color = colour
+        self.default_mode = "color"
+
+    async def set_cycle_colours(self, colour) -> None:
+        """Set static color."""
+        self.color = colour
+        self.default_mode = "movie"
 
     async def interview(self) -> None:
         """Interview."""
@@ -100,6 +109,11 @@ class ClientMock:
     async def set_mode(self, mode: str) -> None:
         """Set mode."""
         if mode == "off":
-            self.turn_off
+            await self.turn_off()
         else:
-            self.turn_on
+            await self.turn_on()
+            self.mode = mode
+
+    async def get_firmware_version(self) -> dict:
+        """Get firmware version."""
+        return {"version": self.version}

tests/components/twinkly/test_light.py

@@ -3,7 +3,7 @@ from __future__ import annotations
 
 from unittest.mock import patch
 
-from homeassistant.components.light import ATTR_BRIGHTNESS
+from homeassistant.components.light import ATTR_BRIGHTNESS, LightEntityFeature
 from homeassistant.components.twinkly.const import (
     CONF_HOST,
     CONF_ID,
@@ -55,9 +55,8 @@ async def test_turn_on_off(hass: HomeAssistant):
     assert hass.states.get(entity.entity_id).state == "off"
 
     await hass.services.async_call(
-        "light", "turn_on", service_data={"entity_id": entity.entity_id}
+        "light", "turn_on", service_data={"entity_id": entity.entity_id}, blocking=True
     )
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)
 
@@ -78,8 +77,8 @@ async def test_turn_on_with_brightness(hass: HomeAssistant):
         "light",
         "turn_on",
         service_data={"entity_id": entity.entity_id, "brightness": 255},
+        blocking=True,
     )
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)
 
@@ -90,8 +89,8 @@ async def test_turn_on_with_brightness(hass: HomeAssistant):
         "light",
         "turn_on",
         service_data={"entity_id": entity.entity_id, "brightness": 1},
+        blocking=True,
     )
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)
 
@@ -99,7 +98,7 @@ async def test_turn_on_with_brightness(hass: HomeAssistant):
 
 
 async def test_turn_on_with_color_rgbw(hass: HomeAssistant):
-    """Test support of the light.turn_on service with a brightness parameter."""
+    """Test support of the light.turn_on service with a rgbw parameter."""
     client = ClientMock()
     client.state = False
     client.device_info["led_profile"] = "RGBW"
@@ -107,23 +106,28 @@ async def test_turn_on_with_color_rgbw(hass: HomeAssistant):
     entity, _, _, _ = await _create_entries(hass, client)
 
     assert hass.states.get(entity.entity_id).state == "off"
+    assert (
+        LightEntityFeature.EFFECT
+        & hass.states.get(entity.entity_id).attributes["supported_features"]
+    )
 
     await hass.services.async_call(
         "light",
         "turn_on",
         service_data={"entity_id": entity.entity_id, "rgbw_color": (128, 64, 32, 0)},
+        blocking=True,
     )
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)
 
     assert state.state == "on"
     assert client.color == (128, 64, 32)
     assert client.default_mode == "color"
+    assert client.mode == "color"
 
 
 async def test_turn_on_with_color_rgb(hass: HomeAssistant):
-    """Test support of the light.turn_on service with a brightness parameter."""
+    """Test support of the light.turn_on service with a rgb parameter."""
     client = ClientMock()
     client.state = False
     client.device_info["led_profile"] = "RGB"
@@ -131,23 +135,28 @@ async def test_turn_on_with_color_rgb(hass: HomeAssistant):
     entity, _, _, _ = await _create_entries(hass, client)
 
     assert hass.states.get(entity.entity_id).state == "off"
+    assert (
+        LightEntityFeature.EFFECT
+        & hass.states.get(entity.entity_id).attributes["supported_features"]
+    )
 
     await hass.services.async_call(
         "light",
         "turn_on",
         service_data={"entity_id": entity.entity_id, "rgb_color": (128, 64, 32)},
+        blocking=True,
     )
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)
 
     assert state.state == "on"
     assert client.color == (128, 64, 32)
     assert client.default_mode == "color"
+    assert client.mode == "color"
 
 
 async def test_turn_on_with_effect(hass: HomeAssistant):
-    """Test support of the light.turn_on service with a brightness parameter."""
+    """Test support of the light.turn_on service with effects."""
     client = ClientMock()
     client.state = False
     client.device_info["led_profile"] = "RGB"
@@ -155,20 +164,116 @@ async def test_turn_on_with_effect(hass: HomeAssistant):
     entity, _, _, _ = await _create_entries(hass, client)
 
     assert hass.states.get(entity.entity_id).state == "off"
-    assert client.current_movie == {}
+    assert not client.current_movie
+    assert (
+        LightEntityFeature.EFFECT
+        & hass.states.get(entity.entity_id).attributes["supported_features"]
+    )
 
     await hass.services.async_call(
         "light",
         "turn_on",
         service_data={"entity_id": entity.entity_id, "effect": "1 Rainbow"},
+        blocking=True,
     )
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)
 
     assert state.state == "on"
     assert client.current_movie["id"] == 1
     assert client.default_mode == "movie"
+    assert client.mode == "movie"
+
+
+async def test_turn_on_with_color_rgbw_and_missing_effect(hass: HomeAssistant):
+    """Test support of the light.turn_on service with rgbw color and missing effect support."""
+    client = ClientMock()
+    client.state = False
+    client.device_info["led_profile"] = "RGBW"
+    client.brightness = {"mode": "enabled", "value": 255}
+    client.version = "2.7.0"
+    entity, _, _, _ = await _create_entries(hass, client)
+
+    assert hass.states.get(entity.entity_id).state == "off"
+    assert (
+        not LightEntityFeature.EFFECT
+        & hass.states.get(entity.entity_id).attributes["supported_features"]
+    )
+
+    await hass.services.async_call(
+        "light",
+        "turn_on",
+        service_data={"entity_id": entity.entity_id, "rgbw_color": (128, 64, 32, 0)},
+        blocking=True,
+    )
+
+    state = hass.states.get(entity.entity_id)
+
+    assert state.state == "on"
+    assert client.color == (0, 128, 64, 32)
+    assert client.mode == "movie"
+    assert client.default_mode == "movie"
+
+
+async def test_turn_on_with_color_rgb_and_missing_effect(hass: HomeAssistant):
+    """Test support of the light.turn_on service with rgb color and missing effect support."""
+    client = ClientMock()
+    client.state = False
+    client.device_info["led_profile"] = "RGB"
+    client.brightness = {"mode": "enabled", "value": 255}
+    client.version = "2.7.0"
+    entity, _, _, _ = await _create_entries(hass, client)
+
+    assert hass.states.get(entity.entity_id).state == "off"
+    assert (
+        not LightEntityFeature.EFFECT
+        & hass.states.get(entity.entity_id).attributes["supported_features"]
+    )
+
+    await hass.services.async_call(
+        "light",
+        "turn_on",
+        service_data={"entity_id": entity.entity_id, "rgb_color": (128, 64, 32)},
+        blocking=True,
+    )
+
+    state = hass.states.get(entity.entity_id)
+
+    assert state.state == "on"
+    assert client.color == (128, 64, 32)
+    assert client.mode == "movie"
+    assert client.default_mode == "movie"
+
+
+async def test_turn_on_with_effect_missing_effects(hass: HomeAssistant):
+    """Test support of the light.turn_on service with effect set even if effects are not supported."""
+    client = ClientMock()
+    client.state = False
+    client.device_info["led_profile"] = "RGB"
+    client.brightness = {"mode": "enabled", "value": 255}
+    client.version = "2.7.0"
+    entity, _, _, _ = await _create_entries(hass, client)
+
+    assert hass.states.get(entity.entity_id).state == "off"
+    assert not client.current_movie
+    assert (
+        not LightEntityFeature.EFFECT
+        & hass.states.get(entity.entity_id).attributes["supported_features"]
+    )
+
+    await hass.services.async_call(
+        "light",
+        "turn_on",
+        service_data={"entity_id": entity.entity_id, "effect": "1 Rainbow"},
+        blocking=True,
+    )
+
+    state = hass.states.get(entity.entity_id)
+
+    assert state.state == "on"
+    assert not client.current_movie
+    assert client.default_mode == "movie"
+    assert client.mode == "movie"
 
 
 async def test_turn_off(hass: HomeAssistant):
@@ -178,9 +283,8 @@ async def test_turn_off(hass: HomeAssistant):
     assert hass.states.get(entity.entity_id).state == "on"
 
     await hass.services.async_call(
-        "light", "turn_off", service_data={"entity_id": entity.entity_id}
+        "light", "turn_off", service_data={"entity_id": entity.entity_id}, blocking=True
     )
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)
 
@@ -199,9 +303,8 @@ async def test_update_name(hass: HomeAssistant):
 
     client.change_name("new_device_name")
     await hass.services.async_call(
-        "light", "turn_off", service_data={"entity_id": entity.entity_id}
+        "light", "turn_off", service_data={"entity_id": entity.entity_id}, blocking=True
     )  # We call turn_off which will automatically cause an async_update
-    await hass.async_block_till_done()
 
     state = hass.states.get(entity.entity_id)