From bb827a60ed27fd74797bd1f5276f2653d7bd3d57 Mon Sep 17 00:00:00 2001 From: Olen Date: Sat, 3 Dec 2022 19:23:29 +0100 Subject: [PATCH] Support older twinkly devices without effects (#83145) fixes undefined --- homeassistant/components/twinkly/const.py | 4 + homeassistant/components/twinkly/light.py | 96 ++++++++++----- tests/components/twinkly/__init__.py | 18 ++- tests/components/twinkly/test_light.py | 135 +++++++++++++++++++--- 4 files changed, 204 insertions(+), 49 deletions(-) diff --git a/homeassistant/components/twinkly/const.py b/homeassistant/components/twinkly/const.py index e48ff165c67..d0d905a5752 100644 --- a/homeassistant/components/twinkly/const.py +++ b/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" diff --git a/homeassistant/components/twinkly/light.py b/homeassistant/components/twinkly/light.py index 8f8e8218d87..c7cf13b6563 100644 --- a/homeassistant/components/twinkly/light.py +++ b/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 kwargs[ATTR_RGBW_COLOR] != self._attr_rgbw_color: - self._attr_rgbw_color = kwargs[ATTR_RGBW_COLOR] + if ( + ATTR_RGBW_COLOR in kwargs + and kwargs[ATTR_RGBW_COLOR] != self._attr_rgbw_color + ): - if isinstance(self._attr_rgbw_color, tuple): - - await self._client.interview() - # Static color only supports rgb - await self._client.set_static_colour( - ( - self._attr_rgbw_color[0], - self._attr_rgbw_color[1], - self._attr_rgbw_color[2], - ) + await self._client.interview() + if LightEntityFeature.EFFECT & self.supported_features: + # Static color only supports rgb + await self._client.set_static_colour( + ( + kwargs[ATTR_RGBW_COLOR][0], + kwargs[ATTR_RGBW_COLOR][1], + kwargs[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 kwargs[ATTR_RGB_COLOR] != self._attr_rgb_color: - self._attr_rgb_color = kwargs[ATTR_RGB_COLOR] + if ATTR_RGB_COLOR in kwargs and kwargs[ATTR_RGB_COLOR] != self._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() - await self._client.set_static_colour(self._attr_rgb_color) - await self._client.set_mode("color") - self._client.default_mode = "color" + self._attr_rgb_color = kwargs[ATTR_RGB_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() - await self.async_update_current_movie() + if LightEntityFeature.EFFECT & self.supported_features: + 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) diff --git a/tests/components/twinkly/__init__.py b/tests/components/twinkly/__init__.py index 351f5509aa2..31d1eff2a61 100644 --- a/tests/components/twinkly/__init__.py +++ b/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} diff --git a/tests/components/twinkly/test_light.py b/tests/components/twinkly/test_light.py index f6fe9e297f6..53e589c564b 100644 --- a/tests/components/twinkly/test_light.py +++ b/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)