Conversation
probot-home-assistant
bot
added
integration: lifx
new-feature
small-pr
| return support | ||
| def color_mode(self) -> str: | ||
| """Return the color mode of the light.""" | ||
| return COLOR_MODE_HS |
There was a problem hiding this comment.
Is this correct, or can the LIFXColor and its subclasses support white with adjustable color temperature too?
There was a problem hiding this comment.
LIFXColor devices all support COLOR_MODE_COLOR_TEMP as well as COLOR_MODE_HS
There was a problem hiding this comment.
Thanks @Djelibeybi 👍
Does it mean this check is not needed, and supported color modes are always hs + color_temp?
if bulb_features["min_kelvin"] != bulb_features["max_kelvin"]:
return COLOR_MODE_COLOR_TEMPAlso, it looks like the API sends a tuple (h, s, b, kelvin) to the light.
What does the light do if saturation is not None, is the kelvin parameter ignored or still relevant?
There was a problem hiding this comment.
For color bulbs, they all support both modes. If min_kelvin == max_kelvin then the bulb only supports brightness adjustment.
if saturation > 0, the kelvin value is ignored.
There was a problem hiding this comment.
OK, so would this be correct:
diff --git a/homeassistant/components/lifx/light.py b/homeassistant/components/lifx/light.py
index 323bec3f1d..5aeeb13d3e 100644
--- a/homeassistant/components/lifx/light.py
+++ b/homeassistant/components/lifx/light.py
@@ -571,10 +571,6 @@ class LIFXLight(LightEntity):
"""Flag supported features."""
support = SUPPORT_BRIGHTNESS | SUPPORT_TRANSITION | SUPPORT_EFFECT
- bulb_features = lifx_features(self.bulb)
- if bulb_features["min_kelvin"] != bulb_features["max_kelvin"]:
- support |= SUPPORT_COLOR_TEMP
-
return support
@property
@@ -728,7 +724,7 @@ class LIFXColor(LIFXLight):
def supported_features(self):
"""Flag supported features."""
support = super().supported_features
- support |= SUPPORT_COLOR
+ support |= SUPPORT_COLOR | SUPPORT_COLOR_TEMP
return supportThere was a problem hiding this comment.
This product seems to be a bulb with adjustable color temperature but without color support though: https://www.lifx.com/products/candle-white-to-warm
There was a problem hiding this comment.
Right, but it wouldn't be discovered as a LIFXColor bulb, though.
There was a problem hiding this comment.
OK updated so LIFXColor and LIFXStrip report support for color modes hs + color_temp.
If saturation is 0, the light will report the current mode as color_temp, else it will report hs.
emontnemery
force-pushed
the
lifx_light_color_mode
branch
from
6e13007 to
67ca003
Compare
Djelibeybi
left a comment
Djelibeybi
left a comment
There was a problem hiding this comment.
Tested locally with my bulbs and it looks good.
epenet
added
the
smash
epenet
removed
the
smash
Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
4 participants
Proposed change
Migrate lifx light to color_mode
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: