[haywardomnilogic] Hayward Omnilogic Pool Automation Binding

[matchews]

Signed-off-by: Matt Myers [email protected]

Hayward Omnilogic binding initial contribution

The Hayward Omnilogic binding integrates the Omnilogic pool controller using the Hayward API.

[TravisBuddy]

Travis tests were successful

Hey @matchews,
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]

Travis tests were successful

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

[matchews]

Forum discussion...
https://community.openhab.org/t/hayward-omnilogic-pool-automation-binding/104105

[fwolter]

Thanks for your contribution! I reviewed your code and here is my feedback.

To add your binding to the official distribution, you need to add your binding to bundles/pom.xml and bom/openhab-addons/pom.xml.

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

There are some formatting issues. You can view them with mvn spotless:check -Dspotless.check.skip=false and fix them with mvn spotless:apply.

[TravisBuddy]

Travis tests were successful

Hey @matchews,
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]

Travis tests were successful

Hey @matchews,
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]

Travis tests were successful

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

[fwolter]

This was a general Jenkins issue, which has been fixed some hours ago.

[TravisBuddy]

Travis tests were successful

Hey @matchews,
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]

Travis tests were successful

Hey @matchews,
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]

Travis tests were successful

Hey @matchews,
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]

Travis tests were successful

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

[fwolter]

Tried running this outside of the IDE on my 2.5.4 RasPi and got the same results plus another hint in the logs:

21:47:52.984 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:01.000-0500' of item PoolChlorOperatingMode with format '%.0f': f != java.time.ZonedDateTime

This is really strange as you haven't configured a pattern of %.0f nor use ZonedDateTime in your code. What does the debugger say when updating the chlorOperatingMode Channel?

[TravisBuddy]

Travis tests have failed

Hey @matchews,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

[matchews]

Tried running this outside of the IDE on my 2.5.4 RasPi and got the same results plus another hint in the logs:
21:47:52.984 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:01.000-0500' of item PoolChlorOperatingMode with format '%.0f': f != java.time.ZonedDateTime

This is really strange as you haven't configured a pattern of %.0f nor use ZonedDateTime in your code. What does the debugger say when updating the chlorOperatingMode Channel?

I can't explain why, but this is now working as intended.

[fwolter]

Do you request another review, when you are finished making the changes?

[matchews]

Do you request another review, when you are finished making the changes?

Not yet. I need to clean up the polling and also need to figure out the Jenkins failure. I'm also getting dates again in my thing.xml items with options on my rasPi working OpenHab instance, but not in the Eclipse IDE.

19:14:18.177 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:01.000-0500' of item PoolBackyardState with format '%.0f': f != java.time.ZonedDateTime 19:14:18.182 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:00.000-0500' of item PoolBOWFlow with format '%.0f': f != java.time.ZonedDateTime 19:14:18.189 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:00.000-0500' of item PoolFilterState with format '%.0f': f != java.time.ZonedDateTime 19:14:19.657 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:00.000-0500' of item PoolHeaterState with format '%.0f': f != java.time.ZonedDateTime 19:14:19.662 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:02.000-0500' of item PoolLightCurrentShow with format '%1f': f != java.time.ZonedDateTime 19:14:19.670 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:02.000-0500' of item PoolLightCurrentShow with format '%1f': f != java.time.ZonedDateTime 19:14:19.673 [WARN ] [.ui.internal.items.ItemUIRegistryImpl] - Exception while formatting value '1969-12-31T19:00:02.000-0500' of item PoolLightCurrentShow with format '%1f': f != java.time.ZonedDateTime

[fwolter]

Sounds like some kind of caching problem on your raspi. The Jenkins build seems to fail due to a general issue.

[Hilbrand]

Thanks for your contribution. I've reviewed the code and you'll get some homework 😉 My general comment is that all logic is in the bridge. You've created all handler subclasses, but these don't add anything. It would make it much more readable and easier to extend or follow the control flow if the device specific logic is done in the handlers. Also the bridge does the discovery, but this should be done by the discovery service, not the bridge. I've made some specific comments were things can be improved. But if you still have questions feel free to ask.

[openhab-bot]

Automatic code migration to openHAB 3 succeeded.

The resulting code can be found at https://ci.openhab.org/job/openHAB-Addons-Migration/29/artifact/bundles/.

You can download the migrated code from there and create a new PR against the master branch of the openhab-addons repo to contribute it for being included in openHAB 3.x.

Please see this issue about the details on how to proceed with your existing PR.

[matchews]

Thanks for your contribution. I've reviewed the code and you'll get some homework 😉 My general comment is that all logic is in the bridge. You've created all handler subclasses, but these don't add anything. It would make it much more readable and easier to extend or follow the control flow if the device specific logic is done in the handlers. Also the bridge does the discovery, but this should be done by the discovery service, not the bridge. I've made some specific comments were things can be improved. But if you still have questions feel free to ask.

Thanks for your time reviewing. The general comments are very logical, but I'm having a hard time determining how to implement. I'm trying to breakup the getTelemetry routine first. I was planning to pass the xml file to each thinghandler (if at least one of these thing types exist) to scan for any things of that type that exist and update them. It seems that you cannot call the thinghandler without creating an instance that is specific to a single thing. That would force me to scan through the XML in the bridgehandler and call the thinghandler once each thing is found. Not much of a gain in segregating the code out to the individual handers. Hopefully I'm missing the trivial solution here. I tried a static routine but that snowballed quickly. Any guidance you can offer would be greatly appreciated. Thanks!

[Hilbrand]

It seems that you cannot call the thinghandler without creating an instance that is specific to a single thing. That would force me to scan through the XML in the bridgehandler and call the thinghandler once each thing is found.

I'm not sure what you mean by this? Why do you need to create an instance?
You can iterate over the things (getThing().getThings()) in the bridge and on each thing get the handler and pass the xml to that handler. The handler can parse the thing specific data out of the xml.