Extend libusb(1?) usage to specify `port=...` as a devfs path to the device node
Currently our USB-capable drivers use port=auto (because ups.conf syntax requires some value) and a number of matchers to select the device at run-time. Discussion in #1273 and other issues suggested that libusb1 may have a way to actually specify a devfs path to the device node instead, to be on par with serial drivers using port=/dev/cuaa, or port=/dev/ttyS0 or port=/dev/ttyUSB1 (for USB->serial converter dongles) instead.
This ticket is a reminder to investigate and hopefully implement this ability, with a caveat that depending on OS setup (use of dynamic enumeration, re-plugging of same or neighboring devices, port resets and sleeps, etc.) such paths might be or not be volatile.
FWIW, libusb1 as of 1.0.16 sports a libusb_get_port_numbers() API that returns the entire device path, rather than just the terminal port number. That would be a stable way to refer to a unique point in the USB topology, and is stable against enumeration order and unplug/replug events (so long as the replug is into the same port).
But I agree that port is probably a more useful, less bespoke approach and probably involves less code. In particular, one can use udev (or devd or similar) to give a symbolic link to a particular point in the USB topology. In udev, for example,
SUBSYSTEM=="usb", KERNEL=="5-1.1", OWNER="nut", SYMLINK+="ups0"
will create
$ ls /dev/ups0
lrwxrwxrwx 1 root root 15 Jun 26 09:37 /dev/ups0 -> bus/usb/005/011
as well as set the owner of the raw bus/usb device. Since 1.0.23, libusb1 has had libusb_wrap_sys_device to allow the use of such device nodes to directly access a given device without enumeration. See
https://github.com/libusb/libusb/blob/e678b3fad58a508cbd0a6e6dc777fb48346f948b/examples/testlibusb.c#L242-L261 for an example of its use.
(ETA: it is purely a coincidence that device path "1.1" has device number "11" as of the commands above. There is no relation between these digit strings.)
Would you like me to try to raise a PR?
Yes, thanks - I suppose that would be welcome (keeping port=auto for guessing as is done now). Just in case, keep in mind that not all world is Linux - though in this case it is rather for libusb1 project's headache to do things right. Just that wordings of such devfs paths can differ a lot.
In any case, if these methods only appeared from some intermediate version of the library, it sounds right to check for their existence in m4/nut_check_libusb.m4 and ifdef the new feature based on that (and also log a message that port!=auto is ignored in libusb0.c). If the provided path does not seem like an expected UPS in builds that support the feature, driver should fail to start; if the feature is not supported - log but ignore (do port=auto as we always did).
OK, I tried and I'm sorry, but I don't think I'm going to be able to raise a PR in the near term.
-
libusb1requires the client to continue to manage the system fd that gets wrapped bylibusb_wrap_sys_device, and to close it after closing the USB device, which is not particularly pleasant and would require violence to the somewhat inconsistent abstraction layer aroundUSBDevice_tandusb_communication_subdriver_tobjects.-
IMHO, it would be much better if the library
dup()'d the descriptor andclose()d the one they had when the device got closed; then the client could optionally retain the fd or not, as desired. Oh well. -
Also IMHO, it's a bit odd to have both
USBDevice_t-s andusb_dev_handle-s as part of theusb_communication_subdriver_tinterface. The use of the latter basically ensures that this interface is a thin shim aroundlibusb[01]and requires that all callers understand the internals (and proper destruction) ofUSBDevice_t.
-
-
The use of
libusb_get_port_numbersmight seem more enticing, but the violence required for theUSBDeviceMatcherobjects is also more than I want to take on right now; the other annoying bit, formatting a temporary string from the array contents for the matchers' use, at least occurs just once. (In particular,USBMATCHER_REGEXP_ARRAY_LIMITis now defined by two nested conditionals and there's not just one optional slot in the resulting array. It's tempting to make all slots non-optional and just leave the regexp asNULLif the feature isn't supported, but even that is... tedious, but probably the easiest answer.)
For my case, busport happens to work, because the two UPSes are siblings on the same hub, but that's not a particularly fantastic requirement. So it goes.