[ondilo] Initial contribution

[MikeTheTux]

[ondilo] Initial contribution

Description

New openHAB binding for Ondilo ICO Pool and Spa monitoring devices, allowing to monitor and automate a pool environment using openHAB’s rules and UI.

• ondilo.com
• Ondilo API Documentation
• openHAB Community Forum

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/request-ondilo-binding/98164/7

[lsiepel]

Thanks @MikeTheTux for another contribution from you! As review capacity is somewhat limited, I would like to ask you to perform a self-review by looking at this checklist.

[MikeTheTux]

self review completed. changes checked in ...

Check	Status Self Review
1. proper bundle name in pom.xml ("openHAB Add-ons :: Bundles :: ... Binding")	OK
2. include new bundles in build (main pom.xml) and karaf feature (in src/main/feature of the bundle)	OK
3. NOTICE: EPLv2 license with NOTICE file	OK
4. NOTICE: all dependencies must be listed in NOTICE file	OK (none)
5. README: Based on our template, same sections should be present	OK
6. README: new line after every sentence	OK
7. README: section headers should be capitalized ("Thing Configuration", not "Thing configuration")	OK
8. README: mention all thing type ids, channel ids, configuration parameter keys in the appropriate section	OK
9. i18n: only original language should be provided, translations should be managed in Crowdin.	OK
10. Check copyright year in source file header. It can be auto corrected with mvn -lp : license:format in the root of the openhab-addons project.	OK
11. Thing/Channel labels should be short (<25 chars, max 2-3 words) and capitalized	OK
12. Thing and Channel's should have semantic tags where possible	OK for Things NOK for Channels
13. conservative use of log levels (mainly debug, unless bugs to report or misconfiguration other than on things)	OK
14. handler.initialize() to return fast and set a valid/correct Thing status	OK
15. use of lambdas for runnables	OK
16. handle REFRESH commands	NOK Not implemented as it would causes >10 channel updates in a row during setup (exceeds given API quota) If you want to update the values, use the poll channel instead
17. All conversions of byte[] to String or vice versa should have the Charset specified as well. This includes converting a Input/OuputStream to a Reader/Writer.	OK
18. Sockets and Input/OutputStreams should be used in try-with-resources statements if possible.	OK
19. Make sure that @NonNullByDefault added to every class with the exception of classes with a DTO suffix or in a dto package.	OK
20. Duplicate code should be refactored where possible.	OK
21. In ThingHandler implementations, all asynchronous futures created during initialize should be cleaned up in dispose.	OK
22. Thing config parameters: Specify units where applicable (e.g. unit="s" for seconds)	OK
23. Thing config parameters: Add min/max value where applicable	OK
24. Thing config parameters: Add context tag where applicable (e.g. network-address)	OK
25. Channels declaration: Use Units of Measure (e.g. Number:Temperature)	OK
26. Non-static fields and variables: Use camel case (i.e. no underscores or prefixes in names)	OK
27. Use primitive over complex types (e.g. int vs. Integer)	OK
28. Log stack traces only on severe errors (e.g. detection of a bug)	OK
29. Don't log if a Thing goes offline due to an error. The text passed to updateStatus() is logged by the framework.	OK
30. Use checked exceptions: Extend custom exceptions from Exception	OK
31. Don't throw RuntimeException on expected errors	OK
32. Don't catch Exception unless an external method throws this exception. To handle unexpected expections catch RuntimeException. This can be helpful in repeated scheduled jobs to avoid stopping a refresh task due to temporary failure.	OK
33. Cache result of getConfigAs() and getConfiguration()	OK
34. When creating a thread, declare it as daemon: Thread.setDaemon(true)	N/R
35. Name createad threads with Thread.setName() or via Thread's constructor	N/R
36. Using synchronized on methods in a Handler should be looked at in detail because the parent class uses synchronized already on for example update of status, and this can result in dead locks.	OK
37. Specify representation property in discovery results	OK
38. Cancelling a task (Future): There's no need to check if it have been cancelled already	OK
39. Any compiler warning that can't be avoided should be annotated with @SuppressWarnings	OK
40. Check checkstyle warnings in target/code-analysis/report.html	OK
41. Check formatting issues with mvn spotless:check -Dspotless.check.skip=false	OK
42. JavaDoc can be checked with: mvn javadoc:javadoc.	OK
43. Any code that catches an IOException should also have a special catch for InterruptedIOException	OK
44. Anytime an InterruptedException or an InterruptedIOException is caught, the code must return from the current method as soon as possible unless code is running in a binding managed thread, in which case use your best judgement.	OK

[lsiepel]

Thansk for performing the self-review. Looked at 16/23 files, will continue later or tomorrow.

[Copilot]

Pull Request Overview

Initial contribution of a new openHAB binding for Ondilo ICO Pool and Spa monitoring devices, enabling OAuth2 integration, device discovery, and real-time data polling.

• Registers the ondilo module in build and feature files
• Defines thing types, channels, and translations for the binding
• Implements OAuth2 bridge handler, discovery service, handlers, API client, and DTOs

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
bundles/pom.xml	Registers the org.openhab.binding.ondilo module
src/main/resources/OH-INF/thing/thing-types.xml	Defines bridge and device thing types with channels
src/main/java/org/openhab/binding/ondilo/internal/OndiloHandler.java	Handles device polling and channel updates
src/main/java/org/openhab/binding/ondilo/internal/OndiloApiClient.java	Implements OAuth2-enabled HTTP client for Ondilo API
README.md	Provides user documentation and configuration examples

Comments suppressed due to low confidence (3)

bundles/org.openhab.binding.ondilo/README.md:76

• The example uses mowerId, which appears copy-pasted; this should reference the pool ID parameter (e.g., id or poolId).

    Thing ondilo "<id_received_from_discovery>" [ mowerId="<id_received_from_discovery>" ] {

bundles/org.openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloApiClient.java:56

• [nitpick] Consider adding unit tests for this method to cover HTTP request handling, response parsing, and token refresh logic.

    public synchronized String get(String endpoint) {

bundles/org.openhab.binding.ondilo/src/main/java/org/openhab/binding/ondilo/internal/OndiloHandler.java:157

• List does not have a getFirst() method; use recommendations.get(0) to access the first element.

                        Recommendation recommendation = recommendations.getFirst();

[lsiepel]

Only the two handlers are left to review

[lsiepel]

Looked at the remaining files. Many small style comments, but one larger design comment about the seperation fo concerns between the pool, bridge and api classes .

[MikeTheTux]

checked and fixed all editorial comments.

[MikeTheTux]

All of the findings are fixed or commented.

The main improvement which is not implemented, is the synchronization of the polling with the expected measure time. Right now, all Things are updated via a single poll-function, ensuring the API rate limit. Polling per Thing would complicate things again ...

[MikeTheTux]

5ae9efb implements:

The binding automatically adjusts the polling schedule to match the expected time of the next measurement, which is typically 1 hour (and 1 minute buffer) after the previous measurement.

[lsiepel]

Thanks for addressing the issues quickly. Besides the comment you had, i also want to add that whenever there is a transient error in the http communcation (like a timeout) the binding does not have some kind of retry mechanism. This might be fine as the polling is rather slow it might also have a big impact.

[lsiepel]

If you are able to fix the open comments before 16:00 it might be part of oh5. Otherwise i'm not sure.

[lsiepel]

Thanks for this binding and the quick follow-ups. While there are some minor things to fix, we need to merge now due to the feature freezen. Please create a follow-up PR for post fixes like the warning: #18914 (comment)

LGTM

Now, you could add the binding's logo to the openHAB website. See https://www.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website