Refactor nhs_ser driver

[jimklimov]

Follows up from #2692

CC @Challado - testing on HW would be welcome, to make sure nothing got broken by this reshuffle

Is my understanding correct that in your code, the first call to upsdrv_updateinfo() would read nothing good from the wire and have no good checksums in "last" packet structs, so it would issue calls at the end of the method ("pkt_hwinfo loss -- Requesting") to get the HW info (and you mentioned there may be a couple of seconds before it replies - so the next cycle via main.c picks up that bit-stream)? Does this also trigger appearance of Data packet readings, or does the UPS just regularly send those?

Here the primary change was moving the code around for conformance to NUT driver design and general maintainability - primarily reduced indention level and improved readability by cutting large code of upsdrv_updateinfo() into methods reconnect_ups_if_needed(), interpret_pkt_hwinfo() and interpret_pkt_data() which now deal each with one aspect of the work, to not repeat everything every time regardless of what was read.

Some actual or suspected bugs were also addressed:

• in upsdrv_updateinfo(): possible overflow of the packet buffer if the end character never comes (e.g. noise on serial cable)
• in upsdrv_updateinfo(): changed reconnection approach, now using datastale/dataok flags; this is one part where testing is important
  • the original could, I think, loop indefinitely as it was incrementing/checking different variables
• relocated much code from upsdrv_initinfo() to upsdrv_initups() to match NUT init approach
  • some code that was in upsdrv_update() belongs really in init, to be read/set once (that is still TODO).
• use of data point names that are not in nut-names.txt (and redundant vs. correct names) - testing welcome that nothing got lost except "wrong" aliases to same data
• unknown packet sizes were ignored, not reported/rejected

Other changes:

• redefined porta[PATH_MAX] size more pedantically
• changed methods that deal with passed buffers and their sizes to account the latter as size_t not int
• the DATA packet analysis should now survive never seeing a HW INFO packet, though then of course not trying to calculate and set some values impacted by that
• experimental.nhs.* datapoints which effectively dump the struct fields are now also populated only if corresponding debug options are set, so by default only the NUT standard names (in their domains like device, ups, input and output) should be seen.
• data points for "input.voltage.low.warning" et al were set from calculated variable that was never changed, fixed to vin_low_warn etc.
• this sequence strewn across the method seemed wrong (beside the naming) - you were re-setting "power" with what seems to be the correct formula for "realpower" instead, at least that's how I fixed it here:

vpower = ((va * pf) * (lastpktdata.potrms / 100.0));
...
dstate_setinfo("power", "%0.2f", vpower);
...
dstate_setinfo("realpower", "%ld", lrint(round(vpower)));
dstate_setinfo("power", "%ld", lrint(round(va * (lastpktdata.potrms / 100.0))));

A couple of data dumps with and without the special debug options would be welcome for reference :)

[AppVeyorBot]

✅ Build nut 2.8.2.2508-master completed (commit 54b7097b94 by @jimklimov)

[jimklimov]

CC @Challado - gentle bump, now that CI has passed.

[Challado]

CC @Challado - gentle bump, now that CI has passed.

Jimmy, good morning.
Compiled and working great here. I'll mantain these new compiled code and reply on possible problems.