apc_modbus: Fix outlet group command handling to be dynamic

[EchterAgo]

Replace hardcoded outlet group commands with dynamic detection and handling based on the SOGRelayConfigSetting register. This ensures commands and variables are only exposed for outlet groups that are actually present on the UPS.

Key changes:

• Add APC_VF_UNAVAILABLE flag to mark registers for absent outlet groups
• Add outlet group info table tracking presence and target bits for each group (MOG, SOG0 to SOG2)
• Dynamically enumerate outlet.group.N.id, outlet.group.N.designator and outlet.group.N.status variables based on detected groups
• Replace static outlet command entries with _apc_modbus_handle_outlet_cmd() which parses load.*, shutdown.*, and outlet.group.N.* commands at runtime
• Global load.* and shutdown.* commands now target all available outlet groups instead of only the main outlet group (MOG)
• Add outlet.group.N.* commands (load.off, load.on, load.cycle, etc.) for each detected group
• Update upsdrv_shutdown() to use the unified command handler
• Add OutletStatus_BF bitfield definitions to header

This fixes issues where commands were exposed for non-existent outlet groups and ensures load/shutdown operations affect all outlets on UPS units with switched outlet groups.

Signed-off-by: Axel Gembe axel@gembe.net

Fixes #2338

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.

• Use of coding helper tools and AI should be disclosed in the commit
  or PR comments (it is interesting to know which ones do a decent job).
  As with other contributions, a human is responsible and thanked for the
  quality and content of the change, and is presumed to have the right to
  post that code to be published further under the project's license terms.

• 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

• Added a bullet point into NEWS.adoc, possibly also UPGRADING.adoc
  if there is something packagers or custom-build users should take into
  account (new driver categories, configuration options, dependencies...)

• 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.

[EchterAgo]

I have successfully tested FSD on a SMX2200RMHV2U (Firmware 15.0, no MOG, 3 SOGs) and SMT1500I (Firmware 9.6, MOG, 1 SOG). The commands all seem to do what I expect but I'd like feedback on this.

[EchterAgo]

IIUC upsmon will start the shutdown when the status has LB set for low battery. Currently we set this based on SimpleSignalingStatus_BF.ShutdownImminent however I'm not sure the conditions of that is what we want. It says this:

ShutdownImminent- 
Indicates that the UPS is committed to disconnecting power from its 
output(s).  The bit is set when UPSStatus_BF.PendingOutputOff is set AND 
RunTimeRemaining is less than or equal to LowRunTimeWarningSetting 
OR any of the following depending upon the UPS configuration. 
 
*  For UPS with an unswitched outlet group - when the 
UPSSystem.UnswitchedOutletGroup.TurnOffCountdown_EN is greater 
than -1 
 
*  For UPS with no unswitched outlet group and with switched outlet 
group(s)  - when the "last commanded" 
UPSSystem.SwitchedOutletGroup[x].TurnOffCountdown_EN is greater 
than -1. 
 
*  For UPS with no unswitched outlet group and with no switched outlet 
groups  - when the  UPSSystem.OutputSystem.TurnOffCountdown_EN is 
greater than -1. 
 
In response to this bit becoming set, the device using the simple signaling 
interface should drive request to shutdown if it hasn't already done so (this 
ensures that TurnOffCountdown_EN timer will be set to at least the 
minimum time needed by the simple signaling host). 

However there doesn't seem to be a documented LowRunTimeWarningSetting. The TurnOffCountdown values are loaded as soon as we send a shutdown / off command with delay enabled. Does anyone have any insights?

[cardil]

Thanks @EchterAgo, this works perfectly on my SMT1500. RPMs built from 44fcf07, driver 0.19.

outlet.group.1.load.off, load.off, load.on all work as expected. The big one: shutdown.return now triggers a proper power cycle with both groups sequencing correctly (OG1 60s, MOG 10s, 4s stayoff, restart).

I also ran it through the full FSD path: nutshutdown hook with POWERDOWNFLAG set, which calls upsdrvctl shutdown, which routes through the running driver via INSTCMD driver.killpower. Everything returns code 0 and the UPS cycles power. This is the exact path that was broken before on units with SOGs.

The per-group timer readback is a nice touch too. During the countdown I could see the timers ticking down in real time via upsc.

[jimklimov]

Great to hear!

@EchterAgo : did this work mature up to un-draft the PR or do you plan any changes?

I'd move the news entry up to other drivers so they are all close (with a* landing on top).

As this positively impacts a driver and has few risks for other NUT users, might even go into the next release that I keep hoping to cut =D maybe by end of Easter holidays...

[EchterAgo]

I definitely want to get this merged ASAP, the current code is broken. Currently I don't really know the CI results because the site loads either extremely slowly or not at all here, even when I connect a VPN or try other browsers. I'll update the NEWS entry soon.

[github-actions]

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

[AppVeyorBot]

✅ Build nut 2.8.4.4473-master completed (commit 1b13909710 by @EchterAgo)

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

[jimklimov]

CI was a bit bogged down lately, I've cleaned and hopefully sped it up a bit.

[AppVeyorBot]

✅ Build nut 2.8.4.4474-master completed (commit 90c3d11b15 by @EchterAgo)

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

[EchterAgo]

FYI, I used AI (Claude Opus 4.5) to help me write the commit message and make sense of and query the spec documents a bit.

[AppVeyorBot]

✅ Build nut 2.8.4.4475-master completed (commit 260a7f0002 by @EchterAgo)

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

[AppVeyorBot]

✅ Build nut 2.8.4.4476-master completed (commit de9b83aa91 by @EchterAgo)

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

[EchterAgo]

CI was a bit bogged down lately, I've cleaned and hopefully sped it up a bit.

It might help if you could set the CI to abort previous builds when the PR branch is pushed.

https://www.jenkins.io/doc/book/pipeline/syntax/#:~:text=disableConcurrentBuilds

options { disableConcurrentBuilds(abortPrevious: true) } maybe?

[AppVeyorBot]

✅ Build nut 2.8.4.4477-master completed (commit 4041360a6f by @EchterAgo)

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

[jimklimov]

Aborting is enabled via milestones at different waiting pounts: if a newer build passes it first, older ones bail out. Also a limited amount of jobs are in flight while othrrs are queued. These levers can be pulled manually, e.g. to boost priority of newer iterations, but sonetimes there is value in completing older svheduled builds. Increasing in-flight job counts helps filter out at least spell-check and Makefile issues quickly, blocking on slower build/test phases - generated and placed last in queue.

Also for master branch etc. we want all merges to have a verdict, even if it catches up much later (e.g. build newest commit of a merge train early, but intermediate points can wait until rebased open PRs are done).

[jimklimov]

apc_modbus.c:1319:10: error: implicit conversion from 'int' to enumeration type 'apc_modbus_outlet_cmd_type_t' is invalid in C++ [-Werror,-Wimplicit-int-enum-cast]
 1319 |         { NULL, 0 }
      |         ~       ^
1 error generated.
make[2]: *** [Makefile:2624: apc_modbus.o] Error 1

I am not sure why new Fedora's clang spews C++ warnings at C code, did not drill down into that yet and rather addressed them directly until now (flaky is flaky anyway). Like here, a named enum const with zero value instead of 0 can do, I hope.

If the rest of workers won't complain about the current commit, this can be patched during merge.