CyberPower UT series do not seem to pass correct 'OB' flag, try to accommodate this with new "onlinedischarge" config flag

[kgizdov]

As the title says. I have CPS UT1500E and unfortunately whatever I do, I cannot get it to show correct online status. I have been in contact with support and they say the UPS is operating as it should. Thus, I have arrived at this workaround.

The issue is as follows, when the UPS loses mains power, it goes from OL into OL DISCHRG mode. Yes, it does not change it's input mode. However, it correctly shows input voltage to be 0. The only way to circumvent this I have found to be encapsulated in this PR.

I am, however, open to suggestions on how to make it better as it clearly only applies to certain types of UPSes. I was thinking to filter on VRANGE, but that somehow seemed inappropriate to me.

Let me know if you require further info or testing and/or logs. Maybe we can find the actual underlying reason this happens and fix it at the core. Cheers.

[jimklimov]

There is a bit of code-style issue in the initial commit (indentation should start with TABs until the level of if on the previous line).

Given that this misbehavior may be not present on all devices supported by the driver, I think it makes sense to define (and document!) a driver option for the "quirk" similar to what was done in https://github.com/networkupstools/nut/pull/788/files - then it is up to end-users suffering same as you to toggle this for their deployment or stay with current behavior.

It may make sense to log or even post ("wall?") the situation, like "We are in a strange OL DISCHRG situation at the moment, do you need the XXX quirk in your driver to handle this as an outage?"

Thinking out loud, an UPS calibration event might also seem like this validly - the device still has wall power, but is intentionally discharging the battery to see how long it lasts with your load.

[kgizdov]

I agree with everything said. I will fix the indentation and will try to refactor as requested. For your last comment, yes, we should detect the UPS calibration and if we cannot, we should issue a warning like "you are in this weird state, if you are not in a calibration mode, maybe you need this setting: bla". How's that sound?

[jimklimov]

Sounds right. See also #817 merged recently that dealt with similar question (whether calibration is an outage).

[kgizdov]

obviously not ready, but is this something in the right direction?

[jimklimov]

General direction looks good, some comments to code posted.
Also lacks a change to manpage, to document the new driver option for the quirk.

[kgizdov]

@jimklimov where would it be appropriate to explain in the manual structure. I can see the general config notes as well as the general hid-driver man. Not sure if either is a great place as this seems specific. Thoughts?

[jimklimov]

A couple of blocking build issues appeared in recent iterations, commented.

[jimklimov]

As for a good place to document the new driver option (added after I initially posted the reference to #788), I think a driver manpage could be good. In case of that PR, its origin could not be modified so a later commit was pushed to master adding the manpage changes for that: 799fd5a

For this PR, the suitable manpage at this time would probably be the USB family's common location: https://github.com/networkupstools/nut/blob/master/docs/man/usbhid-ups.txt

It may be worth adding a mention of the situation (with reference to manpage for specific details) in https://github.com/networkupstools/nut/blob/master/docs/config-notes.txt

[jimklimov]

@kgizdov : Just in case: is this PR's problem area related to #439 (as it evolved in the end) and #1245 as the generic solution to allow hotfixing known invalid mappings?

[kgizdov]

@jimklimov oh, sorry, this MR has completely gone out of my schedule somehow as I jumped into some other work. Allow me a couple of week to have a look. But I think if you've got a more general solution to this then by all means that's the end goal. Let me know if there's anything specific (code-wise) and should be aware of to incorporate or completely refactor the changes.

[jimklimov]

Hi, thanks for the reply. I think, since #1245 was by now merged, it would help if you review its change so that we can decide if your original fix is needed or your goal is achieved by that one, and/or if your solution can be adapted to use that mechanism.

[jimklimov]

Brushed up the PR, it seems to have merit regardless of "general hotfix for invalid mappings". At worst, we would have two solutions for one of the problems :)

[jimklimov]

Thanks for the contribution!