[govee] Fix brightness vs. color synchronization

[andrewfg]

Resolves #17807

Also includes the following..

• Fixed: synchronization of brightness vs color channels (original issue)
• Fixed: communication issue when using textual host names rather than IP addresses
• New: faster update of channels when commanded
• New: support for many more light models
• New: support for configurable min/max color temperature ranges of lamps
• New: added dynamic state description provide to enhance color temperature UI experience
• Enhanced: read me enhanced to include all of the above

Signed-off-by: AndrewFG [email protected]

[andrewfg]

@felixschndr you can download the jar for test purposes below. Uninstall the standard binding, then unzip the file below and drop it in your /addons folder.

org.openhab.binding.govee-4.3.0-SNAPSHOT.jar.zip

[felixschndr]

Can we add the H60A1 to govee.properties? I guess that's why I get the error message thing Handler for 192.168.4.237 couldn't be found.

[felixschndr]

I also noticed something about color temp: The color temp abs works fine, I defined a min and max in the sitemap so I can only send values that are supported by the lamp.
However, the color temp relative only works partially. When sending high values, the lamp goes off because we are sending an abs color temp that is too high.
I don't know if there's anything we can do about this, as the bind probably doesn't know the min and max values for the color temp. In my case they are 2200 <= x <= 6500, but I don't know if they are the same for all Govee lamps.

[andrewfg]

Two things..

1. re the HSB update after brightness command: probably it is only updated on the next polling cycle (?) .. but I will see if I can fix this..

2. re the color temperature range: the binding constraints the limits to 2000 .. 9000 K so if the lamp is more constrained then odd results are expected .. but let me have another look at this..

[andrewfg]

PS you also mentioned that you have a lamp which is not (yet) supported .. can you provide more info about it .. and I will see if I can fix that too..

[andrewfg]

Is the H60A1 the one on 192.168.4.237 ?? You say it works fine. But you also say it reports an error..

[andrewfg]

problem is in both directions

Yeah, it would be..

[felixschndr]

Is the H60A1 the one on 192.168.4.237 ?

Yes it is.

But you also say it reports an error..

You mean the thing Handler for 192.168.4.237 couldn't be found.? I figured this was since the device type is not defined in the govee.properties, isn't it? The communication works flawlessly

[andrewfg]

I will add your H60A1 to the properties file. This simply allows to display a clear text label in the thing properties. However this DOES not solve the "thing Handler .. couldn't be found" error. If you say that 1) it works, and 2) it shows the error message, then this probably means that you have two duplicate things defined on the same IP address, and one is working, and the other is giving the error. Or something like that.

[felixschndr]

If you say that 1) it works, and 2) it shows the error message, then this probably means that you have two duplicate things defined on the same IP address, and one is working, and the other is giving the error. Or something like that.

That's not it. I double checked the MainUI and the config files. I only got this one Govee thing. Also, if I comment out the thing I got, the message disappears.

Edit: Weird… After removing the comment (adding the thing again) the message does not come up again. After restarting the binding the message appears again

[felixschndr]

Can you please create a jar from you last commit for me to test?

[felixschndr]

One thing that could be useful: A string channel which states whether the lamp is in color temp mode or in color color mode. I think that could become pretty useful in some rules. Would it be possible to implement that? Thank you very much for your work!!

[felixschndr]

I noticed that the color temp must be a Number:Temperature item. If it isn't the channel only sends e.g. 6500 (with no K) which leads to no change:

2024-11-27 22:05:27.456 [INFO ] [openhab.event.ItemStateChangedEvent                                             ] - Item 'Licht_Arbeitszimmer_Deckenlampe_Farbtemperatur' changed from 6000.0 to 4900
2024-11-27 22:05:27.463 [DEBUG] [org.openhab.binding.govee.internal.GoveeHandler                                 ] - handleCommand(govee:govee-light:Licht_Arbeitszimmer_Deckenlampe:color-temperature-abs, 4900)
# nothing

This is with the :Temperature attribute:

2024-11-27 22:06:04.506 [INFO ] [openhab.event.ItemCommandEvent                                                  ] - Item 'Licht_Arbeitszimmer_Deckenlampe_Farbtemperatur' received command 3600 K
2024-11-27 22:06:04.511 [INFO ] [openhab.event.ItemStateChangedEvent                                             ] - Item 'Licht_Arbeitszimmer_Deckenlampe_Farbtemperatur' changed from 4900 °C to 3326.85 °C
2024-11-27 22:06:04.517 [DEBUG] [org.openhab.binding.govee.internal.GoveeHandler                                 ] - handleCommand(govee:govee-light:Licht_Arbeitszimmer_Deckenlampe:color-temperature-abs, 3600 K)
2024-11-27 22:06:04.518 [DEBUG] [org.openhab.binding.govee.internal.GoveeHandler                                 ] - sendKelvin(3600)

Can we change this such that a Number without the :Temperature works as well?

[andrewfg]

Edit: Weird… After removing the comment (adding the thing again) the message does not come up again. After restarting the binding the message appears again

Could you provide a trace log around that sequence of events?

[andrewfg]

Can we change this such that a Number without the :Temperature works as well?

I would like to focus on fixing the bug(s) first, before we consider extending the features.

[andrewfg]

A .. channel which states whether the lamp is in color temp mode or in color color mode

Not really. If you look at the CIE color space you will see that color temperatures are points which lie on the Planckian locus within the color space. And when you command a Govee lamp to a particular CT it causes the lamp to select an RGB value (i.e. color XY value) for that point. Therefore when you send a new CT, both the CT channel, and ALSO the Color HSB channel will be updated to match. Or to say it in other words, there is no such thing as Color resp. Color Temperature mode.

---

EDIT: however the color temperature channels will be UNDEF if the color point is NOT on the Planckian locus. So that would perhaps be the indicator that you are looking for.

[andrewfg]

Can you please create a jar from you last commit for me to test?

The Jar in the post above has now been updated to the latest commit.

[andrewfg]

Apropos the "handler couldn't be found" .. when a lamp sends an update, the code takes one of two possible forks -- namely if the handler can be found it processes the update, and if it can't be found it outputs the error message. However you are saying that it does both, which is a kind of Schroedinger cat condition...

[felixschndr]

FWIW there is a Black Friday deal on Amazon for a pair of Govee bulbs for £ 9.99 the pair (£ 4.50 each) and it is astonishing that such a device can run a wifi radio, an Ethernet stack, and a real time control application ;)

Sadly I cannot recommend them. I saw that deal as well and bought them. The ceiling lamps I bought (the H60A1 I am testing with at the moment) are great: For 42€ you get a lamp that is really bright, can do RGB+CTT, shines to the bottom and top and looks good.

Link Germany: https://eu.govee.com/de-de/products/govee-rgbww-rgbic-smart-ceiling-light
Link US: https://us.govee.com/products/govee-rgbww-rgbic-smart-ceiling-light

Can only recommend them!! For the small govee bulbs the story is a bit different... After the success with the ceiling lights I was happy to invest more. However they are pretty dim and dont support the LAN API.

---

Btw, thanks Andrew for picking up the fixes for the binding. <3 Really appreciated as I know you are genie when it comes to lighting and in particular color spaces which is really magic to me ;-)

Yes, thank you @andrewfg !

---

However, I would recommend grabbing some of the led strips like the H6159 which is less than 20 Euros and does support it.

Good to know ;-)

---

by the way, can you really confirm that the H60A1 IS supported, ie. does it have LAN-API setting in the App? At least in the document https://app-h5.govee.com/user-manual/wlan-guide it isn't listed (the H60A0 is listed though)

@stefan-hoehn Yes, it IS supported. Tested it, shows LAN-API in the app, works with OpenHAB. After buying the first one and being so happy I bought 3 more (4 in total now). The light is really nice, can get nice and cozy warm. Also the brightness is incredibly (only use it at like 40-50% during the day) but can get pretty dim too.
TLDR: Buy as many as you can get!

---

I was calling triggerRefresh on the OH thread, so I tried now to call it on the scheduler instead.

New version works! Thank you very much! :)

[maniac103]

the color, brightness, on-off and color temperature commands are UDP unicasts (on port 4003) with the lamp sending no response packet.

Ouch, that's ugly. Your solution seems appropriate in that case.

[felixschndr]

@andrewfg Can you tell me why the binding needs the MAC address of the lamp in addition to the IP/hostname?

[andrewfg]

why the binding needs the MAC address of the lamp in addition to the IP/hostname

Honestly I don't know. The thing xml description mentions the existence of such a property. But I think it is functionally not used. However it could be displayed (for information) on the Main UI page for the thing. Along with the model description etc.

[andrewfg]

Ouch, that's ugly

It is very lightweight in terms of CPU load in the device. It can simply fire-and-forget without any overload of building and maintaining a full TCP socket connection.

[stefan-hoehn]

Can you tell me why the binding needs the MAC address

It is used as the unique identifier for the binding:

.withRepresentationProperty(GoveeBindingConstants.MAC_ADDRESS)

Why are you asking?

[maniac103]

It is very lightweight in terms of CPU load in the device. It can simply fire-and-forget without any overload of building and maintaining a full TCP socket connection.

I wasn't wondering why they used UDP, but why they don't send a response for each request (over UDP). Anyways, we can't change that either way.

[felixschndr]

Why are you asking?

I was wondering why it is necessary to define it. Many binding only use one of the two (mostly IP/hostname). It is one more step for the user to find out the MAC and copy it to the config.
As is understand it it is only an internal identifier, right? Couldn't we the use something else, like an ID of the thing or something and then remove the MAC entirely?

[felixschndr]

One thing regarding hostname vs IP: I define all my things with hostnames, however the logs always state the IP which I then have to look up. Can we change the logs such that it always states the host as it is defined in the thing and only use the IP in the background when we are actually talking to the lamp?

2024-11-29 11:29:48.941 [TRACE] [org.openhab.binding.govee.internal.CommunicationManager                         ] - Sending request to 192.168.4.89 with content = {"msg":{"cmd":"devStatus","data":{}}}
2024-11-29 11:29:48.960 [TRACE] [org.openhab.binding.govee.internal.CommunicationManager                         ] - Received response from 192.168.4.139 with content = {"msg":{"cmd":"devStatus","data":{"onOff":0,"brightness":49,"color":{"r":69,"g":118,"b":255},"colorTemInKelvin":4100}}}
2024-11-29 11:29:48.960 [DEBUG] [org.openhab.binding.govee.internal.CommunicationManager                         ] - Processing response for 192.168.4.139

[andrewfg]

Can we change the logs such that it always states the host

Short answer is no. Not if the handler is not yet found. But I will make some changes..

[andrewfg]

This is for @maniac103 as a connoisseur if gnarly thread synchronization code: I feel deeply uncomfortable calling Thread.sleep() in the handleCommand() method. So I thought it would be better to offload the communication calls onto scheduler threads instead. So I wonder if you can look at the handleCommand() and give your treasured opinion? Note that the communication calls can throw exceptions, so I had to use Callable rather than Runnable, and after all the communications tasks have been scheduled, I schedule another task to collect the results and update the thing status. I am quite proud of it :)

[maniac103]

So I wonder if you can look at the handleCommand() and give your treasured opinion?

I totally agree it's a good idea to move the communication to a scheduler thread. The solution seems slightly over engineered to me though ;-) Wouldn't it be sufficient to collect a list of runnables, and scheduling a single scheduler task which then executes them sequentially, sleeping in between their execution? I realize that means making a thread sleep again (which your solution carefully avoids), but since it's a scheduler thread now and the delay is 300ms at most (4 calls with 100ms in-between) that seems OK. One could likely also shrink the delay to say 50 or 30ms and then it becomes a total non issue. Advantage of doing so would be avoiding any potential threading issues between the various tasks and the final 'collection' of those.

[maniac103]

Another option I just found: CompletableFuture ... that one (with the methods runAsync/thenRunAsync) looks like it has exactly the semantics we need here :-)

[stefan-hoehn]

Just my 2cents: while I am blown away by all the effort you put into that , I am kind of afraid if it still works reliably after such a major change (maybe even better though as well). Anyways, I did a lot of tests in the past after @maniac103 and Kai finalized the binding and made sure it works reliably. I think we should then do quite some testing with a number of devices at the same time in the network.

[andrewfg]

Wouldn't it be sufficient to collect a list of runnables, and scheduling a single scheduler task which then executes them sequentially, sleeping in between their execution?

I can't get my head around CompleteableFuture so I am going with the option mentioned above.

[andrewfg]

I am kind of afraid if it still works reliably after such a major change (maybe even better though as well)

I understand your concerns. But I am, of course, sure it will work better. :)
Therefore I fully agree to test it for some time in various configurations before merging it.

[andrewfg]

@maniac103 I just committed a second much simpler attempt at thread synchronization.

@stefan-hoehn / @felixschndr the revised Jar for testing is in the post at the top.

[andrewfg]

@felixschndr more observations on your question about detecting if the thing is "in color temperature mode"..

• if in that mode the color-temperature channels have a real value, and the color channel h and s values are zero
• if not in that mode the color-temperature channels are UNDEF, and the color channel h and s values are (probably) non zero

[andrewfg]

@stefan-hoehn today I received my H615A Black Friday purchase, and have been testing it this afternoon. I found that the auto discovery does not work (separate issue opened). However when I configured the thing manually, it all seems to work fine.

[stefan-hoehn]

I found that the auto discovery does not work (separate issue opened).

I have a "H6159" which is, AFAIK, pretty similar and worked on auto-discovery reliably. Can you verify if it behaves the same with the milestone version?

[andrewfg]

Can you verify if it behaves the same with the milestone version?

Yes it failed on the milestone release.

[andrewfg]

@stefan-hoehn a few notes on my testing regime => is there anything that you want to add to this?

• test the new functionality for new binding version with all new (manually created) things and items
• clean install of OH official Milestone release; create (manually) old things and items; migrate to new; test
• develop fix for discovery issue; automatically create things and items; test