Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend libusb(1?) usage to specify port=... as a devfs path to the device node #2066

Open
jimklimov opened this issue Sep 19, 2023 · 3 comments
Labels
enhancement USB USB-duplicate-devices Track bugs and issues about monitoring several devices that seem identical to NUT or libusb
Milestone

Comments

@jimklimov
Copy link
Member

jimklimov commented Sep 19, 2023

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.

@jimklimov jimklimov added enhancement USB USB-duplicate-devices Track bugs and issues about monitoring several devices that seem identical to NUT or libusb labels Sep 19, 2023
@jimklimov jimklimov added this to the 2.8.2 milestone Sep 19, 2023
@jimklimov jimklimov modified the milestones: 2.8.2, 2.8.3 Apr 6, 2024
@nwf
Copy link

nwf commented Jun 26, 2024

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?

@jimklimov
Copy link
Member Author

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

@nwf
Copy link

nwf commented Jul 1, 2024

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.

  • libusb1 requires the client to continue to manage the system fd that gets wrapped by libusb_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 around USBDevice_t and usb_communication_subdriver_t objects.

    • IMHO, it would be much better if the library dup()'d the descriptor and close()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 and usb_dev_handle-s as part of the usb_communication_subdriver_t interface. The use of the latter basically ensures that this interface is a thin shim around libusb[01] and requires that all callers understand the internals (and proper destruction) of USBDevice_t.

  • The use of libusb_get_port_numbers might seem more enticing, but the violence required for the USBDeviceMatcher objects 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_LIMIT is 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 as NULL if 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement USB USB-duplicate-devices Track bugs and issues about monitoring several devices that seem identical to NUT or libusb
Projects
None yet
Development

No branches or pull requests

2 participants