[Qbus] Initial contribution #9191

Qbus-Koen · 2020-12-01T10:35:35Z

Thist is te initial contribution for the Qbus binding. This binding has been tested already by multiple users. Especially for openHAB2 an by some people for openHAB(3). The support for this binding was done on the Qbus forum.

This binding does not communicate directly to the home automation controller, but indirectly by a Client/Server application more info can be found here: https://github.com/QbusKoen/QbusClientServer.

We also made an online user guide, explaining the client/server and the basic principles of openHAB on Manual openHAB - Qbus

The JAR file for testing can be found here: https://github.com/QbusKoen/QbusOH3-JAR

Signed-off-by: Koen Schockaert [email protected]

fwolter · 2020-12-08T18:06:49Z

The link to "Manual openHAB - Qbus" leads to a RPI page.

What is the reason for keeping the "core" of this binding in a separate repository?

Qbus-Koen · 2020-12-09T04:46:00Z

The link to "Manual openHAB - Qbus" leads to a RPI page.

What is the reason for keeping the "core" of this binding in a separate repository?

The first page of the manual indeed shows a rpi, as an example of a device capable of runnin openHAB and the Qbus client/server.

The reason for the seperated core is that this core excistetd before i knew about openHAB and was the only way to communicate to the Qbus system. The protocol of the TCP communication with Qbus is a closed protocol for security reasons. Therefore a C# DLL was created to give people with knowledge of using the DLL the possibility to communicate with Qbus.
I created a client/server application from the DLL that allows communication by json data, which makes it easier for people with less programming experience to communicate with Qbus.

So, that's why i decided to use the 'core' as a bridge between Qbus and openHAB.

fwolter · 2020-12-09T09:22:11Z

Just to get an idea. Is the client/server application used in other projects? Are there other maintainers than you?

Qbus-Koen · 2020-12-09T09:40:10Z

Just to get an idea. Is the client/server application used in other projects? Are there other maintainers than you?

The client/server will be used in other projects, but for the moment only in openHAB, and i'm the only maintainer (for the moment - don't know what the future will bring ;-)).

Maybe it is usefull for you to know that i'm an employee of Qbus. I started on the support departement, but with my earlier expieriences in programming and system integration if spend more time in that section. I started experminting to make our system more open.
So now i'm more the IoT specialist for Qbus rather than a support engineer, although i still offer support on our forum but less for Qbus spefic questions - more for openHAB related questions.

fwolter

IMHO, your client/server/mono approach is good to go, as the alternative would be to re-implement the DLLs.

I reviewed 6 of 30 files, that you get an impression what needs to be done.

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

EDIT: Please add yourself to the CODEOWNERS file.

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

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

Qbus-Koen · 2021-01-03T08:13:16Z

@fwolter Thanks for reviewing my binding!
I've updated my code with your requests and checked and changed the other files.
Pushed the changes to github.
Should I now close this pull request and create a new one?
Please advice. Thx!

fwolter · 2021-01-03T09:48:28Z

I will have a look into it soon. This PR will be used for all further review loops concerning this code, so please don't close it.

You need to sign-off all your commits. The necessary commands are listed when you click on "Details" at the DCO check below.

fwolter

Here is the review feedback for the readme and the first few code files.

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

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

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

Qbus-Koen · 2021-01-09T13:14:54Z

@fwolter Thanks for reviewing my binding again!
I've updated my code with your requests.
Just one thing i can't figure out. Left a comment above.

fwolter · 2021-01-19T09:50:54Z

Can you please click on "Resolve conversation" on those you fixed?

The DCO check failed. You need to sign-off your commits. See https://www.openhab.org/docs/developer/contributing.html#sign-your-work The necessary commands are listed when you click on "Details" at the DCO check below.

Qbus-Koen · 2021-02-09T09:50:37Z

@fwolter
It took me some time to make the requested changes to my code, but i completed them now.
Can you check again if you find the time?

Many Thanks!

Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: QbusKoen <[email protected]>

This reverts commit da86aae. Signed-off-by: QbusKoen <[email protected]>

Signed-off-by: QbusKoen <[email protected]>

Had some issues in first commit. Fixed the bugs. Signed-off-by: QbusKoen <[email protected]>

Solved issues reported by fwolter and changed a lot of code, based on the comments. Signed-off-by: QbusKoen <[email protected]> modified: CODEOWNERS Signed-off-by: QbusKoen <[email protected]>

Signed-off-by: QbusKoen <[email protected]>

Update code like requested by fwolter on 3/1/2021 Signed-off-by: QbusKoen <[email protected]>

fwolter

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

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

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

...nding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusBistabielHandler.java

fwolter · 2021-02-10T16:40:19Z

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

+				<context>network-address</context>
+			</parameter>
+			<parameter name="sn" type="text" required="true">
+				<label>Serial nr</label>


Words in labels should be capitalized (except prepositions and so on). See https://www.openhab.org/docs/developer/bindings/thing-xml.html#formatting-labels-and-descriptions

Can you work on this? Please check all.

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

fwolter · 2021-02-10T16:50:16Z

Please note, that your sign-off message must contain your real name.

Updated requested changes by fwolter on 10/02/2021 Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: QbusKoen <[email protected]>

Qbus-Koen · 2021-02-23T08:58:39Z

Hi @fwolter, i've updated my code with your proposals and pushed changes to git.
If you would be so kind to review the code once more...
Thx!

fwolter

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

...openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusHandlerFactory.java

....binding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusDimmerHandler.java

bundles/org.openhab.binding.qbus/src/main/resources/OH-INF/i18n/qbus_nl.properties

fwolter · 2021-02-23T19:40:45Z

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

+				<context>network-address</context>
+			</parameter>
+			<parameter name="sn" type="text" required="true">
+				<label>Serial nr</label>


Can you work on this? Please check all.

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

Updated requested changes by fwolter on 10/02/2021 Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: QbusKoen <[email protected]>

…o main Signed-off-by: Koen Schockaert <[email protected]>

fwolter

The single missing DCO is ok.

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

...nding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusBistabielHandler.java

...hab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusRolHandler.java

...ding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusThermostatHandler.java

...binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCommunication.java

fwolter · 2021-02-24T17:33:01Z

...binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCommunication.java

+            // Delay after sending data to improve scene execution
+            try {
+                TimeUnit.MILLISECONDS.sleep(250);
+            } catch (InterruptedException e) {


Can you work on this?

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

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

Updated changes as requested by Fwolter on 24/02/2021 Signed-off-by: Koen Schockaert <[email protected]>

Qbus-Koen · 2021-02-24T20:57:53Z

Hi @fwolter

As usually I updated the changes you requested an pushed them.

2 warnings are left, but i'm not sure if i can do something about them:
[WARNING] C:\Users\ks\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.qbus\pom.xml [0:0]: Unused Export-Package instructions: [org.openhab.]
[WARNING] C:\Users\ks\Documents\GitHub\openhab-addons\bundles\org.openhab.binding.qbus\pom.xml [0:0]: Unused Import-Package instructions: [io.swagger.v3.oas.annotations., javax.annotation.security., org.eclipse.jdt.annotation., org.openhab.core.automation.annotation., com.google.common.]

Please advice...
Thx!
Kind regards,
Koen

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

....binding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusGlobalHandler.java

...b.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusSceneHandler.java

fwolter · 2021-02-25T16:26:37Z

...binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCommunication.java

+
+        try {
+            startCommunication();
+        } catch (InterruptedException | IOException e) {


You need to remove the InterruptedException from the catch clause to let it be thrown.

...binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCommunication.java

Updated changes reqeusted by fwolter on 25/02/2021 Signed-off-by: Koen Schockaert <[email protected]>

Qbus-Koen · 2021-02-25T17:48:25Z

@fwolter
Check! Updated requested changes.

Qbus-Koen · 2021-03-02T08:19:29Z

Hi @fwolter, any news on the update yet? Should I perform further actions for this Binding?
Thankx in advance for answering.

...hab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusRolHandler.java

...b.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/handler/QbusSceneHandler.java

fwolter · 2021-03-02T20:51:33Z

...binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCommunication.java

+
+        try {
+            startCommunication();
+        } catch (InterruptedException | IOException e) {


Can you work on this?

...binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCommunication.java

Updated changes as requested by fwolter on 2/3/2021 Signed-off-by: Koen Schockaert <[email protected]>

Qbus-Koen · 2021-03-23T09:53:12Z

Dear @fwolter,
I took some time to rework this binding. You will probably notice a lot of changes. When i test the binding now, it works great ;-).
Can you be so kind to do antother review please?

....openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/QbusBridgeHandler.java

...g.openhab.binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCO2.java

...binding.qbus/src/main/java/org/openhab/binding/qbus/internal/protocol/QbusCommunication.java

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

Updates changes as requested by FWolter on 27/03/2021 Signed-off-by: Koen Schockaert <[email protected]>

Qbus-Koen · 2021-03-28T05:02:27Z

@fwolter as requested i updated the binding.
Can you do another check when you find the time?
Many thankx!

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

Update requested as suggested by FWolter on 28/03/2021 Signed-off-by: Koen Schockaert <[email protected]>

fwolter

LGTM

Now it's time to wait for another review, before this can be merged. Please don't add any new features in the meantime.

Qbus-Koen · 2021-03-28T12:29:08Z

Great, thank you very much for your patience and your great advicei I really appreciete this!
What do you mean with another review? Does one of your collegue's will check this to? Or do I have to ask users to test the binding and give comments?

fwolter · 2021-03-28T14:21:48Z

Every new addon needs to be reviewed by two openHAB addons maintainers. The cre label indicates that a second review is pending. You probably need some patience as the backlog is quite full.

fwolter · 2021-04-11T17:22:22Z

Due to lack of review capacity, the rules for new bindings were weakened. New bindings need only the approval of one reviewer instead of two from now on.

Confectrician · 2021-04-12T17:37:06Z

I think this PR broke the website build, since it attempts to load a huge logo file in a doc folder.

Maybe the codeowner can remove this jpg from the readme (and repo) and provide a smaller (not 3500 x 2400 px please) logo picture as a pull request on the openHAB docs repository.

Here is an example of how to add a logo to the documentation properly:
openhab/openhab-docs#1523

The file must be named like the bundle name. "qbus" in this case.

Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: John Marshall <[email protected]>

Signed-off-by: Koen Schockaert <[email protected]>

Qbus-Koen requested a review from a team as a code owner December 1, 2020 10:35

fwolter added the new binding If someone has started to work on a binding. For a new binding PR. label Dec 1, 2020

fwolter requested changes Dec 13, 2020

View reviewed changes

fwolter self-assigned this Dec 13, 2020

fwolter requested changes Jan 3, 2021

View reviewed changes

Qbus-Koen added 9 commits February 10, 2021 08:56

First commit

3970141

Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: QbusKoen <[email protected]>

Revert "First commit"

f41c02e

This reverts commit da86aae. Signed-off-by: QbusKoen <[email protected]>

First Commit

8342d4e

Signed-off-by: QbusKoen <[email protected]>

Second commit

9507c35

Had some issues in first commit. Fixed the bugs. Signed-off-by: QbusKoen <[email protected]>

Solved issues

9de4acb

Solved issues reported by fwolter and changed a lot of code, based on the comments. Signed-off-by: QbusKoen <[email protected]> modified: CODEOWNERS Signed-off-by: QbusKoen <[email protected]>

Update README.md

ed74a2b

Signed-off-by: QbusKoen <[email protected]>

Updated requested changes

3551d12

Signed-off-by: QbusKoen <[email protected]>

Updated headers

ec2e2ac

Signed-off-by: QbusKoen <[email protected]>

Code fixing

d681e87

Update code like requested by fwolter on 3/1/2021 Signed-off-by: QbusKoen <[email protected]>

Qbus-Koen requested a review from fwolter February 10, 2021 09:14

fwolter requested changes Feb 10, 2021

View reviewed changes

Updated requested changes

96d8d83

Updated requested changes by fwolter on 10/02/2021 Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: QbusKoen <[email protected]>

fwolter requested changes Feb 23, 2021

View reviewed changes

Updated requested changes

10bbea6

Updated requested changes by fwolter on 10/02/2021 Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: QbusKoen <[email protected]>

Merge branch 'main' of https://github.com/QbusKoen/openhab-addons int…

78b7add

…o main Signed-off-by: Koen Schockaert <[email protected]>

fwolter requested changes Feb 24, 2021

View reviewed changes

Updated requested changes

f67dda3

Updated changes as requested by Fwolter on 24/02/2021 Signed-off-by: Koen Schockaert <[email protected]>

fwolter requested changes Feb 25, 2021

View reviewed changes

Updated requested changes

34536bd

Updated changes reqeusted by fwolter on 25/02/2021 Signed-off-by: Koen Schockaert <[email protected]>

fwolter requested changes Mar 2, 2021

View reviewed changes

Updated requested changes

0c551c6

Updated changes as requested by fwolter on 2/3/2021 Signed-off-by: Koen Schockaert <[email protected]>

fwolter requested changes Mar 27, 2021

View reviewed changes

Update requested changes

9b408ae

Updates changes as requested by FWolter on 27/03/2021 Signed-off-by: Koen Schockaert <[email protected]>

fwolter requested changes Mar 28, 2021

View reviewed changes

bundles/org.openhab.binding.qbus/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved

Update thing-types.xml

ce46112

Update requested as suggested by FWolter on 28/03/2021 Signed-off-by: Koen Schockaert <[email protected]>

fwolter approved these changes Mar 28, 2021

View reviewed changes

fwolter added the cre Coordinated Review Effort label Mar 28, 2021

fwolter merged commit 16ffeec into openhab:main Apr 11, 2021

fwolter removed the cre Coordinated Review Effort label Apr 11, 2021

fwolter added this to the 3.1 milestone Apr 11, 2021

Qbus-Koen mentioned this pull request Apr 13, 2021

[qbus] - Update logo / Small code changes #10514

Merged

themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021

[Qbus] Initial contribution (openhab#9191)

e0d29e0

Signed-off-by: Koen Schockaert <[email protected]> Signed-off-by: John Marshall <[email protected]>

computergeek1507 pushed a commit to computergeek1507/openhab-addons that referenced this pull request Jul 13, 2021

[Qbus] Initial contribution (openhab#9191)

084ca52

Signed-off-by: Koen Schockaert <[email protected]>

thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021

[Qbus] Initial contribution (openhab#9191)

0384317

Signed-off-by: Koen Schockaert <[email protected]>

marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022

[Qbus] Initial contribution (openhab#9191)

21226fb

Signed-off-by: Koen Schockaert <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Qbus] Initial contribution #9191

[Qbus] Initial contribution #9191

Qbus-Koen commented Dec 1, 2020

fwolter commented Dec 8, 2020

Qbus-Koen commented Dec 9, 2020

fwolter commented Dec 9, 2020

Qbus-Koen commented Dec 9, 2020

fwolter left a comment •

edited

Loading

Qbus-Koen commented Jan 3, 2021

fwolter commented Jan 3, 2021

fwolter left a comment

Qbus-Koen commented Jan 9, 2021

fwolter commented Jan 19, 2021

Qbus-Koen commented Feb 9, 2021

fwolter left a comment

fwolter Feb 10, 2021

fwolter Feb 23, 2021

fwolter commented Feb 10, 2021

Qbus-Koen commented Feb 23, 2021

fwolter left a comment

fwolter Feb 23, 2021

fwolter left a comment

fwolter Feb 24, 2021

Qbus-Koen commented Feb 24, 2021

fwolter Feb 25, 2021

Qbus-Koen commented Feb 25, 2021

Qbus-Koen commented Mar 2, 2021

fwolter Mar 2, 2021

Qbus-Koen commented Mar 23, 2021

Qbus-Koen commented Mar 28, 2021

fwolter left a comment

Qbus-Koen commented Mar 28, 2021

fwolter commented Mar 28, 2021

fwolter commented Apr 11, 2021

Confectrician commented Apr 12, 2021

[Qbus] Initial contribution #9191

[Qbus] Initial contribution #9191

Conversation

Qbus-Koen commented Dec 1, 2020

fwolter commented Dec 8, 2020

Qbus-Koen commented Dec 9, 2020

fwolter commented Dec 9, 2020

Qbus-Koen commented Dec 9, 2020

fwolter left a comment • edited Loading

Choose a reason for hiding this comment

Qbus-Koen commented Jan 3, 2021

fwolter commented Jan 3, 2021

fwolter left a comment

Choose a reason for hiding this comment

Qbus-Koen commented Jan 9, 2021

fwolter commented Jan 19, 2021

Qbus-Koen commented Feb 9, 2021

fwolter left a comment

Choose a reason for hiding this comment

fwolter Feb 10, 2021

Choose a reason for hiding this comment

fwolter Feb 23, 2021

Choose a reason for hiding this comment

fwolter commented Feb 10, 2021

Qbus-Koen commented Feb 23, 2021

fwolter left a comment

Choose a reason for hiding this comment

fwolter Feb 23, 2021

Choose a reason for hiding this comment

fwolter left a comment

Choose a reason for hiding this comment

fwolter Feb 24, 2021

Choose a reason for hiding this comment

Qbus-Koen commented Feb 24, 2021

fwolter Feb 25, 2021

Choose a reason for hiding this comment

Qbus-Koen commented Feb 25, 2021

Qbus-Koen commented Mar 2, 2021

fwolter Mar 2, 2021

Choose a reason for hiding this comment

Qbus-Koen commented Mar 23, 2021

Qbus-Koen commented Mar 28, 2021

fwolter left a comment

Choose a reason for hiding this comment

Qbus-Koen commented Mar 28, 2021

fwolter commented Mar 28, 2021

fwolter commented Apr 11, 2021

Confectrician commented Apr 12, 2021

fwolter left a comment •

edited

Loading