stefan-kaestle / openhab2-addons

Add-ons for openHAB 2.x
Eclipse Public License 2.0
16 stars 1 forks source link

Support the Pairing with Bosch SHC #11

Closed GerdZanker closed 4 years ago

GerdZanker commented 4 years ago

See enhancement #4

Added support for keystore creation and pairing. Documented the new setting and pairing process in readme.md Refactoring of httpClient was done, to take care of SSL context.

Tested on my local setup (Windows + Bosch SHC with latest update from this week - CW26) and working if no jks file is given.

More tests and feedback are very welcome.

GerdZanker commented 4 years ago
coeing commented 4 years ago

Would it be possible to move the new source files into a sub package, e.g. "bridge" or "communication" or so?

stefan-kaestle commented 4 years ago

This is awesome, thank you so much! This part I think was the biggest blocker for merging to master :-) I think with a little bit of testing we should probably start to do a PR against the main repo and see what developers have to say about the addon so far? I'd expect we might have to change a couple of things before we can finally merge :-)

It might also be worth asking on the forum if somebody wants to test the binding and get some feedback from them?

coeing commented 4 years ago

This is awesome, thank you so much! This part I think was the biggest blocker for merging to master :-) I think with a little bit of testing we should probably start to do a PR against the main repo and see what developers have to say about the addon so far? I'd expect we might have to change a couple of things before we can finally merge :-)

It might also be worth asking on the forum if somebody wants to test the binding and get some feedback from them?

👍 Sounds good, a first public version would be great. @stefan-kaestle Do you want to open a separate issue to discuss this?

GerdZanker commented 4 years ago

This is awesome, thank you so much! This part I think was the biggest blocker for merging to master :-) I think with a little bit of testing we should probably start to do a PR against the main repo and see what developers have to say about the addon so far? I'd expect we might have to change a couple of things before we can finally merge :-)

It might also be worth asking on the forum if somebody wants to test the binding and get some feedback from them?

Same from my point of view - if the pairing is working well many OpenHab users can use the PaperUI and install this addon. Preparing the big PR makes sense and I will support you on this way, e.g. with necessary cleanup and other house keeping.

GerdZanker commented 4 years ago

Would it be possible to move the new source files into a sub package, e.g. "bridge" or "communication" or so?

I like the idea to use more sub packages. Can we finish this PR and adding the new "pairing" feature and afterwards reafactor the boschshc.internal package content with a new PR? I started a bit with the new BoschHttpClient class (BTW: in the meantime I like BoschSHCClient more) and several other functions and classes can be separated from the major internal classes like the Bridge and Handler. But if I mix this into the same PR I feel it will be a too big change.

GerdZanker commented 4 years ago

I have handled all the open point and finished your suggsted improvements regarding settings and user feedback. Can you review and test again, before a merge?

coeing commented 4 years ago

@GerdZanker In my opinion feel free to merge your feature :)

GerdZanker commented 4 years ago

Recommited the changes with a "signed-off-by" and forced pushed it to https://github.com/GerdZanker/openhab-addons/tree/bosch-shc-2.5. Not sure what's wrong but new commits are not visible here.

GerdZanker commented 4 years ago

github needs a new commit date and some time (minutes) to update PRs.