[ecobee] Ecobee binding initial contribution

[mhilbush]

Contributing an OH2 version of the ecobee binding.

Binding supports:

• Auto discovery of thermostats and remote sensors.
• Currently uses OAuth process from OH1 version of the binding. Changes to core are needed to use the core OAuth support, so that will be tackled when the binding is ported to OH3.
• Added several thing actions in addition to the actions from OH1.

Things are organized in a Bridge->Bridge->Thing configuration. The first level (Account Bridge) represents the Ecobee Account. The second-level (Thermostat Bridge) represents the Thermostats that are associated with the Ecobee Account bridge. The third-level (Remote Sensor Thing) represents the remote and internal sensors that are associated with the Thermostat bridge.

Signed-off-by: Mark Hilbush mark@hilbush.com

[mhilbush]

Removed the WIP tag.

[Hilbrand]

I see this binding includes OAuth2 support and that you plan to implement the core variant later. What you already could do is to implement the current version using the same interface. So create the same classes with only the methods you need in your binding package. Then later on you can simply change the imports and changing can be done fast. I've done this in the past for the Spotify binding. Here you can see what that looked: https://github.com/astenlund74/openhab2-addons/tree/bee1134692e1c61d2caf38dbfffe92b0cfd8891f/addons/binding/org.openhab.binding.spotify/src/main/java/org/openhab/binding/spotify/internal/oauth2

[mhilbush]

What you already could do is to implement the current version using the same interface.

That's a good suggestion. Wish I had thought of it. 😉

I've done this in the past for the Spotify binding.

I see you used the Authorization Code method in the Spotify binding. I had considered changing from the PIN method, which is what the OH1 version of the binding used, to the Authorization Code method, however, the Ecobee documentation says that in their OAuth implementation redirect URIs cannot have port numbers. This would seem to be a showstopper, as the default redirect URI would need to be something like http://<openhabhost>:8080/connectecobee. Am I understanding this correctly?

That being the case, I guess I would need to stick with the PIN method, and would need to adjust the local OAuth classes to account for this. In addition, as @cweitkamp and I had discussed separately, the OHC OAuth implementation would need to be extended to support a non-standard grant type used by Ecobee (i.e. "grant_type=ecobeePIN"). For example, a new method such as this:

OAuthClientService::getAccessTokenResponseByAuthorizationCode(String grantType)

[mhilbush]

Resetting back to WIP while I rework the OAuth stuff.

[TravisBuddy]

Travis tests were successful

Hey @mhilbush,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[mhilbush]

@cpmeister I've pushed some changes that address most of your review feedback, as well as the rework of EcobeeActions to use your proxy suggestion (with the addition of IEcobeeActions).

For the remaining review feedback not covered in the changes I pushed, I've added answers and/or questions in response to your review comments.

[cpmeister]

I think these are the last changes I can find.

[robnielsen]

LGTM

[TravisBuddy]

Travis tests were successful

Hey @mhilbush,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[mhilbush]

@cpmeister I pushed some additional changes.

There are a couple things above that are still unresolved. If you could have a look at my comments when you get a chance.

[cpmeister]

LGTM

[robnielsen]

@cpmeister, what does the cre label stand for?

[cpmeister]

@robnielsen "Coordinated Review Effort" it basically used to mark PRs that require additional reviewers to review it.

[kaikreuzer]

Awesome work, @mhilbush!
I only had a quick glance over it, but as you are known to be a very experienced binding author and we already have two approvals, there shouldn't be any reason to wait with the merge any longer 🎉.

[mhilbush]

Thanks @kaikreuzer. Very much appreciated. And thanks for fixing up CODEOWNERS.

@cpmeister @robnielsen Thanks again for the reviews!