[hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client #11842

cweitkamp · 2021-12-22T15:45:49Z

Changed discovery to mDNS
Added HTTPS handling
Refactor HTTPClient to use Jetty shared client
Refactor to reduce code and simplify implementation

Depends on openhab/openhab-core#2622 and openhab/openhab-core#2635

Philips announced a brand new HUE API v2 being available. These are the first steps to add support for it.

The new API supports some really interesting new features like streaming SSEs. It uses HTTPS only. Additional, they will ~~deprecate~~ disable UPnP in Q2 2022 in favor of mDNS.

See their Migration Guide to new API.

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

.../main/java/org/openhab/binding/hue/internal/discovery/HueBridgeUPnPDiscoveryParticipant.java

.../main/java/org/openhab/binding/hue/internal/discovery/HueBridgeMDNSDiscoveryParticipant.java

...ue/src/main/java/org/openhab/binding/hue/internal/connection/HueTlsTrustManagerProvider.java

cweitkamp · 2021-12-22T15:59:03Z

...hab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueBridgeHandler.java

+                    // register trustmanager service
+                    HueTlsTrustManagerProvider tlsTrustManagerProvider = new HueTlsTrustManagerProvider(
+                            ip + ":" + hueBridgeConfig.getPort(), hueBridgeConfig.useSelfSignedCertificate());
+                    serviceRegistration = FrameworkUtil.getBundle(getClass()).getBundleContext()
+                            .registerService(TlsTrustManagerProvider.class.getName(), tlsTrustManagerProvider, null);


This is a really ugly implementation and we do not want it. But unfortunately I did not yet found a better solution.

Implementing the HueTlsTrustManagerProvider to be a ThingHandlerService would be my favorite one but when doing so there will be a race condition between activation of the HueTlsTrustManagerProvider and initialization of the HueBridgeHandler.

Registering the HueTlsTrustManagerProvider in the HueThingHandlerFactory does not work because I need access to the configuration of the Thing to retrieve IP address, port and protocol.

Is there a way for HueTlsTrustManagerProvider to return a status in the case that getting the external cert fails (i.e. the server is momentarily unreachable)? It seems like the current code has no provision to retry the cert retrieval process since it attempted only once during initialize().

kaikreuzer · 2021-12-22T16:04:44Z

Wow, that's awesome, thank you - I already hoped that you would look into the new hue APIs! 😄
Especially the new SSE support is really nice to finally get instant state updates.

cweitkamp · 2021-12-22T16:22:13Z

I am planning to separate the implementation into three steps. This is the first one.

Basic refactoring
Restore current behavior on v2 API - meaning new endpoints, new model - and keep v1 for users who own older Hue Bridges in the background
Add handling of SSE

...ue/src/main/java/org/openhab/binding/hue/internal/connection/HueTlsTrustManagerProvider.java

...hab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueBridgeHandler.java

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBridge.java

.../main/java/org/openhab/binding/hue/internal/discovery/HueBridgeMDNSDiscoveryParticipant.java

andrewfg · 2021-12-23T13:10:52Z

Continuing discussion with @lolodomo from this #11834 (comment)

So would make sense in case to set it to 0

You could still do that by means of an entry in the '.cfg' file (rather than via the UI).

My concern was just to keep the parameter for the hue binding.

@lolodomo the main reason to remove the parameter was the brilliant idea of @kaikreuzer quoted below. Which would enable new systems to pre-discover potential Things on the network without any bindings having been installed. However if you insist that it must be a binding parameter (instead of an entry in a .cfg file) then we could not develop such a great feature.

I would have loved for a long long time already to extract different UPnP+mDNS discovery services into an "installed by default" bundle, so that the setup wizard could directly identify devices in your home without the need of installing all the bindings first.)

lolodomo · 2021-12-23T13:22:25Z

My concern was to have a parameter, whatever the file in which to define it.

By the way, now that we move from UPnP to mDNS, is this grace period still useful for mDNS discovery?

lolodomo · 2021-12-23T13:28:51Z

@cweitkamp : do you know if mDMD discovery is available for old v1 bridge?

andrewfg · 2021-12-23T13:35:32Z

now that we move from UPnP to mDNS, is this grace period still useful for mDNS discovery?

I will install the JAR from this PR and test it on my Hue bridge to see if the mDNS disco suffers from the same problem as the UPNP disco.

BTW this time of year is a perfect time to test it. In my case the problem (on UPNP disco) only occurs when the Hue bridge is running "heavy" automations, and presumably the automation causes delays in sending the NOTIFY messages. This time of year is when I run a Christmas light colour loop in the front window of my house. This is quite a "heavy" automation.

cweitkamp · 2021-12-23T13:36:55Z

@lolodomo I am afraid not.

mDNS is available Hue bridge v2.x (squared-shaped) and supported with firmware version 1.23 and beyond.

For older bridges there still is the HueBridgeNupnpDiscovery which retrieves results from https://discovery.meethue.com/. Both methods are recommended by Philips.

andrewfg · 2021-12-23T17:25:52Z

I will install the JAR from this PR and test it on my Hue bridge

Unfortunately I could not build the binding jar because there is some dependency on PEMTrustManager in the core, which I cannot find. => So @cweitkamp could you please give me a link to the built JAR?

cweitkamp · 2021-12-23T18:29:46Z

Here you are: org.openhab.core.io.net-3.3.0-SNAPSHOT.zip

cweitkamp · 2022-10-07T20:11:48Z

How many concurrent connections can be made from shared Jetty client instance?

The client is configured to use at maximum 2 connections per destination. It uses 5 up to 10 threads. Those values are user specific (see https://github.com/openhab/openhab-core/blob/4096df1c6433f10e332632ced9f2e679dd8c340d/bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/internal/WebClientFactoryImpl.java#L156-L163).

andrewfg · 2022-10-19T19:04:07Z

Just to mention that HueTlsTrustManagerProvider.java would still need some work in order for it to support the certificate validation for SSE connections (as I have found in #13570 ) .. however I think that such work could be done in the latter PR rather than this one. And also I would need @cweitkamp to help me please :)

jlaur · 2022-10-20T10:21:38Z

How many concurrent connections can be made from shared Jetty client instance?

The client is configured to use at maximum 2 connections per destination. It uses 5 up to 10 threads. Those values are user specific (see https://github.com/openhab/openhab-core/blob/4096df1c6433f10e332632ced9f2e679dd8c340d/bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/internal/WebClientFactoryImpl.java#L156-L163).

Thanks. I will need to figure out how to add some more detailed logging when this happens, as it does happen consistently a few times per week now. It wouldn't be much of a problem if my rules were more well-written - currently I monitor all my devices and get push messages when they are offline/unreachable, but I don't consider if the reason is the bridge being offline, so that triggers a lot of messages. :-) It might be the number of threads being the limitation as some of my other bindings are quite busy all the time.

jlaur · 2022-10-20T10:41:57Z

Just to report that I am also running the latest build, and I am NOT seeing any exceptions..

To see the exception, DEBUG log level would need to be set. Have your checked in logs if the bridge has come OFFLINE at any moment, even just for a few seconds? I'm suspecting (for me) if may be caused by activity from other bindings using the shared http client.

jlaur · 2022-10-20T10:46:20Z

@lolodomo, @cweitkamp - although the switch to shared Jetty client has changed some behavior related to timeouts in my setup, I believe this is a general problem for which this binding is not to blame, probably it's just a symptom. So IMHO it would be okay to merge if no one else but me experienced any issues. I will still try to track this further.

lolodomo · 2022-10-20T11:29:56Z

My only remaining comments were about exception handling but I believe you concluded there is in fact no problem. As a consequence, that is fine for me,. Feel free to merge @jlaur

jaywiseman1971 · 2022-10-20T12:09:47Z

Can someone post the JAR link so I can test this?

I have 2 HUE bridges with over 50 devices across them. I'm running OH 3.2 and the same HUE version binding, my issue is the HUE bridges go offline/online throughout the day very often (just offline for a few milliseconds). Reading this thread, is this because it's using a shared HTTP client? If this is solved, this would be great!

jlaur · 2022-10-20T12:43:06Z

Can someone post the JAR link so I can test this?

This version of the binding requires core 3.3 or higher, so you would need to upgrade before being able to test it.

I actually experienced the opposite problem. Shared Jetty client was introduced with this PR and I didn't have timeouts before I began testing this.

cweitkamp · 2022-10-20T18:23:27Z

@jlaur Which other bindings do you use in your environment based on the shared Jetty client?

jlaur · 2022-10-20T19:16:46Z

@jlaur Which other bindings do you use in your environment based on the shared Jetty client?

miele: Polling interval 15 seconds, each time one call to get devices and one call per device - I have 5. So 18 calls per minute when idle, but they are relatively fast.
hdpowerview: Polling interval 1 minute, 4 calls per iteration. Relatively fast. However, every 15 minutes a hard refresh per shade. I have 11, and these calls are quite expensive. Additionally every 12 hours 11 very expensive calls to fresh battery status.
boschindego: Polling interval 3 minutes, one call per iteration. Quite cheap.
meater: Polling interval 30 seconds, so 2 calls per minute.
wemo: One call per minute through HttpUtil.executeUrl, I believe.
netatmo
harmonyhub

unifi: Actually doesn't. Mentioned only because I found this interesting comment:

openhab-addons/bundles/org.openhab.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/UniFiThingHandlerFactory.java

Lines 59 to 61 in c9481d5

    
           // [wip] mgb: disabled due to missing common name attributes with certs 
        
           // this.httpClient = httpClientFactory.getCommonHttpClient(); 
        
           httpClient = new HttpClient(new SslContextFactory.Client(true));

cweitkamp · 2022-10-20T19:45:40Z

Interesting. And what about the polling configuration of the hue binding itself? IIRC the binding has three polling jobs. One for lights including groups - frequency is configurable by user in seconds , one for sensors - interval configurable too (in ms, disabled, if 0 or less) and one for scenes - fixed value (10 minutes). Plus one request for each command sent towards Hue Bridge.

jlaur · 2022-10-20T20:23:18Z

And what about the polling configuration of the hue binding itself? IIRC the binding has three polling jobs. One for lights including groups - frequency is configurable by user in seconds , one for sensors - interval configurable too (in ms, disabled, if 0 or less) and one for scenes - fixed value (10 minutes). Plus one request for each command sent towards Hue Bridge.

Polling interval is 10 seconds (default). I remember now that I lowered Sensor Polling Interval to 250 ms. I believe that was an attempt to slightly improve responsiveness of some FoH buttons (in lack of SSE).

andrewfg · 2022-10-21T22:21:44Z

in lack of SSE

=> FYI I am working on SSE for Hue and HDPowerview :)

=> also the Gardena binding uses the shared Jetty client; although it has WebSocket call backs (similar to SSE) so the load is low; but one user has complained that when the WebSocket drops, it breaks other bindings; which may or may not be related ??

https://community.openhab.org/t/gardena-binding-throws-connection-errors-and-seems-to-break-some-bindings/140085

jlaur · 2022-10-22T21:40:56Z

@cweitkamp - thanks again!

…lient to use jetty shared client (openhab#11842) * Changed discovery to MDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

…lient to use jetty shared client (openhab#11842) * Changed discovery to MDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

…lient to use jetty shared client (openhab#11842) * Changed discovery to MDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

cweitkamp added enhancement An enhancement or new feature for an existing add-on work in progress A PR that is not yet ready to be merged awaiting other PR Depends on another PR labels Dec 22, 2021

cweitkamp requested review from lolodomo and a team December 22, 2021 15:45

cweitkamp mentioned this pull request Dec 22, 2021

[miele] Fix mDNS issue where hub repeatedly disappears from, resp. reappears in, the Inbox. #11834

Merged

cweitkamp added the additional testing preferred The change works for the pull request author. A test from someone else is preferred though. label Dec 22, 2021

cweitkamp commented Dec 22, 2021

View reviewed changes

.../main/java/org/openhab/binding/hue/internal/discovery/HueBridgeUPnPDiscoveryParticipant.java Outdated Show resolved Hide resolved

cweitkamp commented Dec 22, 2021

View reviewed changes

.../main/java/org/openhab/binding/hue/internal/discovery/HueBridgeMDNSDiscoveryParticipant.java Outdated Show resolved Hide resolved

cweitkamp commented Dec 22, 2021

View reviewed changes

...ue/src/main/java/org/openhab/binding/hue/internal/connection/HueTlsTrustManagerProvider.java Show resolved Hide resolved

cweitkamp changed the title ~~[WIP][hue] Changed discovery to MDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client~~ [WIP][hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client Dec 22, 2021

cweitkamp commented Dec 22, 2021

View reviewed changes

jlaur reviewed Dec 22, 2021

View reviewed changes

...ue/src/main/java/org/openhab/binding/hue/internal/connection/HueTlsTrustManagerProvider.java Outdated Show resolved Hide resolved

jlaur reviewed Dec 22, 2021

View reviewed changes

...hab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/HueBridgeHandler.java Outdated Show resolved Hide resolved

jlaur reviewed Dec 22, 2021

View reviewed changes

bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/HueBridge.java Outdated Show resolved Hide resolved

cweitkamp force-pushed the feature-hue-v2-api branch 2 times, most recently from 95220ce to f4e3567 Compare December 23, 2021 10:26

cweitkamp changed the title ~~[WIP][hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client~~ [hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client Dec 23, 2021

cweitkamp removed the work in progress A PR that is not yet ready to be merged label Dec 23, 2021

andrewfg reviewed Dec 23, 2021

View reviewed changes

.../main/java/org/openhab/binding/hue/internal/discovery/HueBridgeMDNSDiscoveryParticipant.java Show resolved Hide resolved

cweitkamp force-pushed the feature-hue-v2-api branch from f4e3567 to da2d354 Compare December 23, 2021 19:12

andrewfg mentioned this pull request Oct 19, 2022

[hue] Implement CLIP 2 / API v2 #13570

Merged

jlaur merged commit ee34d92 into openhab:main Oct 22, 2022

jlaur added this to the 3.4 milestone Oct 22, 2022

cweitkamp deleted the feature-hue-v2-api branch October 23, 2022 07:14

lolodomo mentioned this pull request Oct 23, 2022

[hue] Binding broken until you update the new protocol setting to "http" (bridge V1) #13586

Closed

wborn removed the additional testing preferred The change works for the pull request author. A test from someone else is preferred though. label Dec 23, 2022

lolodomo mentioned this pull request Jan 2, 2023

[hue] "scene" channel on a hue group populated with several minutes delay #14137

Closed

jlaur mentioned this pull request Feb 2, 2023

[LGWebOS] Firmware 03.30.16 requires use of secured web socket #14309

Closed

This was referenced Apr 19, 2023

[hue] NUPnP discovery broken #14852

Closed

[hue] Fix NUPnP discovery #14871

Merged

jlaur mentioned this pull request May 1, 2023

[hue] Restore UPnP discovery for old bridges #14914

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client #11842

[hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client #11842

cweitkamp commented Dec 22, 2021 •

edited

Loading

cweitkamp Dec 22, 2021 •

edited

Loading

mlobstein Aug 13, 2022

kaikreuzer commented Dec 22, 2021

cweitkamp commented Dec 22, 2021

andrewfg commented Dec 23, 2021

lolodomo commented Dec 23, 2021 •

edited

Loading

lolodomo commented Dec 23, 2021

andrewfg commented Dec 23, 2021

cweitkamp commented Dec 23, 2021

andrewfg commented Dec 23, 2021

cweitkamp commented Dec 23, 2021

cweitkamp commented Oct 7, 2022

andrewfg commented Oct 19, 2022

jlaur commented Oct 20, 2022 •

edited

Loading

jlaur commented Oct 20, 2022

jlaur commented Oct 20, 2022

lolodomo commented Oct 20, 2022

jaywiseman1971 commented Oct 20, 2022

jlaur commented Oct 20, 2022

cweitkamp commented Oct 20, 2022

jlaur commented Oct 20, 2022

cweitkamp commented Oct 20, 2022

jlaur commented Oct 20, 2022

andrewfg commented Oct 21, 2022

jlaur commented Oct 22, 2022

[hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client #11842

[hue] Changed discovery to mDNS; added HTTPS handling; refactor HTTPClient to use jetty shared client #11842

Conversation

cweitkamp commented Dec 22, 2021 • edited Loading

cweitkamp Dec 22, 2021 • edited Loading

Choose a reason for hiding this comment

mlobstein Aug 13, 2022

Choose a reason for hiding this comment

kaikreuzer commented Dec 22, 2021

cweitkamp commented Dec 22, 2021

andrewfg commented Dec 23, 2021

lolodomo commented Dec 23, 2021 • edited Loading

lolodomo commented Dec 23, 2021

andrewfg commented Dec 23, 2021

cweitkamp commented Dec 23, 2021

andrewfg commented Dec 23, 2021

cweitkamp commented Dec 23, 2021

cweitkamp commented Oct 7, 2022

andrewfg commented Oct 19, 2022

jlaur commented Oct 20, 2022 • edited Loading

jlaur commented Oct 20, 2022

jlaur commented Oct 20, 2022

lolodomo commented Oct 20, 2022

jaywiseman1971 commented Oct 20, 2022

jlaur commented Oct 20, 2022

cweitkamp commented Oct 20, 2022

jlaur commented Oct 20, 2022

cweitkamp commented Oct 20, 2022

jlaur commented Oct 20, 2022

andrewfg commented Oct 21, 2022

jlaur commented Oct 22, 2022

cweitkamp commented Dec 22, 2021 •

edited

Loading

cweitkamp Dec 22, 2021 •

edited

Loading

lolodomo commented Dec 23, 2021 •

edited

Loading

jlaur commented Oct 20, 2022 •

edited

Loading