From 1764c031dfae5f34f7c3285e04a573a7f6437d22 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Mon, 12 Feb 2024 17:43:50 +0100 Subject: [PATCH] Warn when lights violate color mode rules (#110336) * Warn when lights violate color mode rules * Update test * Remove redundant warning, add tests * Improve test coverage --- homeassistant/components/light/__init__.py | 119 +++++++++++++++++---- tests/components/light/test_init.py | 118 +++++++++++++++++++- 2 files changed, 215 insertions(+), 22 deletions(-) diff --git a/homeassistant/components/light/__init__.py b/homeassistant/components/light/__init__.py index 4abe18daa21..28f34e2eec0 100644 --- a/homeassistant/components/light/__init__.py +++ b/homeassistant/components/light/__init__.py @@ -882,6 +882,8 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): _attr_supported_features: LightEntityFeature = LightEntityFeature(0) _attr_xy_color: tuple[float, float] | None = None + __color_mode_reported = False + @cached_property def brightness(self) -> int | None: """Return the brightness of this light between 0..255.""" @@ -897,7 +899,20 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): """Return the color mode of the light with backwards compatibility.""" if (color_mode := self.color_mode) is None: # Backwards compatibility for color_mode added in 2021.4 - # Add warning in 2024.3, remove in 2025.3 + # Warning added in 2024.3, break in 2025.3 + if not self.__color_mode_reported and self.__should_report_light_issue(): + self.__color_mode_reported = True + report_issue = self._suggest_report_issue() + _LOGGER.warning( + ( + "%s (%s) does not report a color mode, this will stop working " + "in Home Assistant Core 2025.3, please %s" + ), + self.entity_id, + type(self), + report_issue, + ) + supported = self._light_internal_supported_color_modes if ColorMode.HS in supported and self.hs_color is not None: @@ -1068,8 +1083,8 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): effect: str | None, ) -> None: """Validate the color mode.""" - if color_mode is None: - # The light is turned off + if color_mode is None or color_mode == ColorMode.UNKNOWN: + # The light is turned off or in an unknown state return if not effect or effect == EFFECT_OFF: @@ -1077,13 +1092,22 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): # color modes if color_mode in supported_color_modes: return - # Increase severity to warning in 2024.3, reject in 2025.3 - _LOGGER.debug( - "%s: set to unsupported color_mode: %s, supported_color_modes: %s", - self.entity_id, - color_mode, - supported_color_modes, - ) + # Warning added in 2024.3, reject in 2025.3 + if not self.__color_mode_reported and self.__should_report_light_issue(): + self.__color_mode_reported = True + report_issue = self._suggest_report_issue() + _LOGGER.warning( + ( + "%s (%s) set to unsupported color mode %s, expected one of %s, " + "this will stop working in Home Assistant Core 2025.3, " + "please %s" + ), + self.entity_id, + type(self), + color_mode, + supported_color_modes, + report_issue, + ) return # When an effect is active, the color mode should indicate what adjustments are @@ -1097,15 +1121,50 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): if color_mode in effect_color_modes: return - # Increase severity to warning in 2024.3, reject in 2025.3 - _LOGGER.debug( - "%s: set to unsupported color_mode: %s, supported for effect: %s", - self.entity_id, - color_mode, - effect_color_modes, - ) + # Warning added in 2024.3, reject in 2025.3 + if not self.__color_mode_reported and self.__should_report_light_issue(): + self.__color_mode_reported = True + report_issue = self._suggest_report_issue() + _LOGGER.warning( + ( + "%s (%s) set to unsupported color mode %s when rendering an effect," + " expected one of %s, this will stop working in Home Assistant " + "Core 2025.3, please %s" + ), + self.entity_id, + type(self), + color_mode, + effect_color_modes, + report_issue, + ) return + def __validate_supported_color_modes( + self, + supported_color_modes: set[ColorMode] | set[str], + ) -> None: + """Validate the supported color modes.""" + if self.__color_mode_reported: + return + + try: + valid_supported_color_modes(supported_color_modes) + except vol.Error: + # Warning added in 2024.3, reject in 2025.3 + if not self.__color_mode_reported and self.__should_report_light_issue(): + self.__color_mode_reported = True + report_issue = self._suggest_report_issue() + _LOGGER.warning( + ( + "%s (%s) sets invalid supported color modes %s, this will stop " + "working in Home Assistant Core 2025.3, please %s" + ), + self.entity_id, + type(self), + supported_color_modes, + report_issue, + ) + @final @property def state_attributes(self) -> dict[str, Any] | None: @@ -1137,7 +1196,7 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): data[ATTR_BRIGHTNESS] = None elif supported_features_value & SUPPORT_BRIGHTNESS: # Backwards compatibility for ambiguous / incomplete states - # Add warning in 2024.3, remove in 2025.3 + # Warning is printed by supported_features_compat, remove in 2025.1 if _is_on: data[ATTR_BRIGHTNESS] = self.brightness else: @@ -1158,7 +1217,7 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): data[ATTR_COLOR_TEMP] = None elif supported_features_value & SUPPORT_COLOR_TEMP: # Backwards compatibility - # Add warning in 2024.3, remove in 2025.3 + # Warning is printed by supported_features_compat, remove in 2025.1 if _is_on: color_temp_kelvin = self.color_temp_kelvin data[ATTR_COLOR_TEMP_KELVIN] = color_temp_kelvin @@ -1191,10 +1250,23 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): def _light_internal_supported_color_modes(self) -> set[ColorMode] | set[str]: """Calculate supported color modes with backwards compatibility.""" if (_supported_color_modes := self.supported_color_modes) is not None: + self.__validate_supported_color_modes(_supported_color_modes) return _supported_color_modes # Backwards compatibility for supported_color_modes added in 2021.4 - # Add warning in 2024.3, remove in 2025.3 + # Warning added in 2024.3, remove in 2025.3 + if not self.__color_mode_reported and self.__should_report_light_issue(): + self.__color_mode_reported = True + report_issue = self._suggest_report_issue() + _LOGGER.warning( + ( + "%s (%s) does not set supported color modes, this will stop working" + " in Home Assistant Core 2025.3, please %s" + ), + self.entity_id, + type(self), + report_issue, + ) supported_features = self.supported_features_compat supported_features_value = supported_features.value supported_color_modes: set[ColorMode] = set() @@ -1251,3 +1323,10 @@ class LightEntity(ToggleEntity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): report_issue, ) return new_features + + def __should_report_light_issue(self) -> bool: + """Return if light color mode issues should be reported.""" + if not self.platform: + return True + # philips_js and zha have known issues, we don't need users to open issues + return self.platform.platform_name not in {"philips_js", "zha"} diff --git a/tests/components/light/test_init.py b/tests/components/light/test_init.py index 0e3bc1332cf..4becbd87c1b 100644 --- a/tests/components/light/test_init.py +++ b/tests/components/light/test_init.py @@ -21,7 +21,7 @@ from homeassistant.exceptions import HomeAssistantError, Unauthorized from homeassistant.setup import async_setup_component import homeassistant.util.color as color_util -from tests.common import MockUser, async_mock_service +from tests.common import MockEntityPlatform, MockUser, async_mock_service orig_Profiles = light.Profiles @@ -2657,6 +2657,62 @@ def test_deprecated_supported_features_ints(caplog: pytest.LogCaptureFixture) -> assert "is using deprecated supported features values" not in caplog.text +@pytest.mark.parametrize( + ("color_mode", "supported_color_modes", "warning_expected"), + [ + (None, {light.ColorMode.ONOFF}, True), + (light.ColorMode.ONOFF, {light.ColorMode.ONOFF}, False), + ], +) +def test_report_no_color_mode( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + color_mode: str, + supported_color_modes: set[str], + warning_expected: bool, +) -> None: + """Test a light setting no color mode.""" + + class MockLightEntityEntity(light.LightEntity): + _attr_color_mode = color_mode + _attr_is_on = True + _attr_supported_features = light.LightEntityFeature.EFFECT + _attr_supported_color_modes = supported_color_modes + + entity = MockLightEntityEntity() + entity._async_calculate_state() + expected_warning = "does not report a color mode" + assert (expected_warning in caplog.text) is warning_expected + + +@pytest.mark.parametrize( + ("color_mode", "supported_color_modes", "warning_expected"), + [ + (light.ColorMode.ONOFF, None, True), + (light.ColorMode.ONOFF, {light.ColorMode.ONOFF}, False), + ], +) +def test_report_no_color_modes( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + color_mode: str, + supported_color_modes: set[str], + warning_expected: bool, +) -> None: + """Test a light setting no color mode.""" + + class MockLightEntityEntity(light.LightEntity): + _attr_color_mode = color_mode + _attr_is_on = True + _attr_supported_features = light.LightEntityFeature.EFFECT + _attr_supported_color_modes = supported_color_modes + + entity = MockLightEntityEntity() + entity._async_calculate_state() + expected_warning = "does not set supported color modes" + assert (expected_warning in caplog.text) is warning_expected + + @pytest.mark.parametrize( ("color_mode", "supported_color_modes", "effect", "warning_expected"), [ @@ -2701,5 +2757,63 @@ def test_report_invalid_color_mode( entity = MockLightEntityEntity() entity._async_calculate_state() - expected_warning = f"set to unsupported color_mode: {color_mode}" + expected_warning = f"set to unsupported color mode {color_mode}" + assert (expected_warning in caplog.text) is warning_expected + + +@pytest.mark.parametrize( + ("color_mode", "supported_color_modes", "platform_name", "warning_expected"), + [ + ( + light.ColorMode.ONOFF, + {light.ColorMode.ONOFF}, + "test", + False, + ), + ( + light.ColorMode.ONOFF, + {light.ColorMode.ONOFF, light.ColorMode.BRIGHTNESS}, + "test", + True, + ), + ( + light.ColorMode.HS, + {light.ColorMode.HS, light.ColorMode.BRIGHTNESS}, + "test", + True, + ), + ( + light.ColorMode.HS, + {light.ColorMode.COLOR_TEMP, light.ColorMode.HS}, + "test", + False, + ), + ( + light.ColorMode.ONOFF, + {light.ColorMode.ONOFF, light.ColorMode.BRIGHTNESS}, + "zha", # We don't log issues for zha + False, + ), + ], +) +def test_report_invalid_color_modes( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + color_mode: str, + supported_color_modes: set[str], + platform_name: str, + warning_expected: bool, +) -> None: + """Test a light setting an invalid color mode.""" + + class MockLightEntityEntity(light.LightEntity): + _attr_color_mode = color_mode + _attr_is_on = True + _attr_supported_features = light.LightEntityFeature.EFFECT + _attr_supported_color_modes = supported_color_modes + platform = MockEntityPlatform(hass, platform_name=platform_name) + + entity = MockLightEntityEntity() + entity._async_calculate_state() + expected_warning = "sets invalid supported color modes" assert (expected_warning in caplog.text) is warning_expected