-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[pentair] SerialBridge does not set configured id and documented default is not applied #16874
Comments
@jsjames as you have a PR pending, is this something you can confirm (and or fix?) |
@BillBaird there is a significant update coming, but that will not make it into 4.2 that will be out in a couple of weeks. I expect it to be in one of the first snapshots of 4.3 |
@BillBaird - As @lsiepel mentioned, there is a significant update to the binding coming which adds quite a few additional features and is much more stable. I verified that this issue should not exist in the new code since I restructured the code better so there is more common code between the 2 bridge types. If you want to try the new version out, you can download this file and put in your addons directory. Note, the configuration of the binding is different. The updated code/documentation is here: https://github.com/jsjames/openhab-addons/tree/pentair2 https://drive.google.com/file/d/13QnBZr2yHtC9pNmJQkOW-T33YmFvKg2y/view?usp=sharing |
Thank you @jsjames. I have reviewed your changes to the initialization of
both the IPBridge and SerialBridge handlers and your refactoring does
appear to address the issue I raised. Hopefully I can try it this weekend
and safely go back to the serial interface.
Looking quickly at the docs for the new version, I see you added support
for Pentair programs (1 through 9). I thought I had previously read that
Intellitouch systems supported up to 12 programs. I just searched for this
and see in this doc
https://www.pentair.com/content/dam/extranet/nam/pentair-pool/residential/automation/intellitouch/manual/intellitouch-pool-control-system-users-guide-english.pdf
on page 28 that "Up to 99 total programs may be created for all circuits
combined.". I've only created/removed schedules using the Screenlogic
interface, so I look forward to trying what you have added.
I have also noticed that when using your current Pentair binding that pump
rpms sometimes report differently than expected. For instance, I may have
the pump set at 2600 rpm (using a Pentair schedule) and while the Pentair
pump status (in ScreenLogic) reports it as running at 2600, the Pentair
binding it reports it at 2744. I haven't worried about it, but will
certainly check once I've tried your updated binding.
Thanks for writing / supporting it.
Bill
…On Fri, Jun 28, 2024 at 3:35 PM jsjames ***@***.***> wrote:
@BillBaird <https://github.com/BillBaird> - As @lsiepel
<https://github.com/lsiepel> mentioned, there is a significant update to
the binding coming which adds quite a few additional features and is much
more stable. I verified that this issue should not exist in the new code
since I restructured the code better so there is more common code between
the 2 bridge types. If you want to try the new version out, you can
download this file and put in your addons directory. Note, the
configuration of the binding is different. The updated code/documentation
is here: https://github.com/jsjames/openhab-addons/tree/pentair2
https://drive.google.com/file/d/13QnBZr2yHtC9pNmJQkOW-T33YmFvKg2y/view?usp=sharing
—
Reply to this email directly, view it on GitHub
<#16874 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGVCIOBF4SM2KLTKBXC6PDZJXQK5AVCNFSM6AAAAABJMAXQCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJXG4ZTOMBTGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I do recall the RPM being an issue that I corrected. My pump does not output the RPM, so I couldn't test this originally. Regarding the number of programs, if you have an intellitouch, we could work on a future enhancement to the binding to support more programs. Right now the binding does not determine the type of pentair controller, so it assumes they are all an easytouch. |
As #13485 has been merged and @BillBaird confirmed this was fixed. I close this. If needed to discuss new features please open a new issue per enhancement. |
Issue
The pentair binding, when using the Serial Bridge ignores the "id" thing configuration. The makes the binding when using the serial bridge configuration essentially read-only. It should be noted that the IP Bridge binding properly sets the "id" and behaves properly.
Expected Behavior
The thing's "id" is included in commands sent using the Pentair binding, enabling the Pentair EasyTouch system to accept and act on the command.
Current Behavior
Currently, commands written to the RS485 bus use a value of 00 for the source id, as seen in this log entry:
[DEBUG] [rnal.handler.PentairBaseBridgeHandler] - Writing packet: FF FF FF 00 FF A5 01 10 00 86 02 09 01 01 48
The correct behavior should have a configured value of 34 (hex 22) for the source id, as seen in this log entry generated using the IP Bridge:
[DEBUG] [rnal.handler.PentairBaseBridgeHandler] - Writing packet: FF FF FF 00 FF A5 01 10 22 86 02 09 01 01 6A
The difference can be seen in the incorrect
10 00 86
sequence and the correct10 22 86
sequence.Possible Solution
Add this line in
PentairSerialBridgeHandler.java
at line 53:This would make the
connect()
method begin withwhich is exactly what the
PentairIPBridgeHandler.java
connect()
method does.It should also be noted that
PentairBaseBridgeHandler.java
indicates that subclasses need to assign, but thePentairSerialBridgeHandler.java
does not.Interestingly, the
README.md
documentation indicates that it defaults to 34 for both the ip_bridge and serial_bridgeThe code not only does not set the default of 34 (at least as I read it), nor does it set the id at all for the serial_bridge. Either the code should actually implement a default of 34, or the docs should be changed to indicate that id should typically be set to 34.
Steps to Reproduce (for Bugs)
Prerequisite - you would need a Pentair EasyTouch setup on your swimming pool.
Context
I had a hard time believing this was the issue as this binding has been around for a number of years. In an effort to standardize my installation, I recently converted to it from an early version which I had modified several years ago. I could not get the current binding to work. Observing the log I saw the my ancient working version was sending the hex 22 source address, but the new version was not. I dug into the new code and noticed that the address was only set when using the IP_bridge. I changed my configuration from using a serial bridge to an IP bridge (with the id set to 34), the log entries looked correct, and the things started working.
The good news is that I have a workaround (use the IP bridge). My EasyTouch (I suspect all EasyTouches) require the source address. I don't see how anyone could get the serial binding to work without this fix.
Your Environment
The text was updated successfully, but these errors were encountered: