Skip to content

Validate light supported_color_modes #48247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
emontnemery wants to merge 1 commit into from
Closed

Validate light supported_color_modes #48247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions homeassistant/components/light/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@
COLOR_MODES_BRIGHTNESS = VALID_COLOR_MODES - {COLOR_MODE_ONOFF}
COLOR_MODES_COLOR = {COLOR_MODE_HS, COLOR_MODE_RGB, COLOR_MODE_XY}


def validate_supported_color_modes(color_modes):
"""Validate the given color modes."""
color_modes = set(color_modes)
if (
not color_modes
or COLOR_MODE_UNKNOWN in color_modes
or (COLOR_MODE_BRIGHTNESS in color_modes and len(color_modes) > 1)
or (COLOR_MODE_ONOFF in color_modes and len(color_modes) > 1)
):
raise vol.Error(f"Invalid supported_color_modes {sorted(color_modes)}")
return color_modes


# Float that represents transition time in seconds to make change.
ATTR_TRANSITION = "transition"

Expand Down Expand Up @@ -609,9 +623,16 @@ def capability_attributes(self):
if supported_features & SUPPORT_EFFECT:
data[ATTR_EFFECT_LIST] = self.effect_list

data[ATTR_SUPPORTED_COLOR_MODES] = sorted(
self._light_internal_supported_color_modes
)
supported_color_modes = self._light_internal_supported_color_modes
try:
supported_color_modes = validate_supported_color_modes(

@balloob balloob Mar 23, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't validate attributes at runtime as it can get too costly to do it for everything. Instead I once had this idea #37663

@emontnemery emontnemery Mar 24, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular case, it's a capability attribute which should rarely change, if ever.
If the attribute has not changed, there's no need to re-validate.

In #37663 it's mentioned that:

It's up to each integration to implement a validator platform.

So it's opt-in per integration? Can't it be done in base components instead?

supported_color_modes
)
except vol.Error as ex:
_LOGGER.warning("Light %s: %s", self.entity_id, ex)

@emontnemery emontnemery Mar 23, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may spam a lot, should the warning be limited to once per entity?

supported_color_modes = {COLOR_MODE_ONOFF}

@emontnemery emontnemery Mar 23, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, should the state update be rejected?


data[ATTR_SUPPORTED_COLOR_MODES] = sorted(supported_color_modes)

return data

Expand Down
51 changes: 51 additions & 0 deletions tests/components/light/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,57 @@ async def test_light_backwards_compatibility_color_mode(hass):
assert state.attributes["color_mode"] == light.COLOR_MODE_HS


async def test_invalid_color_modes(hass, caplog):
"""Test invalid combinations of color modes."""
platform = getattr(hass.components, "test.light")
platform.init(empty=True)

platform.ENTITIES.append(platform.MockLight("Test_0", STATE_ON))
platform.ENTITIES.append(platform.MockLight("Test_1", STATE_ON))
platform.ENTITIES.append(platform.MockLight("Test_2", STATE_ON))
platform.ENTITIES.append(platform.MockLight("Test_3", STATE_ON))

entity0 = platform.ENTITIES[0]
entity0.supported_color_modes = []

entity1 = platform.ENTITIES[1]
entity1.supported_color_modes = [light.COLOR_MODE_ONOFF, light.COLOR_MODE_RGB]

entity2 = platform.ENTITIES[2]
entity2.supported_color_modes = [light.COLOR_MODE_BRIGHTNESS, light.COLOR_MODE_RGB]

entity3 = platform.ENTITIES[3]
entity3.supported_color_modes = [light.COLOR_MODE_UNKNOWN]

assert await async_setup_component(hass, "light", {"light": {"platform": "test"}})
await hass.async_block_till_done()

assert "Light light.test_0: Invalid supported_color_modes []" in caplog.text
assert (
"Light light.test_1: Invalid supported_color_modes ['onoff', 'rgb']"
in caplog.text
)
assert (
"Light light.test_2: Invalid supported_color_modes ['brightness', 'rgb']"
in caplog.text
)
assert (
"Light light.test_3: Invalid supported_color_modes ['unknown']" in caplog.text
)

state = hass.states.get(entity0.entity_id)
assert state.attributes["supported_color_modes"] == [light.COLOR_MODE_ONOFF]

state = hass.states.get(entity1.entity_id)
assert state.attributes["supported_color_modes"] == [light.COLOR_MODE_ONOFF]

state = hass.states.get(entity2.entity_id)
assert state.attributes["supported_color_modes"] == [light.COLOR_MODE_ONOFF]

state = hass.states.get(entity3.entity_id)
assert state.attributes["supported_color_modes"] == [light.COLOR_MODE_ONOFF]


async def test_light_service_call_rgbw(hass):
"""Test backwards compatibility for rgbw functionality in service calls."""
platform = getattr(hass.components, "test.light")
Expand Down