Add kelvin/brightness_pct alternatives to light.turn_on#7596
Conversation
There was a problem hiding this comment.
undefined name 'preprocess_turn_on_alternatives'
There was a problem hiding this comment.
Why would LIFX need this logic?
There was a problem hiding this comment.
This is because LIFX effects (and also lifx_set_state now) accept the same parameter names as light.turn_on, without actually going through the turn_on handler.
I previously wrote it out for color_name, but now with four alternative names I thought it to be too much duplication.
There was a problem hiding this comment.
line too long (80 > 79 characters)
amelchio
force-pushed
the
light-kelvin-brightness_pct
branch
from
d9caf0e to
3bfbcc5
Compare
amelchio
force-pushed
the
light-kelvin-brightness_pct
branch
from
3bfbcc5 to
ebd9719
Compare
amelchio
force-pushed
the
light-kelvin-brightness_pct
branch
from
ebd9719 to
7534b10
Compare
This avoids the color_name duplication and gains support for profile.
amelchio
force-pushed
the
light-kelvin-brightness_pct
branch
from
7534b10 to
687d330
Compare
|
|
||
| def preprocess_turn_on_alternatives(params): | ||
| """Processing extra data for turn light on request.""" | ||
| profile = Profiles.get(params.pop(ATTR_PROFILE, None)) |
There was a problem hiding this comment.
I wonder if anyone knows about this and uses it.
There was a problem hiding this comment.
Maybe we should deprecate it now that we have scenes, scripts etc. (out of scope of this PR)
There was a problem hiding this comment.
I have actually been thinking about extending it instead :-). I hope to make it possible to have multiple colors in a profile. This would create a "mood" that could be applied to a group of lights so each light gets one of the colors (without them all being identical). It could also be used for an animation effect where lights randomly change between colors in the profile.
|
|
||
| color_name = params.pop(ATTR_COLOR_NAME, None) | ||
| if color_name is not None: | ||
| params[ATTR_RGB_COLOR] = color_util.color_name_to_rgb(color_name) |
There was a problem hiding this comment.
Would it make sense to just mark rgb_color and color_name exclusive in the voluptuous schema? That way they can never be specified together. Now it might be confusing to the user why one parameter would overrule the other.
There was a problem hiding this comment.
Yes, I think that would make sense but I would prefer it to be a different PR?
The xy_color, color_temp, profile and maybe more parameters are also mutually exclusive.
|
@balloob profiles are realy cool. I hope we do that not remove 👍 |
|
Alrighty, looks good 🐬 |
ghost
added
6 participants
Description:
The existing
color_temp(in mired) andbrightness(arbitrary 8-bit scale) parameters tolight.turn_onusually require calculations that most people cannot do in their head.This PR adds alternative and well-known ways to specify white temperature (
kelvin) and brightness (brightness_pct).I guess the best way to express brightness would be with a calibrated unit like lumen but I do not see any way to provide that.
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code does not interact with devices:
toxrun successfully.