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

[boschshc] Initial contribution - Bindings for Bosch Smart Home devices #8371

Closed
wants to merge 121 commits into from

Conversation

coeing
Copy link
Contributor

@coeing coeing commented Aug 30, 2020

Hi there,

@stefan-kaestle started a fork of your repository in November 2019 to implement the bindings for Bosch Smart Home devices which are pretty popular in Germany. @GerdZanker and I joined earlier this year.

Together we managed to built a good base for a first release, which already has the base structure to setup and communicate with the Bosch Smart Home controller and supports a lot of devices:

  • Bosch In-Wall switches
  • Bosch Smart Plugs (use "in-wall-switch" thing too).
  • Bosch TwinGuard smoke detector
  • Bosch Window/Door contacts
  • Bosch Motion Detector
  • Bosch Shutter Control in-wall
  • Bosch Thermostat
  • Bosch Climate Control

Before we continue our development we'd like to do a first public release to gather user feedback and your feedback as openHAB maintainers :)

We already found some fellow openHAB users who tested pre-release versions and apart from some minor issues which we fixed, they were already quite content with the result (and had some ideas for enhancements for the next version of course :) ).

We tried to get some upfront feedback about our code base from one of your maintainers, but got no answer, so we thought we just create a pull request to discuss any further issues here: https://community.openhab.org/t/towards-merging-the-new-bosch-shc-binding/101952

You can find the fork here: https://github.com/stefan-kaestle/openhab2-addons We already tried to sign-off each commit and follow your coding guidelines (https://www.openhab.org/docs/developer/development/guidelines.html), but might have missed some points as it is our first contribution to your project.

We have a thread in the community forum: https://community.openhab.org/t/will-there-be-a-bosch-smart-home-binding

The last pre-release is up-to-date apart from some file moving: https://github.com/stefan-kaestle/openhab2-addons/releases/tag/v1.0-beta.1

Feel free to ask any questions and let us know what work needs to be done before our work can be merged. Thanks for your help and for your great open-source Smart Home platform!

@coeing coeing requested a review from a team as a code owner August 30, 2020 18:53
@TravisBuddy
Copy link

TravisBuddy commented Aug 30, 2020

Travis tests have failed

Hey @coeing,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

@fwolter fwolter added the new binding If someone has started to work on a binding. For a new binding PR. label Aug 30, 2020
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
…roller

Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
This gives basic functionality to receive updates from the Bosch Smart Home Controller

Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Otherwise, code would get stuck on requesting second subscription ID
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
Signed-off-by: Stefan Kaestle <stefan@mad-kow.de>
…from them

Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
@Hilbrand
Copy link
Member

Can you post a comment if you think all comments are addressed so we can have a second look?

@GerdZanker
Copy link
Contributor

Another comment. The added library is rather big, and it looks only a small part is used. When you add an empty file shrinkBundle.profile next to this bindings pom.xml the packaging will only include thoses classes from the library that are actually used. This does shrink this bindings jar significantly.

I will give it a try.

@coeing
Copy link
Contributor Author

coeing commented Sep 14, 2020

Can you post a comment if you think all comments are addressed so we can have a second look?

Sure, will do. Right now we are still busy fixing some of your commented code. Once we resolved every conversation, I'll post a comment and reference you and @fwolter :)

@GerdZanker
Copy link
Contributor

Another comment. The added library is rather big, and it looks only a small part is used. When you add an empty file shrinkBundle.profile next to this bindings pom.xml the packaging will only include thoses classes from the library that are actually used. This does shrink this bindings jar significantly.

I will give it a try.

Unfortunately this results in

java.lang.ClassFormatError: LVTT entry for 'stateClass' in class file org/openhab/binding/boschshc/internal/devices/bridge/BoschSHCBridgeHandler does not match any LVT entry

The size is related to the used bouncycastle libs for the certificate creation.

Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
…CBridgeHandler.updateSwitchState

Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
…et the thing to offline

Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
…y-rating channels

Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
Signed-off-by: Christian Oeing <christian.oeing@slashgames.org>
@openhab-bot
Copy link
Collaborator

Automatic code migration to openHAB 3 succeeded.

The resulting code can be found at https://ci.openhab.org/job/openHAB-Addons-Migration/31/artifact/bundles/.

You can download the migrated code from there and create a new PR against the master branch of the openhab-addons repo to contribute it for being included in openHAB 3.x.

Please see this issue about the details on how to proceed with your existing PR.

…kdown

the HTTPClient requests were broken
updated exception handling and logging for connection and pairing
describing source of certificates

Signed-off-by: Gerd Zanker <gerd.zanker@web.de>
@GerdZanker
Copy link
Contributor

Hello @coeing,
looks like we are too slow to keep track with openHAB. But I'm sorry, I can't spent more time on the code even if a would like to.

You can download the migrated code from there and create a new PR against the master branch of the openhab-addons repo

It seems to be the best to switch asap to openHAB3 with Java11, new base code and the migrated add-on code, because up to now only my last "http fix" needs to be ported.

What do you think?

@coeing
Copy link
Contributor Author

coeing commented Sep 21, 2020

Hello @coeing,
looks like we are too slow to keep track with openHAB. But I'm sorry, I can't spent more time on the code even if a would like to.

You can download the migrated code from there and create a new PR against the master branch of the openhab-addons repo

It seems to be the best to switch asap to openHAB3 with Java11, new base code and the migrated add-on code, because up to now only my last "http fix" needs to be ported.

What do you think?

Yes, probably the best way to go. We would have to support openHAB 3 anyway, so it would be easier to develop against one code base only. The changes from 2.5 to 3 do not seem to be too big for bindings, so I'm pretty confident we can get it done quickly. I just downloaded the artifacts and copied them over. I will push them to a new branch which I rebase on openHAB 3.

@coeing
Copy link
Contributor Author

coeing commented Oct 1, 2020

@Hilbrand @fwolter We created a new branch "bosch-shc-3.0" which is based on openhab/main. Should we start a new pull request for that branch and we close this one?

@Hilbrand
Copy link
Member

Hilbrand commented Oct 1, 2020

Do what you feel comfortable with. In general we suggest people not to open new pr's as this would make reviewing more difficult because reviews comments are more difficult to track. But with the breaking history changes this might be more difficult for people to know how to do it correctly. If you create a new pr, please do close this pr.

@coeing
Copy link
Contributor Author

coeing commented Oct 1, 2020

Do what you feel comfortable with. In general we suggest people not to open new pr's as this would make reviewing more difficult because reviews comments are more difficult to track. But with the breaking history changes this might be more difficult for people to know how to do it correctly. If you create a new pr, please do close this pr.

Alright, thanks for the quick message! I started a draft of a new pull request (#8629). We need to fix one more issue, afterwards I will close this pull request and open the other one.

@fwolter
Copy link
Member

fwolter commented Oct 18, 2020

Closing in favor of #8629

@fwolter fwolter closed this Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new binding If someone has started to work on a binding. For a new binding PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants