From 3b6bc6654f869311b0a524d465a79cd4a7a1487b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" <nick@koston.org> Date: Sun, 23 Jan 2022 20:53:17 -1000 Subject: [PATCH] Fix unexpected color mode switch in flux_led when brightness is near 0 (#64812) Co-authored-by: Chris Talkington <chris@talkingtontech.com> --- homeassistant/components/flux_led/const.py | 2 + homeassistant/components/flux_led/light.py | 12 ++-- .../components/flux_led/manifest.json | 2 +- homeassistant/components/flux_led/util.py | 53 +++++++++++++---- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/flux_led/test_light.py | 59 +++++++++++++++++-- 7 files changed, 105 insertions(+), 27 deletions(-) diff --git a/homeassistant/components/flux_led/const.py b/homeassistant/components/flux_led/const.py index 69663555d36..95121b5685b 100644 --- a/homeassistant/components/flux_led/const.py +++ b/homeassistant/components/flux_led/const.py @@ -20,6 +20,8 @@ from homeassistant.components.light import ( DOMAIN: Final = "flux_led" +MIN_RGB_BRIGHTNESS: Final = 1 +MIN_CCT_BRIGHTNESS: Final = 2 FLUX_COLOR_MODE_TO_HASS: Final = { FLUX_COLOR_MODE_RGB: COLOR_MODE_RGB, diff --git a/homeassistant/components/flux_led/light.py b/homeassistant/components/flux_led/light.py index 4e1fe00a18a..85b74616b32 100644 --- a/homeassistant/components/flux_led/light.py +++ b/homeassistant/components/flux_led/light.py @@ -44,6 +44,8 @@ from .const import ( CONF_TRANSITION, DEFAULT_EFFECT_SPEED, DOMAIN, + MIN_CCT_BRIGHTNESS, + MIN_RGB_BRIGHTNESS, MULTI_BRIGHTNESS_COLOR_MODES, TRANSITION_GRADUAL, TRANSITION_JUMP, @@ -290,7 +292,7 @@ class FluxLight(FluxOnOffEntity, CoordinatorEntity, LightEntity): # We previously had a problem with the brightness # sometimes reporting as 0 when an effect was in progress, # however this has since been resolved in the upstream library - return max(1, brightness) + return max(MIN_RGB_BRIGHTNESS, brightness) async def _async_set_mode(self, **kwargs: Any) -> None: """Set an effect or color mode.""" @@ -308,7 +310,7 @@ class FluxLight(FluxOnOffEntity, CoordinatorEntity, LightEntity): ): # When switching to color temp from RGBWW or RGB&W mode, # we do not want the overall brightness of the RGB channels - brightness = max(self._device.rgb) + brightness = max(MIN_CCT_BRIGHTNESS, *self._device.rgb) await self._device.async_set_white_temp(color_temp_kelvin, brightness) return # Handle switch to RGB Color Mode @@ -322,8 +324,7 @@ class FluxLight(FluxOnOffEntity, CoordinatorEntity, LightEntity): if rgbw := kwargs.get(ATTR_RGBW_COLOR): if ATTR_BRIGHTNESS in kwargs: rgbw = rgbw_brightness(rgbw, brightness) - if not self._device.requires_turn_on: - rgbw = _min_rgbw_brightness(rgbw) + rgbw = _min_rgbw_brightness(rgbw, self._device.rgbw) await self._device.async_set_levels(*rgbw) return # Handle switch to RGBWW Color Mode @@ -331,8 +332,7 @@ class FluxLight(FluxOnOffEntity, CoordinatorEntity, LightEntity): if ATTR_BRIGHTNESS in kwargs: rgbcw = rgbcw_brightness(kwargs[ATTR_RGBWW_COLOR], brightness) rgbwc = rgbcw_to_rgbwc(rgbcw) - if not self._device.requires_turn_on: - rgbwc = _min_rgbwc_brightness(rgbwc) + rgbwc = _min_rgbwc_brightness(rgbwc, self._device.rgbww) await self._device.async_set_levels(*rgbwc) return if (white := kwargs.get(ATTR_WHITE)) is not None: diff --git a/homeassistant/components/flux_led/manifest.json b/homeassistant/components/flux_led/manifest.json index 0205e493a4c..95164015a4f 100644 --- a/homeassistant/components/flux_led/manifest.json +++ b/homeassistant/components/flux_led/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "dependencies": ["network"], "documentation": "https://www.home-assistant.io/integrations/flux_led", - "requirements": ["flux_led==0.28.8"], + "requirements": ["flux_led==0.28.10"], "quality_scale": "platinum", "codeowners": ["@icemanch", "@bdraco"], "iot_class": "local_push", diff --git a/homeassistant/components/flux_led/util.py b/homeassistant/components/flux_led/util.py index da51727fbef..70983433e18 100644 --- a/homeassistant/components/flux_led/util.py +++ b/homeassistant/components/flux_led/util.py @@ -9,8 +9,9 @@ from homeassistant.components.light import ( COLOR_MODE_ONOFF, COLOR_MODE_WHITE, ) +from homeassistant.util.color import color_hsv_to_RGB, color_RGB_to_hsv -from .const import FLUX_COLOR_MODE_TO_HASS +from .const import FLUX_COLOR_MODE_TO_HASS, MIN_RGB_BRIGHTNESS def _hass_color_modes(device: AIOWifiLedBulb) -> set[str]: @@ -54,24 +55,50 @@ def _str_to_multi_color_effect(effect_str: str) -> MultiColorEffects: assert False # pragma: no cover +def _is_zero_rgb_brightness(rgb: tuple[int, int, int]) -> bool: + """RGB brightness is zero.""" + return all(byte == 0 for byte in rgb) + + def _min_rgb_brightness(rgb: tuple[int, int, int]) -> tuple[int, int, int]: """Ensure the RGB value will not turn off the device from a turn on command.""" - if all(byte == 0 for byte in rgb): - return (1, 1, 1) + if _is_zero_rgb_brightness(rgb): + return (MIN_RGB_BRIGHTNESS, MIN_RGB_BRIGHTNESS, MIN_RGB_BRIGHTNESS) return rgb -def _min_rgbw_brightness(rgbw: tuple[int, int, int, int]) -> tuple[int, int, int, int]: - """Ensure the RGBW value will not turn off the device from a turn on command.""" - if all(byte == 0 for byte in rgbw): - return (1, 1, 1, 0) - return rgbw +def _min_scaled_rgb_brightness(rgb: tuple[int, int, int]) -> tuple[int, int, int]: + """Scale an RGB tuple to minimum brightness.""" + return color_hsv_to_RGB(*color_RGB_to_hsv(*rgb)[:2], 1) + + +def _min_rgbw_brightness( + rgbw: tuple[int, int, int, int], current_rgbw: tuple[int, int, int, int] +) -> tuple[int, int, int, int]: + """Ensure the RGBW value will not turn off the device from a turn on command. + + For RGBW, we also need to ensure that there is at least one + value in the RGB fields or the device will switch to CCT mode unexpectedly. + + If the new value being set is all zeros, scale the current + color to brightness of 1 so we do not unexpected switch to white + """ + if _is_zero_rgb_brightness(rgbw[:3]): + return (*_min_scaled_rgb_brightness(current_rgbw[:3]), rgbw[3]) + return (*_min_rgb_brightness(rgbw[:3]), rgbw[3]) def _min_rgbwc_brightness( - rgbwc: tuple[int, int, int, int, int] + rgbwc: tuple[int, int, int, int, int], current_rgbwc: tuple[int, int, int, int, int] ) -> tuple[int, int, int, int, int]: - """Ensure the RGBWC value will not turn off the device from a turn on command.""" - if all(byte == 0 for byte in rgbwc): - return (1, 1, 1, 0, 0) - return rgbwc + """Ensure the RGBWC value will not turn off the device from a turn on command. + + For RGBWC, we also need to ensure that there is at least one + value in the RGB fields or the device will switch to CCT mode unexpectedly + + If the new value being set is all zeros, scale the current + color to brightness of 1 so we do not unexpected switch to white + """ + if _is_zero_rgb_brightness(rgbwc[:3]): + return (*_min_scaled_rgb_brightness(current_rgbwc[:3]), rgbwc[3], rgbwc[4]) + return (*_min_rgb_brightness(rgbwc[:3]), rgbwc[3], rgbwc[4]) diff --git a/requirements_all.txt b/requirements_all.txt index fc8dd2fbdbd..a3652676265 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -681,7 +681,7 @@ fjaraskupan==1.0.2 flipr-api==1.4.1 # homeassistant.components.flux_led -flux_led==0.28.8 +flux_led==0.28.10 # homeassistant.components.homekit fnvhash==0.1.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index d171e336135..446934e56ca 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -427,7 +427,7 @@ fjaraskupan==1.0.2 flipr-api==1.4.1 # homeassistant.components.flux_led -flux_led==0.28.8 +flux_led==0.28.10 # homeassistant.components.homekit fnvhash==0.1.0 diff --git a/tests/components/flux_led/test_light.py b/tests/components/flux_led/test_light.py index c77cfa956c0..83a76311e8a 100644 --- a/tests/components/flux_led/test_light.py +++ b/tests/components/flux_led/test_light.py @@ -28,6 +28,8 @@ from homeassistant.components.flux_led.const import ( CONF_TRANSITION, CONF_WHITE_CHANNEL_TYPE, DOMAIN, + MIN_CCT_BRIGHTNESS, + MIN_RGB_BRIGHTNESS, TRANSITION_JUMP, ) from homeassistant.components.flux_led.light import ( @@ -352,7 +354,12 @@ async def test_rgb_light_auto_on(hass: HomeAssistant) -> None: # If the bulb is off and we are using existing brightness # it has to be at least 1 or the bulb won't turn on bulb.async_turn_on.assert_not_called() - bulb.async_set_levels.assert_called_with(1, 1, 1, brightness=1) + bulb.async_set_levels.assert_called_with( + MIN_RGB_BRIGHTNESS, + MIN_RGB_BRIGHTNESS, + MIN_RGB_BRIGHTNESS, + brightness=MIN_RGB_BRIGHTNESS, + ) bulb.async_set_levels.reset_mock() bulb.async_turn_on.reset_mock() @@ -498,10 +505,30 @@ async def test_rgbw_light_auto_on(hass: HomeAssistant) -> None: ) # If the bulb is on and we are using existing brightness # and brightness was 0 we need to set it to at least 1 - # or the device may not turn on + # or the device may not turn on. In this case we scale + # the current color to brightness of 1 to ensure the device + # does not switch to white since otherwise we do not have + # enough resolution to determine which color to display bulb.async_turn_on.assert_not_called() bulb.async_set_brightness.assert_not_called() - bulb.async_set_levels.assert_called_with(1, 1, 1, 0) + bulb.async_set_levels.assert_called_with(2, 0, 0, 0) + bulb.async_set_levels.reset_mock() + + await hass.services.async_call( + LIGHT_DOMAIN, + "turn_on", + {ATTR_ENTITY_ID: entity_id, ATTR_RGBW_COLOR: (0, 0, 0, 56)}, + blocking=True, + ) + # If the bulb is on and we are using existing brightness + # and brightness was 0 we need to set it to at least 1 + # or the device may not turn on. In this case we scale + # the current color to brightness of 1 to ensure the device + # does not switch to white since otherwise we do not have + # enough resolution to determine which color to display + bulb.async_turn_on.assert_not_called() + bulb.async_set_brightness.assert_not_called() + bulb.async_set_levels.assert_called_with(2, 0, 0, 56) bulb.async_set_levels.reset_mock() bulb.brightness = 128 @@ -613,10 +640,13 @@ async def test_rgbww_light_auto_on(hass: HomeAssistant) -> None: ) # If the bulb is on and we are using existing brightness # and brightness was 0 we need to set it to at least 1 - # or the device may not turn on + # or the device may not turn on. In this case we scale + # the current color so we do not unexpectedly switch to white + # since other we do not have enough resolution to determine + # which color to display bulb.async_turn_on.assert_not_called() bulb.async_set_brightness.assert_not_called() - bulb.async_set_levels.assert_called_with(1, 1, 1, 0, 0) + bulb.async_set_levels.assert_called_with(2, 0, 0, 0, 0) bulb.async_set_levels.reset_mock() bulb.brightness = 128 @@ -1267,6 +1297,25 @@ async def test_rgbcw_light(hass: HomeAssistant) -> None: bulb.async_set_brightness.assert_called_with(255) bulb.async_set_brightness.reset_mock() + await async_mock_device_turn_off(hass, bulb) + bulb.color_mode = FLUX_COLOR_MODE_RGBWW + bulb.brightness = MIN_RGB_BRIGHTNESS + bulb.rgb = (MIN_RGB_BRIGHTNESS, MIN_RGB_BRIGHTNESS, MIN_RGB_BRIGHTNESS) + await async_mock_device_turn_on(hass, bulb) + state = hass.states.get(entity_id) + assert state.state == STATE_ON + assert state.attributes[ATTR_COLOR_MODE] == COLOR_MODE_RGBWW + assert state.attributes[ATTR_BRIGHTNESS] == 1 + + await hass.services.async_call( + LIGHT_DOMAIN, + "turn_on", + {ATTR_ENTITY_ID: entity_id, ATTR_COLOR_TEMP: 170}, + blocking=True, + ) + bulb.async_set_white_temp.assert_called_with(5882, MIN_CCT_BRIGHTNESS) + bulb.async_set_white_temp.reset_mock() + async def test_white_light(hass: HomeAssistant) -> None: """Test a white light."""