Skip to content

Disallow ambiguous color descriptors in the light.turn_on schema#7765

Merged
balloob merged 2 commits into
home-assistant:devfrom
amelchio:exclusive-colors
Jun 2, 2017
Merged

Disallow ambiguous color descriptors in the light.turn_on schema#7765
balloob merged 2 commits into
home-assistant:devfrom
amelchio:exclusive-colors

Conversation

@amelchio
Copy link
Copy Markdown
Contributor

@amelchio amelchio commented May 25, 2017

Description:

There are quite a lot of different ways to specify colors and making them mutally exclusive seems simpler than making rules of precedence.

Strictly speaking, this is a breaking change: previously you could have an XY and an RGB light in a group and make one light red and the other one blue with a single service call. I don't think losing that ability will be an issue. Also, profiles can no longer have their xy-value overwritten, only the brightness. Again – not a real loss.

The real breaking change is that it will now point out some configurations that have previously been ambiguous or redundant.

I have pooled color_temp with the color-setting ones because I think it should imply a white color.

The white_value is not included due to it being separate from all the others. In an RGBW light, the white_value affects the W while all the others affect the RGB.

Opinion can probably differ on the above points and I am prepared to adjust given some sensible arguments :)

Breaking change notice: The light component will now refuse turn_on calls that specify redundant or ambiguous colors, such as using both color_name and color_rgb in the same call.

Checklist:

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

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@mention-bot
Copy link
Copy Markdown

@amelchio, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @tomduijf and @mxtra to be potential reviewers.

Comment thread tests/components/light/test_init.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

statement ends with a semicolon

Comment thread tests/components/light/test_init.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

statement ends with a semicolon

@amelchio
Copy link
Copy Markdown
Contributor Author

I'm torn about color_temp and would like agitated feedback :)

Copy link
Copy Markdown
Contributor

@emlove emlove left a comment

Choose a reason for hiding this comment

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

I like this idea.

I think color temp is an appropriate inclusion. It's really just another definition of a color. A light can't use an RGB color and a color temp at the same time.

@balloob balloob merged commit e2cfdbf into home-assistant:dev Jun 2, 2017
@balloob balloob mentioned this pull request Jun 2, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants