I finally got around to running a UPS using its data connection to
shut down systems when a power failure occurs using sensorsd, and
while trying to build some scripts to manage the peculiarities of
my setup, I found that I could not read directly from the UPS, though
I could write to it.
Root cause is that devices with report IDs are getting wrong return
values on the sync path (async works fine), in uhi_get_report,
lines 841-872 of uhidev.c. It's returning -1 with errno EIO,
even on success, while USB_SET_REPORT works correctly.
uhidev_get_report prepends room for the report ID byte (len++),
does the transfer, and copies len-1 bytes past the ID back to
the caller, but then returns actlen without subtracting the report
ID byte.
uhidev_set_report() returns the caller's original len and
uhidev_get_report_async_cb() already decrements len for id > 0,
so those paths are already working properly.
My testing found all uhid children failed on read, while upd0 sensors
work. To show the issue, attach an USB HID device with report IDs and
a feature report (in my case it's an APC BackUPS 1500), and for any
one of its uhidN:
struct usb_ctl_report r;
memset(&r, 0, sizeof(r));
r.ucr_report = UHID_FEATURE_REPORT;
if (ioctl(fd, USB_GET_REPORT, &r) == -1)
err(1, "USB_GET_REPORT"); /* always EIO */
Patch: Subtract the report ID byte from actlen, as the async callback
does:
Index: sys/dev/usb/uhidev.c
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -864,6 +864,8 @@ uhidev_get_report(struct uhidev_softc *sc, int type, int
id, void *data,
/* Skip the reportID. */
if (id > 0) {
+ if (actlen > 0)
+ actlen--;
memcpy(data, buf + 1, len - 1);
free(buf, M_TEMP, len);
}
--
Jim Lippard [email protected] https://discord.org/
GPG Key ID: 0x99FD5CD6
