full RGB support for users of tradfri GW by spektren · Pull Request #9411 · home-assistant/core

spektren · 2017-09-13T07:36:36Z

the use of set_rgb_color() seems to be advantageous over set_hex_color() for full RGB support

Description:

Issue:
pytradfri's set_hex_color() does not work for arbitrary colors with the current IKEA tradfri gateway. Only setting rgb hex values (param 5706) of some predefined colors has the desired effect. Others will fall back to one predefined value. I assume, the GW doesn't allow for values deviating from the predefined values.

However, pytradfri's set_rgb_color() does also work for arbitrary colors. Latest pytradfri (2.2/PR51?) will convert rgb to xy and send xy thru the GW (param 5709 and 5710).

-> changed the function used from set_hex_color() to set_rgb_color() in HA's component\light\tradfri

Result:
Full RGB support with arbitrary colors with my setup.

Setup:
Home Assistant 0.53.0
pytradfri 2.2
IKEA tradfri gateway fw 1.1.0015
zigbee rgb PWM module (dresden elektronik FLS-PP lp)

Unfortunately I cannot test tradfri GW with other bulbs (no have hue/lightify bulbs).

Predefined colors from <home-assistant-libs/pytradfri#51>:
this.f3891b = new HashMap();
this.f3891b.put("f5faf6", new C1386c(0.3804d, 0.3804d, "f5faf6", 0.54d));
this.f3891b.put("f1e0b5", new C1386c(0.4599d, 0.4106d, "CCT_LIGHT_NEUTRAL", 0.61d));
this.f3891b.put("efd275", new C1386c(0.5056d, 0.4152d, "efd275", 0.66d));
this.f3891b.put("dcf0f8", new C1386c(0.3221d, 0.3317d, "dcf0f8", 0.45d));
this.f3891b.put("eaf6fb", new C1386c(0.3451d, 0.3451d, "eaf6fb", 0.48d));
this.f3891b.put("f5faf6", new C1386c(0.3804d, 0.3804d, "f5faf6", 0.54d));
this.f3891b.put("f2eccf", new C1386c(0.4369d, 0.4041d, "f2eccf", 0.59d));
this.f3891b.put("CCT_LIGHT_NEUTRAL", new C1386c(0.4599d, 0.4106d, "CCT_LIGHT_NEUTRAL", 0.61d));
this.f3891b.put("efd275", new C1386c(0.5056d, 0.4152d, "efd275", 0.66d));
this.f3891b.put("ebb63e", new C1386c(0.5516d, 0.4075d, "ebb63e", 0.68d));
this.f3891b.put("e78834", new C1386c(0.58d, 0.38d, "e78834", 0.69d));
this.f3891b.put("e57345", new C1386c(0.58d, 0.35d, "e57345", 0.67d));
this.f3891b.put("da5d41", new C1386c(0.62d, 0.34d, "da5d41", 0.7d));
this.f3891b.put("dc4b31", new C1386c(0.66d, 0.32d, "dc4b31", 0.73d));
this.f3891b.put("e491af", new C1386c(0.5d, 0.28d, "e491af", 0.57d));
this.f3891b.put("e8bedd", new C1386c(0.45d, 0.28d, "e8bedd", 0.53d));
this.f3891b.put("d9337c", new C1386c(0.5d, 0.24d, "d9337c", 0.55d));
this.f3891b.put("c984bb", new C1386c(0.34d, 0.19d, "c984bb", 0.38d));
this.f3891b.put("8f2686", new C1386c(0.31d, 0.12d, "8f2686", 0.33d));
this.f3891b.put("4a418a", new C1386c(0.17d, 0.05d, "4a418a", 0.18d));
this.f3891b.put("6c83ba", new C1386c(0.2d, 0.1d, "6c83ba", 0.22d));
this.f3891b.put("a9d62b", new C1386c(0.4099999964237213d, 0.5099999904632568d, "a9d62b", 0.654d));
this.f3891b.put("d6e44b", new C1386c(0.44999998807907104d, 0.4699999988079071d, "d6e44b", 0.65d));

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

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:

Documentation added/updated in home-assistant.github.io

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

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
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.

## 201709013: set_hex_color() seems not to work in pytradfri api - set_rgb_color() does ## -> changed function set_hex_color() to set_rgb_color() ## tested w. IKEA tradfri GW and zigbee rgb PWM module (dresden elektronik FLS-PP lp)

homeassistant · 2017-09-13T07:36:38Z

Hi @spektren,

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!

mention-bot · 2017-09-13T07:36:38Z

@spektren, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ggravlingen, @matemaciek and @balloob to be potential reviewers.

houndci-bot · 2017-09-13T07:36:46Z

+            ## 201709013: set_hex_color() seems not to work in pytradfri api - set_rgb_color() does 
+            ## -> changed function set_hex_color() to set_rgb_color()
+            ## tested w. IKEA tradfri GW and zigbee rgb PWM module (dresden elektronik FLS-PP lp) 
+            self._api(self._light.light_control.set_rgb_color(*kwargs[ATTR_RGB_COLOR]))


line too long (87 > 79 characters)

houndci-bot · 2017-09-13T07:36:46Z

+            #~    color_util.color_rgb_to_hex(*kwargs[ATTR_RGB_COLOR])))
+            ## 201709013: set_hex_color() seems not to work in pytradfri api - set_rgb_color() does 
+            ## -> changed function set_hex_color() to set_rgb_color()
+            ## tested w. IKEA tradfri GW and zigbee rgb PWM module (dresden elektronik FLS-PP lp) 


too many leading '#' for block comment
line too long (97 > 79 characters)
trailing whitespace

houndci-bot · 2017-09-13T07:36:46Z

+            #~ self._api(self._light.light_control.set_hex_color(
+            #~    color_util.color_rgb_to_hex(*kwargs[ATTR_RGB_COLOR])))
+            ## 201709013: set_hex_color() seems not to work in pytradfri api - set_rgb_color() does 
+            ## -> changed function set_hex_color() to set_rgb_color()


too many leading '#' for block comment

houndci-bot · 2017-09-13T07:36:46Z

-                color_util.color_rgb_to_hex(*kwargs[ATTR_RGB_COLOR])))
+            #~ self._api(self._light.light_control.set_hex_color(
+            #~    color_util.color_rgb_to_hex(*kwargs[ATTR_RGB_COLOR])))
+            ## 201709013: set_hex_color() seems not to work in pytradfri api - set_rgb_color() does 


too many leading '#' for block comment
line too long (99 > 79 characters)
trailing whitespace

houndci-bot · 2017-09-13T07:36:46Z

-            self._api(self._light.light_control.set_hex_color(
-                color_util.color_rgb_to_hex(*kwargs[ATTR_RGB_COLOR])))
+            #~ self._api(self._light.light_control.set_hex_color(
+            #~    color_util.color_rgb_to_hex(*kwargs[ATTR_RGB_COLOR])))


block comment should start with '# '

houndci-bot · 2017-09-13T07:36:46Z

        if ATTR_RGB_COLOR in kwargs and self._light_data.hex_color is not None:
-            self._api(self._light.light_control.set_hex_color(
-                color_util.color_rgb_to_hex(*kwargs[ATTR_RGB_COLOR])))
+            #~ self._api(self._light.light_control.set_hex_color(


block comment should start with '# '

Setup: Home Assistant 0.53.0 pytradfri 2.2 IKEA tradfri gateway fw 1.1.0015 zigbee rgb PWM module (dresden elektronik FLS-PP lp) Issue: pytradfri's set_hex_color() does not work for arbitrary colors with the current IKEA tradfri gateway. Only setting rgb hex values (param 5706) of some predefined colors has the desired effect. Others will fall back to one predefined value. I assume, the GW doesn't allow for values deviating from the predefined values. However, pytradfri's set_rgb_color() does also work for arbitrary colors. Latest pytradfri (2.2/PR51?) will convert rgb to xy and send xy thru the GW (param 5709 and 5710). -> changed the function used from set_hex_color() to set_rgb_color() in HA's component\light\tradfri Result: Full RGB support with arbitrary colors with my setup. Unfortunately I cannot test tradfri GW with other bulbs (no have hue/lightify bulbs). ___ Predefined colors from <home-assistant-libs/pytradfri#51>: this.f3891b = new HashMap(); this.f3891b.put("f5faf6", new C1386c(0.3804d, 0.3804d, "f5faf6", 0.54d)); this.f3891b.put("f1e0b5", new C1386c(0.4599d, 0.4106d, "CCT_LIGHT_NEUTRAL", 0.61d)); this.f3891b.put("efd275", new C1386c(0.5056d, 0.4152d, "efd275", 0.66d)); this.f3891b.put("dcf0f8", new C1386c(0.3221d, 0.3317d, "dcf0f8", 0.45d)); this.f3891b.put("eaf6fb", new C1386c(0.3451d, 0.3451d, "eaf6fb", 0.48d)); this.f3891b.put("f5faf6", new C1386c(0.3804d, 0.3804d, "f5faf6", 0.54d)); this.f3891b.put("f2eccf", new C1386c(0.4369d, 0.4041d, "f2eccf", 0.59d)); this.f3891b.put("CCT_LIGHT_NEUTRAL", new C1386c(0.4599d, 0.4106d, "CCT_LIGHT_NEUTRAL", 0.61d)); this.f3891b.put("efd275", new C1386c(0.5056d, 0.4152d, "efd275", 0.66d)); this.f3891b.put("ebb63e", new C1386c(0.5516d, 0.4075d, "ebb63e", 0.68d)); this.f3891b.put("e78834", new C1386c(0.58d, 0.38d, "e78834", 0.69d)); this.f3891b.put("e57345", new C1386c(0.58d, 0.35d, "e57345", 0.67d)); this.f3891b.put("da5d41", new C1386c(0.62d, 0.34d, "da5d41", 0.7d)); this.f3891b.put("dc4b31", new C1386c(0.66d, 0.32d, "dc4b31", 0.73d)); this.f3891b.put("e491af", new C1386c(0.5d, 0.28d, "e491af", 0.57d)); this.f3891b.put("e8bedd", new C1386c(0.45d, 0.28d, "e8bedd", 0.53d)); this.f3891b.put("d9337c", new C1386c(0.5d, 0.24d, "d9337c", 0.55d)); this.f3891b.put("c984bb", new C1386c(0.34d, 0.19d, "c984bb", 0.38d)); this.f3891b.put("8f2686", new C1386c(0.31d, 0.12d, "8f2686", 0.33d)); this.f3891b.put("4a418a", new C1386c(0.17d, 0.05d, "4a418a", 0.18d)); this.f3891b.put("6c83ba", new C1386c(0.2d, 0.1d, "6c83ba", 0.22d)); this.f3891b.put("a9d62b", new C1386c(0.4099999964237213d, 0.5099999904632568d, "a9d62b", 0.654d)); this.f3891b.put("d6e44b", new C1386c(0.44999998807907104d, 0.4699999988079071d, "d6e44b", 0.65d));

balloob · 2017-09-14T05:25:00Z

Thanks! And congratulations on your first PR ! 🐬 🎉

spektren · 2017-10-04T20:27:02Z

Thank you. I'm happy to contribute.

Update tradfri.py

4fc3d51

## 201709013: set_hex_color() seems not to work in pytradfri api - set_rgb_color() does ## -> changed function set_hex_color() to set_rgb_color() ## tested w. IKEA tradfri GW and zigbee rgb PWM module (dresden elektronik FLS-PP lp)

homeassistant added platform: light.tradfri cla-needed labels Sep 13, 2017

houndci-bot reviewed Sep 13, 2017

View reviewed changes

homeassistant added cla-signed and removed cla-needed labels Sep 13, 2017

spektren changed the title ~~Update tradfri.py~~ full RGB support for users of tradfri GW Sep 13, 2017

balloob approved these changes Sep 14, 2017

View reviewed changes

balloob merged commit 28d3128 into home-assistant:dev Sep 14, 2017

balloob mentioned this pull request Sep 22, 2017

0.54 #9532

Merged

home-assistant locked and limited conversation to collaborators Mar 3, 2018

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

full RGB support for users of tradfri GW#9411

full RGB support for users of tradfri GW#9411
balloob merged 2 commits into
home-assistant:devfrom
spektren:patch-1

spektren commented Sep 13, 2017 •

edited

Loading

Uh oh!

homeassistant commented Sep 13, 2017

Uh oh!

mention-bot commented Sep 13, 2017

Uh oh!

houndci-bot Sep 13, 2017

Uh oh!

houndci-bot Sep 13, 2017

Uh oh!

houndci-bot Sep 13, 2017

Uh oh!

houndci-bot Sep 13, 2017

Uh oh!

houndci-bot Sep 13, 2017

Uh oh!

houndci-bot Sep 13, 2017

Uh oh!

balloob commented Sep 14, 2017

Uh oh!

spektren commented Oct 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

spektren commented Sep 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

homeassistant commented Sep 13, 2017

Uh oh!

mention-bot commented Sep 13, 2017

Uh oh!

houndci-bot Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

houndci-bot Sep 13, 2017

Choose a reason for hiding this comment

Uh oh!

balloob commented Sep 14, 2017

Uh oh!

spektren commented Oct 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

spektren commented Sep 13, 2017 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):