Skip to content

Fix color setting of tplink lights#14108

Merged
balloob merged 1 commit intohome-assistant:devfrom
amelchio:tplink-hsv-fix
Apr 28, 2018
Merged

Fix color setting of tplink lights#14108
balloob merged 1 commit intohome-assistant:devfrom
amelchio:tplink-hsv-fix

Conversation

@amelchio
Copy link
Copy Markdown
Contributor

@amelchio amelchio commented Apr 27, 2018

Description:

This hopefully fixes TPLink lights after the hue/sat update. I noticed a few other issues as well.

I do not have hardware to test this so please review closely.

  • Cast hue/sat to integers like before the hue/sat conversion.
  • Remove ATTR_KELVIN as it should not reach platforms.
  • Allow setting of brightness and color in a single call.
  • Use current/provided brightness (and not a constant) for HSV.

CC @Arkadyf, @armills

Related issue (if applicable): fixes #13924, also reported in #11288 (comment)

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox.

@amelchio amelchio added this to the 0.68 milestone Apr 27, 2018
@amelchio amelchio requested a review from rytilahti as a code owner April 27, 2018 06:03
@balloob balloob removed this from the 0.68 milestone Apr 27, 2018
@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 27, 2018

I'm removing this from the milestone as the release is going out today and no one with hardware seems to have tested this.

@Arkadyf
Copy link
Copy Markdown

Arkadyf commented Apr 27, 2018

I was able to load this tplink.py and can confirm that it fixed my color issue.
Tested on LB230 lights, HA version 0.67.1.

  • I'm new to this, so in case it matters, to test this I created a a tplink.py file in the following location:
    /config/custom_components/light/
    Then I copy-pasted the contents of amelchio's edit. Restarted HA, and the lights started working with color.

@amelchio
Copy link
Copy Markdown
Contributor Author

@balloob That's fine, I agree with not adding untested code to the final release. As it is tested now and a fairly serious bug for those affected, are you okay with adding it to 0.68.1 even if the regression happened a few releases ago?

@Arkadyf Thanks for testing. That is a fine way to do it with minimal impact to the running system.

BTW, I found an existing issue for this bug: #13924

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 28, 2018

Yes, fine with 0.68.1

@balloob balloob added this to the 0.68.1 milestone Apr 28, 2018
@balloob balloob merged commit e6d4501 into home-assistant:dev Apr 28, 2018
balloob pushed a commit that referenced this pull request Apr 30, 2018
@balloob balloob mentioned this pull request Apr 30, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tplink LB130 Color LED Bulb

4 participants