Skip to content

rtu-usb: claim the HID interface, send full-length reports, allow disabling reset - #12

Open
d01 wants to merge 4 commits into
networkupstools:rtu_usbfrom
d01:upstream-submission
Open

rtu-usb: claim the HID interface, send full-length reports, allow disabling reset#12
d01 wants to merge 4 commits into
networkupstools:rtu_usbfrom
d01:upstream-submission

Conversation

@d01

@d01 d01 commented Aug 9, 2026

Copy link
Copy Markdown

Updated 2026-08-16: the description of the reset (now section 5) was revised —
continued instrumented testing disproved my "fatal" characterization, and the
reset handling has since been reworked into commit 48797e4. Earlier update:
the original version of this PR removed the device reset outright, which was
wrong of me — see networkupstools/nut#2609.

Four commits for the rtu_usb backend: two bugs in the basic I/O path, two
more found in day-long soak testing, and a rework of how reset-on-open is
scoped. Found getting NUT's apc_modbus working with an APC Smart-UPS X1500
(051d:0003, firmware "UPS 16.0").

1. The HID interface is never claimed for I/O

_usb_get_hid_descriptor() claims the interface just long enough to read the
report descriptor and then releases it. On a match, _modbus_rtu_usb_connect()
stores the handle and breaks without claiming anything, so every subsequent
interrupt transfer runs 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.

Now claimed for the life of the connection and released in
_modbus_rtu_usb_close().

2. Short interrupt OUT transfers are ignored

The report descriptor declares output report 0x90 as 63 bytes, but the send
path transfers payload_chunk_len + 1 — 7 bytes for a 6-byte request. This
firmware accepts the short transfer at the host-controller level and then
ignores it: no reply arrives, and the Modbus engine is left unresponsive until
the USB cable is reseated. Sending the full declared length, zero padded, makes
the identical request work.

The OUT transfer also passed timeout 0 (wait forever), so an unresponsive
endpoint blocked the caller indefinitely rather than returning an error — which
is how this bug presented, and made it much harder to find. Now finite.

3. Flush until a full inter-frame interval of silence (63583ff)

The pre-send flush drained for a fixed 10 ms, which lets a straggling reply
from an earlier exchange survive into the next one and get mistaken for its
reply. APC's AN176 documents a 35 ms inter-frame minimum for these devices;
the flush now drains until it has seen a full 45 ms of continuous silence.

4. One receive deadline across skipped reports (a832970)

The receive path restarted the full response timeout for every non-Modbus HID
report it skipped. Against a device that broadcasts periodic notification
reports (this one emits several per second when degraded), the configured
timeout could effectively never fire — a "2 s timeout" only fired on total
device muteness. One deadline now spans the whole receive.

5. Reset-on-open: only the matched device, after the callback (48797e4), plus modbus_rtu_usb_set_reset_on_open()

Two problems with the reset added in #11, one of scoping and one I
mischaracterized:

Scoping: the reset ran inside enumeration, on every USB device on the bus
the code could open, before the device-selection callback was consulted —
so unrelated devices got reset while probing, and a callback had no way to
veto the reset for the device actually selected. 48797e4 moves it to fire
only on the matched device, after the callback has run. The default stays
enabled, so existing behavior for matched devices is preserved; the new
modbus_rtu_usb_set_reset_on_open() lets a caller that knows its hardware
opt out.

The mischaracterization: an earlier version of this PR called the reset
"fatal" on the X1500 and disabled it by default. Longer instrumented testing
showed that was wrong, and I want the record straight since it maligned the
mechanism beyond the data. A single reset is survivable on this model — it
typically goes deaf for up to 2.5 minutes and recovers fully, sometimes
resuming within seconds; it has absorbed more than a dozen isolated resets.
What actually wedged it (until a physical cable reseat) was resets in close
succession: my "fatal" case was a hidden restart loop — every driver start
issued this reset before the callback could veto it, the post-reset deaf
window beat the first-read timeout, systemd restarted the driver, and the
next start reset it again.

One data point on the #11 rationale (recovering a desynced device serving
stale replies), for whoever weighs the default: on this model the reset
usually does not shed the queued stale reply — across a dozen resets issued
against a known standing desync, two shed it, and the pattern is consistent
with the reset discarding only a reply already staged in the endpoint buffer
while a reply the firmware still withholds internally survives. Other
hardware may well behave differently; whether the default should change, or
this should be a device quirk or desync-triggered, is a judgement call for
someone with visibility across more hardware than I have — happy to rework
it whichever way you prefer.

Testing

All against NUT 2.8.4's apc_modbus, statically linked against this branch,
on the one X1500 (so review by anyone with other rtu_usb hardware would be
valuable):

  • Fixes 1 and 2: with both, the driver starts, reads the inventory block and
    polls cleanly; with either missing, the 516:636 inventory read fails
    immediately and the driver exits with "Can't read inventory information
    from the UPS".
  • Fixes 3 and 4 have run continuously for several days; a 24-hour soak came
    back with zero faults (no reconnects, no staleness, no read failure
    surviving a single retry).
  • 48797e4 validated live since: seven deliberately armed resets each fired
    on the matched device only, after the callback; a full host reboot with a
    vetoing callback produced zero resets (confirmed in the kernel log).

Note on authorship

Developed with assistance from Claude (Anthropic); the commit carries a
Co-Authored-By trailer. All findings were verified on real hardware.

@d01
d01 force-pushed the upstream-submission branch from fedd0ad to 4553446 Compare August 9, 2026 21:15
@d01 d01 changed the title rtu-usb: fix three bugs preventing Modbus over USB on APC Smart-UPS X1500 rtu-usb: claim the HID interface, send full-length reports, allow disabling reset Aug 9, 2026
…abling 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) <noreply@anthropic.com>
Signed-off-by: d01 <d01@users.noreply.github.com>
d01 and others added 3 commits August 14, 2026 14:23
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 <noreply@anthropic.com>
Signed-off-by: d01 <d01@users.noreply.github.com>
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 <noreply@anthropic.com>
Signed-off-by: d01 <d01@users.noreply.github.com>
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 <noreply@anthropic.com>
Signed-off-by: d01 <d01@users.noreply.github.com>
@d01

d01 commented Aug 15, 2026

Copy link
Copy Markdown
Author

Pushed three more commits from continued testing against an APC Smart-UPS (051d:0003):

  • 63583ff — flush drains until a full 45 ms inter-frame interval of silence (AN176 documents a 35 ms interframe minimum; the previous 10 ms window let stragglers from an earlier exchange survive into the next one and get mistaken for its reply).
  • a832970 — the receive path enforces one deadline across skipped non-Modbus reports. Previously the full timeout restarted for every non-Modbus HID report, so against a device emitting periodic notification reports the configured response timeout could effectively never fire.
  • 48797e4 — reset-on-open resets only the matched device, and only after the selection callback has run. Before, enumeration reset every USB device on the bus it could open, before the callback could veto via modbus_rtu_usb_set_reset_on_open(). The default stays enabled, so existing behavior for matched devices is preserved.

All three have been running against the APC unit for several days through NUT's apc_modbus; the latest 24 h soak came back with zero faults.

@EchterAgo

Copy link
Copy Markdown

I'll test this on my units

@d01

d01 commented Aug 16, 2026

Copy link
Copy Markdown
Author

@EchterAgo Thanks! One heads-up before you start, so you're not testing against a stale description: I've just revised the PR body in light of further instrumented testing. The original "the reset is fatal" characterization was wrong — what wedged my unit was a restart loop clustering resets (every driver start issued a hidden reset before the callback could veto it); isolated resets are survivable here. The body now covers all four commits, including the 48797e4 ordering rework and its live validation, plus measurements on how often the reset actually sheds a queued stale reply on this model. Full correction with the data posted on networkupstools/nut#2609.

No code has changed since your comment — only the description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants