Skip to content

Introduce support for color temperature #25503

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

Merged
balloob merged 3 commits into from
Jul 31, 2019
Merged

Introduce support for color temperature #25503

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc71c19
Introduce support for color temperature
yeralin Jul 26, 2019
37b9bc2
Fix linter errors
yeralin Jul 31, 2019
bfd89c2
Remove extra whitespace for pylint
yeralin Jul 31, 2019
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
32 changes: 28 additions & 4 deletions homeassistant/components/flux_led/light.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
from homeassistant.const import CONF_DEVICES, CONF_NAME, CONF_PROTOCOL
from homeassistant.components.light import (
ATTR_BRIGHTNESS, ATTR_HS_COLOR, ATTR_EFFECT, ATTR_WHITE_VALUE,
EFFECT_COLORLOOP, EFFECT_RANDOM, SUPPORT_BRIGHTNESS, SUPPORT_EFFECT,
SUPPORT_COLOR, SUPPORT_WHITE_VALUE, Light, PLATFORM_SCHEMA)
ATTR_COLOR_TEMP, EFFECT_COLORLOOP, EFFECT_RANDOM, SUPPORT_BRIGHTNESS,
SUPPORT_EFFECT, SUPPORT_COLOR, SUPPORT_WHITE_VALUE, SUPPORT_COLOR_TEMP,
Light, PLATFORM_SCHEMA)
import homeassistant.helpers.config_validation as cv
import homeassistant.util.color as color_util

Expand All @@ -27,7 +28,7 @@
DOMAIN = 'flux_led'

SUPPORT_FLUX_LED = (SUPPORT_BRIGHTNESS | SUPPORT_EFFECT |
SUPPORT_COLOR)
SUPPORT_COLOR | SUPPORT_COLOR_TEMP)
Copy link
Copy Markdown
Contributor

@amelchio amelchio Aug 1, 2019

Choose a reason for hiding this comment

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

This is wrong as now we are advertising support for color temperature with RGB (not just RGBW).


MODE_RGB = 'rgb'
MODE_RGBW = 'rgbw'
Expand All @@ -36,6 +37,11 @@
# RGB value is ignored when this mode is specified.
MODE_WHITE = 'w'

# Constant color temp values for 2 flux_led special modes
# Warm-white and Cool-white. Details on #23704
COLOR_TEMP_WARM_WHITE = 333
COLOR_TEMP_COOL_WHITE = 250
Comment thread
yeralin marked this conversation as resolved.

# List of supported effects which aren't already declared in LIGHT
EFFECT_RED_FADE = 'red_fade'
EFFECT_GREEN_FADE = 'green_fade'
Expand Down Expand Up @@ -269,15 +275,19 @@ def _turn_on(self, **kwargs):
brightness = kwargs.get(ATTR_BRIGHTNESS)
effect = kwargs.get(ATTR_EFFECT)
white = kwargs.get(ATTR_WHITE_VALUE)
color_temp = kwargs.get(ATTR_COLOR_TEMP)

if all(item is None for item in [hs_color, brightness, effect, white]):
if all(item is None for item in
[hs_color, brightness, effect, white, color_temp]):
return

# handle W only mode (use brightness instead of white value)
if self._mode == MODE_WHITE:
if brightness is not None:
self._bulb.setWarmWhite255(brightness)
return

# handle effects
if effect is not None:
# Random color effect
if effect == EFFECT_RANDOM:
Expand All @@ -294,6 +304,20 @@ def _turn_on(self, **kwargs):
elif effect in EFFECT_MAP:
self._bulb.setPresetPattern(EFFECT_MAP[effect], 50)
return

# handle special modes
if color_temp is not None:
if brightness is None:
brightness = self.brightness
if color_temp == COLOR_TEMP_WARM_WHITE:
Copy link
Copy Markdown
Contributor

@amelchio amelchio Aug 1, 2019

Choose a reason for hiding this comment

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

I don't like this magic value. Using the frontend slider will give erratic behavior when this value is hit.

If only two temperatures are available I think it is better to decide a cutoff (like 3500 kelvin) and make all temperatures above/below the same.

But I will be surprised if only two temperatures are available since mixing w and w2 should give the full range of temperatures and that seems like a nice bullet on a feature list.

Copy link
Copy Markdown
Contributor Author

@yeralin yeralin Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

You cannot mix w and w2 see #23704:

1. In order to activate warm white state, all values *but* `w` should be `None`, while `w` can be between 0 and 255. If any other parameter is not `None`, the call will not result in *any action*

2. In order to activate cool white state, all values *but* `w2` should be `None`, while `w2` can be between 0 and 255. If any other parameter is not `None`, the call will not result in *any action*

Copy link
Copy Markdown
Contributor

@amelchio amelchio Aug 2, 2019

Choose a reason for hiding this comment

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

Okay, maybe there is some product differentiation or maybe you found a bug. There is so much different flux_led hardware, I still don't think this is true for it all.

For example, this commit suggests white mixing is possible.

self._bulb.setRgbw(w=brightness)
elif color_temp == COLOR_TEMP_COOL_WHITE:
self._bulb.setRgbw(w2=brightness)
else:
self._bulb.setRgbw(
*color_util.color_temperature_to_rgb(color_temp))
Copy link
Copy Markdown
Contributor

@amelchio amelchio Aug 1, 2019

Choose a reason for hiding this comment

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

Don't do this conversion, it gives poor results and setting RGB is not what ATTR_COLOR_TEMP is for.

Copy link
Copy Markdown
Contributor Author

@yeralin yeralin Aug 1, 2019

Choose a reason for hiding this comment

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

Hmmm, what can be the alternative?

Copy link
Copy Markdown
Contributor

@amelchio amelchio Aug 2, 2019

Choose a reason for hiding this comment

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

Always set some white light even if it is the wrong temperature.

return

# Preserve current brightness on color/white level change
if hs_color is not None:
if brightness is None:
Expand Down