diff --git a/NEWS.adoc b/NEWS.adoc index 14d648e929..1faeb7e267 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -70,6 +70,10 @@ https://github.com/networkupstools/nut/milestone/13 option bypasses the check altogether, so driver instances handling well behaved devices would also not show their manufacturer). [issue #3436] + - `richcomm_usb` driver updates: + * Completed support for NUT standard USB configuration options, and fixed + some problems possible when iterating devices. [issue #1768, PR #3477] + - `usbhid-ups` driver updates: * When reconnecting, report success more visibly (not only in debug), and do not reset driver state to "quiet" when it will loop trying. [PR #3423] diff --git a/docs/man/richcomm_usb.txt b/docs/man/richcomm_usb.txt index 638b7d5a0a..8e1cb0ca5a 100644 --- a/docs/man/richcomm_usb.txt +++ b/docs/man/richcomm_usb.txt @@ -26,14 +26,10 @@ As such, all the limitations of the underlying contact closure interface apply. This means that you will only get the essentials in `ups.status`: `OL`, `OB`, and `LB`. See also linkman:genericups[8]. -//////// -TODO: Uncomment after solving https://github.com/networkupstools/nut/issues/1768 - EXTRA ARGUMENTS --------------- include::nut_usb_addvars.txt[] -//////// BUGS ---- diff --git a/drivers/richcomm_usb.c b/drivers/richcomm_usb.c index c20e1b6ea2..09c7d4939d 100644 --- a/drivers/richcomm_usb.c +++ b/drivers/richcomm_usb.c @@ -30,7 +30,7 @@ /* driver version */ #define DRIVER_NAME "Richcomm dry-contact to USB driver" -#define DRIVER_VERSION "0.17" +#define DRIVER_VERSION "0.18" /* driver description structure */ upsdrv_info_t upsdrv_info = { @@ -67,7 +67,12 @@ static usb_device_id_t richcomm_usb_id[] = { static usb_dev_handle *udev = NULL; static USBDevice_t usbdevice; +static USBDeviceMatcher_t *reopen_matcher = NULL; +static USBDeviceMatcher_t *regex_matcher = NULL; static unsigned int comm_failures = 0; +#if WITH_LIBUSB_1_0 +static int libusb_inited = 0; +#endif /* Forward decls */ static int instcmd(const char *cmdname, const char *extra); @@ -305,7 +310,6 @@ static int usb_device_close(usb_dev_handle *handle) #if WITH_LIBUSB_1_0 libusb_close(handle); - libusb_exit(NULL); #else ret = usb_close(handle); #endif @@ -321,10 +325,12 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev #if WITH_LIBUSB_1_0 libusb_device **devlist; ssize_t devcount = 0; - libusb_device_handle *handle; + libusb_device_handle *handle = NULL; struct libusb_device_descriptor dev_desc; - uint8_t bus_num; - /* TODO: consider device_addr */ + uint8_t bus_num, device_addr; +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + uint8_t bus_port; +#endif int i; #else /* => WITH_LIBUSB_0_1 */ struct usb_bus *bus; @@ -332,9 +338,11 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev /* libusb base init */ #if WITH_LIBUSB_1_0 - if (libusb_init(NULL) < 0) { - libusb_exit(NULL); - fatal_with_errno(EXIT_FAILURE, "Failed to init libusb 1.0"); + if (!libusb_inited) { + if (libusb_init(NULL) < 0) { + fatal_with_errno(EXIT_FAILURE, "Failed to init libusb 1.0"); + } + libusb_inited = 1; } #else /* => WITH_LIBUSB_0_1 */ usb_init(); @@ -358,7 +366,21 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev USBDeviceMatcher_t *m; libusb_device *dev = devlist[i]; + USBDevice_t descriptor_device; libusb_get_device_descriptor(dev, &dev_desc); + + memset(&descriptor_device, 0, sizeof(descriptor_device)); + descriptor_device.VendorID = dev_desc.idVendor; + descriptor_device.ProductID = dev_desc.idProduct; + descriptor_device.bcdDevice = dev_desc.bcdDevice; + + if (is_usb_device_supported(richcomm_usb_id, &descriptor_device) != SUPPORTED) { + upsdebugx(4, "Device [%04x:%04x] is not supported by this driver - skipping open", + dev_desc.idVendor, dev_desc.idProduct); + continue; + } + + handle = NULL; ret = libusb_open(dev, &handle); *handlep = handle; #else /* => WITH_LIBUSB_0_1 */ @@ -371,13 +393,23 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev int i; USBDeviceMatcher_t *m; + USBDevice_t descriptor_device; upsdebugx(3, "Checking USB device [%04x:%04x] (%s/%s)", dev->descriptor.idVendor, dev->descriptor.idProduct, bus->dirname, dev->filename); - /* supported vendors are now checked by the supplied matcher */ + memset(&descriptor_device, 0, sizeof(descriptor_device)); + descriptor_device.VendorID = dev->descriptor.idVendor; + descriptor_device.ProductID = dev->descriptor.idProduct; + descriptor_device.bcdDevice = dev->descriptor.bcdDevice; + + if (is_usb_device_supported(richcomm_usb_id, &descriptor_device) != SUPPORTED) { + upsdebugx(4, "Device [%04x:%04x] is not supported by this driver - skipping open", + dev->descriptor.idVendor, dev->descriptor.idProduct); + continue; + } /* open the device */ *handlep = handle = usb_open(dev); @@ -398,12 +430,18 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev free(device->Product); free(device->Serial); free(device->Bus); + free(device->Device); +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + free(device->BusPort); +#endif memset(device, 0, sizeof(*device)); #if WITH_LIBUSB_1_0 device->VendorID = dev_desc.idVendor; device->ProductID = dev_desc.idProduct; + device->bcdDevice = dev_desc.bcdDevice; + bus_num = libusb_get_bus_number(dev); device->Bus = (char *)malloc(4); if (device->Bus == NULL) { @@ -411,13 +449,56 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev fatal_with_errno(EXIT_FAILURE, "Out of memory"); } snprintf(device->Bus, 4, "%03d", bus_num); + + device_addr = libusb_get_device_address(dev); + device->Device = (char *)malloc(4); + if (device->Device == NULL) { + libusb_free_device_list(devlist, 1); + fatal_with_errno(EXIT_FAILURE, "Out of memory"); + } + if (device_addr > 0) { + snprintf(device->Device, 4, "%03d", device_addr); + } else { + upsdebugx(1, "%s: invalid libusb device address %i", + __func__, device_addr); + free(device->Device); + device->Device = NULL; + } + +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + bus_port = libusb_get_port_number(dev); + device->BusPort = (char *)malloc(4); + if (device->BusPort == NULL) { + libusb_free_device_list(devlist, 1); + fatal_with_errno(EXIT_FAILURE, "Out of memory"); + } + if (bus_port > 0) { + snprintf(device->BusPort, 4, "%03d", bus_port); + } else { + upsdebugx(1, "%s: invalid libusb bus port %i", + __func__, bus_port); + free(device->BusPort); + device->BusPort = NULL; + } +#endif + iManufacturer = dev_desc.iManufacturer; iProduct = dev_desc.iProduct; iSerialNumber = dev_desc.iSerialNumber; #else /* => WITH_LIBUSB_0_1 */ device->VendorID = dev->descriptor.idVendor; device->ProductID = dev->descriptor.idProduct; + device->bcdDevice = dev->descriptor.bcdDevice; device->Bus = xstrdup(bus->dirname); + device->Device = xstrdup(dev->filename); +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + device->BusPort = (char *)malloc(4); + if (device->BusPort == NULL) { + fatal_with_errno(EXIT_FAILURE, "Out of memory"); + } + upsdebugx(2, "%s: NOTE: BusPort is always zero with libusb0", __func__); + snprintf(device->BusPort, 4, "%03d", 0); +#endif iManufacturer = dev->descriptor.iManufacturer; iProduct = dev->descriptor.iProduct; iSerialNumber = dev->descriptor.iSerialNumber; @@ -474,6 +555,11 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev upsdebugx(4, "- Product : %s", device->Product ? device->Product : "unknown"); upsdebugx(4, "- Serial Number: %s", device->Serial ? device->Serial : "unknown"); upsdebugx(4, "- Bus : %s", device->Bus ? device->Bus : "unknown"); + upsdebugx(4, "- Device : %s", device->Device ? device->Device : "unknown"); +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + upsdebugx(4, "- Bus Port : %s", device->BusPort ? device->BusPort : "unknown"); +#endif + upsdebugx(4, "- Device release number: %04x", device->bcdDevice); for (m = matcher; m; m = m->next) { @@ -590,11 +676,41 @@ static int usb_device_open(usb_dev_handle **handlep, USBDevice_t *device, USBDev void upsdrv_initups(void) { char reply[REPLY_PACKETSIZE]; - int i; + char *regex_array[USBMATCHER_REGEXP_ARRAY_LIMIT]; + int i, ret; warn_if_bad_usb_port_filename(device_path); - for (i = 0; usb_device_open(&udev, &usbdevice, &device_matcher, &driver_callback) < 0; i++) { + regex_array[0] = getval("vendorid"); + regex_array[1] = getval("productid"); + regex_array[2] = getval("vendor"); + regex_array[3] = getval("product"); + regex_array[4] = getval("serial"); + regex_array[5] = getval("bus"); + regex_array[6] = getval("device"); +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + regex_array[7] = getval("busport"); +#else + if (getval("busport")) { + upslogx(LOG_WARNING, "\"busport\" is configured for the device, but is not actually handled by current build combination of NUT and libusb (ignored)"); + } +#endif + + ret = USBNewRegexMatcher(®ex_matcher, regex_array, REG_ICASE | REG_EXTENDED); + switch (ret) + { + case -1: + fatal_with_errno(EXIT_FAILURE, "USBNewRegexMatcher"); + case 0: + break; /* all is well */ + default: + fatalx(EXIT_FAILURE, "invalid regular expression: %s", regex_array[ret - 1]); + } + + /* Apply user USB filters first, then confirm the device is supported. */ + regex_matcher->next = &device_matcher; + + for (i = 0; usb_device_open(&udev, &usbdevice, regex_matcher, &driver_callback) < 0; i++) { #ifndef WIN32 if ((i < 32) && (sleep(5) == 0)) { @@ -618,6 +734,13 @@ void upsdrv_initups(void) "Fatal error: unusable configuration"); } + ret = USBNewExactMatcher(&reopen_matcher, &usbdevice); + if (ret) { + fatal_with_errno(EXIT_FAILURE, "USBNewExactMatcher"); + } + + reopen_matcher->next = regex_matcher; + /* * Read rubbish data a few times; the UPS doesn't seem to respond properly * the first few times after connecting @@ -631,11 +754,24 @@ void upsdrv_initups(void) void upsdrv_cleanup(void) { usb_device_close(udev); + USBFreeExactMatcher(reopen_matcher); + USBFreeRegexMatcher(regex_matcher); free(usbdevice.Vendor); free(usbdevice.Product); free(usbdevice.Serial); free(usbdevice.Bus); + free(usbdevice.Device); +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + free(usbdevice.BusPort); +#endif + +#if WITH_LIBUSB_1_0 + if (libusb_inited) { + libusb_exit(NULL); + libusb_inited = 0; + } +#endif } void upsdrv_initinfo(void) @@ -664,7 +800,7 @@ void upsdrv_updateinfo(void) if (!udev) { dstate_setinfo("driver.state", "reconnect.trying"); - ret = usb_device_open(&udev, &usbdevice, &device_matcher, &driver_callback); + ret = usb_device_open(&udev, &usbdevice, reopen_matcher, &driver_callback); if (ret < 0) { return; @@ -808,8 +944,5 @@ void upsdrv_tweak_prognames(void) void upsdrv_makevartable(void) { /* allow -x vendor=X, vendorid=X, product=X, productid=X, serial=X */ - /* TODO: Uncomment while addressing https://github.com/networkupstools/nut/issues/1768 - * When fixing, see also tools/nut-scanner/scan_usb.c "exceptions". - * nut_usb_addvars(); - */ + nut_usb_addvars(); } diff --git a/tools/nut-scanner/scan_usb.c b/tools/nut-scanner/scan_usb.c index 29bd9baf52..069580fa3d 100644 --- a/tools/nut-scanner/scan_usb.c +++ b/tools/nut-scanner/scan_usb.c @@ -773,7 +773,6 @@ nutscan_device_t * nutscan_scan_usb(nutscan_usb_t * scanopts) * do not otherwise support these options: */ if (strncmp(driver_name, "bcmxcp_usb", 10) - && strncmp(driver_name, "richcomm_usb", 12) && strncmp(driver_name, "nutdrv_atcl_usb", 15) ) { snprintf(string, sizeof(string), "%04X", VendorID); @@ -837,7 +836,6 @@ nutscan_device_t * nutscan_scan_usb(nutscan_usb_t * scanopts) /* NOTE: nutdrv_atcl_usb recognizes * "vendor" but not other opts */ if (strncmp(driver_name, "bcmxcp_usb", 10) - && strncmp(driver_name, "richcomm_usb", 12) ) { nutscan_add_option_to_device(nut_dev, "vendor",