Skip to content

Fix flux_led only-white controllers (and remove explicit declaration as RGBW in automatic add) #22210

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 13 commits into from
Apr 16, 2019
Merged

Fix flux_led only-white controllers (and remove explicit declaration as RGBW in automatic add) #22210

Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
08e9322
Remove explicit declaration of automatic found devices as RGBW
Mar 20, 2019
5f9ccd2
fixes for Magic Home only-white controllers
Apr 4, 2019
e23c366
mode is now set to None instead of removed entirely
Apr 4, 2019
52cb222
flux_led now changes no values when turned on from off state.
Apr 4, 2019
b66a628
Merge 'upstream/dev'
amelchio Apr 9, 2019
05d8200
better checking for changed values in turn_on
Apr 10, 2019
0254abb
Merge branch 'flux_led_fix' of github.com:autinerd/home-assistant int…
Apr 10, 2019
f568097
Reduce waiting time to 1 second
Apr 10, 2019
b46cbbe
Correction of turn on logic
Apr 11, 2019
afd673d
Merge branch 'flux_led_fix' of github.com:autinerd/home-assistant int…
Apr 11, 2019
f780d73
Remove accidentally inserted 'not'
Apr 11, 2019
f104fc0
Remove lint
amelchio Apr 11, 2019
b476390
Remove redundant code
amelchio Apr 15, 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
25 changes: 15 additions & 10 deletions homeassistant/components/flux_led/light.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
if ipaddr in light_ips:
continue
device['name'] = '{} {}'.format(device['id'], ipaddr)
device[ATTR_MODE] = MODE_RGBW

@amelchio amelchio Mar 25, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The device[ATTR_MODE] value is always read in __init__() so this will make it always raise an exception. I think it could work if you set it to None instead.

Just a note for others, the mode is set automatically if not explicitly defined: https://github.com/home-assistant/home-assistant/blob/08e93224e0689c9a20a2b6b43057f60348bb7c14/homeassistant/components/flux_led/light.py#L185-L191

(whether that works for all devices, I don't know)

device[ATTR_MODE] = None
device[CONF_PROTOCOL] = None
device[CONF_CUSTOM_EFFECT] = None
light = FluxLight(device)
Expand Down Expand Up @@ -266,13 +266,21 @@ async def async_turn_on(self, **kwargs):

def _turn_on(self, **kwargs):
"""Turn the specified or all lights on."""
self._bulb.turnOn()
if not self.is_on:

@amelchio amelchio Apr 10, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason for this condition? As I said, the state can be stale so it is better to not trust it.

self._bulb.turnOn()
if kwargs.get(ATTR_BRIGHTNESS) is None:
return

@amelchio amelchio Apr 9, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is wrong. I think you are addressing a UI issue but we can turn on with any parameters using light.turn_on.

Also, preferably turn on unconditionally because our local state could be stale.

I am not quite sure what you are trying to do so I cannot advise on a better way :)

@autinerd autinerd Apr 9, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

One (in my opinion) not so optimal thing is, that turn_on is for turning stuff on and changing the state of an entity. What I want to do is that when the lamp is off and the user want to turn it on, it just turns on and nothing else (except when brightness is set, because it's possible to turn the light on with the brightness slider as well). When the lamp is already on, then the state should be changed.

@amelchio amelchio Apr 9, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, light.turn_on is indeed overloaded but each light platform should not work around that. It is quite possible to turn on to an RGB color, for example with an automation.

Instead, only call self._bulb.setRgb() and friends if relevant kwargs are actually passed.

@autinerd autinerd Apr 10, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, I forgot the automation, thank you. I will correct this.


hs_color = kwargs.get(ATTR_HS_COLOR)
brightness = kwargs.get(ATTR_BRIGHTNESS)
effect = kwargs.get(ATTR_EFFECT)
white = kwargs.get(ATTR_WHITE_VALUE)

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

# Show warning if effect set with rgb, brightness, or white level
if effect and (brightness or white or hs_color):
_LOGGER.warning("RGB, brightness and white level are ignored when"
Expand All @@ -284,7 +292,6 @@ def _turn_on(self, **kwargs):
random.randint(0, 255),
random.randint(0, 255))
return

if effect == EFFECT_CUSTOM:
if self._custom_effect:
self._bulb.setCustomPattern(
Expand All @@ -308,12 +315,8 @@ def _turn_on(self, **kwargs):
self._color = (self._color[0], self._color[1],
brightness / 255 * 100)

# handle W only mode (use brightness instead of white value)
if self._mode == MODE_WHITE:
self._bulb.setRgbw(0, 0, 0, w=brightness)

# handle RGBW mode
elif self._mode == MODE_RGBW:
if self._mode == MODE_RGBW:
if white is None:
self._bulb.setRgbw(*color_util.color_hsv_to_RGB(*self._color))
else:
Expand All @@ -333,7 +336,7 @@ def update(self):
try:
self._connect()
self._error_reported = False
if self._bulb.getRgb() != (0, 0, 0):
if self._mode != MODE_WHITE and self._bulb.getRgb() != (0, 0, 0):
color = self._bulb.getRgbw()
self._color = color_util.color_RGB_to_hsv(*color[0:3])
self._white_value = color[3]
Expand All @@ -345,7 +348,9 @@ def update(self):
self._error_reported = True
return
self._bulb.update_state(retry=2)
if self._bulb.getRgb() != (0, 0, 0):
if self._mode != MODE_WHITE and self._bulb.getRgb() != (0, 0, 0):
color = self._bulb.getRgbw()
self._color = color_util.color_RGB_to_hsv(*color[0:3])
self._white_value = color[3]
elif self._mode == MODE_WHITE:
self._white_value = self._bulb.getRgbw()[3]