[boschspexor] Initial contribution #12706

marcfischerboschio · 2022-05-09T15:42:33Z

Hello guys,

Description

this is the initial contribution to support the Bosch spexor with openHAB 3.2.x.
The binding requires a short simple configuration and provides a servlet which guides any user to setup the plugin.
I already provided an community post with a bit of information about the Bosch spexor here: https://community.openhab.org/t/bosch-spexor-binding-openhab3-beta/135090

The goal of the new Binding is that we would like to enable a) openHAB with a smart sensor that measures Air Quality - Acceleration, Temperature etc. and also b) enable the spexor to other systems easily to get a better user experience.
We do think that also the openHAB could also be used on mobile devices like camper to also get thus smart.

Used libraries

The addon is having a servlet for easy setup and is storing and handling an OAuth 2.0 device code flow mechanism.
Therefore I included some apache 2.0 licensed libraries for better and stable inclusion.
I did some bounding to the internal http client of openHAB to consider system setups of openHAB. It also contains embedded jquery and a QRcode library in javascript based on MIT licenses. I added them also into the source code for traceability.
For more details please see the attached commit information file LICENSES_USED_LIBRARIES

Testing

The code was originally developed based on the 3.1.x version and is now linked to 3.2.x version.
The code is tested with 3.1.x as well as 3.2.x version on Mac - Linux and Windows based openHAB installations.
I created a few unit tests / integration tests to reflect server communication and also the mechanism of the authorization flow to ensure that everything is working proper. The tests are do need a while due to a simulated state.

Code

I checked all coding guidelines and applied them as I understood. Findings are only on Unit tests but the marked issues are wished and should consider error cases. The code is also shared through my department and I'm contributing this we rules of Bosch.IO. Therefore we did also lots of checks regarding security & open source compliance.

Please let me know if I need to change something. My team and me are using this quite a long time and didn't had any bad experience but if so, let me know!!

Marc

openhab-bot · 2022-05-09T16:30:53Z

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

https://community.openhab.org/t/bosch-spexor-binding-openhab3-beta/135090/1

lsiepel

Thank you for contributing!
I mainly focused on housekeeping and checked allmost all files. I think other reviewers would need to check licences, oauth/servlet and thingstate handling etc If i have more time i try to do another pass.

bundles/org.openhab.binding.boschspexor/pom.xml

bundles/org.openhab.binding.boschspexor/README.md

bundles/org.openhab.binding.boschspexor/RELEASE_NOTE

...or/src/main/java/org/openhab/binding/boschspexor/internal/BoschSpexorThingConfiguration.java

...chspexor/src/main/java/org/openhab/binding/boschspexor/internal/BoschSpexorThingHandler.java

bundles/org.openhab.binding.boschspexor/src/main/resources/OH-INF/thing/thing-types.xml

...les/org.openhab.binding.boschspexor/src/main/resources/OH-INF/i18n/boschspexor_xx.properties

...ain/java/org/openhab/binding/boschspexor/internal/discovery/BoschSpexorDiscoveryService.java

marcfischerboschio · 2022-05-11T11:12:13Z

Latest changes on code due to the recommendation of @lsiepel applied

lsiepel

Some small feedback. It's allways good to use a spell checker on documentation before a commit :-)

Please also mark feeedback that is resolved.

bundles/org.openhab.binding.boschspexor/README.md

marcfischerboschio

added some comment to request more details

bundles/org.openhab.binding.boschspexor/README.md

...chspexor/src/main/java/org/openhab/binding/boschspexor/internal/BoschSpexorThingHandler.java

...xor/src/main/java/org/openhab/binding/boschspexor/internal/api/service/SpexorAPIService.java

Hilbrand · 2022-05-16T10:18:34Z

Something went wrong, the pr now seems to include changes for other bindings?

marcfischerboschio · 2022-05-16T12:01:25Z

@Hilbrand will try to fix it. Didn't added this - only due to the CDO recommendation...

…ents Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

marcfischerboschio · 2022-05-16T12:36:10Z

sorry for having some unwished commits in the PR.
I was trying to resolve the DCO findings but it seemed like there were also other PR's then include to mine.

Big sorry but I saved everything and applied your PR recommendations. Only topic currently is the point by @lsiepel regarding the topic of returning an empty list and and maybe confuse the user. But in this case I tested a lot and adjusted the behavior and it was working as expected. Including behavior of detaching things that are not any more available.

Hilbrand

Here is a first review. I haven't reviewed everything, but wanted to give this. One part not completely reviewed is regarding the channels dynamically added. The sensors channels could also be of a QuantityType and given them units, but this will be more work, as each unit type you need to define a separate channels.

Regarding OAuth2. openHAB has built in support for OAuth2 authentication flow. Did that not work? or did it miss anything?

bundles/org.openhab.binding.boschspexor/LICENCES_USED_LIBRARIES

bundles/org.openhab.binding.boschspexor/src/main/feature/feature.xml

bundles/org.openhab.binding.boschspexor/src/main/resources/html/resources/QRCODE_LICENSE_MIT

bundles/org.openhab.binding.boschspexor/src/main/resources/OH-INF/config/configs.xml

.../main/java/org/openhab/binding/boschspexor/internal/api/service/BoschSpexorBridgeConfig.java

...chspexor/src/main/java/org/openhab/binding/boschspexor/internal/BoschSpexorThingHandler.java

marcfischerboschio · 2022-05-17T06:09:06Z

Here is a first review. I haven't reviewed everything, but wanted to give this. One part not completely reviewed is regarding the channels dynamically added. The sensors channels could also be of a QuantityType and given them units, but this will be more work, as each unit type you need to define a separate channels.

Regarding OAuth2. openHAB has built in support for OAuth2 authentication flow. Did that not work? or did it miss anything?

Hello @Hilbrand, thanks for your review.
Regarding the OAuth2: Yes I did now that there is a general implementation of the OAuth2 mechanism in it. I tried it quite a long time to map all my data in the corresponding services but I had to include "internal" classes to store and use the refresh urls. Due to the OAuth2 device code flow, which is not supported in the implementation, I then decided to use another library and store the tokens on my own.

I also requested a bit of support here: openHAB Community: OAuth2.0 Device Authorization Grant (device_code) but also there was the same outcome to use custom implementation.

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

...c/main/java/org/openhab/binding/boschspexor/internal/api/service/auth/SpexorAuthServlet.java

.../src/main/java/org/openhab/binding/boschspexor/internal/api/service/SpexorBridgeHandler.java

...chspexor/src/main/java/org/openhab/binding/boschspexor/internal/BoschSpexorThingHandler.java

…s to a better fitting one Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

…ndation for better UX' Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

...hspexor/src/main/java/org/openhab/binding/boschspexor/internal/BoschSpexorBridgeHandler.java

...va/org/openhab/binding/boschspexor/internal/api/service/auth/SpexorAuthorizationService.java

...chspexor/src/main/java/org/openhab/binding/boschspexor/internal/BoschSpexorThingHandler.java

bundles/org.openhab.binding.boschspexor/README.md

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

bundles/org.openhab.binding.boschspexor/pom.xml

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

lolodomo · 2022-11-06T10:32:53Z

There were already several review steps. What is the current status ? @Hilbrand : is your review finished ?

jlaur · 2022-12-23T15:40:53Z

bundles/org.openhab.binding.boschspexor/pom.xml

@@ -0,0 +1,72 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">


Suggested change

xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">

xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">

See #11833

wborn · 2022-12-29T10:47:22Z

bundles/org.openhab.binding.boschspexor/pom.xml

+  <parent>
+    <groupId>org.openhab.addons.bundles</groupId>
+    <artifactId>org.openhab.addons.reactor.bundles</artifactId>
+    <version>3.4.0-SNAPSHOT</version>


Suggested change

<version>3.4.0-SNAPSHOT</version>

<version>4.2.0-SNAPSHOT</version>

lsiepel · 2023-01-16T15:45:32Z

bundles/org.openhab.binding.boschspexor/src/main/resources/OH-INF/binding/binding.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>


This needs to be adapted to the new addon.xml, the PR to adapt the developer docs is not merged yet, so if you need more details, the corresponding issue is here: openhab/openhab-core#2058
But you can also update your branch and look at another binding's addon.xml and adapt it to this initial contribution.

lsiepel · 2024-06-21T12:45:09Z

Unfortunately it looks like we cannot bring this PR to the finish line. After two years of inactivity i close this PR. If there is a need to restart, feel free to re-open or start a new PR.

marcfischerboschio changed the title ~~Boschspexor main~~ [boschspexor] initial commit of spexor plugin May 9, 2022

marcfischerboschio changed the title ~~[boschspexor] initial commit of spexor plugin~~ [boschspexor] initial contribution of spexor plugin May 9, 2022

marcfischerboschio marked this pull request as ready for review May 9, 2022 16:21

marcfischerboschio requested a review from a team as a code owner May 9, 2022 16:21

jlaur added new binding If someone has started to work on a binding. For a new binding PR. labels May 9, 2022

jlaur changed the title ~~[boschspexor] initial contribution of spexor plugin~~ [boschspexor] Initial contribution May 9, 2022

lsiepel requested changes May 10, 2022

View reviewed changes

lsiepel reviewed May 11, 2022

View reviewed changes

marcfischerboschio commented May 16, 2022

View reviewed changes

marcfischerboschio force-pushed the boschspexor-main branch from 7fa8f5b to 555ec9d Compare May 16, 2022 08:45

marcfischerboschio requested review from mvalla, clinique, cweitkamp and lolodomo as code owners May 16, 2022 08:45

marcfischerboschio requested a review from lsiepel May 16, 2022 08:57

initial commit. Fixed signoff issues and applied pull request adjustm…

c91a5b2

…ents Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

marcfischerboschio force-pushed the boschspexor-main branch from 6be0d20 to c91a5b2 Compare May 16, 2022 12:26

Hilbrand reviewed May 16, 2022

View reviewed changes

mvalla removed their request for review May 17, 2022 04:18

marcfischerboschio added 2 commits May 17, 2022 14:04

applied pull request recommendation and refined documentation

3f34912

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

applied recommended usage of ID access and adjusted discovery behavior

77e462f

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

Hilbrand reviewed May 20, 2022

View reviewed changes

marcfischerboschio added 2 commits May 23, 2022 10:36

refactored mechanism of channel creation and moved and renamed classe…

08d9ae8

…s to a better fitting one Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

adjusted predefined layout of state of charge. Tried to apply recomme…

57188fc

…ndation for better UX' Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

marcfischerboschio requested a review from Hilbrand May 31, 2022 06:17

Hilbrand reviewed Jun 1, 2022

View reviewed changes

marcfischerboschio added 3 commits June 9, 2022 12:02

applied recommendations due to the pull request.

b1a01d7

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

Removed additional log entry

3bf53df

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

refactored exception handling. Changed status tracking

08a63cc

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

marcfischerboschio requested a review from Hilbrand June 9, 2022 14:14

changed behavior of reporting false input by the user

48f13be

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

wborn reviewed Jul 10, 2022

View reviewed changes

bundles/org.openhab.binding.boschspexor/pom.xml Outdated Show resolved Hide resolved

marcfischerboschio added 2 commits August 10, 2022 08:21

Merge branch 'main' into boschspexor-main

eea61db

updated to main 3.4.0-SNAPSHOT

a001f04

Signed-off-by: Marc Fischer <marc.fischer2@bosch.io>

marcfischerboschio requested a review from wborn August 10, 2022 06:38

jlaur reviewed Dec 23, 2022

View reviewed changes

wborn reviewed Dec 29, 2022

View reviewed changes

lsiepel reviewed Jan 16, 2023

View reviewed changes

jlaur added the awaiting feedback Awaiting feedback from the pull request author label Mar 22, 2023

lsiepel added the stale As soon as a PR is marked stale, it can be removed 6 months later. label Apr 29, 2024

lsiepel closed this Jun 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[boschspexor] Initial contribution #12706

[boschspexor] Initial contribution #12706

marcfischerboschio commented May 9, 2022

openhab-bot commented May 9, 2022

lsiepel left a comment •

edited

Loading

marcfischerboschio commented May 11, 2022

lsiepel left a comment •

edited

Loading

marcfischerboschio left a comment

Hilbrand commented May 16, 2022

marcfischerboschio commented May 16, 2022

marcfischerboschio commented May 16, 2022

Hilbrand left a comment

marcfischerboschio commented May 17, 2022

lolodomo commented Nov 6, 2022

jlaur Dec 23, 2022

wborn Dec 29, 2022 •

edited

Loading

lsiepel Jan 16, 2023

lsiepel commented Jun 21, 2024

	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">

	<version>3.4.0-SNAPSHOT</version>
	<version>4.2.0-SNAPSHOT</version>

[boschspexor] Initial contribution #12706

[boschspexor] Initial contribution #12706

Conversation

marcfischerboschio commented May 9, 2022

Description

Used libraries

Testing

Code

openhab-bot commented May 9, 2022

lsiepel left a comment • edited Loading

Choose a reason for hiding this comment

marcfischerboschio commented May 11, 2022

lsiepel left a comment • edited Loading

Choose a reason for hiding this comment

marcfischerboschio left a comment

Choose a reason for hiding this comment

Hilbrand commented May 16, 2022

marcfischerboschio commented May 16, 2022

marcfischerboschio commented May 16, 2022

Hilbrand left a comment

Choose a reason for hiding this comment

marcfischerboschio commented May 17, 2022

lolodomo commented Nov 6, 2022

jlaur Dec 23, 2022

Choose a reason for hiding this comment

wborn Dec 29, 2022 • edited Loading

Choose a reason for hiding this comment

lsiepel Jan 16, 2023

Choose a reason for hiding this comment

lsiepel commented Jun 21, 2024

lsiepel left a comment •

edited

Loading

lsiepel left a comment •

edited

Loading

wborn Dec 29, 2022 •

edited

Loading