feat: Add support for Liebert PSI5 #2369
Conversation
Adding support for the Liebert PSI5 model of UPS. Signed-off-by: James R. Parks <jrjparks@zathera.com>
jrjparks
force-pushed
the
feat-liebert-psi5
branch
from
58079aa
to
31e9979
Compare
There was a problem hiding this comment.
Looks quite solid to me. Is there a particular reason this is posted as a "draft" PR? Are you in local testing so far?
Compared to #2271 discussion I see you went for a separate method to "maybe apply" a correction factor (in the PoC there you added logic to an existing method). On one hand, this way you surely should not introduce errors into existing support for other devices, on another - I still wonder if some other reports about Liebert/Belkin HID devices could benefit from an exponent fix for those earlier HID tree node readings. And whether that original method did have an error about 1e7 vs. 1e9 comparisons and multiplications. At least, something to keep in mind if more bug reports come eventually...
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
|
Passing to NUT CI farm to test this change... |
I left it in draft before I went to bed to make sure it would pass the approvals before I published it.
Yeah, I thought about how to extend the existing function without possibly breaking others, but I wouldn't be able to be sure of any changes other than "it works on my UPS" 😆. This seemed to be the safer path as it's also using different HID lookup paths, so I assumed Liebert is using different maths for different models. I was also trying to think of a way to just set the line voltage multiplier based on the model or some factor, but I'm still not very familiar with the code and it's been a while since I last touched some C. |
…-up] Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ert_psi5_line_voltage_fun() into common liebert_line_voltage_fun() Hopefully should help more devices than those which serve this or that HID subtree only. Follows up from PR networkupstools#2369 Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
|
Just wanted to add that this is confirmed working on the PST5 now as well (this model is different than the PSI5, though reports as the same vendor and product id). Thanks! |
Adding support for the Liebert PSI5 model of UPS, as discussed in #2271 initially.
Maybe addresses similar complaints posted in #2024 (but that is for another driver related just by vendor)
Closes: #2271