Skip to content
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.

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

tplink bulb consecutively dims brightness at each color change #15582

Closed
tempse opened this issue Jul 20, 2018 · 4 comments
Closed

tplink bulb consecutively dims brightness at each color change #15582

tempse opened this issue Jul 20, 2018 · 4 comments
Assignees

Comments

@tempse
Copy link

tempse commented Jul 20, 2018

Home Assistant release with the issue:

0.74.0

Last working Home Assistant release (if known):
unknown

Operating environment (Hass.io/Docker/Windows/etc.):

Raspbian GNU/Linux 9 (stretch), installation of homeassistant via pip (Python 3.5.3)

Component/platform:

issue with the tplink light component: https://www.home-assistant.io/components/light.tplink/

Description of problem:
Tested device: TP-Link Bulb TP130.
Every time the color of the bulb is changed from the Home Assistant Interface, the bulb brightness is reduced (in addition to the successful change of color). For example, starting from 100% brightness, switching to a different color results in the brightness being reduced to 39%. Changing color again, yields 15% brightness, etc... (see my remarks about these specific numbers at the end of this issue).

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

light:
  - platform: tplink
    name: SmartBulb
    host: !secret smart_bulb_ip

Traceback (if applicable):


Additional information:
My best guess for the cause of this problem is the following: The brightness can be given in percent or values between 1 and 255. Internally, only one of those two options (i.e, percent values) is used, if I read the source code correctly. Now, if at some point, one assumes the brightness to be in the wrong format and does the conversion, the brightness will be updated to wrong values. In the case of the brightness actually being in percent, but assumed as values between 1 and 255, this would result in brightness reductions to values of 100/255=39%, 39/255=15%, etc... - which is exactly the steps in brightness reduction I observe.
Is it possible that the following lines cause this problem by assuming the brightness values to be in absolute 1..255 values (whereas they actually are in percent already)? https://github.com/home-assistant/home-assistant/blob/7bc2362e33f86d41769d753d1701a53ae60224d5/homeassistant/components/light/tplink.py#L96-L97
Thanks, and keep up the excellent work!

@amelchio
Copy link
Contributor

That line is correct, ATTR_BRIGHTNESS is indeed 0..255.

This seems like a bug in the third party library. The HSV setter is documented to accept a percentage but actually converts from byte.

I don't know if this should be fixed in pyHS100 or Home Assistant. Probably @rytilahti can say more.

@rytilahti
Copy link
Member

Unfortunately I don't really know much about those lights, nor do I have a test device. @tempse is the pyhs100 cli tool and it's hsv command working as expected for you?

tempse pushed a commit to tempse/pyHS100 that referenced this issue Jul 22, 2018
The HSV setter should accept a percentage for the brightness
value but actually assumed the brightness to be in absolute values
between 1 and 255.
This resulted in brightness reductions at each HSV update, in
steps of 100% -> 100/255=39% -> 39/255=15% -> ... (see also
home-assistant/core#15582,
where I originally reported this bug).
@tempse
Copy link
Author

tempse commented Jul 22, 2018

Thanks for your prompt answers, @amelchio and @rytilahti!

The strange behavior I originally observed in Home Assistant is indeed coming from the pyHS100 tool, where I can reproduce everything I reported above.

As you can see, I created a PR in the pyHS100 repository which should fix this bug. I will keep you posted regarding any updates on this matter.

rytilahti pushed a commit to GadgetReactor/pyHS100 that referenced this issue Sep 5, 2018
* Fix bug that changed brightness at each hsv update

The HSV setter should accept a percentage for the brightness
value but actually assumed the brightness to be in absolute values
between 1 and 255.
This resulted in brightness reductions at each HSV update, in
steps of 100% -> 100/255=39% -> 39/255=15% -> ... (see also
home-assistant/core#15582,
where I originally reported this bug).

* Modify HSV property to return brightness in percent

Switch from reported brightness values of 1..255 to percentage
values, for consistency with the apidoc and 8761dd8.

* Add checks and tests for the hsv setter

- make sure that new (hue, saturation, brightness) values are
  within their valid ranges (0..255, 0..100, 0..100) and raise
  SmartDeviceException if they are not
- add test function for the hsv setter
@rytilahti
Copy link
Member

I just released 0.3.3: https://github.com/GadgetReactor/pyHS100/releases/tag/0.3.3

@amelchio amelchio mentioned this issue Sep 8, 2018
6 tasks
@ghost ghost assigned amelchio Sep 8, 2018
@ghost ghost added the in progress label Sep 8, 2018
@ghost ghost removed the in progress label Sep 8, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants