Add support for shutdown variables, mode selector, PC reset timing, a…

[haulfi]

…nd battery switch threshold

• Introduced new variables to the phoenixcontact_modbus driver especially for QUINTS 4 USB:

  • shutdown variables
  • mode selector 9 = PC_MODE
  • PC reset delay after main power recovers
  • automatic switch to battery mode if main power is below or above a defined threshold.

  Signed-off-by: Uli [email protected]

General points

• Described the changes in the PR submission or a separate issue, e.g.
  known published or discovered protocols, applicable hardware (expected
  compatible and actually tested/developed against), limitations, etc.

• There may be multiple commits in the PR, aligned and commented with
  a functional change. Notably, coding style changes better belong in a
  separate PR, but certainly in a dedicated commit to simplify reviews
  of "real" changes in the other commits. Similarly for typo fixes in
  comments or text documents.

• Please star NUT on GitHub, this helps with sponsorships! ;)

Frequent "underwater rocks" for driver addition/update PRs

• Revised existing driver families and added a sub-driver if applicable
  (nutdrv_qx, usbhid-ups...) or added a brand new driver in the other
  case.

• Did not extend obsoleted drivers with new hardware support features
  (notably blazer and other single-device family drivers for Qx protocols,
  except the new nutdrv_qx which should cover them all).

• For updated existing device drivers, bumped the DRIVER_VERSION macro
  or its equivalent.

• For USB devices (HID or not), revised that the driver uses unique
  VID/PID combinations, or raised discussions when this is not the case
  (several vendors do use same interface chips for unrelated protocols).

• For new USB devices, built and committed the changes for the
  scripts/upower/95-upower-hid.hwdb file

• Proposed NUT data mapping is aligned with existing docs/nut-names.txt
  file. If the device exposes useful data points not listed in the file, the
  experimental.* namespace can be used as documented there, and discussion
  should be raised on the NUT Developers mailing list to standardize the new
  concept.

• Updated data/driver.list.in if applicable (new tested device info)

Frequent "underwater rocks" for general C code PRs

• Did not "blindly assume" default integer type sizes and value ranges,
  structure layout and alignment in memory, endianness (layout of bytes and
  bits in memory for multi-byte numeric types), or use of generic int where
  language or libraries dictate the use of size_t (or ssize_t sometimes).

• Progress and errors are handled with upsdebugx(), upslogx(),
  fatalx() and related methods, not with direct printf() or exit().
  Similarly, NUT helpers are used for error-checked memory allocation and
  string operations (except where customized error handling is needed,
  such as unlocking device ports, etc.)

• Coding style (including whitespace for indentations) follows precedent
  in the code of the file, and examples/guide in docs/developers.txt file.

• For newly added files, the Makefile.am recipes were updated and the
  make distcheck target passes.

General documentation updates

• Updated docs/acknowledgements.txt (for vendor-backed device support)

• Added or updated manual page information in docs/man/*.txt files
  and corresponding recipe lists in docs/man/Makefile.am for new pages

• Passed make spellcheck, updated spell-checking dictionary in the
  docs/nut.dict file if needed (did not remove any words -- the make
  rule printout in case of changes suggests how to maintain it).

Additional work may be needed after posting this PR

• Propose a PR for NUT DDL with detailed device data dumps from tests
  against real hardware (the more models, the better).

• Address NUT CI farm build failures for the PR: testing on numerous
  platforms and toolkits can expose issues not seen on just one system.

• Revise suggestions from LGTM.COM analysis about "new issues" with
  the changed codebase.

[AppVeyorBot]

❌ Build nut 2.8.3.3288-master failed (commit 44e1837c49 by @)

[jimklimov]

Thanks! Code changes look reasonable, there are some syntactic/markup problems to take care of, as commented additionally.

Why did all code get indented and end-of-file blank line get lost? Please fix this back too :)

Also, some additions are due into docs/man/... page for the driver, and into NEWS.adoc about changes to this driver that would get published in a future NUT release.

[jimklimov]

Here and below, please use block comments /* ... */ even for single line texts

[jimklimov]

Can you please add these explanations also to the man page?

[jimklimov]

I believe these strings are also driver configuration parameters (per addvar() below)?

If any of these concepts are already described by standard NUT variable/command names listed in docs/nut-names.txt, it would be beneficial to add corresponding code so upsrw/upscmd can be used to set them at run-time, same way as for any other supported UPS with such features. But this may be improved later.

[haulfi]

Thanks @jimklimov for your feedback.

I updated the code, and please dont hesitate to add more feedback :)

[AppVeyorBot]

✅ Build nut 2.8.3.3312-master completed (commit 1339a1e957 by @jimklimov)

[AppVeyorBot]

✅ Build nut 2.8.3.3318-master completed (commit c4e8d885fd by @jimklimov)

[AppVeyorBot]

❌ Build nut 2.8.3.3319-master failed (commit b0c36621f4 by @jimklimov)

[jimklimov]

@haulfi : Now that the driver code looks good for CI, I wonder if these added driver-configuration names (for concepts that overlap with existing standard NUT variable or instant command names in docs/nut-names.txt) can be reconciled? Just to have this done on main branch before merging the PR, to minimize the surprise for end-users about their meaning ("is X conceptually same as Y or...?")

And as noted in earlier review comments, I'm also wondering if for at least some of these it would make sense to back them as ordinary NUT variables (rather than internal C numeric variables), which can be set at run-time by upsrw and other clients (so the initial value via ups.conf configuration would be like override.battery.voltage.low=11.5 etc.?) Note there are input.transfer.(trim|boost).(low|high) - one of these is likely equivalent to what you named ups.mains.voltage_high, for example.

[jimklimov]

Note: AppVeyor CI build itself was OK, just overflowed their time allocation so could not finish some job book-keeping.