Skip to content

usbhid-ups: do not crash due to auto-reconnecting #1632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jimklimov merged 4 commits into from
Aug 30, 2022
Merged

usbhid-ups: do not crash due to auto-reconnecting #1632

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
89dbdd1
drivers/usbhid-ups.c: only report "Closing comm_driver previous handl…
jimklimov Aug 27, 2022
d4db5a9
drivers/usbhid-ups.c: if reconnecting with use_interrupt_pipe mode, s…
jimklimov Aug 27, 2022
c96f32b
drivers/usbhid-ups.c: account interrupt_pipe_EIO_count for suggestion…
jimklimov Aug 27, 2022
515d00d
drivers/usbhid-ups.c: cosmetic fixes
jimklimov Aug 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions drivers/usbhid-ups.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ bool_t use_interrupt_pipe = TRUE;
#else
bool_t use_interrupt_pipe = FALSE;
#endif
static size_t interrupt_pipe_EIO_count = 0; /* How many times we had I/O errors since last reconnect? */
static time_t lastpoll; /* Timestamp the last polling */
hid_dev_handle_t udev = HID_DEV_HANDLE_CLOSED;

Expand Down Expand Up @@ -833,7 +834,12 @@ void upsdrv_updateinfo(void)
return;
}

upsdebugx(1, "Got to reconnect!\n");
upsdebugx(1, "Got to reconnect!");
if (use_interrupt_pipe == TRUE && interrupt_pipe_EIO_count > 0) {
upsdebugx(0, "\nReconnecting. If you saw \"nut_libusb_get_interrupt: Input/Output Error\" "
"or similar message in the log above, try setting \"pollonly\" flag in \"ups.conf\" "
"options section for this driver!\n");
}

if (!reconnect_ups()) {
lastpoll = now;
Expand All @@ -842,6 +848,7 @@ void upsdrv_updateinfo(void)
}

hd = &curDevice;
interrupt_pipe_EIO_count = 0;

if (hid_ups_walk(HU_WALKMODE_INIT) == FALSE) {
hd = NULL;
Expand All @@ -851,6 +858,7 @@ void upsdrv_updateinfo(void)
#ifdef DEBUG
interval();
#endif

/* Get HID notifications on Interrupt pipe first */
if (use_interrupt_pipe == TRUE) {
evtCount = HIDGetEvents(udev, event, MAX_EVENT_NUM);
Expand All @@ -864,7 +872,6 @@ void upsdrv_updateinfo(void)
#endif
case LIBUSB_ERROR_NO_DEVICE: /* No such device */
case LIBUSB_ERROR_ACCESS: /* Permission denied */
case LIBUSB_ERROR_IO: /* I/O error */
#if WITH_LIBUSB_0_1 /* limit to libusb 0.1 implementation */
case -ENXIO: /* No such device or address */
#endif
Expand All @@ -874,6 +881,11 @@ void upsdrv_updateinfo(void)
/* Uh oh, got to reconnect! */
hd = NULL;
return;
case LIBUSB_ERROR_IO: /* I/O error */
/* Uh oh, got to reconnect, with a special suggestion! */
interrupt_pipe_EIO_count++;
hd = NULL;
return;
default:
upsdebugx(1, "Got %i HID objects...", (evtCount >= 0) ? evtCount : 0);
break;
Expand All @@ -900,10 +912,10 @@ void upsdrv_updateinfo(void)

/* Skip Input reports, if we don't use the Feature report */
found_data = FindObject_with_Path(pDesc, &(event[i]->Path), interrupt_only ? ITEM_INPUT:ITEM_FEATURE);
if(!found_data && !interrupt_only) {
if (!found_data && !interrupt_only) {
found_data = FindObject_with_Path(pDesc, &(event[i]->Path), ITEM_INPUT);
}
if(!found_data) {
if (!found_data) {
upsdebugx(2, "Could not find event as either ITEM_INPUT or ITEM_FEATURE?");
continue;
}
Expand All @@ -919,7 +931,7 @@ void upsdrv_updateinfo(void)
upsdebugx(1, "took %.3f seconds handling interrupt reports...\n",
interval());
#endif
/* clear status buffer before begining */
/* clear status buffer before beginning */
status_init();

/* Do a full update (polling) every pollfreq
Expand Down Expand Up @@ -1482,7 +1494,6 @@ static bool_t hid_ups_walk(walkmode_t mode)
#endif
case LIBUSB_ERROR_NO_DEVICE: /* No such device */
case LIBUSB_ERROR_ACCESS: /* Permission denied */
case LIBUSB_ERROR_IO: /* I/O error */
#if WITH_LIBUSB_0_1 /* limit to libusb 0.1 implementation */
case -ENXIO: /* No such device or address */
#endif
Expand All @@ -1493,6 +1504,12 @@ static bool_t hid_ups_walk(walkmode_t mode)
hd = NULL;
return FALSE;

case LIBUSB_ERROR_IO: /* I/O error */
/* Uh oh, got to reconnect, with a special suggestion! */
interrupt_pipe_EIO_count++;
Copy link
Copy Markdown
Member Author

@jimklimov jimklimov Aug 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if the count should go down/zero if there were successful reads later?.. So far this is mostly informative (suggest pollonly if there were any interrupt errors)...

hd = NULL;
return FALSE;

case 1:
break; /* Found! */

Expand Down Expand Up @@ -1571,10 +1588,14 @@ static int reconnect_ups(void)
wait_before_reconnect = atoi(val);
}

upsdebugx(4, "Closing comm_driver previous handle");
/* Try to close the previous handle */
if (udev)
if (udev == HID_DEV_HANDLE_CLOSED) {
upsdebugx(4, "Not closing comm_driver previous handle: already closed");
} else {
upsdebugx(4, "Closing comm_driver previous handle");
comm_driver->close_dev(udev);
udev = HID_DEV_HANDLE_CLOSED;
}

upsdebugx(4, "===================================================================");
if (wait_before_reconnect > 0 ) {
Expand Down