Add support for light color modes by emontnemery · Pull Request #47720 · home-assistant/core

emontnemery · 2021-03-10T16:01:44Z

Proposed change

Add support for light color modes, background in home-assistant/architecture#519.
The color mode will be stored in the light's state, but should not be passed to the turn_on service call.
When turning on the light, the color mode is non ambiguous because only one color may be given.

Colors will no longer be force-converted through hs color space when updating state or in a turn_on call, instead the light's native color space will be respected.

New attributes:

ATTR_COLOR_MODE
ATTR_SUPPORTED_COLOR_MODES
ATTR_RGBW_COLOR
ATTR_RGBWW_COLOR

Defined color modes:

COLOR_MODES = {
    COLOR_MODE_ONOFF,  # The light can be turned on or off. This mode must be the only supported mode if supported by the light
    COLOR_MODE_DIMMER,  # The light can be dimmed. This mode must be the only supported mode if supported by the light
    COLOR_MODE_COLOR_TEMP,  # The light can be dimmed and its color temperature is present in the state.
    COLOR_MODE_HS,  # The light can be dimmed and its color is present in the state as an (h, s) tuple (no brightness).
    COLOR_MODE_XY,  # The light can be dimmed and its color is present in the state as an (x, y) tuple (no brightness).
    COLOR_MODE_RGB,  # The light can be dimmed and its color is present in the state as an (r, g, b) tuple (not normalized).
    COLOR_MODE_RGBW,  # The light can be dimmed and its color is present in the state as an (r, g, b, w) tuple (not normalized).
    COLOR_MODE_RGBWW,  # The light can be dimmed and its color is present in the state as an (r, g, b, cw, ww) tuple (not normalized).
    COLOR_MODE_UNKNOWN,  # The light has reported an ambiguous state (e.g. no brightness for a light which supports `COLOR_MODE_DIMMER`)
}

New methods added to the base component LightEntity:

color_mode
supported_color_modes
rgb_color
rgbw_color
rgbww_color
xy_color

For backwards compatibility:

If the light does not implement supported_color_modes, it will be calculeted based on supported_features
If the light does not implement color_mode, current color_mode will be guessed in the following order:
- COLOR_MODE_RGBW if the light's white_value and hs_color are not None
- COLOR_MODE_HS if the light's hs_color is not None
- COLOR_MODE_COLOR_TEMP if the light's color_temp is not None
- COLOR_MODE_DIMMER if the light's brihtness is not None
- COLOR_MODE_NONE otherwise
If the light does not implement supported_color_modes, a turn_on service call with ATTR_RGBW_COLOR will be converted to ATTR_HS_COLOR + ATTR_WHITE_VALUE.

Deprecated:

SUPPORT_BRIGHTNESS = 1
SUPPORT_COLOR_TEMP = 2
SUPPORT_COLOR = 16

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example entry for `configuration.yaml`:

# Example configuration.yaml

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

probot-home-assistant · 2021-03-10T16:01:53Z

Hey there @emlove, mind taking a look at this pull request as its been labeled with an integration (zerproc) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

probot-home-assistant · 2021-03-10T16:01:53Z

Hey there @rytilahti, @zewelor, @shenxn, mind taking a look at this pull request as its been labeled with an integration (yeelight) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

balloob · 2021-03-10T19:08:27Z

I would expect Google, Alexa and Homekit requiring updates because of this PR

balloob · 2021-03-10T19:13:03Z

homeassistant/components/light/__init__.py

This tests if ATTR_HS_COLOR is truthy, ATTR_RGB_COLOR is truthy, or ATTR_XY_COLOR in params. The first two are defined so this code is always executed?

Suggested change

if ATTR_HS_COLOR or ATTR_RGB_COLOR or ATTR_XY_COLOR in params:

if ATTR_HS_COLOR in params or ATTR_RGB_COLOR in params or ATTR_XY_COLOR in params:

Oops, tests are needed for the new code

Couldn't you use intersection here?

Not sure if more readable:

if params.keys() & {ATTR_HS_COLOR, ATTR_RGB_COLOR, ATTR_XY_COLOR}:

You can also do

if any(key in params for key in (ATTR_HS_COLOR, ATTR_RGB_COLOR, ATTR_XY_COLOR)):

balloob · 2021-03-10T19:14:40Z

homeassistant/components/light/__init__.py

Make sure every fallback has a test.

Sure, the tests for the light component needs to be updated to test the new features as well as all the backwards compatibility cases.

balloob · 2021-03-10T19:15:37Z

homeassistant/components/light/__init__.py

Suggested change

r, g, b = color_util.color_hs_to_RGB(*self.hs_color)

# pylint: disable=invalid-name

r, g, b = color_util.color_hs_to_RGB(*self.hs_color)

emontnemery · 2021-03-10T21:08:34Z

I would expect Google, Alexa and Homekit requiring updates because of this PR

Does it mean those integrations will need to be updated to handle the new colour modes, rgbw and rgbww?

emontnemery · 2021-03-11T19:40:59Z

@Koenkk could you have a look at this from a z2m perspective?

emontnemery · 2021-03-12T15:38:32Z

@dmulcahey, @Kane610 Can you review the proposal from the perspective of zha / deconz?

zha · 2021-03-12T15:46:00Z

@zha, @Kane610 Can you review the proposal from the perspective of zha / deconz?

I am sure you mentioned the wrong person 😊. Very interesting project though.

Adminiuga · 2021-03-12T15:56:42Z

Nothing jumps out for ZHA. LGTM

Please notify me when this gets merged (in case I miss it), and I'll test it, update ZHA if needed.

Koenkk · 2021-03-12T18:59:25Z

@emontnemery looks good, 2 things however:

COLOR_MODE_BRIGHTNESS looks like a more logical name for COLOR_MODE_DIMMER
Based on this: https://github.com/home-assistant/core/pull/47720/files#diff-1d3290e4b1b6fbd972cf52246c5138aef77c3dc2419099576d5d2e0f7cc842cdR433 I expect the MQTT payload should contain the color_mode like this (lets say we have a bulb which is currently in XY mode):

{"state": "ON", "brightness": 100, "color": {"x": 0.7, "y": 0.3"}, "color_mode": "color_mode_xy"}

Question: what happens if there is no color_mode property in the payload? I would expect COLOR_MODE_UNKNOWN to be used in this case.

The "color_mode": "color_mode_xy" looks a bit strange to me since color_mode is in both the property and value. What about just: "color_mode": "xy"?

balloob · 2021-03-12T20:20:41Z

homeassistant/components/light/__init__.py

+        elif ATTR_HS_COLOR in params and COLOR_MODE_HS not in supported_color_modes:
+            hs_color = params.pop(ATTR_HS_COLOR)


You could go all wallrus:

Suggested change

elif ATTR_HS_COLOR in params and COLOR_MODE_HS not in supported_color_modes:

hs_color = params.pop(ATTR_HS_COLOR)

elif COLOR_MODE_HS not in supported_color_modes and (hs_color := params.pop(ATTR_HS_COLOR, None) is not None:

It becomes 4 lines instead of 2 after formatting though:

elif ( COLOR_MODE_HS not in supported_color_modes and (hs_color := params.pop(ATTR_HS_COLOR, None)) is not None ):

balloob · 2021-03-12T20:23:09Z

homeassistant/components/light/__init__.py

+        color_mode = self._light_internal_color_mode
+
+        if color_mode not in self._light_internal_supported_color_modes:
+            _LOGGER.debug(


Should we log this as error the first time per-entity?

I think we should start doing that in 1-2 releases, not yet. I added a comment in 63e4f4d.

…_BRIGHTNESS

emontnemery · 2021-03-15T07:14:27Z

@Koenkk

COLOR_MODE_BRIGHTNESS looks like a more logical name for COLOR_MODE_DIMMER

I agree, changed in 577f787.

Question: what happens if there is no color_mode property in the payload? I would expect COLOR_MODE_UNKNOWN to be used in this case.

For an MQTT JSON light, I think we should add two new configuration variables:

color_mode: set to True if the light will report its color mode, False otherwise.
supported_color_modes: A list of supported color modes

For a light which is configured with color_mode=True, it would be an error if we receive a state update dict without the color_mode, or a color_mode not included in supported_color_modes.

The "color_mode": "color_mode_xy" looks a bit strange to me since color_mode is in both the property and value. What about just: "color_mode": "xy"?

I agree, changed in 577f787.

Koenkk · 2021-03-16T18:15:06Z

~~@emontnemery big thanks, when looking at the changes files I don't see anything changed in MQTT. Does this mean a new merge request has to be done for this? (or can I already test this out somehow)~~

Forget about this, just saw #47993

homeassistant added the cla-signed label Mar 10, 2021

probot-home-assistant bot added core has-tests integration: kulersky integration: light integration: yeelight integration: zerproc new-feature labels Mar 10, 2021

probot-home-assistant bot assigned emlove and rytilahti and unassigned emlove Mar 10, 2021

balloob reviewed Mar 10, 2021

View reviewed changes

emontnemery added 4 commits March 11, 2021 11:46

Add support for light color modes

0bdf790

Update tests

d1c24fd

Update comments

d8a28e6

Fix bugs, add tests

7c843a9

emontnemery force-pushed the light_color_modes branch from 8f9a1da to 7c843a9 Compare March 11, 2021 10:47

emontnemery added 2 commits March 11, 2021 13:24

Suppress lint errors

e4b7694

Don't suppress brightness when state is ambiguous

4604ed1

Improve reproduce_state + add tests

7b66919

balloob reviewed Mar 12, 2021

View reviewed changes

balloob approved these changes Mar 12, 2021

View reviewed changes

emontnemery added 2 commits March 15, 2021 08:04

Add comment

63e4f4d

Change COLOR_MODE_* constants, rename COLOR_MODE_DIMMER to COLOR_MODE…

577f787

…_BRIGHTNESS

Fix tests

d8ecb3b

emontnemery closed this Mar 16, 2021

emontnemery reopened this Mar 16, 2021

Tweaks

af7f5eb

emontnemery merged commit 5f2326f into home-assistant:dev Mar 16, 2021

sjorge mentioned this pull request Mar 16, 2021

Allow /get for color_mode Koenkk/zigbee-herdsman-converters#2283

Merged

github-actions bot locked and limited conversation to collaborators Mar 17, 2021

	if ATTR_HS_COLOR or ATTR_RGB_COLOR or ATTR_XY_COLOR in params:
	if ATTR_HS_COLOR in params or ATTR_RGB_COLOR in params or ATTR_XY_COLOR in params:

	r, g, b = color_util.color_hs_to_RGB(*self.hs_color)
	# pylint: disable=invalid-name
	r, g, b = color_util.color_hs_to_RGB(*self.hs_color)

		elif ATTR_HS_COLOR in params and COLOR_MODE_HS not in supported_color_modes:
		hs_color = params.pop(ATTR_HS_COLOR)

	elif ATTR_HS_COLOR in params and COLOR_MODE_HS not in supported_color_modes:
	hs_color = params.pop(ATTR_HS_COLOR)
	elif COLOR_MODE_HS not in supported_color_modes and (hs_color := params.pop(ATTR_HS_COLOR, None) is not None:

Uh oh!

Conversation

emontnemery commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

Uh oh!

probot-home-assistant bot commented Mar 10, 2021

Uh oh!

probot-home-assistant bot commented Mar 10, 2021

Uh oh!

balloob commented Mar 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emontnemery commented Mar 10, 2021

Uh oh!

emontnemery commented Mar 11, 2021

Uh oh!

emontnemery commented Mar 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zha commented Mar 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Adminiuga commented Mar 12, 2021

Uh oh!

Koenkk commented Mar 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emontnemery Mar 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emontnemery Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emontnemery commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Koenkk commented Mar 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

emontnemery commented Mar 10, 2021 •

edited

Loading

Example entry for `configuration.yaml`:

emontnemery commented Mar 12, 2021 •

edited

Loading

zha commented Mar 12, 2021 •

edited

Loading

Koenkk commented Mar 12, 2021 •

edited

Loading

emontnemery Mar 13, 2021 •

edited

Loading

emontnemery Mar 15, 2021 •

edited

Loading

emontnemery commented Mar 15, 2021 •

edited

Loading

Koenkk commented Mar 16, 2021 •

edited

Loading