[ISM8] New binding for Wolf heating systems using an ISM8 interface #6336

Hans-Reiner · 2019-11-03T13:37:52Z

[ISM8] Initial contribution

I was looking into the available options to integrate my heating system into OpenHab. As I didn't want to place own hardware modifications or 3rd party interfaces into my heating system, I decided to create a binding communicating with the original Wolf interface. The ISM8 interface sends a set of parameters and values to a partner using standard TCP/IP protocols.

With this binding, you are able to integrate your Wolf heating system. This allows you to check the temperature values as well as changing set points.

As this is my first new binding as well as I'm usually not working with Java, I'd like to ask you to contribute as well in the development of this binding if this is in any interest..

TravisBuddy · 2019-11-03T13:42:00Z

Travis tests were successful

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

TravisBuddy · 2019-11-03T14:38:50Z

Travis tests were successful

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

cpmeister

Before I get too far in the review I just want you to look at openhab's coding guidelines and make sure that you adhere to those guidelines as much as possible.
There are several places that you are going against the guidelines for logging levels and the naming guidelines.
You should avoid catching Exceptions and instead should catch specific exceptions.
Also if you can please format the code such that you minimize nested indentations.

Once you have addressed as much as you can please ping me for a review again.

cpmeister · 2020-03-21T03:20:28Z

...es/org.openhab.binding.ism8/src/main/java/org/openhab/binding/ism8/internal/Ism8Handler.java

+                        } catch (Exception e) {
+                        }


What are you expecting to catch here?

Basically nothing, but Eclipse initially requested this in order to build it successfully. I could solve this by reviewing the Exception handling as previously were a lot of methods decorated by the ‘throws Exception’ statement. I removed them at all possible methods and only a few had to stay.

cpmeister · 2020-03-21T03:22:26Z

...es/org.openhab.binding.ism8/src/main/java/org/openhab/binding/ism8/server/DataPointBase.java

+    private int __Id;
+    private String __KnxDataType = new String();
+    private String __Description = new String();
+    private T __Value;
+    private String __Unit = new String();


Please make sure that all field and variable names are camelcase.

Also, instead of new String() just use an empty string ""

cpmeister · 2020-03-21T03:26:47Z

...es/org.openhab.binding.ism8/src/main/java/org/openhab/binding/ism8/internal/Ism8Handler.java

+        logger.debug("Ism8: Channel Link {}", channelUID.getId());
+    }
+
+    @SuppressWarnings("null")


Please remove all warning suppressions from your code

I removed the suppressions, but received afterwards a few warnings in Eclipse. There were null-checks in an IF statement, but I could only solve it by copying the object reference in a local variable. Please let me know if this is the right way (e.g. Ism8Handler.java line 61, 69, 78).

TravisBuddy · 2020-04-10T22:54:32Z

Travis tests were successful

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

TravisBuddy · 2020-04-11T20:35:43Z

Travis tests were successful

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

Hilbrand · 2020-09-19T13:04:15Z

Can you fix the conflict and CODEOWNERS today (19 September 2020)? So it can be shipped with 2.5.9. 2.5.9 is planned for 20 September.

ISM8 Binding added Signed-off-by: Hans-Reiner <57272516+Hans-Reiner@users.noreply.github.com> Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

…changes Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

… the UI. Removed write-blocking for read-only parameter. Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Hans-Reiner · 2020-09-19T15:31:51Z

I hope the merge was executed correctly. After the --rebase I was using git rebase --continue and checking the files after each step. Finally it was resulting back in having the latest files of the binding and there was only 2 files left to be checked in. Surprisingly now are all steps/commits listed above again. Please let me know if this was correct or what to do if not. I created manually a copy to be sure to not loose any file.

bundles/pom.xml

Hilbrand

LGTM. rebase went well. There was a indentation in the bundle pom, but I applied the fix so that should work. The build should work now.

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Hans-Reiner · 2020-09-19T16:48:33Z

Hilbrand, Fabian, many thanks for your great support, hints and to be so patient. This was my first code in Java and in contributing in an open source project. Very much appreciated.
As it looks that the code is accepted could you shortly give me a hint if there anything more I need to do now or basically what will happen next. Will this directly become part of the master?

fwolter · 2020-09-19T17:01:00Z

Good job! As two maintainers approved your code, it's ready to merge in general. When Travis and Jenkins have built the code sucessfully, your PR will be merged into the 2.5.x branch. That means your binding will be released with openHAB 2.5.9 tomorrow as part of the official openHAB distro.

kaikreuzer · 2020-09-19T20:57:29Z

Travis and Jenkins are both happy! 👍

…penhab#6336) Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Hans-Reiner requested a review from a team as a code owner November 3, 2019 13:37

Hans-Reiner force-pushed the master branch from 55ec316 to e90ba97 Compare November 3, 2019 14:35

Hans-Reiner changed the title ~~New binding for Wolf heating systems using an ISM8 interface~~ [ISM8] New binding for Wolf heating systems using an ISM8 interface Nov 3, 2019

Hilbrand added the new binding If someone has started to work on a binding. For a new binding PR. label Nov 3, 2019

cpmeister changed the base branch from master to 2.5.x March 21, 2020 01:51

cpmeister requested changes Mar 21, 2020

View reviewed changes

cpmeister added the awaiting feedback label Apr 7, 2020

Hans-Reiner force-pushed the master branch from 609ae3a to 342a37a Compare April 11, 2020 20:36

Hans-Reiner requested review from 3cky, BobRak, caffineehacker, cdjackson, Cossey, cschneider, cweitkamp, davidgraeff, digitaldan, dominicdesu, fa2k, FlorianSW, fruggy83, gbicskei, gedejong, gerrieg, Hilbrand and J-N-K as code owners April 11, 2020 20:36

Hans-Reiner and others added 18 commits September 19, 2020 17:19

Initial commit of the ISM8 Binding

7df6b1e

ISM8 Binding added Signed-off-by: Hans-Reiner <57272516+Hans-Reiner@users.noreply.github.com> Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Updated coding guidelines and exception handling

b13934d

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

POM updated to current version

0555931

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Updated after second review.

72e99d0

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Changes triggered by the review

90f30a4

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Changes triggered by the review

7ef3467

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Simplified getting the channel within the handler

50625bb

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Solved some warning and Nullable/NonNullByDefault attributes.

7e1355b

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Changed DataPointBase generic parameter to Nullable

51f9f45

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Changes triggered by the review

662638f

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Changes in styling and list handling

0a99fd9

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Changed to snapshot 2.5.9

d2b215e

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Fixing spacing violations in the bundles/pom.xml

c86ab08

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Optimized exception handling and logging.

4034d9d

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Corrected upper cases for static fields. Removed redundant null check.

aa4f8bd

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Optimized search loops, updating Readme.md and other minor requested …

d68b87f

…changes Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Update thing-type configuration in order to configure the binding via…

cd1401c

… the UI. Removed write-blocking for read-only parameter. Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Updated CODEOWNERS file and merged conflicts

28cbc7d

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Hans-Reiner force-pushed the master branch from d5c7131 to 28cbc7d Compare September 19, 2020 15:24

Hilbrand reviewed Sep 19, 2020

View reviewed changes

bundles/pom.xml Outdated Show resolved Hide resolved

Hilbrand approved these changes Sep 19, 2020

View reviewed changes

Correct indention of the bundles/pom.xml

807f482

Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Hans-Reiner force-pushed the master branch from e2be0f4 to 807f482 Compare September 19, 2020 16:42

kaikreuzer merged commit ee6537c into openhab:2.5.x Sep 19, 2020

andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Oct 8, 2020

[ISM8] New binding for Wolf heating systems using an ISM8 interface (o…

c0e335b

…penhab#6336) Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISM8] New binding for Wolf heating systems using an ISM8 interface #6336

[ISM8] New binding for Wolf heating systems using an ISM8 interface #6336

Hans-Reiner commented Nov 3, 2019

TravisBuddy commented Nov 3, 2019 •

edited

Loading

TravisBuddy commented Nov 3, 2019

cpmeister left a comment

cpmeister Mar 21, 2020

Hans-Reiner Apr 11, 2020

cpmeister Mar 21, 2020

cpmeister Mar 21, 2020

Hans-Reiner Apr 11, 2020

cpmeister Mar 21, 2020

Hans-Reiner Apr 11, 2020

TravisBuddy commented Apr 10, 2020

TravisBuddy commented Apr 11, 2020

Hilbrand commented Sep 19, 2020

Hans-Reiner commented Sep 19, 2020

Hilbrand left a comment

Hans-Reiner commented Sep 19, 2020

fwolter commented Sep 19, 2020

kaikreuzer commented Sep 19, 2020

[ISM8] New binding for Wolf heating systems using an ISM8 interface #6336

[ISM8] New binding for Wolf heating systems using an ISM8 interface #6336

Conversation

Hans-Reiner commented Nov 3, 2019

TravisBuddy commented Nov 3, 2019 • edited Loading

Travis tests were successful

TravisBuddy commented Nov 3, 2019

Travis tests were successful

cpmeister left a comment

Choose a reason for hiding this comment

cpmeister Mar 21, 2020

Choose a reason for hiding this comment

Hans-Reiner Apr 11, 2020

Choose a reason for hiding this comment

cpmeister Mar 21, 2020

Choose a reason for hiding this comment

cpmeister Mar 21, 2020

Choose a reason for hiding this comment

Hans-Reiner Apr 11, 2020

Choose a reason for hiding this comment

cpmeister Mar 21, 2020

Choose a reason for hiding this comment

Hans-Reiner Apr 11, 2020

Choose a reason for hiding this comment

TravisBuddy commented Apr 10, 2020

Travis tests were successful

TravisBuddy commented Apr 11, 2020

Travis tests were successful

Hilbrand commented Sep 19, 2020

Hans-Reiner commented Sep 19, 2020

Hilbrand left a comment

Choose a reason for hiding this comment

Hans-Reiner commented Sep 19, 2020

fwolter commented Sep 19, 2020

kaikreuzer commented Sep 19, 2020

TravisBuddy commented Nov 3, 2019 •

edited

Loading