openhab-addons
openhab-addons copied to clipboard
[haywardomnilogic] Added support ColorLogic V2 Lights, Updated Chlor Enable, Alert, Error, Status
Added support for ColorLogic V2 Lights (Speed & Brightness) Updated Chlor Enable based on updated Hayward API Updated Chlor Alert, Error & Status to be string bit arrays based on new API intel Updated miscellaneous units and patterns Updated getTelemetry poll time based on Hayward API recommendations
https://community.openhab.org/t/hayward-omnilogic-pool-automation-binding/104105/119?u=matchews
This pull request has been mentioned on openHAB Community. There might be relevant details there:
https://community.openhab.org/t/hayward-omnilogic-pool-automation-binding/104105/119
Please note there is a file conflict to resolve.
Please note there is a file conflict to resolve.
Resolved.
My apologies for having all changes in a single commit. My local repo blew up.
Anyone available to review this?
@lolodomo would you be able to proceed with the review? If you are too busy, i can spend some time on this if needed.
Some comments to fix. Mosty looks good, did you test the upgrade instructions? That is the last requirement i think before it can be merged.
Good catch! I tested (and fixed) the upgrade instructions. Is the breaking change alert no longer needed?
https://github.com/openhab/openhab-addons/pull/16733/
From: lsiepel @.> Sent: Thursday, May 9, 2024 7:09 AM To: openhab/openhab-addons @.> Cc: Matt @.>; State change @.> Subject: Re: [openhab/openhab-addons] [haywardomnilogic] Add support for ColorLogic V2 Lights, Update Chlor Enable, Alert, Error, Status (PR #15478)
@lsiepel commented on this pull request.
In bundles/org.openhab.binding.haywardomnilogic/src/main/resources/OH-INF/thing/chlorinator.xml https://github.com/openhab/openhab-addons/pull/15478#discussion_r1595308505 :
@@ -51,7 +50,7 @@
Number:Dimensionless Current salt output setting for the chlorinator (%).
-
<state min="0" max="100" step="1.0" pattern="%d %unit%" readOnly="false"/>
-
<state min="0" max="100" step="1.0" pattern="%d %" readOnly="false"/>
Ah, sorry i missed this open comment and allready merged it. If you can create a follow up PR with this fix i can merge it really soon.
— Reply to this email directly, view it on GitHub https://github.com/openhab/openhab-addons/pull/15478#discussion_r1595308505 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQXZPSZBSOE2FZWFCQGPULZBNKK5AVCNFSM6AAAAAA3ZZGYSSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANBXG4YTMNBSGM . You are receiving this because you modified the open/close state. https://github.com/notifications/beacon/ACQXZPWPTJ4TWKBUBBJZS7LZBNKK5A5CNFSM6AAAAAA3ZZGYSSWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTT2BWWEO.gif Message ID: @.*** @.***> >
-- This email has been checked for viruses by AVG antivirus software. www.avg.com
Some comments to fix. Mosty looks good, did you test the upgrade instructions? That is the last requirement i think before it can be merged.
Good catch! I tested (and fixed) the upgrade instructions. Is the breaking change alert no longer needed?
I don't see any breaking changes that should be mentioned. The thing upgrade instructions should do it's work.