From 2d03b6710669fb8b70ecf457b5cebbd252fd947a Mon Sep 17 00:00:00 2001 From: d01 Date: Sun, 9 Aug 2026 16:11:35 -0400 Subject: [PATCH 1/4] rtu-usb: claim the HID interface, send full-length reports, allow disabling reset Found while getting NUT's apc_modbus working with an APC Smart-UPS X1500 (051d:0003, firmware "UPS 16.0"). Two bugs, plus an option needed because the existing device reset is fatal on this hardware. 1. Claim the HID interface before doing I/O on it. _usb_get_hid_descriptor() claims the interface only long enough to read the report descriptor and then releases it, so the interrupt transfers that follow run against an interface still owned by the kernel HID driver. The OUT transfer is not delivered and the device's input reports go to usbhid rather than to us. The interface is now held for the life of the connection and released in _modbus_rtu_usb_close(). 2. Send full-length, zero-padded OUT reports. The report descriptor declares the output report as 63 bytes. A short transfer (report id plus only the Modbus bytes -- 7 bytes for a 6-byte request) is accepted by the host controller but ignored by this firmware, and leaves the device's Modbus engine unresponsive until the USB cable is physically reseated. The interrupt OUT transfer also used timeout 0, i.e. wait forever, so an unresponsive endpoint blocked the caller indefinitely instead of returning an error. Now a finite timeout. 3. Make the reset on open optional (modbus_rtu_usb_set_reset_on_open). Default is unchanged -- the reset still happens -- because it is there for a reason: it recovers devices whose framing has desynchronised and which return stale data from earlier requests. On the Smart-UPS X1500 it is fatal. After the reset the device never services its interrupt OUT endpoint again, every register read times out, and only physically reseating the USB cable restores it. A sysfs-level re-enumeration reproduces the same state; neither idle time nor draining the endpoint recovers it. Worth noting the reset runs for every device on the bus during enumeration, before the match callback is consulted. So rather than removing it, this adds a way to turn it off. Whether the default should change, or whether it would be better driven by a quirk or by detecting the desync it is meant to fix, is a judgement call for someone with visibility across more hardware than I have. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: d01 --- src/modbus-rtu-usb-private.h | 8 +++ src/modbus-rtu-usb.c | 129 +++++++++++++++++++++++++++++++++-- src/modbus-rtu-usb.h | 6 ++ 3 files changed, 137 insertions(+), 6 deletions(-) diff --git a/src/modbus-rtu-usb-private.h b/src/modbus-rtu-usb-private.h index 23ed86002..a6140407e 100644 --- a/src/modbus-rtu-usb-private.h +++ b/src/modbus-rtu-usb-private.h @@ -34,6 +34,10 @@ static const uint8_t _MODBUS_RTU_USB_Default_TX_Report_Id = 0x89; static const int _MODBUS_USB_REPORT_SIZE = _MODBUS_USB_REPORT_SIZE_VAL; static const int _MODBUS_USB_PAYLOAD_SIZE = _MODBUS_USB_REPORT_SIZE_VAL - 1; +/* Timeout for the interrupt OUT transfer, in milliseconds. A device that never + * services the endpoint would otherwise block the caller forever. */ +#define _MODBUS_USB_SEND_TIMEOUT_MS 5000 + typedef struct _modbus_rtu_usb { /* Device selection callback */ modbus_usb_device_selection_callback_t callback; @@ -41,6 +45,10 @@ typedef struct _modbus_rtu_usb { char *path; /* Libusb device handle */ struct libusb_device_handle *device_handle; + /* Interface claimed for I/O, or -1 if none is held */ + int claimed_interface; + /* Reset the device when opening it (default on) */ + int reset_on_open; /* USB endpoint */ int endpoint; /* RTU USB report ids */ diff --git a/src/modbus-rtu-usb.c b/src/modbus-rtu-usb.c index 204bcec04..7d2322288 100644 --- a/src/modbus-rtu-usb.c +++ b/src/modbus-rtu-usb.c @@ -162,6 +162,13 @@ static ssize_t _modbus_rtu_usb_send(modbus_t *ctx, const uint8_t *req, int req_l while (total_remaining > 0) { /* USB 2.0 Full Speed only supports 64 bytes per transfer, * one byte is used for the report ID, leaving 63 to the payload*/ + + /* Send a full-length report, zero padded. The HID report descriptor + * declares the output report as 63 bytes; a short transfer is accepted + * by the host controller but ignored by at least some devices (an APC + * Smart-UPS X1500 never answers one, and its Modbus engine is left + * unresponsive until the USB cable is physically reseated). */ + memset(usb_report, 0, _MODBUS_USB_REPORT_SIZE_VAL); usb_report[0] = ctx_rtu_usb->rx_report_id; /* Transfer in payload chunks of 63 until we have less than 63 left */ payload_chunk_len = (total_remaining > _MODBUS_USB_PAYLOAD_SIZE) @@ -171,20 +178,20 @@ static ssize_t _modbus_rtu_usb_send(modbus_t *ctx, const uint8_t *req, int req_l r = libusb_interrupt_transfer(ctx_rtu_usb->device_handle, LIBUSB_ENDPOINT_OUT | ctx_rtu_usb->endpoint, usb_report, - payload_chunk_len + 1, /* +1 for report ID */ + _MODBUS_USB_REPORT_SIZE_VAL, &transferred, - 0); + _MODBUS_USB_SEND_TIMEOUT_MS); if (r != LIBUSB_SUCCESS) { errno = _usb_error_to_errno(r); return -1; } - if (transferred < payload_chunk_len + 1) { + if (transferred < _MODBUS_USB_REPORT_SIZE_VAL) { break; } - total_remaining -= transferred - 1; - total_transferred += transferred - 1; + total_remaining -= payload_chunk_len; + total_transferred += payload_chunk_len; } return total_transferred; @@ -414,6 +421,65 @@ static int _usb_get_hid_descriptor(modbus_t *ctx, return -1; } +/* Detach any kernel driver from the device's HID interface and claim it for + * ourselves, returning the interface number or a negative libusb error. + * + * Unlike _usb_get_hid_descriptor(), which releases the interface as soon as it + * has read the descriptor, the caller holds this one for the life of the + * connection: interrupt transfers issued on an interface owned by the kernel + * HID driver are not delivered, and the device's input reports go to that + * driver rather than to us. */ +static int _usb_claim_hid_interface(modbus_t *ctx, libusb_device_handle *dev_handle) +{ + int r, iface_idx, alt_idx, iface_num; + struct libusb_config_descriptor *conf_desc; + const struct libusb_interface *iface; + const struct libusb_interface_descriptor *alt_iface_desc; + + if ((r = libusb_get_active_config_descriptor(libusb_get_device(dev_handle), + &conf_desc)) != LIBUSB_SUCCESS) { + return r; + } + + for (iface_idx = 0; iface_idx < conf_desc->bNumInterfaces; iface_idx++) { + iface = &conf_desc->interface[iface_idx]; + + for (alt_idx = 0; alt_idx < iface->num_altsetting; alt_idx++) { + alt_iface_desc = &iface->altsetting[alt_idx]; + + if (alt_iface_desc->bInterfaceClass != LIBUSB_CLASS_HID) { + continue; + } + + iface_num = alt_iface_desc->bInterfaceNumber; + + if (libusb_kernel_driver_active(dev_handle, iface_num) == 1) { + r = libusb_detach_kernel_driver(dev_handle, iface_num); + if (r != LIBUSB_SUCCESS) { + libusb_free_config_descriptor(conf_desc); + return r; + } + } + + r = libusb_claim_interface(dev_handle, iface_num); + if (r != LIBUSB_SUCCESS) { + libusb_free_config_descriptor(conf_desc); + return r; + } + + if (ctx->debug) { + printf("Claimed interface %d for Modbus I/O\n", iface_num); + } + + libusb_free_config_descriptor(conf_desc); + return iface_num; + } + } + + libusb_free_config_descriptor(conf_desc); + return LIBUSB_ERROR_NOT_FOUND; +} + static int _modbus_rtu_usb_connect(modbus_t *ctx) { libusb_device **devs, *d; @@ -516,7 +582,15 @@ static int _modbus_rtu_usb_connect(modbus_t *ctx) continue; } - libusb_reset_device(dev_handle); + /* Resetting the device on open recovers hardware whose host/device + * framing has lost synchronisation, but it is fatal on others: an APC + * Smart-UPS X1500 (051d:0003) never services its interrupt OUT + * endpoint again afterwards, and only physically reseating the USB + * cable restores it. Note this runs for every device on the bus, + * before the match callback is consulted. */ + if (ctx_rtu_usb->reset_on_open) { + libusb_reset_device(dev_handle); + } if (dev_desc.iManufacturer) { memset(&vendor_buffer, 0, sizeof(vendor_buffer)); @@ -579,6 +653,19 @@ static int _modbus_rtu_usb_connect(modbus_t *ctx) printf(" Product ID: 0x%04x\n", ud.pid); } + /* Claim the HID interface before any I/O on it. */ + r = _usb_claim_hid_interface(ctx, dev_handle); + if (r < 0) { + if (ctx->debug) { + fprintf(stderr, + "failed to claim HID interface: %s\n", + libusb_strerror(r)); + } + libusb_close(dev_handle); + continue; + } + ctx_rtu_usb->claimed_interface = r; + ctx_rtu_usb->device_handle = dev_handle; #if defined HAVE_LIBUSB_POLLFD && HAVE_LIBUSB_POLLFD if (usb_ctx) { @@ -643,6 +730,11 @@ static void _modbus_rtu_usb_close(modbus_t *ctx) modbus_rtu_usb_t *ctx_rtu_usb = ctx->backend_data; if (ctx_rtu_usb->device_handle != NULL) { + if (ctx_rtu_usb->claimed_interface >= 0) { + libusb_release_interface(ctx_rtu_usb->device_handle, + ctx_rtu_usb->claimed_interface); + ctx_rtu_usb->claimed_interface = -1; + } libusb_close(ctx_rtu_usb->device_handle); ctx_rtu_usb->device_handle = NULL; _usb_exit(); @@ -756,6 +848,8 @@ static modbus_t *_modbus_new_rtu_usb_common(modbus_usb_modes mode) memset(ctx_rtu_usb, 0, sizeof(modbus_rtu_usb_t)); ctx_rtu_usb->endpoint = 1; + ctx_rtu_usb->claimed_interface = -1; + ctx_rtu_usb->reset_on_open = TRUE; ctx_rtu_usb->confirmation_to_ignore = FALSE; _modbus_rtu_usb_clear_buffers(ctx); @@ -893,6 +987,29 @@ int modbus_rtu_usb_set_callback(modbus_t *ctx, return 0; } +/* Enable or disable the USB reset performed when opening the device. + * + * Enabled by default, preserving existing behaviour: the reset recovers + * devices whose framing has desynchronised, where reads otherwise return stale + * data belonging to earlier requests. It is fatal on some hardware, however -- + * an APC Smart-UPS X1500 stops servicing its interrupt OUT endpoint entirely + * once reset, recoverable only by reseating the USB cable -- so callers + * working with such a device need a way to turn it off. */ +int modbus_rtu_usb_set_reset_on_open(modbus_t *ctx, int enabled) +{ + modbus_rtu_usb_t *ctx_rtu_usb; + + if (ctx == NULL) { + errno = EINVAL; + return -1; + } + + ctx_rtu_usb = ctx->backend_data; + ctx_rtu_usb->reset_on_open = enabled ? TRUE : FALSE; + + return 0; +} + int modbus_rtu_usb_set_report_ids(modbus_t *ctx, uint8_t rx, uint8_t tx) { modbus_rtu_usb_t *ctx_rtu_usb = ctx->backend_data; diff --git a/src/modbus-rtu-usb.h b/src/modbus-rtu-usb.h index 3dd3ce9a3..67b236219 100644 --- a/src/modbus-rtu-usb.h +++ b/src/modbus-rtu-usb.h @@ -62,6 +62,12 @@ modbus_rtu_usb_set_callback(modbus_t *ctx, MODBUS_API int modbus_rtu_usb_set_report_ids(modbus_t *ctx, uint8_t rx, uint8_t tx); +/* Enable or disable the USB device reset performed when opening the device. + * Enabled by default. The reset recovers devices whose host/device framing has + * lost synchronisation, but is fatal on some hardware -- see + * modbus_rtu_usb_set_reset_on_open() notes in modbus-rtu-usb.c. */ +MODBUS_API int modbus_rtu_usb_set_reset_on_open(modbus_t *ctx, int enabled); + MODBUS_END_DECLS #endif /* MODBUS_RTU_USB_H */ From 63583ffef6296bde6767bd590f7c90aa64fdef47 Mon Sep 17 00:00:00 2001 From: d01 Date: Fri, 14 Aug 2026 14:23:34 -0400 Subject: [PATCH 2/4] rtu-usb: flush until a full inter-frame interval of silence The flush's drain-until-idle window was 10 ms, which returns before a late reply can arrive (~40 ms observed on an APC Smart-UPS X1500), so a reply abandoned by a timed-out request survived every flush and was mistaken for the answer to the next request. APC's AN176 (sec 4.2.2) specifies a 35 ms minimum inter-frame interval for these devices; apcupsd's driver has used 45 ms in the field since 2014 for the same reason. Use 45 ms. Co-Authored-By: Claude Fable 5 Signed-off-by: d01 --- src/modbus-rtu-usb.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/modbus-rtu-usb.c b/src/modbus-rtu-usb.c index 7d2322288..8c9b04db5 100644 --- a/src/modbus-rtu-usb.c +++ b/src/modbus-rtu-usb.c @@ -746,8 +746,16 @@ static int _modbus_rtu_usb_flush(modbus_t *ctx) int rc; int rc_sum = 0; + /* Drain until the line has been idle for a full Modbus inter-frame + * interval. APC's AN176 (sec 4.2.2) specifies a 35 ms minimum inter-frame + * for these devices, and apcupsd's field-proven driver waits 45 ms ("spec + * is 35, increase due to UPS missing messages occasionally"). The previous + * 10 ms window returned before a late reply could arrive, which made every + * flush-based recovery a measured no-op: a reply abandoned by a timed-out + * request would survive the flush and be mistaken for the answer to the + * next request. */ for (;;) { - rc = _modbus_rtu_usb_recv_more(ctx, 10); + rc = _modbus_rtu_usb_recv_more(ctx, 45); if (rc < 0) { if (errno == ETIMEDOUT) From a832970bdbdab27d4aa2645e341aa56e124fc305 Mon Sep 17 00:00:00 2001 From: d01 Date: Fri, 14 Aug 2026 15:36:17 -0400 Subject: [PATCH 3/4] rtu-usb: enforce one receive deadline across skipped reports recv_more granted a fresh full timeout to every interrupt transfer it retried after skipping a non-Modbus report, and select granted one to every report of a multi-report reply. The device interleaves unsolicited HID notification reports on the same interrupt endpoint (AN178 sec 3.2.2 rate-limits them to one per 15 s, but a degraded device has been measured emitting them at 1 Hz), so a notification stream could defer the response deadline indefinitely: a 2 s response timeout only ever expired if the device went completely silent, notifications included. Compute the deadline once on entry and give each transfer only the time remaining, as apcupsd's ModbusRx does. A timeout of 0 still waits forever. This also gives flush true fixed-window semantics: 45 ms of Modbus silence now ends the drain regardless of notification traffic. Co-Authored-By: Claude Fable 5 Signed-off-by: d01 --- src/modbus-rtu-usb.c | 59 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/src/modbus-rtu-usb.c b/src/modbus-rtu-usb.c index 8c9b04db5..36b0d7fd5 100644 --- a/src/modbus-rtu-usb.c +++ b/src/modbus-rtu-usb.c @@ -9,6 +9,7 @@ #include #include #include +#include #ifndef _MSC_VER #include #endif @@ -225,12 +226,31 @@ static int _modbus_rtu_usb_receive(modbus_t *ctx, uint8_t *req) return rc; } +static uint64_t _modbus_rtu_usb_now_usecs(void) +{ + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + return (uint64_t) ts.tv_sec * 1000000 + (uint64_t) (ts.tv_nsec / 1000); +} + +/* Wait for one Modbus report and append its payload to the receive buffer. + * timeout_msecs bounds the whole wait (0 waits forever): the device + * interleaves unsolicited HID notification reports on the same interrupt + * endpoint, and granting each skipped report a fresh timeout lets a + * notification stream defer the deadline indefinitely (APC's AN178 sec 3.2.2 + * rate-limits notifications to one per 15 s, but a degraded device has been + * measured emitting them at 1 Hz, which would keep a 2 s response timeout + * from ever expiring). */ static ssize_t _modbus_rtu_usb_recv_more(modbus_t *ctx, unsigned int timeout_msecs) { uint8_t usb_report[_MODBUS_USB_REPORT_SIZE]; int transferred, r, payload_len; + uint64_t deadline_usecs = 0; modbus_rtu_usb_t *ctx_rtu_usb = ctx->backend_data; + if (timeout_msecs > 0) + deadline_usecs = _modbus_rtu_usb_now_usecs() + (uint64_t) timeout_msecs * 1000; + if (ctx_rtu_usb->device_handle == NULL) { errno = EINVAL; return -1; @@ -256,12 +276,24 @@ static ssize_t _modbus_rtu_usb_recv_more(modbus_t *ctx, unsigned int timeout_mse } while (1) { + unsigned int remaining_msecs = 0; /* libusb: 0 waits forever */ + + if (timeout_msecs > 0) { + uint64_t now_usecs = _modbus_rtu_usb_now_usecs(); + if (now_usecs >= deadline_usecs) { + errno = ETIMEDOUT; + return -1; + } + remaining_msecs = + (unsigned int) ((deadline_usecs - now_usecs + 999) / 1000); + } + r = libusb_interrupt_transfer(ctx_rtu_usb->device_handle, LIBUSB_ENDPOINT_IN | ctx_rtu_usb->endpoint, usb_report, sizeof(usb_report), &transferred, - timeout_msecs); + remaining_msecs); if (r != LIBUSB_SUCCESS) { errno = _usb_error_to_errno(r); return -1; @@ -787,6 +819,31 @@ static int _modbus_rtu_usb_select(modbus_t *ctx, timeout_msecs = (tv->tv_sec * 1000) + (tv->tv_usec / 1000); + /* One deadline for the whole wait: without it, each report of a + * multi-report reply grants the device a fresh full timeout. */ + if (timeout_msecs > 0) { + uint64_t deadline_usecs = + _modbus_rtu_usb_now_usecs() + (uint64_t) timeout_msecs * 1000; + + while ((ctx_rtu_usb->usb_buffer_end - ctx_rtu_usb->usb_buffer_start) < + length_to_read) { + uint64_t now_usecs = _modbus_rtu_usb_now_usecs(); + unsigned int remaining_msecs; + + if (now_usecs >= deadline_usecs) { + errno = ETIMEDOUT; + return -1; + } + remaining_msecs = + (unsigned int) ((deadline_usecs - now_usecs + 999) / 1000); + if (_modbus_rtu_usb_recv_more(ctx, remaining_msecs) <= 0) { + return -1; + } + } + + return 0; + } + while ((ctx_rtu_usb->usb_buffer_end - ctx_rtu_usb->usb_buffer_start) < length_to_read) { if (_modbus_rtu_usb_recv_more(ctx, timeout_msecs) <= 0) { From 48797e48daeca86638d47a92acd50d5a7db18c24 Mon Sep 17 00:00:00 2001 From: d01 Date: Fri, 14 Aug 2026 17:39:43 -0400 Subject: [PATCH 4/4] rtu-usb: reset only the matched device, after the selection callback The reset-on-open in _modbus_rtu_usb_connect ran inside the enumeration loop, immediately after libusb_open of each candidate: while hunting for the target it reset every USB device on the bus it could open, and it ran before the device-selection callback was consulted, so a callback that disables it via modbus_rtu_usb_set_reset_on_open() could only ever protect within-process reconnects -- the first open of the process had already reset the device. Move the reset inside the match branch, after the selection callback has accepted the device and before the HID interface is claimed. Only the matched device is ever reset, and the callback can now veto the reset per open. The default stays enabled, preserving existing behavior for paths that rely on the reset to recover lost host/device framing. The veto matters on hardware where resets are expensive or fatal: an APC Smart-UPS (051d:0003) goes deaf for 1.5-2.5 minutes after each reset (measured 2026-08-14), and three resets within four minutes -- two of them 26 s apart -- left one servicing nothing on its interrupt endpoints until its USB cable was physically reseated. A hidden reset on every process start is also indistinguishable from run-to-run degradation to anyone measuring driver behavior. Co-Authored-By: Claude Fable 5 Signed-off-by: d01 --- src/modbus-rtu-usb.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/modbus-rtu-usb.c b/src/modbus-rtu-usb.c index 36b0d7fd5..4ce186059 100644 --- a/src/modbus-rtu-usb.c +++ b/src/modbus-rtu-usb.c @@ -614,16 +614,6 @@ static int _modbus_rtu_usb_connect(modbus_t *ctx) continue; } - /* Resetting the device on open recovers hardware whose host/device - * framing has lost synchronisation, but it is fatal on others: an APC - * Smart-UPS X1500 (051d:0003) never services its interrupt OUT - * endpoint again afterwards, and only physically reseating the USB - * cable restores it. Note this runs for every device on the bus, - * before the match callback is consulted. */ - if (ctx_rtu_usb->reset_on_open) { - libusb_reset_device(dev_handle); - } - if (dev_desc.iManufacturer) { memset(&vendor_buffer, 0, sizeof(vendor_buffer)); r = libusb_get_string_descriptor_ascii(dev_handle, @@ -685,6 +675,19 @@ static int _modbus_rtu_usb_connect(modbus_t *ctx) printf(" Product ID: 0x%04x\n", ud.pid); } + /* Reset only the matched device, and only after the selection + * callback has run, so the callback can veto it with + * modbus_rtu_usb_set_reset_on_open(ctx, 0): a reset recovers + * hardware whose host/device framing has lost synchronisation, + * but on some devices it is expensive or fatal -- an APC + * Smart-UPS (051d:0003) goes deaf for 1.5-2.5 minutes after + * each reset, and consecutive resets in close succession left + * one servicing nothing on its interrupt endpoints until its + * USB cable was physically reseated. */ + if (ctx_rtu_usb->reset_on_open) { + libusb_reset_device(dev_handle); + } + /* Claim the HID interface before any I/O on it. */ r = _usb_claim_hid_interface(ctx, dev_handle); if (r < 0) {