Add nutdrv_qx driver for Gtec ZP120N

[turekl]

Gtec ZP120N is already supported by the blazer_usb and nutdrv_qx drivers, but both have two problems:

1. They use the simple "Q1" query, which doesn't report the result of the latest battery test. For some reason the UPS reports normal battery voltage even when the battery is completely disconnected. So you won't know the battery is dead until power failure. This driver sends the more advanced "Q4" request instead. Here the answer includes status letters with more information than the Q1 binary flags, including battery status.
2. Reply is read in 8-byte chunks, which causes the UPS to disconnect from USB. The UPS reconnects in a second, but it still breaks the initialization of nutdrv_qx, where the query is sent twice in a short time (unlike blazer_usb). The solution is simple: read the whole reply at once.

The new protocol and subdriver need to be both enabled manually in ups.conf:

driver = "nutdrv_qx"
subdriver = "gtec"
protocol = "gtec"

[jimklimov]

Thanks, looks nice. Some comments:

• can you also please update the man page
• ...and NEWS.adoc?
• while some dstate setters of ups.alarm directly still exist, we're trying to eradicate them - prefer (status|alarm)_(init|set|commit) around loops
• bump main nutdrv_qx driver version
• I think we started converting to variable (not hard-coded) interface, endpoint, etc. numbers. Maybe this is not changed everywhere though. Can you quickly check if other subdrivers are already fixed this way, and if yes - also do it here? If not, can wait...

[jimklimov]

I've posted some fixes to the formality points I've raised earlier, and whatever CI was unhappy about.

@turekl : One big new question for me is that in gtec_process_status() you seem to handle many status letters, but only return the val of one of them (latest seen, or OFF by default). It seems to make sense to status_set(val) whenever we pick up a new value, so the resulting ups.status committed after a data collection loop in nutdrv_qx.c::upsdrv_updateinfo() would contain several tokens if several states co-existed. I do not know if this device can report many status letters at once though (e.g. "OL + BYPASS" or "OB + LB").

While at it, you assign an "!OL" value at one point. Is it same as "OB" (on-battery) generally used elsewhere? Or is there some nuance? Or were you just following the (obsolete) precedent of a few other nutdrv_qx_*.c files?

In any case, after some fixes already done on master for issue #2708, it should be safe to both explicitly status_set() something here and return a value, I think - ultimately the status_commit() would assign the set of unique recently-raised tokens into ups.status string.

I am not quickly sure if an intermediate single token returned via mapping table would be assigned AND announced on the bus - so the value would flip around on short notice, though. If yes, maybe mapping tables handling like this (in various drivers) should use an experimental or otherwise temporary value to delete at end of loop, or have a way (flag) to not actually assign it during the loop.

[turekl]

Thanks for the comments and fixes!

There can probably be many combinations of status letters, but I have seen only these: I, IM, AI, CKP, KMP. It seems A/C/M are mutually exclusive, as well as I/J/K/L. The Q4 answer includes at least one letter which sets the status, except when the UPS is off, that's why "OFF" is the default. OL + BYPASS is not possible, I'm not sure about OB + LB, I will test it tomorrow. The "!OL" status was taken from blazer_process_status_bits(), I will change it to OB if it's more appropriate.

It was really hard to find some documentation, I used description of the status letters from this document. Now I have found another one, which also describes the 'P' flag, but it seems redundant (it was always in conjunction with 'K'). What's the best way to report that the battery is disconnected, alarm_set()?

[jimklimov]

Update: found in update_status() that ! is handled to "clear" the status when converting into a status_item->status_mask, so !OL should not pop up in the final reports. Ooh is it a can of worms :)

[jimklimov]

What's the best way to report that the battery is disconnected, alarm_set()?

If you know that disconnection to be the case - yes, I suppose. Alternately (at least with same physical effect that manipulation is needed) you can use the RB (replace battery) status that nutdrv_qx.c (ups_status_set() and ups_alarm_set()) would eventually convert into both the status token and common alarm message.

[turekl]

Thanks! I did the test I promised, and the status letters when the battery was running out were "ABI", so OB + LB is really possible. We've had a few dead batteries and the battery test flags were either 'J' or 'K', so these should set the RB status. I will send a new pull request next week, but first I need to understand how the status really works (I thought new status replaces the previous one).

[jimklimov]

Thanks!

With status_set("xxx") the new (not seen before) token is added to the list. The list itself is cleared by status_init() at start of data update loop, and finalized/published by status_commit() in the end. Likewise for alerts (code should alert_commit before status_commit). In nutdrv_qx this is handled commonly in upsdrv_updateinfo() method.