Allow independent control of white level on flux_led component by oblogic7 · Pull Request #13985 · home-assistant/core

oblogic7 · 2018-04-18T15:47:22Z

Description:

Allows independent control of white level on flux_led lights in RGBW mode. Also preserve brightness on color change.

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

Also preserve brightness on color change.

homeassistant · 2018-04-18T15:47:23Z

Hi @oblogic7,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

houndci-bot · 2018-04-18T15:47:31Z

                              random.randint(0, 255),
                              random.randint(0, 255))
+
+        #effect selection


block comment should start with '# '

houndci-bot · 2018-04-18T15:47:31Z

+        if rgb is not None:
+            self._bulb.setRgb(*tuple(rgb), brightness=self.brightness)
+
+        #brightness change only


block comment should start with '# '

houndci-bot · 2018-04-18T15:47:31Z

 from homeassistant.const import CONF_DEVICES, CONF_NAME, CONF_PROTOCOL
 from homeassistant.components.light import (
-    ATTR_BRIGHTNESS, ATTR_HS_COLOR, ATTR_EFFECT, EFFECT_COLORLOOP,
+    ATTR_BRIGHTNESS, ATTR_HS_COLOR, ATTR_EFFECT, ATTR_WHITE_VALUE, EFFECT_COLORLOOP,


line too long (84 > 79 characters)

syssi · 2018-04-19T05:14:01Z

Please check the travis builds for lint errors, too:

C:195, 0: Unnecessary parens after 'if' keyword (superfluous-parens)
C:196, 0: Unnecessary parens after 'return' keyword (superfluous-parens)

./homeassistant/components/light/flux_led.py:192:1: D202 No blank lines allowed after function docstring

balloob · 2018-05-01T18:39:26Z

+    @property
+    def white_value(self):
+        """Return the white value of this light between 0..255."""
+        return self._bulb.getRgbw()[3]


Currently, the light entity will request the white_value even if not SUPPORT_WHITE_VALUE. Will this raise an error ?

Retrieving index 3 should not directly result in an error because the flux_led library will always return something on that index even if None. So it would depend on how the code calling white_level handles the values. I can add a sanity check if None or non integer values are not supported.

If it's None, then it's fine as that is the expected return value for "no value"

tadly · 2018-05-12T23:47:45Z

Seeing as this is the most recent change to the flux component, I'm kind of certain it's what broke it for me.

As it stands all I'm able to do is turn the device on/off and change the brightness of the rgb part. I can't control the w portion at all now.
Setting colors, color temp, kelvin etc. is all broken.

Log isn't throwing any obvious errors.
I might do more investigation tomorrow as it's quite late right now.

oblogic7 · 2018-05-13T01:54:58Z

The change on this PR separated the W channel on RGBW to the white level slider instead of controlling W via the brightness slider. This makes sense because RGB brightness should be controllable independently of W brightness.

After looking at this for just a few minutes, I see that the flux.switch component uses xy_color and brightness to control color temperature, so I expect that the flux.switch component is expecting the W channel to respond to brightness level. Don't have time to dig any further tonight, but I think that adding support to flux.switch for RGBW mode would possibly make this work as expected. I will hopefully have time to look into it more tomorrow.

kineticscreen · 2018-05-14T01:12:55Z

Also since the latest update, it's broken scenes for me with the flux_LED component. Specifically if I send both RGB and Brightness settings, it changes the RGB but ignores the brightness change. This is true for both RGB and RGBW units. If I send the brightness change on its own it works fine, but coupled with colour change it ignores the brightness.

So:

    light.TV_Backlight:
        state: on
        rgb_color: [255, 100, 230]
        brightness_pct: 50

....alters the colours only.

    light.TV_Backlight:
        state: on
        brightness_pct: 50

.... changes the brightness.

The YeeLights in this same scene operate as normal, so it does appear to be a specific problem with the lux_LED component. And it ocurrs whether using brightness or brightness_pct

This problem is also present when directly doing this via the Services tab in Hassio.

oblogic7 · 2018-05-15T18:35:01Z

I've opened a new PR that should correct the issues that were introduced. If either of you can confirm the fixes it would be great.

#14476

kineticscreen · 2018-05-16T00:26:54Z

Total noob, so have no idea how to install this sorry. Thanks for getting onto it so quickly.

oblogic7 added 2 commits April 18, 2018 10:19

Allow independent control of white level on flux_led component.

540d305

Also preserve brightness on color change.

Limit white value support to RGBW mode.

4ce939b

homeassistant added cla-needed platform: light.flux_led small-pr PRs with less than 30 lines. labels Apr 18, 2018

houndci-bot reviewed Apr 18, 2018

View reviewed changes

oblogic7 changed the title ~~Flux led white level~~ Allow independent control of white level on flux_led component Apr 18, 2018

homeassistant added cla-signed and removed cla-needed labels Apr 18, 2018

Requested changes.

79c82b7

homeassistant added the cla-signed label Apr 18, 2018

oblogic7 added 2 commits April 19, 2018 08:57

Correct liniting issues

f89e792

Formatting

62eef59

balloob reviewed May 1, 2018

View reviewed changes

balloob merged commit c2d00be into home-assistant:dev May 1, 2018

balloob mentioned this pull request May 11, 2018

0.69 #14392

Merged

oblogic7 deleted the flux_led_white_level branch May 15, 2018 18:11

oblogic7 mentioned this pull request May 15, 2018

Flux bug fix #14476

Merged

2 tasks

Mariusthvdb mentioned this pull request May 31, 2018

Tradfri - Unexpected keyword argument brightness #14525

Closed

home-assistant locked and limited conversation to collaborators Sep 5, 2018

ghost added integration: flux_led and removed platform: light.flux_led labels Mar 21, 2019

Uh oh!

Conversation

oblogic7 commented Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Checklist:

Uh oh!

homeassistant commented Apr 18, 2018

Uh oh!

houndci-bot Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

houndci-bot Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

houndci-bot Apr 18, 2018

Choose a reason for hiding this comment

Uh oh!

syssi commented Apr 19, 2018

Uh oh!

balloob May 1, 2018

Choose a reason for hiding this comment

Uh oh!

oblogic7 May 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob May 1, 2018

Choose a reason for hiding this comment

Uh oh!

tadly commented May 12, 2018

Uh oh!

oblogic7 commented May 13, 2018

Uh oh!

kineticscreen commented May 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oblogic7 commented May 15, 2018

Uh oh!

kineticscreen commented May 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

oblogic7 commented Apr 18, 2018 •

edited

Loading

oblogic7 May 1, 2018 •

edited

Loading

kineticscreen commented May 14, 2018 •

edited

Loading