From 5e43416c9b4ea3ca9c311f58a584868405751394 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 29 Jan 2025 15:59:05 +0100 Subject: [PATCH 1/6] drivers/apc_modbus.c: _apc_modbus_create_reopen_matcher(): make sure the freed reopen_matcher does not remain in the chain from best_matcher upwards [#2609] Signed-off-by: Jim Klimov --- drivers/apc_modbus.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/apc_modbus.c b/drivers/apc_modbus.c index 978948b3ea..b04bd896f5 100644 --- a/drivers/apc_modbus.c +++ b/drivers/apc_modbus.c @@ -925,6 +925,18 @@ static void _apc_modbus_create_reopen_matcher(void) int r; if (reopen_matcher != NULL) { + USBDeviceMatcher_t *curr, *prev = NULL; + for (curr = best_matcher; curr != NULL; curr = curr->next) { + if (curr == reopen_matcher) { + if (prev) { + prev->next = curr->next; + } else { + best_matcher = curr->next; + } + } else { + prev = curr; + } + } USBFreeExactMatcher(reopen_matcher); reopen_matcher = NULL; } From 2f33a4d3df983a525aa1930ed151c2c5072da515 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 29 Jan 2025 16:00:05 +0100 Subject: [PATCH 2/6] drivers/apc_modbus.c: upsdrv_cleanup(): cleaner release of regex_matcher and reopen_matcher [#2609] Signed-off-by: Jim Klimov --- drivers/apc_modbus.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/apc_modbus.c b/drivers/apc_modbus.c index b04bd896f5..acf487ca32 100644 --- a/drivers/apc_modbus.c +++ b/drivers/apc_modbus.c @@ -2000,6 +2000,9 @@ void upsdrv_cleanup(void) #if defined NUT_MODBUS_HAS_USB USBFreeExactMatcher(reopen_matcher); + reopen_matcher = NULL; + USBFreeExactMatcher(regex_matcher); + regex_matcher = NULL; #endif /* defined NUT_MODBUS_HAS_USB */ } From c0a45c02cd7b38d732d354ae376b22a518f75d0c Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 29 Jan 2025 16:00:53 +0100 Subject: [PATCH 3/6] drivers/apc_modbus.c: _apc_modbus_usb_callback(): debug return codes from current_matcher->match_function() [#2609] Signed-off-by: Jim Klimov --- drivers/apc_modbus.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/apc_modbus.c b/drivers/apc_modbus.c index acf487ca32..27912b45d1 100644 --- a/drivers/apc_modbus.c +++ b/drivers/apc_modbus.c @@ -1719,8 +1719,11 @@ static int _apc_modbus_usb_callback(const modbus_usb_device_t *device) _apc_modbus_usb_lib_to_nut(device, &usbdevice); current_matcher = best_matcher; + upsdebugx(5, "%s: current_matcher=%p", __func__, (void*)current_matcher); while (current_matcher != NULL) { - if (current_matcher->match_function(&usbdevice, current_matcher->privdata) == 1) { + int ret = current_matcher->match_function(&usbdevice, current_matcher->privdata); + upsdebugx(5, "%s: Tried matcher %p returned %d", __func__, (void*)current_matcher, ret); + if (ret == 1) { break; } From fbf65941cb5ac834a5963e8406cafa9501757743 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 29 Jan 2025 16:50:51 +0100 Subject: [PATCH 4/6] drivers/apc_modbus.c: _apc_modbus_usb_callback(): import USB device detail printout from libusb1.c [#2609] Signed-off-by: Jim Klimov --- drivers/apc_modbus.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/apc_modbus.c b/drivers/apc_modbus.c index 27912b45d1..8e5ea3653d 100644 --- a/drivers/apc_modbus.c +++ b/drivers/apc_modbus.c @@ -1718,12 +1718,26 @@ static int _apc_modbus_usb_callback(const modbus_usb_device_t *device) _apc_modbus_usb_lib_to_nut(device, &usbdevice); + upsdebugx(2, "- VendorID: %04x", usbdevice.VendorID); + upsdebugx(2, "- ProductID: %04x", usbdevice.ProductID); + upsdebugx(2, "- Manufacturer: %s", usbdevice.Vendor ? usbdevice.Vendor : "unknown"); + upsdebugx(2, "- Product: %s", usbdevice.Product ? usbdevice.Product : "unknown"); + upsdebugx(2, "- Serial Number: %s", usbdevice.Serial ? usbdevice.Serial : "unknown"); + upsdebugx(2, "- Bus: %s", usbdevice.Bus ? usbdevice.Bus : "unknown"); +#if (defined WITH_USB_BUSPORT) && (WITH_USB_BUSPORT) + upsdebugx(2, "- Bus Port: %s", usbdevice.BusPort ? usbdevice.BusPort : "unknown"); +#endif + upsdebugx(2, "- Device: %s", usbdevice.Device ? usbdevice.Device : "unknown"); + upsdebugx(2, "- Device release number: %04x", usbdevice.bcdDevice); + + upsdebugx(2, "Trying to match device"); current_matcher = best_matcher; upsdebugx(5, "%s: current_matcher=%p", __func__, (void*)current_matcher); while (current_matcher != NULL) { int ret = current_matcher->match_function(&usbdevice, current_matcher->privdata); upsdebugx(5, "%s: Tried matcher %p returned %d", __func__, (void*)current_matcher, ret); if (ret == 1) { + /* known good hit */ break; } From 2c8663f0ac7688883f77f00e29afefa95fd48b6a Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 29 Jan 2025 16:51:31 +0100 Subject: [PATCH 5/6] NEWS.adoc, drivers/apc_modbus.c: _apc_modbus_usb_callback(): handle common return codes from matcher methods, like libusb1.c does [#2609] Notably, handle active rejection of a candidate device by the matcher properly - go try the next device! Signed-off-by: Jim Klimov --- NEWS.adoc | 4 ++++ drivers/apc_modbus.c | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/NEWS.adoc b/NEWS.adoc index 4653a34e14..00c11d8fec 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -374,6 +374,10 @@ https://github.com/networkupstools/nut/milestone/11 to have fixed a Segmentation Fault seen in earlier NUT releases with some of the devices supported by this driver. [#2427] + - apc_modbus driver should now properly handle mismatches of criteria passed + to the regular expression matcher (e.g. serial number) allowing several USB + devices to be monitored without conflict. [#2609] + - phoenixcontact_modbus driver: Introduced Phoenix Contact QUINT4-UPS/24DC management (only new modbus addresses). [#2689, #2716] diff --git a/drivers/apc_modbus.c b/drivers/apc_modbus.c index 8e5ea3653d..3e9d6537af 100644 --- a/drivers/apc_modbus.c +++ b/drivers/apc_modbus.c @@ -1739,6 +1739,16 @@ static int _apc_modbus_usb_callback(const modbus_usb_device_t *device) if (ret == 1) { /* known good hit */ break; + } else if (ret == 0) { + /* known active rejection */ + upsdebugx(2, "%s: Device does not match - skipping", __func__); + /* go to next device that libmodbus would suggest and use this callback again */ + return -1; + } else if (ret == -1) { + fatal_with_errno(EXIT_FAILURE, "%s: matcher", __func__); + } else if (ret == -2) { + upsdebugx(2, "%s: matcher: unspecified error", __func__); + return -1; } current_matcher = current_matcher->next; From 4458262ed29358c4457faa152dbdaa9c277c0dd2 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Wed, 29 Jan 2025 18:09:55 +0100 Subject: [PATCH 6/6] drivers/apc_modbus.c: _apc_modbus_reopen(): do not assume the first matcher in the list is the old reopen_matcher(), rely on _apc_modbus_create_reopen_matcher() to do the right job [#2609] Signed-off-by: Jim Klimov --- drivers/apc_modbus.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/apc_modbus.c b/drivers/apc_modbus.c index 3e9d6537af..e7da827146 100644 --- a/drivers/apc_modbus.c +++ b/drivers/apc_modbus.c @@ -963,8 +963,9 @@ static int _apc_modbus_reopen(void) #if defined NUT_MODBUS_HAS_USB /* We might have matched a new device in the modbus_connect callback. - * Because of this we want a new exact matcher. */ - best_matcher = best_matcher->next; + * Because of this we want a new exact matcher. The method will drop + * the old reopen_matcher from our list starting at best_matcher, and + * from memory. */ _apc_modbus_create_reopen_matcher(); #endif /* defined NUT_MODBUS_HAS_USB */