Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hue] Implement CLIP 2 / API v2 #13570

Merged
merged 155 commits into from
Jul 1, 2023
Merged

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Oct 19, 2022

Resolves #13456

Description

This Pull Request adds support for the new Hue CLIP 2 API.

It comprises the following..

  1. New CLIP 2 DTO classes
  2. New bridge thing handler that supports HTTPS, CLIP 2 end points, and SSE, via HTTP 2.0.
  3. New thing handler that can represent any CLIP 2 device (e.g. light, button, sensor, rotary dial, etc.), or grouped light, and has channels dynamically created according to its capabilities.
  4. Added support to the thing handler factory for creating CLIP 2 thing handlers.
  5. Added support to the MDNS discovery service for discovering CLIP 2 bridges.
  6. Added thing discovery service to detect CLIP 2 things within a CLIP 2 bridge.
  7. Added JUNIT tests, and respective JSON test files, for CLIP 2 DTO classes.

Note: the prior API v1 DTO classes were (sorry to say it) not well structured, and have also accumulated too much 'spaghetti code', so that it was impossible to directly map between API V1 and CLIP 2 things. So this is a 'breaking change' as far as migration of existing API V1 installations to CLIP 2. (This is presumably in line with why Philips Signify also chose to make a breaking change between API V1 and CLIP 2).

Nevertheless it includes all the code from API V1 so it is still backwards compatible for existing API V1 installations that will not be migrated.

Furthermore when new API v2 things are created via the discovery services, then if a legacy API v1 thing exists, the new v2 thing will attempt too clone the attributes of the existing v1 thing. And also, if a legacy API v1 thing exists and has items linked to its channels, then the new v2 thing will replicate the links between those items and the respective new v2 channels.

JAR Files for OH v4.0

You can install the binding from the Marketplace. The binding requires the UPnP transport feature which is sometimes not automatically installed, so you may need to execute the following command..

feature:install openhab-transport-upnp

JAR Files for OH v3.4.x

You can install the binding from the Marketplace. In OH 3.4.x the binding requires the Jetty Http2 features which are not part of openHAB v3.4.x distribution, and the UPnP transport feature which is sometimes not automatically installed. So before installing please i) download the Jetty-Http2-Feature-Jars, ii) drop them in the /addons folder, iii) enter the following commands at the console command prompt, and restart your system.

feature:install openhab-transport-upnp
bundle:list | grep "HPACK" // note the bundle id (e.g. 999)
bundle:start-level 999 30 // use the bundle id from above

Migration

The Things in API v2 are created differently than in API v1; for testing purposes see the migration guide HERE.

Useful Links

The following are some useful resources..

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

@andrewfg andrewfg added enhancement An enhancement or new feature for an existing add-on additional testing preferred The change works for the pull request author. A test from someone else is preferred though. work in progress A PR that is not yet ready to be merged awaiting other PR Depends on another PR labels Oct 19, 2022
@andrewfg andrewfg requested a review from cweitkamp as a code owner October 19, 2022 17:30
@andrewfg andrewfg self-assigned this Oct 19, 2022
@andrewfg
Copy link
Contributor Author

andrewfg commented Oct 19, 2022

Example of CLIP 2 supported things

image

@andrewfg
Copy link
Contributor Author

andrewfg commented Oct 19, 2022

Example of thing properties

image

@andrewfg
Copy link
Contributor Author

andrewfg commented Oct 19, 2022

Example of channels for resource thing of type 'light'

image

@andrewfg
Copy link
Contributor Author

andrewfg commented Oct 19, 2022

Example of channels for resource thing of type 'motion detector'

image

@andrewfg
Copy link
Contributor Author

Pinging @cweitkamp @kaikreuzer @lolodomo @jlaur => for your information :)

@andrewfg
Copy link
Contributor Author

@cweitkamp can you please help me fix the hue integration tests for this PR? The CLIP 2 bridge requires two additional OSGI bundles to support the SSE functionality -- namely javax.ws.rs.client.ClientBuilder and org.osgi.service.jaxrs.client.SseEventSourceFactory -- and it looks like the current itests cannot load those bundles, and consequently freeze the CI build.

@andrewfg andrewfg requested a review from a team as a code owner October 20, 2022 14:00
@andrewfg andrewfg added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Oct 20, 2022
@cweitkamp
Copy link
Contributor

The TLS certificate checker introduced in #11842 does not permit the SSE feature to function. The certificate checker requires the host address to consist of IP and PORT whereas the Hue CLIP 2 SSE end point rejects calls with a PORT in it.

Hm, it is not the implementation of the HueTlsTrustManagerProvider which need the port it is deeper in openHAB core. The ExtensibleTrustManager implementation either identifies the trustManager by the sslEngine peer/host or the common name of the certificate. We count on the finding a match on the first condition here.

Reading Philips documentation it might be possible to change this because

... the common name on the certificate matches the bridge id of the bridge you are trying to connect to.

I will look into that.

@cweitkamp
Copy link
Contributor

One more question. Why do you think that the request with port are rejected? I am able to call either CLIP 2 API or the SSE when adding a port via linux console and curl commands.

@andrewfg
Copy link
Contributor Author

@cweitkamp apropos the SSE connection via HTTPS: Oops! I think I misled you concerning the certificate checker. It seems the issue was not due to the HueTlsTrustManagerProvider but actually due to the HostnameVerifier. I solved the issue by implementing a custom HostnameVerifier.verify() method interface in the Clip2Bridge class. (Not completely sure why though..)

@andrewfg
Copy link
Contributor Author

andrewfg commented Oct 22, 2022

Color Temperature Channels: Fixed vs Relative Scale

At some point, sooner or later, we will need to have a discussion about what is the correct scale to use for light's color temperature channel values..

It differs between API V1 and CLIP 2 as follows..

  • On CLIP 2, the color temperature is measured in 'mirek' with a scale of MIN: 153 to MAX: 500 (where both MIN and MAX are fixed). And I have coded it so that the color temperature channels use a PercentType scale from mirek:MIN:153 = 0% to mirek:MAX:500 = 100%.
  • In real life, different models of lights have different capabilities of mirek range, so some cannot go all the way down to mirek:153, and some cannot go all the way up to mirek:500. In API V1 the CT channel PercentType scale represents the percentage relative to the actual MIN/MAX capabilities of the light concerned (i.e. NOT relative to the 153/500 range).
  • So on a CLIP 2 channel a 50% colour temperature is always relative to the MIN:153..MAX:500 scale regardless of the light model (meaning that in reality some lamps cannot actually go down to 0% or some cannot go up to 100%). Whereas on an API V1 channel a 50% colour temperature value is relative its own capabilities (meaning that in reality all lamps can go down to 0% or up to 100%).

Obviously in the CLIP 2 binding we can code it either way. But we need to agree. => Any thoughts?


EDIT: so if you have one room containing lights of different models with different actual mirek ranges, then on API V1 if you set all such lights to CT 50% you would observe the lights to have different actual color temperatures; whereas if you set them to 50% on CLIP 2 then all the lights should be observed to have the same actual color temperatures..

@NorbertHD
Copy link

@ccutrer has just implemented 'mired' in openhab-core (mired, mirek or MK⁻¹ are the same). Perhaps we should use his implementation.
He also has a scaledMireds PercentType:
https://github.com/openhab/openhab-addons/pull/13578/files#diff-9b85d96666742f051b83f5d10d21773ab3f26dfdc822053b198085aeaf7cd60dR250

@andrewfg
Copy link
Contributor Author

just implemented 'mired' in openhab-core (mired, mirek or MK⁻¹ are the same). Perhaps we should use his implementation.

Yes. That makes a lot of sense. :)

But I feel less comfortable about @ccutrer 's 'scaled mired' (see below) on a scale {153 .. 370} which is in contrast to Philips Hue scale of {153 .. 500} on CLIP 2, or {model-dependent-minimum .. model-dependent-maximum} on API V1 .. ??

private static PercentType scaleMireds(int mireds) {
        // range in mireds is 153-370
        // 100 - (mireds - 153) / (370 - 153) * 100
        if (mireds >= 370) {
            return PercentType.HUNDRED;
        } else if (mireds <= 153) {
            return PercentType.ZERO;
        }
        return new PercentType(BIG_DECIMAL_100.subtract(new BigDecimal(mireds).subtract(BIG_DECIMAL_153)
                .divide(BIG_DECIMAL_217, MathContext.DECIMAL128).multiply(BIG_DECIMAL_100)));
    }

Note: in the existing Hue API V1 binding there are actually two channels -- namely a 'scaled mired' (where we "just" have a debate about the range of the scale) and colour temperature absolute (which is currently shown in Kelvin, but could easily be re-cast using UoM to mirek)

@andrewfg
Copy link
Contributor Author

andrewfg commented Oct 22, 2022

^
BTW in the Hue CLIP 2 REST API the 'raw' JSON colour temperature values from the bridge are as follows. And currently I convert to Percent and/or Kelvin inside the binding code. But probably it makes more sense for the core to do the conversion from Mirek to Kelvin. (But we still have a discussion about 'percent scaled range mireks' ... )

lamp type A (currently on)
			"color_temperature": {
				"mirek": 443,
				"mirek_valid": true,
				"mirek_schema": {
					"mirek_minimum": 153,
					"mirek_maximum": 454
				}
..
lamp type B (currently off)
			"color_temperature": {
				"mirek": null,
				"mirek_valid": false,
				"mirek_schema": {
					"mirek_minimum": 153,
					"mirek_maximum": 500
				}

@ccutrer
Copy link
Contributor

ccutrer commented Oct 22, 2022

Might I chip in? I agree that the espmilight scaled mireds isn't exactly a shining example. While I understand the simplicity of a 0-100% scale when someone isn't familiar with (or a device doesn't claim to have) a particular set of units, it seems to usually present more of a problem than not. A better example would be the HomeKit add-on: https://github.com/openhab/openhab-addons/pull/13538/files. It's flexible to work with however various bindings would implement it, but my "ideal" scenario would be a device that exposes a Number channel, with a stateDescription describing the minimum and maximum, and the binding supplying a QuantityType in its native units (probably mireds). This would allow HomeKit to construct the color temp most simply:

Group gMyBulb "My Bulb" { homekit="Lighting" }
Dimmer Bulb_Dimmer "My Bulb" (gMyBulb) { channel="...", homekit="Lighting.Brightness" }
Number:Temperature Bulb_ColorTemp "My Bulb [%d K]" (gMyBulb) { channel="...", homekit="Lighting.ColorTemperature" }

Exposing as a number also gives precise units (and now that core supports mireds, the user's preference - K or mireds) so that you can color-match multiple bulbs, regardless of their native range. It also makes it easy to display K (what's more natural to a common user who is familiar with 2700/3000/4000K bulbs), but do math with mireds (which is more appropriate for "make it a bit warmer"). You can still leave the Dimmer channel for the simple case, hiding the mired channel under "advanced" as has been discusses on the forum once. In that case, it's up for debate if the min/max should be what the binding supports (so at least it would be consistent among bulbs in the same binding), or what each individual bulb supports.

@andrewfg
Copy link
Contributor Author

On Hue the color temperature absolute channel is an advanced channel and currently has raw data in kelvin but I would change that to your mirek now. And probably the second percent channel should be in percent of the range of the actual lamp itself.

@lolodomo
Copy link
Contributor

Color temperature in Kelvin is something more natural and known by final users.
Mired or mirek is a unit I had myself never heard until now.
So I would suggest preferring kelvin over mirek.

Or mirek is maybe a common unit in US/UK ?

@ccutrer
Copy link
Contributor

ccutrer commented Oct 22, 2022

I agree. Especially if you already have the channel in Kelvin, keep it in Kelvin. Just make sure to use a QuantityType when providing it, instead of a DecimalType, then conversions will "just work".

And no, mired/mirek is not a common layman's unit in the US. It's more common when you start talking academic color theory because it's kind of like a logarithmic scale- 10 Mireds anywhere along the scale is the same amount of perceptible difference between colors. Whereas 5000K to 6000K is barely a difference, but 2700k to 3500k is significant.

@lolodomo
Copy link
Contributor

lolodomo commented Jul 1, 2023

That is fine for my review comments.

I just had a new look to OH-INF files and I have just a very minor comment for you @andrewfg : can you check the channel descriptions, you use sometimes "group of lights", sometimes "group of the lights". Please make this "consistent". My feeling is that "group of lights" is fine.

@lolodomo
Copy link
Contributor

lolodomo commented Jul 1, 2023

@fwolter : is there something still open for you ?

@fwolter
Copy link
Member

fwolter commented Jul 1, 2023

I'm fine!

@lolodomo
Copy link
Contributor

lolodomo commented Jul 1, 2023

@kaikreuzer @J-N-K : other involved people said they are finally ok with the unusual channels approach. If you are not ok, please tell us quickly ... before I merge this PR ;)

PS: I also do not pronounce myself because I stopped following the discussion (after having initiated it) ;)

@lolodomo
Copy link
Contributor

lolodomo commented Jul 1, 2023

@andrewfg : please consider my 2 last comments (even if they are minor) and then I merge your PR.

@andrewfg
Copy link
Contributor Author

andrewfg commented Jul 1, 2023

please consider my 2 last comments (even if they are minor) and then I merge your PR.

Ok. I will do it this evening, when I get back home.

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from lolodomo July 1, 2023 17:49
@lolodomo lolodomo merged commit bd4a838 into openhab:main Jul 1, 2023
@lolodomo
Copy link
Contributor

lolodomo commented Jul 1, 2023

Thank you @andrewfg

@ccutrer
Copy link
Contributor

ccutrer commented Jul 1, 2023

🎉 🎉 🎉

@lolodomo
Copy link
Contributor

lolodomo commented Jul 1, 2023

A long journey for @andrewfg and I imagine a lof of efforts and patience.
Thanks also to @jlaur who made the effort to review this massive PR (more than 23000 new lines) and everybody that helped to avoid the "clash".

@andrewfg andrewfg deleted the #13456-hue-clip2 branch July 2, 2023 10:59
@kaikreuzer kaikreuzer added this to the 4.0 milestone Jul 2, 2023
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Jul 8, 2023
* [hue] Implement CLIP 2 / API v2

---------

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
…rature (openhab#3129)

refs openhab/openhab-addons#13570 (comment)

now that Kelvin and mireds/mireks are easily convertible

Signed-off-by: Cody Cutrer <cody@cutrer.us>
GitOrigin-RevId: 221c80b
@andrewfg andrewfg removed the additional testing preferred The change works for the pull request author. A test from someone else is preferred though. label Jul 24, 2023
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
* [hue] Implement CLIP 2 / API v2

---------

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Matt Myers <mmyers75@icloud.com>
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/philips-hue-clip-2-api-v2-discussion-thread/142111/367

austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
* [hue] Implement CLIP 2 / API v2

---------

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cre Coordinated Review Effort enhancement An enhancement or new feature for an existing add-on
Projects
None yet