belkin-hid.c: auto-select interface 1 for Liebert PSI5 / PowerSure PST (10af:0002)

[jimklimov]

Aims to supplant PR #3345 by addressing the issue detailed there at the more proper location.

Fixes: #1252
Fixes: #1028

Not sure if really related to #3340 or a different issue (could simplify the config there though).

[dcgrove]

Compatibility note: this approach breaks NUT 2.8.0 (Debian bookworm stable)

I tested the belkin_claim() approach from this PR against NUT 2.8.0 (the version in Debian bookworm, which is also what Raspberry Pi OS ships) and found it fails silently — the driver starts but reports no device found. Here's why.

Root cause: dual use of hid_rep_index in 2.8.0's libusb1.c

In NUT 2.8.0, libusb1.c uses hid_rep_index for two distinct purposes:

1. As the USB configuration descriptor index passed to libusb_get_config_descriptor():

   // drivers/libusb1.c ~line 331 (NUT 2.8.0)
   upsdebugx(2, "Reading first configuration descriptor");
   ret = libusb_get_config_descriptor(device,
       (uint8_t)usb_subdriver.hid_rep_index,   // ← config index, not interface!
       &conf_desc);

2. As the USB interface number to claim (later in the same function):

   libusb_claim_interface(udev, usb_subdriver.hid_rep_index);

In NUT master, these have been correctly separated: libusb_get_config_descriptor() now uses a dedicated usb_config_index field (defaulting to 0), so hid_rep_index is free to hold only the interface number. The belkin_claim() approach in this PR works correctly on master for exactly that reason.

What goes wrong on 2.8.0

The call order is:

libusb_open_device()
  → match_function_subdriver()      ← calls belkin_claim(), which sets hid_rep_index = 1
  → libusb_get_config_descriptor(device, hid_rep_index=1, ...)   ← tries config #1, not #0!
      → returns LIBUSB_ERROR_NOT_FOUND (-5)  (device only has one config, index 0)
  → libusb_claim_interface(udev, hid_rep_index=1)  ← succeeds (interface 1 exists)
  → retrieve HID descriptor → fails (conf_desc is invalid/null from the -5 error)
  → device rejected, driver exits with "No matching HID UPS found"

Debug output from a 2.8.0 build with this PR's patch applied:

[D1] belkin_claim: defaulting usb_hid_rep_index=1 for Liebert PSI5/PST 10af:0002
[D2] Device matches
[D2] Reading first configuration descriptor
[D2] result: -5 (Entity not found)        ← config descriptor #1 doesn't exist
[D2] Claimed interface 1 successfully     ← interface 1 is fine, but conf_desc is bad
[D2]   Couldn't retrieve descriptors      ← HID descriptor fetch fails
[D2] libusb1: No appropriate HID device found
libusb1: Could not open any HID devices: insufficient permissions on everything

What works on 2.8.0

Setting hid_rep_index = 1 after libusb_get_config_descriptor() returns (i.e., in libusb1.c itself, right after the config read) avoids the conflict:

// drivers/libusb1.c — after libusb_get_config_descriptor() call
/* Now we have matched the device we wanted. Claim it. */

/* Liebert PSI5/PowerSure PST (10af:0002): the HID Power Device Class
 * descriptor is on interface 1; interface 0 exposes only a 27-byte
 * vendor-specific report with no usable data. */
if ((curDevice->VendorID == 0x10af) && (curDevice->ProductID == 0x0002)) {
    usb_subdriver.hid_rep_index = 1;
}

This is the approach in PR #3345 (targeting master's libusb1.c, which at that point has already used usb_config_index for the descriptor read).

Suggestion

If a 2.8.x backport or stable-branch fix is ever needed, the libusb1.c placement is the safe one for that era of code. The belkin_claim() placement in this PR is correct and clean for master.

For what it's worth, I've been running the libusb1.c-based fix on a Raspberry Pi 4 (Debian bookworm, NUT 2.8.0) with a Liebert PSI5 1100VA since this thread started and it's rock-solid — ups.status: OL CHRG throughout.

[jimklimov]

Thanks, good points. But why really care about 2.8.0 if it's old and set in stone 4 years ago or so?

I think the assumption here is that NUT versioning follows proper SEMVER (MAJOR-MINOR-PATCH releases), then 2.8 feature set is a reasonable baseline to code a patch against.

Alas, at the moment we're diverged from that, and whole X.Y.Z numbers are distinct releases. Hope we'll get back to truer semvers after some planned refactoring/hardening works are complete (a decade in the making)...

[jimklimov]

So if this fix is confirmed working on master, it is good to be merged to master (eventually to be part of v2.8.5 release).

@dcgrove : was this change only analyzed by Claude, or you actually built and tested the PR source branch with your UPS?

[jimklimov]

Per #3345 (comment) :

Just had Claude reinstall with PR #3349 and post a comment on that PR with the results. Feel free to close/reject/accept whatever you want with this one if 3349 is better.

Aha, ok :)