8000 improve RGB support for tradfri by spektren · Pull Request #9698 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

improve RGB support for tradfri #9698
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

Jump to bottom

Closed
wants to merge 7 commits into from
Closed

improve RGB support for tradfri #9698

wants to merge 7 commits into from

Conversation

spektren
Copy link
Contributor
@spektren spektren commented Oct 4, 2017 •
edited
Loading

Added feature detection (SUPPORT_RGB_COLOR) for IKEA tradfri RGB bulbs.

String comparison with manufacturer/device id used because I think there isn't a suitable property/param to identify the RGB capability. Both IKEA WS and RGB have data in the same parameters (x, y, rgb).

Added on the fly conversion of xyb values to rgb list to update method resulting in:
- bulb icon in UI is color scaled with current color (tested with IKEA WS+RGB and FLS-pp RGB)
- UI color picker or color temperature slider is displayed for respective bulb types

see also:
#9603

Tested with the following setup:
Home Assistant 0.53.1
pytradfri 2.2
IKEA tradfri gateway fw 1.1.0015
zigbee rgb PWM module (dresden elektronik FLS-PP lp)
IKEA RGB, WS and dimmable-only bulbs

Description:

Related issue (if applicable): fixes #9603

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#9411

Example entry for configuration.yaml (if applicable):

scene:
  - name: daytime
    entities:
      light.kitchen:
        state: on
        brightness: 63
        rgb_color: [255, 191, 0]

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ na ] Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • [ na ] New dependencies have been added to the REQUIREMENTS variable (example).
  • [ na ] New dependencies are only imported inside functions that use them (example).
  • [ na ] New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • [ na ] New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@spektren
Update tradfri.py
3f3536c 
Added feature detection (SUPPORT_RGB_COLOR) for IKEA tradfri RGB bulbs.

String comparison with manufacturer/device id used because I think there isn't a suitable property/param to identify the RGB capability. Both IKEA WS and RGB have data in the same parameters (x, y, rgb).

Color picker in UI is only offered after one of the default RGB colors (i.e. efd275) is set in param 5706. For now I couldn't find the section in polymer where the decision (to show a color picker or not) is made.
houndci-bot
houndci-bot reviewed Oct 4, 2017
View reviewed changes
homeassistant/components/light/tradfri.py Outdated


_LOGGER.debug("features detected for %s %s: %d", self._name, self._light.device_info.model_number, self._features)

Copy link

Choose a reason for hiding this comment

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

blank line contains whitespace

homeassistant/components/light/tradfri.py Outdated
else:
self._features |= SUPPORT_RGB_COLOR


_LOGGER.debug("features detected for %s %s: %d", self._name, self._light.device_info.model_number, self._features)
Copy link

Choose a reason for hiding this comment

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

line too long (122 > 79 characters)

homeassistant/components/light/tradfri.py Outdated
else:
self._features |= SUPPORT_RGB_COLOR


Copy link

Choose a reason for hiding this comment

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

blank line contains whitespace

homeassistant/components/light/tradfri.py Outdated
@@ -104,13 +104,18 @@ def __init__(self, light, api):
self._name = self._light.name
self._rgb_color = None
self._features = SUPPORT_BRIGHTNESS

Copy link

Choose a reason for hiding this comment

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

blank line contains whitespace

houndci-bot
houndci-bot reviewed Oct 4, 2017
View reviewed changes
homeassistant/components/light/tradfri.py Outdated
else:
self._features |= SUPPORT_RGB_COLOR

_LOGGER.debug("features detected for %s %s: %d",
self._name, self._light.device_info.model_number, self._features)
Copy link

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

homeassistant/components/light/tradfri.py Outdated
else:
self._features |= SUPPORT_RGB_COLOR

_LOGGER.debug("features detected for %s %s: %d",
Copy link

Choose a reason for hiding this comment

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

trailing whitespace

houndci-bot
houndci-bot reviewed Oct 4, 2017
View reviewed changes
homeassistant/components/light/tradfri.py Outdated
else:
self._features |= SUPPORT_RGB_COLOR

_LOGGER.debug("features detected for %s %s: %d",
self._name, self._light.device_info.model_number, self._features)
Copy link

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent
line too long (81 > 79 characters)

@spektren spektren changed 8000 the title Update tradfri.py add RGB support feature detection for IKEA rgb bulbs Oct 4, 2017
pvizeli
pvizeli requested changes Oct 4, 2017
View reviewed changes
homeassistant/components/light/tradfri.py Outdated
_LOGGER.debug(
"features detected for %s %s: %d",
self._name, self._light.device_info.model_number, self._features
)
Copy link
Member

Choose a reason for hiding this comment

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

You see the feature on UI, there need no debug log...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed logging of detected features

@MartinHjelmare
Copy link
Member

The color selector will be shown when the RGB property of the entity does not return None after a state update.

@spektren
improve support of RGB bulbs
ef9e3a1 
- debug logging of detected features removed (change requested)
- on the fly conversion of xyb values to rgb list added to update method
    resulting in: 
    - current rgb values will made be available for all bulbs/entities from current xyb values (i.e. IKEA WS and RGB) 
    - bulb icon in UI is color scaled with current color (tested with IKEA WS+RGB and FLS-pp RGB) 
    - UI color picker or color temperature slider is displayed for respective bulb types
houndci-bot
houndci-bot reviewed Oct 5, 2017
View reviewed changes
homeassistant/components/light/tradfri.py Outdated
self._light_data.dimmer not in (None, '0'):
self._rgb_color = color_util.color_xy_brightness_to_RGB(
self._light_data.xy_color[0]/65536,
self._light_data.xy_color[1]/65536,
Copy link

Choose a reason for hiding this comment

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

trailing whitespace

homeassistant/components/light/tradfri.py Outdated
elif self._light_data.xy_color not in (None, '0') and \
self._light_data.dimmer not in (None, '0'):
self._rgb_color = color_util.color_xy_brightness_to_RGB(
self._light_data.xy_color[0]/65536,
Copy link

Choose a reason for hiding this comment

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

trailing whitespace

homeassistant/components/light/tradfri.py Outdated

# handle GW behavoiur which does report hex color '0' whenevener
# a bulb is set to a color_xy which isn't one of the predefined ones
# -> convert xyb to rgb when available
Copy link

Choose a reason for hiding this comment

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

trailing whitespace

homeassistant/components/light/tradfri.py Outdated
self._rgb_color = color_util.rgb_hex_to_rgb_list(
self._light_data.hex_color)

# handle GW behavoiur which does report hex color '0' whenevener
Copy link

Choose a reason for hiding this comment

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

trailing whitespace

homeassistant/components/light/tradfri.py Outdated
@@ -198,6 +201,16 @@ def update(self):

# Handle Hue lights paired with the gateway
# hex_color is 0 when bulb is unreachable
if self._light_data.hex_color not in (None, '0'):
if self._light_data.hex_color not in (None, '0'):
Copy link

Choose a reason for hiding this comment

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

trailing whitespace

@spektren
improve support of RGB bulbs with tradfri
7b72bef 
- debug logging of detected features removed (change requested)
- on the fly conversion of xyb values to rgb list added to update method
    resulting in: 
    - current rgb values will made be available for all bulbs/entities from current xyb values (i.e. IKEA WS and RGB) 
    - bulb icon in UI is color scaled with current color (tested with IKEA WS+RGB and FLS-pp RGB) 
    - UI color picker or color temperature slider is displayed for respective bulb types
@spektren spektren changed the title add RGB support feature detection for IKEA rgb bulbs improve RGB support for tradfri Oct 5, 2017
@balloob
Copy link
Member
balloob commented Oct 5, 2017

I don't want to merge this before #7815 gets in, which is a major refactor of Tradfri and should land soon.

@balloob balloob mentioned this pull request Oct 5, 2017
Merged
2 tasks
spektren added a commit to spektren/home-assistant that referenced this pull request Oct 6, 2017
@spektren
home-assistant#9698 for tradfri async [055.0.dev0]
637bc82
spektren added a commit to spektren/home-assistant that referenced this pull request Oct 6, 2017
@spektren
home-assistant#9698 for tradfri async [0.55.0.dev0]
a5cdace
spektren added a commit to spektren/home-assistant that referenced this pull request Oct 6, 2017
@spektren
home-assistant#9698 and home-assistant#9703 for tradfri async [0.55.0…
53a1fd9 
….dev0]
spektren added a commit to spektren/home-assistant that referenced this pull request Oct 6, 2017
@spektren
home-assistant#9698 and home-assistant#9703 for tradfri async [0.55.0…
d72a7a1 
….dev0]
@spektren spektren mentioned this pull request Oct 6, 2017
Closed
8 tasks
@spektren spektren closed this Oct 6, 2017
@spektren
Copy link
Contributor Author
spektren commented Oct 6, 2017

Reincarnated in #9717

spektren added a commit to spektren/home-assistant that referenced this pull request Oct 6, 2017
@spektren
home-assistant#9698 and home-assistant#9703 for tradfri async [0.55.0…
9fac372 
….dev0]
spektren added a commit to spektren/home-assistant that referenced this pull request Oct 6, 2017
@spektren
home-assistant#9698 and home-assistant#9703 for tradfri async [0.55.0…
5027d33 
….dev0]
@home-assistant home-assistant locked and limited conversation to collaborators Mar 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@pvizeli pvizeli pvizeli requested changes

Assignees
No one assigned
Labels
cla-signed Hacktoberfest integration: tradfri platform via-github
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

IKEA Tradfri RGB - No color picker
8 participants
@spektren @MartinHjelmare @balloob @houndci-bot @pvizeli @fabaff @balloobbot @homeassistant
0