openhab-addons icon indicating copy to clipboard operation
openhab-addons copied to clipboard

[haywardomnilogic] Added support ColorLogic V2 Lights, Updated Chlor Enable, Alert, Error, Status

Open matchews opened this issue 1 year ago • 7 comments

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

matchews avatar Aug 22 '23 11:08 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

openhab-bot avatar Aug 23 '23 14:08 openhab-bot

Please note there is a file conflict to resolve.

lolodomo avatar Oct 18 '23 06:10 lolodomo

Please note there is a file conflict to resolve.

Resolved.

matchews avatar Oct 27 '23 20:10 matchews

My apologies for having all changes in a single commit. My local repo blew up.

matchews avatar Nov 21 '23 20:11 matchews

Anyone available to review this?

matchews avatar Dec 23 '23 15:12 matchews

@lolodomo would you be able to proceed with the review? If you are too busy, i can spend some time on this if needed.

lsiepel avatar Feb 25 '24 13:02 lsiepel

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?

matchews avatar Apr 09 '24 21:04 matchews

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

matchews avatar May 09 '24 23:05 matchews

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.

lsiepel avatar May 10 '24 17:05 lsiepel