Openhab-addons: [yeelight] Add support for new devices

Created on 15 Jun 2018  Â·  38Comments  Â·  Source: openhab/openhab-addons

This is an issue for further tracking the integration of new devices as discussed in https://github.com/openhab/openhab2-addons/pull/3582#issuecomment-397418891

Model names:

  • ceiling1
  • ct_bulb
discussion enhancement

All 38 comments

@daw87 and others owning this devices: One thing you could check is whether adding the devices manually using ceiling and dolphin as model instead. If that works than just the name has changed and the change can be integrated pretty easy.

@claell tried that already, did not work.
Yesterday, I had a closer look at the source code. From what I understand I not only have to modify the binding but also the yeelight lib. Because based on the SSDP response (model field) the lib decides what actions are possible, right? So I guess I need to add the model types to the lib, change the handlers and after that I change the binding constants. In my opinion the lib is not very flexible when it comes to adding new devices....

Yes, that is also what I think now after having a closer look at the code.

In my opinion the lib is not very flexible when it comes to adding new devices....
What do you mean with that? That it requires rebuilding the lib every time? Or is it hard to integrate new devices into the existing code?

There has also been a discussion about including the lib directly into the binding instead of a jar file. Since this will probably require some testing until the lib works as expected and rebuilding it every time requires extra effort, I will probably do that now.

Yes, you always have to change the lib, rebuild it and additionally change the binding code. The lib code hasn't been changed for 2 years and I don't think that anyone is still working on it...
I was thinking about forking the lib and making the changes, also adding support for IP based configuration instead of device id based. But that would take me quite some time.
Since at least I only need it for OpenHab anyway, we also could include the lib code into the binding and go from there.

Besides, my new LED Bulb V2 (white) does not respond to SSDP requests. I see the advertisement message after enabling the developer mode but nothing more. So if the behaviour was changed intentionally, we cannot rely on the lib code to connect new devices any more.

Ok. I already started to include the lib into the code directly. Will create a PR soon from which we can start working as it seems you are willing to contribute as well?

Besides, my new LED Bulb V2 (white) does not respond to SSDP requests. I see the advertisement message after enabling the developer mode but nothing more. So if the behaviour was changed intentionally, we cannot rely on the lib code to connect new devices any more.

So that means the old lib will definitely not work by just changing the models there while leaving the rest untouched? Do you think that means no communication is possible at all using the local network?

My LED Bulb v2 (color) works with the binding, so they did not change anything there on communication side.

as it seems you are willing to contribute as well?

Yes, but as I said reading code is easier for my than writing it. But I will try.

So that means the old lib will definitely not work by just changing the models there while leaving the rest untouched?

If I don't just have a faulty bulb, then yes. But it was only an observation I made. I will reset the bulb and check again.

Alright. I just pushed a new branch to my repo and created a PR: #3673

I will give you write access to that repo, so you can directly push changes there.
Edit: Done.

If I don't just have a faulty bulb, then yes. But it was only an observation I made. I will reset the bulb and check again.

Alright, fingers crossed.

Changed the lib and the binding now, but I don't know whether these changes are enough or whether some extra work is required. The latest builds are available from https://openhab.jfrog.io/openhab/libs-pullrequest-local/org/openhab/binding/org.openhab.binding.yeelight/2.4.0-SNAPSHOT/ so maybe you can give it a try @daw87
Also log output (set to debug) might help if the build does not work.

@claell great news. first of all, your changes worked like a charm and I was able to discover my ceiling light.
Furthermore, after resetting my white bulb the SSDP communication started working and OpenHab was able to discover it too. The only option missing at the moment is the light temperature control that is supported by version 2 of the white bulb. So from a device perspective it should be treated like the ceiling lamp.

The only "bug" I found so far is that it takes quite some time until OH recognizes a lamp as "disconnected/offline" (my ceiling lamp is still triggered with an old light switch). So when turning off the ceiling lamp OH still shows it as "online" and I can change settings. The changes result in an connection error after a timeout (see debug log).
But that's really more an inconvienience than a bug. A keepalive request could solve that.
I believe you already mentioned this in the other issue #3666

2018-06-16 21:39:54.180 [DEBUG] [yeelight.handler.YeelightHandlerBase] - Handle Ceiling Command OFF
2018-06-16 21:39:54.188 [DEBUG] [lib.device.connection.WifiConnection] - WifiConnection: Write Success!
2018-06-16 21:39:55.603 [DEBUG] [yeelight.handler.YeelightHandlerBase] - Handle Ceiling Command ON
2018-06-16 21:39:55.617 [DEBUG] [lib.device.connection.WifiConnection] - WifiConnection: Write Success!
2018-06-16 21:40:36.973 [DEBUG] [lib.device.connection.WifiConnection] - Exception: {}
java.net.SocketException: Connection reset
        at java.net.SocketInputStream.read(SocketInputStream.java:210) [?:?]
        at java.net.SocketInputStream.read(SocketInputStream.java:141) [?:?]
        at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284) [?:?]
        at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326) [?:?]
        at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178) [?:?]
        at java.io.InputStreamReader.read(InputStreamReader.java:184) [?:?]
        at java.io.BufferedReader.fill(BufferedReader.java:161) [?:?]
        at java.io.BufferedReader.readLine(BufferedReader.java:324) [?:?]
        at java.io.BufferedReader.readLine(BufferedReader.java:389) [?:?]
        at org.openhab.binding.yeelight.lib.device.connection.WifiConnection$1.run(WifiConnection.java:84) [212:org.openhab.binding.yeelight:2.4.0.201806161908]
        at java.lang.Thread.run(Thread.java:748) [?:?]
2018-06-16 21:40:36.981 [DEBUG] [nding.yeelight.lib.device.DeviceBase] - DeviceBase: set connection state to: DISCONNECTED
2018-06-16 21:40:36.984 [DEBUG] [nding.yeelight.lib.device.DeviceBase] - DeviceBase: CheckAutoConnect: online: false, autoConnect: true, connection state: CONNECTED, device = org.openhab.binding.yeelight.lib.device.CeilingDevice@170e3e6, device id: 0x0000000004a0af6b
2018-06-16 21:40:36.987 [DEBUG] [yeelight.handler.YeelightHandlerBase] - onConnectionStateChanged -> DISCONNECTED
2018-06-16 21:40:36.996 [DEBUG] [yeelight.handler.YeelightHandlerBase] - Thing OFFLINE. Initiated discovery
2018-06-16 21:40:37.009 [DEBUG] [ng.yeelight.lib.device.DeviceFactory] - DeviceManager: send discovery message!
2018-06-16 21:41:22.798 [DEBUG] [yeelight.handler.YeelightHandlerBase] - Handle Ceiling Command OFF
2018-06-16 21:41:22.807 [DEBUG] [lib.device.connection.WifiConnection] - WifiConnection: write exception, set device to disconnected!

Great news, indeed. Did not expect that it would be that easy :slightly_smiling_face:

I will try to add the temperature control to the white bulb when I have time, maybe best thing is to add a new device class for it.

For the bug you found: I am already aware of it, it is tracked in #3666 (edit: just saw your edit, where you mention that issue already). I think this has to be changed in the library and until now the lib was hard to modify. If you know how to change that feel free to give it a try (same for the bulb temperature).

Added a commit that will hopefully bring full support for the white bulb v2. It is available at the same link I posted above when the build has finished. I assume that you currently used the device with mono set as model, right? That now changes, so the model should be set to ct_bulb in order to make the ct channel available.

@miklosandras Just saw that you are already aware of this issue. Did you try the linked binding version from above? If your device model is ceiling1 it should work for you.

Also TODO:

  • [x] Add log output with model for currently unsupported devices

    • Discovered devices are already logged (in debug level), also with model info, so there is no need to add further logging

@claell Yes, I am using the latest version from https://openhab.jfrog.io/openhab/libs-pullrequest-local/org/openhab/binding/org.openhab.binding.yeelight/2.4.0-SNAPSHOT/ Just updated again, and unfortunately this is not find my Yeelight LED Ceiling Lamps. One off my lamp details:

hardwareVersion | MTK7697
modelId | yeelink.light.ceiling1
wifiFirmware | 4.3.0

Hm, @daw87 had success with the same model. Maybe problems got introduced with the following commits. You can try this build: https://openhab.jfrog.io/openhab/webapp/#/builds/PR-openHAB2-Addons/10160/1529176156363/published/ which should be the one that @daw87 confirmed working for him.

@claell I can not download the jar file from the above link. May the problem is that, I am using openhab 2.3 instead of 2.4 (as this is not available for Synology yet).

Sorry, did not check whether it is available. So then I will need to build it again with the old settings for testing.

It should be no problem that you are using 2.3. This should work afaik.

org.openhab.binding.yeelight-2.4.0-SNAPSHOT.zip

Just rebuilt it. Unfortunately Github did not support upload of the .jar file, so I changed the extension to .zip. You have to rename it again with .jar extension (on Windows you have to enable the displaying of extensions in the Explorer for that).

@claell Great work, now my Yeelight LED Ceiling Lamps are discovered successful. And both of my lamp working fine with this version. Only ColorTemperature what is not working quite well :) Thank you.

So there apparently got problems introduced with the commits in between. I will check that. For the not properly working CT can you describe the problem in more detail? One thing I could think of is that your lamp model has a different range of min and max CT.

Hi Guys

Just wanted to check, I have downloaded the above 2.4.0 snapshot version but when I discover my Yeelight LED Stripe it only show me Color & ColorTemperature Channels. No Brightness Channel, I have tried to add it manually but I am unable to control the Brightness or on/off?

@menzy81 This is wanted (changed from previous binding version). The color channel already supports brightness, so you can use the brightness slider from color instead of the old extra brightness slider. This is the standard usage for all openHAB light items.

org.openhab.binding.yeelight-2.4.0-SNAPSHOT.zip
@miklosandras I checked and did not find any part of the commits after the working version that could have introduced a problem. So I built again with the latest version, maybe you can give it another try to see whether it works now.

No problems I have manually configured using the color channel & all is working. Is it possible to have On/Off & Brightness populate to those channels in the PaperUI automatically?

From: Claudius Ellsel notifications@github.com
Sent: Thursday, 21 June 2018 10:52 PM
To: openhab/openhab2-addons openhab2-addons@noreply.github.com
Cc: Adam Menzel Adam@almenzel.com; Mention mention@noreply.github.com
Subject: Re: [openhab/openhab2-addons] [yeelight] Add support for new devices (#3665)

@menzy81https://github.com/menzy81 This is wanted (changed from previous binding version). The color channel already supports brightness, so you can use the brightness slider from color instead of the old extra brightness slider. This is the standard usage for all openHAB light items.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/openhab/openhab2-addons/issues/3665#issuecomment-399101376, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AkizPxazaDy0b-I-v_0v4DdQ0AyO2YNiks5t-53qgaJpZM4UpbTG.

I don't understand the question. A color item can be connected to the color channel. This item supports setting the color, saturation and brightness. If you only change the brightness of the color item (while having the bulb set to white) only the brigtness of the bulb will get changed.

Maybe you can elaborate what you expect/what the old binding did in more detail.

Thankyou for the explanation, I have got it working. The only issue I have found is that the brightness slider always returns to 100% after making a change. It does set the brightness correctly using the slider but the slider on my sitemap always shows 100%.

-------- Original message --------
From: Claudius Ellsel notifications@github.com
Date: 21/6/18 10:51 pm (GMT+09:30)
To: openhab/openhab2-addons openhab2-addons@noreply.github.com
Cc: Adam Menzel Adam@almenzel.com, Mention mention@noreply.github.com
Subject: Re: [openhab/openhab2-addons] [yeelight] Add support for new devices (#3665)

@menzy81https://github.com/menzy81 This is wanted (changed from previous binding version). The color channel already supports brightness, so you can use the brightness slider from color instead of the old extra brightness slider. This is the standard usage for all openHAB light items.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/openhab/openhab2-addons/issues/3665#issuecomment-399101376, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AkizPxazaDy0b-I-v_0v4DdQ0AyO2YNiks5t-53qgaJpZM4UpbTG.

Thanks for bringing this up. For me the slider works, although I only tested it with the one on PaperUI, not on a sitemap. Is it also happening on PaperUI for you (probably yes)?

Since I don't own a stripe device, I cannot test it, but I assume there is a problem handling this device type properly.

Yes it also does this in the PaperUI. Slider does work in the PaperUI & on the Sitemap but it always returns to 100% once the adjustment to the desired brightness is completed.

Further to the Slider always returning to 100%, I have rules that automatically dim my Stripe's over the evening which are working but the sitemap & paperui do not reflect the new percentage after the rule fires.

After further testing I can report my rules fire correctly but do not adjust the brightness of the LED Stripe. If I unplug the LED Stripe from power my rules fire & show correctly on the sitemap & paper UI.

Thanks for the feedback. So regarding the stripe you basically report two problems:

  • Changing brightness does not work from rules
  • Sliders don't reflect the state of the thing, but always jump back to 100%

Changing brightness manually works for you?

That's correct, Manually changing the brightness on the Sitemap or in PaperUI sometimes works but usually takes 2 or 3 slides to adjust and then goes back to 100% on the slider while keeping the brightness level.

So it is bugged there as well apparently. Have you tested it with the older binding version from @coasterli?

Unfortunately I don't have much time currently for investigating into this and also lack a device to test. So if you have some experience in coding giving this a try yourself will probably be the fastest way to get it fixed.

@claell I got issue with celling on FW 1.5.90184,
discovery is not showing Celling, also not discover by binding.miio
there is something wired as I still can control celling by python yeecli, but by cli miio, discover show as below

Device ID: 80605122
Model info: Unknown
Address: 192.168.x.x
Token: ???
Support: Unknown

and inspect request token as below
miio inspect 80605122
INFO Attempting to inspect 80605122
ERROR Could inspect device. Error was: Could not connect to device, token needs to be specified

I have openhab 2.4 and it wont discover my ceiling light.
The plugin was normally installed via paper ui. I did not compile or download any special version.

This is the dump from the notify.

NOTIFY * HTTP/1.1
Host: 239.255.255.250:1982
Cache-Control: max-age=3600
Location: yeelight://192.168.178.39:55443
NTS: ssdp:alive
Server: POSIX, UPnP/1.0 YGLC/1
id: 0x000000000xxxxxxx
model: ceiling3
fw_ver: 40
support: get_prop set_default set_power toggle set_bright set_scene cron_add cron_get cron_del start_cf stop_
cf set_ct_abx set_name set_adjust adjust_bright adjust_ct
power: off
bright: 5
color_mode: 2
ct: 4000
rgb: 0
hue: 0
sat: 0
name:

Is model ceiling3 not supported?

daw87
In my opinion the lib is not very flexible when it comes to adding new devices....

I can only recon that. This yeelight lib uses the wrong design imho. It's not the 'model' that defines features but the 'support' string (maybe other items from response).

I have ceiling1, ct_bulb, color and iirc ceiling3 devices and none were detected and manually adding them didn't work either due to fixed device types in binding/lib.
(color wasn't detected due to invalid enum exception so scanning was aborted)

This legacy binding uses a way better design as it doesn't care what model a lamp is.
https://github.com/octa22/org.openhab.binding.yeelight

@hamwong Sorry for my late reply. I fear I don't understand your problem properly. Unfortunately I also don't have much time for adding further devices.

@bugficks The linked legacy binding looks to be a rather pure approach. While it might be true that the current binding is overloaded it also seems to be easier to use (when it works). Or am I wrong here?

So if you have some experience and know what can be done better (like the current lib code) or the treatment of new device models, it would be cool, if you can do a PR or make a proposal on how to change the current design.

@claell
What I mean is, don't let DeviceType enum define features but use 'support' from response and 'get_prop' requests to determine what a device can do.
This way you even could add generic/unkown DeviceType.

e.g.

Yeelight.has_brightness()
Yeelight.has_color()
Yeelight.has_color_temp()
Yeelight.has_night_mode()
Yeelight.has_background()

etc.

@bugficks Ah, I see. This would probably be easier, indeed. Unfortunately I am not very experienced with the library and don't have time for implementation an testing.

Are you interested in giving this a try, maybe there can also be some combination of this effort and the one from @octa22 so the work does not have to be done twice.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

doandzhi picture doandzhi  Â·  6Comments

martinvw picture martinvw  Â·  5Comments

openPhiL picture openPhiL  Â·  5Comments

trailblazer2006 picture trailblazer2006  Â·  6Comments

IOOOTAlan picture IOOOTAlan  Â·  3Comments