openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.57k forks source link

[yeelight] Yeelight Color Bulb Gen 1 - Switch and Brigthness needed #4607

Closed Selters closed 5 years ago

Selters commented 5 years ago

Expected Behavior

How can one switch this bulb ("wonder") on/off and adjust the brightness?

Current Behavior

There is no switch to switch the bulb on/off

Possible Solution

With the color picker item it is possible to switch the bulb. “Arrow up” turns on the light and “arrow down” turns it off.

Steps to Reproduce (for Bugs)

When switching off the “wonder” bulb via yeelight app I got the following output: [INFO ] [light.internal.lib.device.DeviceBase] - status = DeviceStatus [isPowerOff=true, r=255, g=0, b=0, color=16711680, brightness=100, ct=3908, hue=359, sat=100, isFlowing=false, delayOff=-1, mFlowItems=null, mode=MODE_SUNHINE, isMusicOn=false, name=]

Context

This workaround doesn't work

Your Environment

claell commented 5 years ago

The "workaround" you linked is actually how it is meant to be used. To me it seems to be rather that switching on and off (plus brightness -> see below) does not work for you.

Just for clarification, brightness is also not working for you (using the slider from the color picker, maybe with saturation and hue set to not zero)?

Selters commented 5 years ago

According to the doc, there are only color and colorTemperature channels for wonder bulb.

image 107

So, brightness is missing - which - at least - is needed to switch the bulb on and off. But an extra switch/power channel would be great too ...

It seems that the Xiaomi MIIO Binding provides several channels. But this binding requires the token which isn't easy to extract.

claell commented 5 years ago

So, brightness is missing

I am rather sure, I already wrote you in the Forum that the channels are intentional. You should be able to change brightness with the brightness slider from the color channel. No extra brightness channel is needed for that.

which - at least - is needed to switch the bulb on and off.

This is wrong. You can do it with the color channel. If not there is a bug.

But an extra switch/power channel would be great too ...

Why? It might be a bit easier to use when not knowing that the same can already be done with the color channel. That is true. But in that case I rather vote for improving the documentation instead of "randomly" introducing channels that add redundant code, which is harder to maintain.

As for brightness you can also assign a switch item to the color channel. If that does not turn your bulb on/off there is a bug.

sverzijl commented 5 years ago

Why? It might be a bit easier to use when not knowing that the same can already be done with the color channel.

I've come across this issue too.

The problem is that other apps (like the official one or Google Home) do not use brightness to turn off the light. The issue then is that there is no way to tell if the light is on or off when it's been controlled elsewhere.

claell commented 5 years ago

Hm, that might be a point. I am wondering though, whether this can be still achieved with the current configuration. Maybe the binding has to be changed to make it working with the color channel.

At least the LIFX binding does it like that. So either it has the same problem meaning the concept of only having color and color temperature channel has to be overthought or it handles it in a different way that can then be implemented for the Yeelight binding as well.

sverzijl commented 5 years ago

So when Google Home, for example, turns off the light the binding returns this: 2019-01-29 22:15:25.797 [INFO ] [light.internal.lib.device.DeviceBase] - status = DeviceStatus [isPowerOff=true, r=255, g=127, b=0, color=16744192, brightness=50, ct=4000, hue=359, sat=100, isFlowing=false, delayOff=-1, mFlowItems=null, mode=MODE_COLOR, isMusicOn=false, name=]

isPowerOff=true

The other positive side-effect is when Google Home turns back on the device all the colours and brightness return to their values prior to the light turning off - something that I don't believe can be done in the current set up.

I spent the last hour reading the source code however my knowledge of Java goes as far as its similarities to Python and I'm not sure how everything connects with openhab itself (any information on that last point would be great). I can see that the binding is aware of the status update (at least to produce that log entry). Is it just a matter of making another channel that supports the 'ispoweroff' flag which produces an OnOff result?

claell commented 5 years ago

Ok, so at least the binding gets the information.

The question is, whether another channel is really needed or whether there can simply be a switch item linked to the color channel that can then be turned off. At least for turning on and off from inside openHAB that works and afaik it also remembers brightness and color settings, so that might also work in the reverse direction.

If you want to learn more about how bindings work, I think a good starting point is the openHAB documentation pages where they have a section about developing bindings.

sverzijl commented 5 years ago

Can you use only the colour channel but have it register the 'ispoweroff' flag?

My issue is that I have no way to indicate that the light is off if it has been turn off by something like Google Home.

On Wed, 30 Jan. 2019, 12:34 am Claudius Ellsel <notifications@github.com wrote:

Ok, so at least the binding gets the information.

The question is, whether another channel is really needed or whether there can simply be a switch item linked to the color channel that can then be turned off. At least for turning on and off from inside openHAB that works and afaik it also remembers brightness and color settings, so that might also work in the reverse direction.

If you want to learn more about how bindings work, I think a good starting point is the openHAB documentation pages where they have a section about developing bindings.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openhab/openhab2-addons/issues/4607#issuecomment-458541236, or mute the thread https://github.com/notifications/unsubscribe-auth/AULNSBvIs8PuvsEprFieP_mshRkGAOQUks5vIE3vgaJpZM4aBxJr .

claell commented 5 years ago

At least you can link a switch item to the channel that turns the light on and off. But I don't know whether that switch can also then be used to indicate the bulb status (on or off). That should however be answered first before creating an additional channel.

sverzijl commented 5 years ago

Okay, so after a bit more reading of the source code - it looks like the intention is for 'ispoweroff' status to change brightness to 0:

In YeelightHandlerBase.java void updateBrightnessAndColorUI(DeviceStatus status) { if (status.isPowerOff()) { updateState(YeelightBindingConstants.CHANNEL_BRIGHTNESS, new PercentType(0));

And when I look in DeviceBase.java and at onNotify there is code where the status is updated: if (prop.getKey().equals("power")) { updateProp += " power"; if (prop.getValue().toString().equals("\"off\"")) { mDeviceStatus.setPowerOff(true); } else if (prop.getValue().toString().equals("\"on\"")) { mDeviceStatus.setPowerOff(false); }

I've noticed a 'listener' class (DeviceStatusChangeListener.java) which I assume is meant to tell classes that contain devices, like YeelightHandlerBase, that a device instance state has changed. public void onStatusChanged(String prop, DeviceStatus status) { logger.debug("UpdateState->{}", status); updateUI(status); }

However it obviously isn't being called despite it being mentioned in DeviceBase: if (needNotify) { logger.info("status = {}", mDeviceStatus.toString()); for (DeviceStatusChangeListener statusChangeListener : mStatusChangeListeners) { statusChangeListener.onStatusChanged(updateProp.trim(), mDeviceStatus); } }

Maybe something wrong with the 'needNotify'?

I don't have the tools to probe this further - is this something that can be looked at? Looks like it is true that you don't need a separate channel. Now that the need for a separate channel may be the result of a bug should I raise this as something separate?

claell commented 5 years ago

Thanks for taking the time to dig in the source code. Interesting to hear that it seemed to be intended to forward the status, but it just is not done properly. That might indeed be some bug (I think these parts you quoted are from a former external library which got now integrated as code directly into the binding. This library is not that nicely written and might contain some bugs).

Feel free to post a separate issue for that to further track this!

claell commented 5 years ago

Just for the record, the new issue for that: #4767

goodfore commented 5 years ago

@Selters and @claell can we close this issue? I submitted a PR to close #4767

claell commented 5 years ago

This resulted from https://community.openhab.org/t/yeelight-binding/13735/233?u=clel which was a different issue. @sverzijl only commented with his issue which was not related, so he created another one for it.

However, I think I found the problem of @Selters now. If I am right it has nothing to do with the binding, but he did not create the correct items.

He tried Switch Color_Toggle (Gyeelight,Glicht) { channel="yeelight:wonder:0x000000000xxxxx:colorTemperature" } which obviously did not work, since it was the colorTemperature, not the color channel. I did not notice that back then. However I will wait for @Selters to confirm that.

davidgraeff commented 5 years ago

Please reopen if this is still an Issue, but from the sound of it, it is solved by now.