[siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging #7769

andrewfg · 2020-05-24T10:41:55Z

ENHANCED all classes are now declared @NonNullByDefault and the code has been refactored throughout to eliminate all null related compiler warnings.
ENHANCED to comply with null annotation rules, the point type classes previously embedded in the RdsDataPoints class have been refactored into individual classes of their own.
NEW JUnit test suite added.
BUG FIX as a result of JUnit testing, the JSON de-serializer needed to be extensively rewritten to properly parse all known test cases; it is also faster.
ENHANCED improvements in the clarity of logging messages.
BUG FIX added support for Units of Measure on reading from, and writing to, the cloud server; temperature values have been changed from DecimalType to QuantityType.
BUG FIX characters for the temperature degree sign have been recoded as escaped Unicode characters (since previously the spotless formatter was messing things up).
ENHANCED many methods and variables have been renamed for better self- documentation of the code.
ENHANCED the configuration parameter for polling interval has been moved to advanced level to avoid confusing regular users.
BUG FIX the present priority of data points was not being refreshed.
BUG FIX in case of unexpected errors, raise an exception instead of returning dummy values; and catch the exceptions at higher levels.
ENHANCED faster search for data points by point class; it was done by iteration, but now done (faster) through a second hash table lookup.

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

…toring; logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

TravisBuddy · 2020-05-24T10:50:08Z

Travis tests were successful

Hey @andrewfg,
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

Thanks for doing all this. I really love a lot of these changes.
Here are some things I noticed from my first pass.

...penhab.binding.siemensrds/src/test/java/org/openhab/binding/siemensrds/test/RdsTestData.java

...ding.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsHandlerFactory.java

...ng.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsDiscoveryService.java

...b.binding.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsDebouncer.java

....binding.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsDataPoints.java

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

TravisBuddy · 2020-05-25T17:51:04Z

Travis tests were successful

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

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

TravisBuddy · 2020-05-26T10:54:44Z

Travis tests were successful

Hey @andrewfg,
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

final changes

...inding.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsCloudHandler.java

...hab.binding.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsHandler.java

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

TravisBuddy · 2020-05-26T16:50:46Z

Hey @andrewfg,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 0a209610-9f71-11ea-a34f-dd9ab62511aa

andrewfg · 2020-05-26T17:03:46Z

I don't know why Travis errored :( but I think it wasn't anything to do with my code :)

cpmeister · 2020-05-26T17:31:09Z

The travis error wasn't caused by you, builds have been unstable lately for various reasons. Don't worry about it.

cpmeister

LGTM

cpmeister · 2020-05-26T17:33:34Z

Just waiting to see if the jenkins build succeeds.

andrewfg · 2020-05-26T19:24:55Z

Great news! Many thanks @cpmeister for your support!

andrewfg · 2020-05-28T13:08:12Z

Hi @cpmeister in your review of #7767 you suggested me to use QuantityType Percent on one channel; and this reminded me that I need to do the same for this [siemensrds] binding.

I have made a three line code modification, but I don't know how to add it -- either a) add to this already merged PR, of b) create a new micro PR for it.

The reason why I ask the question is that you merged this #7769 PR into 2.5.6 but if I try to create a new PR it currently can only be based on 2.5.x where the #7769 changes have not (yet) appeared.

cpmeister · 2020-05-28T16:18:48Z

Please create a new PR for those new changes.

andrewfg · 2020-05-28T16:36:14Z

When will the changes that you merged from this PR actually become visible in the 2.5.x branch? I don’t see the merged changes yet, and so if I try make a new PR the diff shows everything that was changed since May 24th (including the so called already merged changes).

andrewfg · 2020-05-28T17:45:02Z

When will the changes that you merged from this PR actually become visible in the 2.5.x branch?

Ok, I think it is solved. Probably I had some Git synchronization issue, but I resolved it by creating a clean new branch on my repo.

The new PR is here #7814

* 2.5.x: (174 commits) [hpprinter] Add additional data points and refactoring (openhab#7805) [neohub] new/legacy API; null annotations; enhancements; bugs; logging (openhab#7767) [meteoalerte] Initial contribution (openhab#7200) [lgwebos] Console command to show the access key (openhab#7801) [hue] Refactored state handling and fix polling after command (openhab#7518) [telegram] add attachment URL (openhab#7816) [siemensrds] readme adjusted to match openhab#7814 (openhab#7819) [lametrictime] correctly parse response (openhab#7818) [Seneye] Bug fix for using Pond or Home sensors. (openhab#7797) [siemensrds] apply UoM quantityType percent for relative humidity (openhab#7814) [alarmdecoder] Add vzone thing for virtual zone control (openhab#7800) [hue] Channel alert added for groups (openhab#7810) [hue] Keep compatibility with hue emulation for groups (openhab#7809) [dscalarm] Bridge/things management refactored (openhab#7748) [avmfritz] Add link to Fensterkontakt (magnetisch) to docs (openhab#7806) [deconz] add light/blind support and additional sensors (openhab#7608) [homekit] add support for min/max values for temperature (openhab#7782) [tesla] Use CXF JAX-RS client builder, if available (openhab#7804) [mqtt.homie] Improve Homie discovery time (openhab#7760) [siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging (openhab#7769) ...

…ring; logging (openhab#7769) * [siemensrds] null annotations; JUnit; U-o-M; enhancements; bug; refactoring; logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

…ring; logging (openhab#7769) * [siemensrds] null annotations; JUnit; U-o-M; enhancements; bug; refactoring; logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch> Signed-off-by: CSchlipp <christian@schlipp.de>

…ring; logging (openhab#7769) * [siemensrds] null annotations; JUnit; U-o-M; enhancements; bug; refactoring; logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

…ring; logging (openhab#7769) * [siemensrds] null annotations; JUnit; U-o-M; enhancements; bug; refactoring; logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch> Signed-off-by: Daan Meijer <daan@studioseptember.nl>

…ring; logging (openhab#7769) * [siemensrds] null annotations; JUnit; U-o-M; enhancements; bug; refactoring; logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

[siemensrds] null annotations; JUnit; U-o-M; enhancements; bug; refac…

e361d34

…toring; logging Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

andrewfg added bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels May 24, 2020

cpmeister requested changes May 25, 2020

View reviewed changes

[siemensrds] modifications requested in code review by @cpmeister

dfa93af

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

[siemensrds] adopted code review suggestions by @cpmeister

27eed7c

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

cpmeister requested changes May 26, 2020

View reviewed changes

...inding.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsCloudHandler.java Outdated Show resolved Hide resolved

...hab.binding.siemensrds/src/main/java/org/openhab/binding/siemensrds/internal/RdsHandler.java Outdated Show resolved Hide resolved

[siemensrds] removed orphan semi- colons

ae9a4f6

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

cpmeister approved these changes May 26, 2020

View reviewed changes

cpmeister merged commit bfe0feb into openhab:2.5.x May 26, 2020

cpmeister added this to the 2.5.6 milestone May 26, 2020

cpmeister removed the bug An unexpected problem or unintended behavior of an add-on label May 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging #7769

[siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging #7769

andrewfg commented May 24, 2020

TravisBuddy commented May 24, 2020

cpmeister left a comment

TravisBuddy commented May 25, 2020

TravisBuddy commented May 26, 2020

cpmeister left a comment

TravisBuddy commented May 26, 2020

andrewfg commented May 26, 2020

cpmeister commented May 26, 2020

cpmeister left a comment

cpmeister commented May 26, 2020

andrewfg commented May 26, 2020

andrewfg commented May 28, 2020

cpmeister commented May 28, 2020

andrewfg commented May 28, 2020 •

edited

Loading

andrewfg commented May 28, 2020

[siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging #7769

[siemensrds] null annotations; JUnit; UoM; enhancements; bug; refactoring; logging #7769

Conversation

andrewfg commented May 24, 2020

TravisBuddy commented May 24, 2020

Travis tests were successful

cpmeister left a comment

Choose a reason for hiding this comment

TravisBuddy commented May 25, 2020

Travis tests were successful

TravisBuddy commented May 26, 2020

Travis tests were successful

cpmeister left a comment

Choose a reason for hiding this comment

TravisBuddy commented May 26, 2020

TravisBuddy Request Identifier: 0a209610-9f71-11ea-a34f-dd9ab62511aa

andrewfg commented May 26, 2020

cpmeister commented May 26, 2020

cpmeister left a comment

Choose a reason for hiding this comment

cpmeister commented May 26, 2020

andrewfg commented May 26, 2020

andrewfg commented May 28, 2020

cpmeister commented May 28, 2020

andrewfg commented May 28, 2020 • edited Loading

andrewfg commented May 28, 2020

andrewfg commented May 28, 2020 •

edited

Loading