[opensprinkler] Improvements: Remove apache.commons, fix bugs and warnings #9869

Skinah · 2021-01-19T01:58:53Z

Remove: apache commons library #7722
Refactor: Auto Discovery to improve experience and now uses withRepresentationProperty.
Refactor: All classes now use nonNullByDefault and fixed many compiler warnings from this change.
Refactor: Use a single file for all constants to make it easier to find them.
Fix: Online and Offline detection was not working correctly. #8942
Fix: Password could get MD5 hashed twice in a row causing the binding to stop working.
Fix: Current sensing channel was not getting displayed in OH3 due to being added on the fly to the thing. #9664
Fix: Many NPE errors were occurring and not getting caught, reported on forum.
Fix: java.lang.IndexOutOfBoundsException When the station Index is higher than the total number of stations.
Improve: Added bindings ability to report to the user when the password was wrong and not crash with a NPE.
Improve: Added new RSSI signalStrength channel.
Improve: Added new flowSensorCount channel so openHAB can see when the flow sensor detects water or not.
Improve: Added new channel programs to allow a selectable list of programs to be displayed and run.
Improve: Added new channel stations to allow a selectable list of stations to be displayed and run.
Improve: Added new channel rainDelay.
Improve: Added new channel resetStations to stop all stations and remove everything from the queue.
Improve: Added new channel enablePrograms.

Signed-off-by: Matthew Skinner matt@pcmus.com

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

...er/src/main/java/org/openhab/binding/opensprinkler/internal/api/OpenSprinklerApiFactory.java

FlorianSW

Apart from that I didn't see anything, yet, where I would say it should block the merge. However, two caveats, though:

I don't have an easy way anymore to check if the binding still works as expected with a real opensprinkler device :/
This PR got a bit big :D It's hard to review it in one shoot tbh. This is, however, only a side note and maybe next time we can split up the PR in multiple smaller ones :-)

Thanks for your contribution!

.../src/main/java/org/openhab/binding/opensprinkler/internal/OpenSprinklerBindingConstants.java

Skinah · 2021-01-30T23:32:10Z

Thank you for looking it over.

I have posted on the forum asking for testers and I am confident that there are no breaking changes introduced and will still work the exact same way, only with extended functionality. A few users have reported it works already.
Yes it did get large but I feel it was worth it to address the code analysis and compiler warnings to bring it up to current standards to be merged.

Skinah · 2021-02-04T04:57:30Z

All requested changes have been addressed.

Signed-off-by: Matthew Skinner <matt@pcmus.com>

…enhab/binding/opensprinkler/internal/discovery/OpenSprinklerDiscoveryService.java Signed-off-by: Matthew Skinner <matt@pcmus.com> Co-authored-by: Connor Petty <mistercpp2000@gmail.com>

Signed-off-by: Matthew Skinner <matt@pcmus.com>

fwolter · 2021-07-10T12:14:41Z

What's the reason for the force-push? That lead to marking the whole code as changed. So, what did you really change?

Skinah · 2021-07-11T05:15:11Z

What's the reason for the force-push? That lead to marking the whole code as changed. So, what did you really change?

I did a rebase with openhab main and changed it to 3.2snapshot so it will be fine to merge now that 3.1 stable is released. I also did one small change as a separate commit after the force push to remove a suppress warnings. This can be seen as it is a separate commit.

fwolter

OK! LGTM

…nings (openhab#9869) * Fix: current channel would not get added. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Fix: online and offline now detect correctly. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Improve discovery. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Bug fixes. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Misc Improvements and log cleanup. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Make current sensor channel dynamically get removed if not supported. Signed-off-by: Matthew Skinner <matt@pcmus.com> * fix compiler warning. Signed-off-by: Matthew Skinner <matt@pcmus.com> * readme updates. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Shift refresh() Signed-off-by: Matthew Skinner <matt@pcmus.com> * Add RSSI and water flow meter count channels. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Change to flowSensorCount Signed-off-by: Matthew Skinner <matt@pcmus.com> * Fix bug Signed-off-by: Matthew Skinner <matt@pcmus.com> * Fix NPE on parsing jsReplies. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Add new programs channel. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Fix wrong api use. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Change to using max time if the time is null. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Add device withRepresentationProperty Signed-off-by: Matthew Skinner <matt@pcmus.com> * update readme Signed-off-by: Matthew Skinner <matt@pcmus.com> * Fix rain sensor not working in 219 firmware. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Spotless fixes. Signed-off-by: Matthew Skinner <matt@pcmus.com> * fix gson double up error. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Add more channels Signed-off-by: Matthew Skinner <matt@pcmus.com> * refresh completed before bridge goes online. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Simplify. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Allow non default password to not stop discovery. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Add ignoresRain channel Signed-off-by: Matthew Skinner <matt@pcmus.com> * Simplify commands in station handler. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Add new rainDelay channel. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Make constants consistent. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Add categories. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Changes requested by FlorianSW Signed-off-by: Matthew Skinner <matt@pcmus.com> * Stop catching and throwing new exp. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Create a state class. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Update bundles/org.openhab.binding.opensprinkler/src/main/java/org/openhab/binding/opensprinkler/internal/discovery/OpenSprinklerDiscoveryService.java Signed-off-by: Matthew Skinner <matt@pcmus.com> Co-authored-by: Connor Petty <mistercpp2000@gmail.com> * Fix for EOF exception that stops polling with bad wifi. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Change signal strength over to 0-4 range Signed-off-by: Matthew Skinner <matt@pcmus.com> * Handle toUnit null returns. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Support discovery on subnets that are not /24 Signed-off-by: Matthew Skinner <matt@pcmus.com> * update readme for recent changes to signalStrength channel. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Fix code anal issue and readme. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Remove info logging. Signed-off-by: Matthew Skinner <matt@pcmus.com> * remove traces of old dependancy. Signed-off-by: Matthew Skinner <matt@pcmus.com> * Changes for fwolter Signed-off-by: Matthew Skinner <matt@pcmus.com> * support RSSI and rssi Signed-off-by: Matthew Skinner <matt@pcmus.com> * Update after new command sent. Signed-off-by: Matthew Skinner <matt@pcmus.com> * improve delayedRefresh Signed-off-by: Matthew Skinner <matt@pcmus.com> * Remove supressWarnings Signed-off-by: Matthew Skinner <matt@pcmus.com> Co-authored-by: Connor Petty <mistercpp2000@gmail.com>

Skinah requested review from CrackerStealth and FlorianSW as code owners January 19, 2021 01:58

Skinah added bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels Jan 19, 2021

This was linked to issues Jan 19, 2021

[opensprinkler] Offline detection does not work reliably #8942

Closed

[opensprinkler] Opensprinkler currentDraw channel not appearing for "device" thing in OH3 despite being available in OH2 #9664

Closed

Skinah added the work in progress A PR that is not yet ready to be merged label Jan 19, 2021

wborn added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 19, 2021

Skinah added rebuild Triggers Jenkins PR build and removed work in progress A PR that is not yet ready to be merged rebuild Triggers Jenkins PR build labels Jan 20, 2021

FlorianSW requested changes Jan 20, 2021

View reviewed changes

wborn added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 23, 2021

Skinah added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 23, 2021

Skinah requested a review from FlorianSW January 26, 2021 01:55

FlorianSW reviewed Jan 30, 2021

View reviewed changes

.../src/main/java/org/openhab/binding/opensprinkler/internal/OpenSprinklerBindingConstants.java Show resolved Hide resolved

Hilbrand requested a review from FlorianSW February 6, 2021 17:04

Skinah and others added 14 commits July 3, 2021 16:17

Create a state class.

3e933f1

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Update bundles/org.openhab.binding.opensprinkler/src/main/java/org/op…

aa83d13

…enhab/binding/opensprinkler/internal/discovery/OpenSprinklerDiscoveryService.java Signed-off-by: Matthew Skinner <matt@pcmus.com> Co-authored-by: Connor Petty <mistercpp2000@gmail.com>

Fix for EOF exception that stops polling with bad wifi.

0280571

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Change signal strength over to 0-4 range

4242f3b

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Handle toUnit null returns.

290c5c5

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Support discovery on subnets that are not /24

6b5d435

Signed-off-by: Matthew Skinner <matt@pcmus.com>

update readme for recent changes to signalStrength channel.

120119b

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Fix code anal issue and readme.

b76d76c

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Remove info logging.

710c0ee

Signed-off-by: Matthew Skinner <matt@pcmus.com>

remove traces of old dependancy.

fd67b5a

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Changes for fwolter

6ec36a3

Signed-off-by: Matthew Skinner <matt@pcmus.com>

support RSSI and rssi

6f2709b

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Update after new command sent.

ae2a253

Signed-off-by: Matthew Skinner <matt@pcmus.com>

improve delayedRefresh

f470809

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Skinah force-pushed the opensprinkler branch from e6181d5 to f470809 Compare July 3, 2021 06:18

Remove supressWarnings

7b90d86

Signed-off-by: Matthew Skinner <matt@pcmus.com>

Skinah added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jul 4, 2021

Skinah added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jul 11, 2021

fwolter approved these changes Jul 11, 2021

View reviewed changes

fwolter merged commit 6fc24e4 into openhab:main Jul 11, 2021

fwolter added this to the 3.2 milestone Jul 11, 2021

Skinah deleted the opensprinkler branch August 8, 2021 02:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[opensprinkler] Improvements: Remove apache.commons, fix bugs and warnings #9869

[opensprinkler] Improvements: Remove apache.commons, fix bugs and warnings #9869

Skinah commented Jan 19, 2021 •

edited

Loading

FlorianSW left a comment

Skinah commented Jan 30, 2021

Skinah commented Feb 4, 2021

fwolter commented Jul 10, 2021

Skinah commented Jul 11, 2021

fwolter left a comment

[opensprinkler] Improvements: Remove apache.commons, fix bugs and warnings #9869

[opensprinkler] Improvements: Remove apache.commons, fix bugs and warnings #9869

Conversation

Skinah commented Jan 19, 2021 • edited Loading

FlorianSW left a comment

Choose a reason for hiding this comment

Skinah commented Jan 30, 2021

Skinah commented Feb 4, 2021

fwolter commented Jul 10, 2021

Skinah commented Jul 11, 2021

fwolter left a comment

Choose a reason for hiding this comment

Skinah commented Jan 19, 2021 •

edited

Loading