plasticrake / homebridge-tplink-smarthome

TP-Link Smarthome Plugin for Homebridge
MIT License
467 stars 70 forks source link

No longer able to set dimmer brightness without triggering the light to turn on #333

Closed shugass closed 4 months ago

shugass commented 5 months ago

Is there an existing issue for this?

What happened?

Starting with version 7.1 it is not possible to set a dimmer brightness level without triggering the switch to turn on. In version 7.0.1 and previous, one could set the brightness level and the switch would not change state. This is useful for setting all your dimmers to say 5% at night, so that if you do turn it on you won't be blinded, but also ensuring that you don't turn on all your dimmer in your house when doing so. This can't be done with the stock Apple Home app, but can be done using "Controller for Home" app on iOS, where the brightness and power toggle are exposed as 2 seperate and independent functions.

What, if anything, has recently changed?

All versions after 7.0.1 have this issue. Version 7.1 introduced duplicate switches for all devices which limited functionality. Latest version of plugin fixed the duplicate switches issue, but the "dimming independant of power state" issue still remains.

Version

8.0.2

Node Version

20.10.0

Homebridge Version

4.55.1

Which OS/Platform?

Linux

Relevant log output

Log shows light turning on when brightness is changed as opposed to previously where a dimmer level change did not also trigger light to turn on.

Configuration

"name": "TPLink",
            "addCustomCharacteristics": false,
            "deviceTypes": [
                "plug",
                "bulb"
            ],
            "timeout": 20,
            "devicesUseDiscoveryPort": false,
            "_bridge": {
                "username": "XX:XX:XX:XX:XX:XX",
                "port": XXXXX
            },
            "platform": "TplinkSmarthome"
aj-may commented 5 months ago

https://github.com/plasticrake/homebridge-tplink-smarthome/commit/d267e40ee0ab11f6182269dea16d5a0a2b328063

Here is the change that introduced this bug. I heavily use the ability to adjust my dimmer brightness throughout the day without the lights turning on.

@plasticrake happy to open a PR, but wanted to check what the reason was for the change in the first place.

aj-may commented 5 months ago

Patched it locally and it is working as expected for me.

plasticrake commented 5 months ago

Thanks @aj-may, but If I merge this then this person's devices (#306) will start throwing errors (maybe yours too?).

Unfortunately I've noticed that different versions of tp-link devices, and even different versions of firmware on the same models or from different countries, will behave differently and its hard to have the same code work for everything. On the device from #306 if you do not use dimmer.set_dimmer_transition then it throws an error when setting brightness to 0. The code was originally calling dimmer.set_brightness, but I had to switch to dimmer.set_dimmer_transition (which I believe is what the kasa app calls when changing brightness).

I downloaded "Controller for HomeKit" and I tested setting the brightness separately on my Hue bulbs and it did not work either (nothing happens, it doesn't turn on but the brightness setting just went back to the old value). So I feel this is a niche use case especially since you need a 3rd party app and even some official HomeKit devices do not support it.

But if we can get this working I'm all for it!

I do not have a switch to experiment with, but can you tell me if when brightness is set to 0 you get this error (in the older code)? Perhaps we just need to do something different when brightness is 0. I'm guessing maybe the device doesn't support set_brightness to 0 when its on, but using set_dimmer_transition works (which also applies the brightness immediately).

From #306:

[4/23/2023, 5:34:19 PM] [TplinkSmarthome] [Main Hall Way] Setting Brightness to: 1
[4/23/2023, 5:34:19 PM] [TplinkSmarthome] [Main Hall Way] Setting Brightness to: 0
[4/23/2023, 5:34:19 PM] [homebridge-tplink-smarthome] This plugin threw an error from the characteristic 'Brightness': Unhandled error thrown inside write handler for characteristic: err_code not zero response: {"err_code":-3,"err_msg":"invalid argument"} command: {"smartlife.iot.dimmer":{"set_brightness":{"brightness":0}}}. See https://homebridge.io/w/JtMGR for more info.
aj-may commented 5 months ago

Actually yes, I do have the HS230 in use as well. I am able to reproduce. For me at least it still works as expected even though it is throwing the errors.

Based on the logs, I believe this is because the home app sends first a command to set the brightness to 0, which is invalid, but then also sends a command to set the power state to false.

image

Given this I can see potentially 2 workable solutions:

  1. Just add a bounds check to ensure the value is greater than 0 before calling setBrightness. This will avoid the invalid call, but at least in my case the light will still be turned off based on the second command to set the power state.
  2. In the case that the value is 0 we can instead call the setPowerState command and turn off the device. This Might be safer if we think that in some instances only the setBrightness command will be called and not the setPowerState like I am seeing in my logs. In my instance there would just then be 2 calls to set the powerState to false.

IMO it feels cleaner to go with option 1, but I don't have other versions of home bridge or the home app to test with.

aj-may commented 5 months ago

Gave it a bit more thought and looked over the logs shared in #306 and decided that option 2 is likely the better choice.

I updated the PR with those changes, and tested them on my own instance of homebrige with the HS230 and it is working perfectly with no errors.