huawei-ups2000: fix broken instant command invocations and undefined Modbus behaviors.

[biergaizi]

huawei-ups2000: fix broken instant command invocations, close #3603

A serious regression was introduced during the "fightwarn" campaign that completely broke bypass.start, shutdown.return, shutdown.reboot, and shutdown.reboot.graceful. Any attempt to use them will fail, with an error logged to syslog:

huawei-ups2000: instcmd: command [bypass.start] reg1 is negative
huawei-ups2000: instcmd: command [shutdown.return] reg1 is negative
huawei-ups2000: instcmd: command [shutdown.reboot] reg1 is negative
huawei-ups2000: instcmd: command [shutdown.reboot.graceful] reg1 is negative

Because the regression was introduced shortly after the initial driver was merged into the upstream, these instant commands do no work in any NUT version (with the exception of the earliest development build). This problem was not discovered during pre-merge test for the same reason: it's a regression that occurred after the Pull Request was merged.

In huawei-ups2000, all instant commands are driven by a lookup table to consolidate the logic to a central dispatcher. Some commands are dispatched to a (*handler_func)(uint16_t reg1) function, which is used to handle commands that needs additional processing.

Not all *handler_func actually reads reg1, it's an optional argument. Because declaring the function to accept a variadic or a (void *) would be over-engineering, a simple unsigned 16-bit value is used, *handler_func may or may not use it.

During the project-wide "fightwarn" refactor campaign, it was noticed that reg1 is stored in the lookup table as int16_t (-1 indicates an unneeded register), but *handler_func accepts a uint16_t. In case that *handler_func doesn't need the register ID from the lookup table, reg1 is implicitly converted from -1 to 65535. This is harmless, since reg1 is not actually used in this case.

Unfortunately, in an attempt to make the code "safe" by suppressing compiler warning, an incorrect range check was added to the function, which refuses to invoke *handler_func if reg1 is negative.

As a result, all instant commands that don't use reg1 are broken. These commands include bypass.start, shutdown.return, shutdown.reboot, and shutdown.reboot.graceful.

To fix this problem, fix the implicit type conversion correctly by change the reg1 data type in the lookup table from int16_t to uint16_t, and remove the "negative reg1" check. As defensive programming. all *handler_func that actually need reg1 would check and abort if reg1 is 0.

Fixes: e9f02e2 ("drivers/huawei-ups2000.c: instcmd(): range-check and cast for ups2000_write_register() and handler_func()")

huawei-ups2000: use modbus_write_register, not modbus_write_registers, fix #3593.

On some Huawei UPS2000 models, no instant commands or variable writes are possible, because modbus_write_registers() doesn't work correctly.

Upon investigation, Huawei UPS2000 doesn't support multiple-register writes at all (Modbus command 0x10), the official datasheet only supports single-register writes (Modbus command 0x06). Multiple-register writes worked on some models, but this turned out to be undefined behavior, it doesn't work with all models.

This commit switches modbus_write_registers() to modbus_write_register() to fix the problem.

huawei-ups2000: bump driver version to v0.14.

Dump driver versions to reflect changes.

docs/man/huawei-ups2000.txt: document bug #3593 and bug #3603.

Two bugs #3593, #3603 can prevent users from sending any instant commands or changing any variables at all. Document these known bugs in the man page.

[github-actions]

A ZIP file with standard source tarball and another tarball with pre-built docs for commit 37c545c is temporarily available: NUT-tarballs-PR-3604.zip.

[AppVeyorBot]

✅ Build nut 2.8.5.5171-master completed (commit c0f1a898fd by @biergaizi)

• artifacts
• A 7-zip archive with built NUT for Windows debug binaries is temporarily available: NUT-for-Windows-x86_64-SNAPSHOT-2.8.5.5171-master.7z

[AppVeyorBot]

✅ Build nut 2.8.5.5172-master completed (commit d1ef3757d2 by @biergaizi)

• artifacts
• A 7-zip archive with built NUT for Windows debug binaries is temporarily available: NUT-for-Windows-x86_64-SNAPSHOT-2.8.5.5172-master.7z

[AppVeyorBot]

✅ Build nut 2.8.5.5174-master completed (commit f8de6c84ed by @biergaizi)

• artifacts
• A 7-zip archive with built NUT for Windows debug binaries is temporarily available: NUT-for-Windows-x86_64-SNAPSHOT-2.8.5.5174-master.7z

[AppVeyorBot]

✅ Build nut 2.8.5.5175-master completed (commit 05161a3126 by @biergaizi)

• artifacts
• A 7-zip archive with built NUT for Windows debug binaries is temporarily available: NUT-for-Windows-x86_64-SNAPSHOT-2.8.5.5175-master.7z

[AppVeyorBot]

✅ Build nut 2.8.5.5176-master completed (commit 883344e1d4 by @biergaizi)

• artifacts
• A 7-zip archive with built NUT for Windows debug binaries is temporarily available: NUT-for-Windows-x86_64-SNAPSHOT-2.8.5.5176-master.7z

[jimklimov]

LGTM, but a NEWS.adoc entry would be welcome.

Maybe even add a note in UPGRADING.adoc to announce that all earlier versions were broken in this regard. That file usually is about changes that can adversely impact previous release users/packagers, but here users of the driver have all been adversely impacted already and can get fixed now, so maybe it belongs there too.

[AppVeyorBot]

✅ Build nut 2.8.5.5189-master completed (commit 64f3cb330f by @biergaizi)

• artifacts
• A 7-zip archive with built NUT for Windows debug binaries is temporarily available: NUT-for-Windows-x86_64-SNAPSHOT-2.8.5.5189-master.7z

[biergaizi]

Documentation files NEWS.adoc and UPGRADING.adoc updated. Let's merge it. I've tested it with a simple command (the server is too important to shut down right now), without seeing any breakage, so these changes should be at least neutral. Let's depend on user report for the final words.