Skip to content

Improve usb.core.Device error handling REV 2 #10616

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
merged 8 commits into from
Sep 9, 2025
Merged

Improve usb.core.Device error handling REV 2 #10616

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
05db6ee
fix bad xfer_result_t enum values
samblenny Sep 2, 2025
b076077
use switch for ctrl_transfer result check
samblenny Sep 2, 2025
e9da6bc
return actual length from ctrl_transfer
samblenny Sep 2, 2025
f3560f9
handle XFER_RESULT_FAILED with exception
samblenny Sep 3, 2025
7d26fe9
factor out timed transfer setup & err handling
samblenny Sep 3, 2025
e8a6d38
fix idVendor and idProduct error check
samblenny Sep 6, 2025
99f244e
fix comment
samblenny Sep 6, 2025
ad745c2
error checks for usb device string properties
samblenny Sep 7, 2025
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
214 changes: 137 additions & 77 deletions shared-module/usb/core/Device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This file is part of the CircuitPython project: https://circuitpython.org
//
// SPDX-FileCopyrightText: Copyright (c) 2022 Scott Shawcroft for Adafruit Industries
// SPDX-FileCopyrightText: Copyright (c) 2025 Sam Blenny
//
// SPDX-License-Identifier: MIT

Expand Down Expand Up @@ -45,7 +46,7 @@ bool common_hal_usb_core_device_construct(usb_core_device_obj_t *self, uint8_t d
}
self->device_address = device_address;
self->first_langid = 0;
_xfer_result = 0xff;
_xfer_result = XFER_RESULT_INVALID;
return true;
}

Expand All @@ -70,14 +71,18 @@ void common_hal_usb_core_device_deinit(usb_core_device_obj_t *self) {
uint16_t common_hal_usb_core_device_get_idVendor(usb_core_device_obj_t *self) {
uint16_t vid;
uint16_t pid;
tuh_vid_pid_get(self->device_address, &vid, &pid);
if (!tuh_vid_pid_get(self->device_address, &vid, &pid)) {
mp_raise_usb_core_USBError(NULL);
}
return vid;
}

uint16_t common_hal_usb_core_device_get_idProduct(usb_core_device_obj_t *self) {
uint16_t vid;
uint16_t pid;
tuh_vid_pid_get(self->device_address, &vid, &pid);
if (!tuh_vid_pid_get(self->device_address, &vid, &pid)) {
mp_raise_usb_core_USBError(NULL);
}
return pid;
}

Expand All @@ -91,14 +96,86 @@ static void _transfer_done_cb(tuh_xfer_t *xfer) {

static bool _wait_for_callback(void) {
while (!mp_hal_is_interrupted() &&
_xfer_result == 0xff) {
_xfer_result == XFER_RESULT_INVALID) {
// The background tasks include TinyUSB which will call the function
// we provided above. In other words, the callback isn't in an interrupt.
RUN_BACKGROUND_TASKS;
}
if (mp_hal_is_interrupted()) {
// Handle case of VM being interrupted by Ctrl-C or autoreload
return false;
}
// Handle callback result code from TinyUSB
xfer_result_t result = _xfer_result;
_xfer_result = XFER_RESULT_INVALID;
switch (result) {
case XFER_RESULT_SUCCESS:
return true;
case XFER_RESULT_FAILED:
mp_raise_usb_core_USBError(NULL);
break;
case XFER_RESULT_STALLED:
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
break;
case XFER_RESULT_TIMEOUT:
case XFER_RESULT_INVALID:
mp_raise_usb_core_USBTimeoutError();
break;
}
return false;
}

static void _prepare_for_transfer(void) {
// Prepare for transfer. Unless there is a timeout, these static globals will
// get modified by the _transfer_done_cb() callback when tinyusb finishes the
// transfer or encounters an error condition.
_xfer_result = XFER_RESULT_INVALID;
_actual_len = 0;
}

static size_t _handle_timed_transfer_callback(tuh_xfer_t *xfer, mp_int_t timeout) {
if (xfer == NULL) {
mp_raise_usb_core_USBError(NULL);
return 0;
}
uint32_t start_time = supervisor_ticks_ms32();
while ((timeout == 0 || supervisor_ticks_ms32() - start_time < (uint32_t)timeout) &&
!mp_hal_is_interrupted() &&
_xfer_result == XFER_RESULT_INVALID) {
// The background tasks include TinyUSB which will call the function
// we provided above. In other words, the callback isn't in an interrupt.
RUN_BACKGROUND_TASKS;
}
if (mp_hal_is_interrupted()) {
// Handle case of VM being interrupted by Ctrl-C or autoreload
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
return 0;
}
// Handle transfer result code from TinyUSB
xfer_result_t result = _xfer_result;
_xfer_result = 0xff;
return result == XFER_RESULT_SUCCESS;
_xfer_result = XFER_RESULT_INVALID;
switch (result) {
case XFER_RESULT_SUCCESS:
return _actual_len;
case XFER_RESULT_FAILED:
mp_raise_usb_core_USBError(NULL);
break;
case XFER_RESULT_STALLED:
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
break;
case XFER_RESULT_TIMEOUT:
// This timeout comes from TinyUSB, so assume that it has stopped the
// transfer (note: timeout logic may be unimplemented on TinyUSB side)
mp_raise_usb_core_USBTimeoutError();
break;
case XFER_RESULT_INVALID:
// This timeout comes from CircuitPython, not TinyUSB, so tell TinyUSB
// to stop the transfer
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
mp_raise_usb_core_USBTimeoutError();
break;
}
return 0;
}

static mp_obj_t _get_string(const uint16_t *temp_buf) {
Expand All @@ -115,41 +192,75 @@ static void _get_langid(usb_core_device_obj_t *self) {
}
// Two control bytes and one uint16_t language code.
uint16_t temp_buf[2];
if (!tuh_descriptor_get_string(self->device_address, 0, 0, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
!_wait_for_callback()) {
return;
_prepare_for_transfer();
if (!tuh_descriptor_get_string(self->device_address, 0, 0, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
mp_raise_usb_core_USBError(NULL);
} else if (_wait_for_callback()) {
self->first_langid = temp_buf[1];
}
self->first_langid = temp_buf[1];
}

mp_obj_t common_hal_usb_core_device_get_serial_number(usb_core_device_obj_t *self) {
uint16_t temp_buf[127];
_get_langid(self);
if (!tuh_descriptor_get_serial_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
!_wait_for_callback()) {
tusb_desc_device_t descriptor;
// First, be sure not to ask TinyUSB for a non-existent string (avoid error)
if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) {
return mp_const_none;
}
return _get_string(temp_buf);
if (descriptor.iSerialNumber == 0) {
return mp_const_none;
}
// Device does provide this string, so continue
_get_langid(self);
_prepare_for_transfer();
if (!tuh_descriptor_get_serial_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
mp_raise_usb_core_USBError(NULL);
} else if (_wait_for_callback()) {
return _get_string(temp_buf);
}
return mp_const_none;
}

mp_obj_t common_hal_usb_core_device_get_product(usb_core_device_obj_t *self) {
uint16_t temp_buf[127];
_get_langid(self);
if (!tuh_descriptor_get_product_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
!_wait_for_callback()) {
tusb_desc_device_t descriptor;
// First, be sure not to ask TinyUSB for a non-existent string (avoid error)
if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) {
return mp_const_none;
}
if (descriptor.iProduct == 0) {
return mp_const_none;
}
return _get_string(temp_buf);
// Device does provide this string, so continue
_get_langid(self);
_prepare_for_transfer();
if (!tuh_descriptor_get_product_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
mp_raise_usb_core_USBError(NULL);
} else if (_wait_for_callback()) {
return _get_string(temp_buf);
}
return mp_const_none;
}

mp_obj_t common_hal_usb_core_device_get_manufacturer(usb_core_device_obj_t *self) {
uint16_t temp_buf[127];
_get_langid(self);
if (!tuh_descriptor_get_manufacturer_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
!_wait_for_callback()) {
tusb_desc_device_t descriptor;
// First, be sure not to ask TinyUSB for a non-existent string (avoid error)
if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) {
return mp_const_none;
}
if (descriptor.iManufacturer == 0) {
return mp_const_none;
}
return _get_string(temp_buf);
// Device does provide this string, so continue
_get_langid(self);
_prepare_for_transfer();
if (!tuh_descriptor_get_manufacturer_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
mp_raise_usb_core_USBError(NULL);
} else if (_wait_for_callback()) {
return _get_string(temp_buf);
}
return mp_const_none;
}


Expand Down Expand Up @@ -225,38 +336,13 @@ void common_hal_usb_core_device_set_configuration(usb_core_device_obj_t *self, m
}

static size_t _xfer(tuh_xfer_t *xfer, mp_int_t timeout) {
_xfer_result = 0xff;
_prepare_for_transfer();
xfer->complete_cb = _transfer_done_cb;
if (!tuh_edpt_xfer(xfer)) {
mp_raise_usb_core_USBError(NULL);
return 0;
}
uint32_t start_time = supervisor_ticks_ms32();
while ((timeout == 0 || supervisor_ticks_ms32() - start_time < (uint32_t)timeout) &&
!mp_hal_is_interrupted() &&
_xfer_result == 0xff) {
// The background tasks include TinyUSB which will call the function
// we provided above. In other words, the callback isn't in an interrupt.
RUN_BACKGROUND_TASKS;
}
if (mp_hal_is_interrupted()) {
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
return 0;
}
xfer_result_t result = _xfer_result;
_xfer_result = 0xff;
if (result == XFER_RESULT_STALLED) {
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
}
if (result == 0xff) {
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
mp_raise_usb_core_USBTimeoutError();
}
if (result == XFER_RESULT_SUCCESS) {
return _actual_len;
}

return 0;
return _handle_timed_transfer_callback(xfer, timeout);
}

static bool _open_endpoint(usb_core_device_obj_t *self, mp_int_t endpoint) {
Expand Down Expand Up @@ -355,38 +441,12 @@ mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self,
.complete_cb = _transfer_done_cb,
};

_xfer_result = 0xff;

_prepare_for_transfer();
if (!tuh_control_xfer(&xfer)) {
mp_raise_usb_core_USBError(NULL);
return 0;
}
uint32_t start_time = supervisor_ticks_ms32();
while ((timeout == 0 || supervisor_ticks_ms32() - start_time < (uint32_t)timeout) &&
!mp_hal_is_interrupted() &&
_xfer_result == 0xff) {
// The background tasks include TinyUSB which will call the function
// we provided above. In other words, the callback isn't in an interrupt.
RUN_BACKGROUND_TASKS;
}
if (mp_hal_is_interrupted()) {
tuh_edpt_abort_xfer(xfer.daddr, xfer.ep_addr);
return 0;
}
xfer_result_t result = _xfer_result;
_xfer_result = 0xff;
if (result == XFER_RESULT_STALLED) {
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
}
if (result == 0xff) {
tuh_edpt_abort_xfer(xfer.daddr, xfer.ep_addr);
mp_raise_usb_core_USBTimeoutError();
}
if (result == XFER_RESULT_SUCCESS) {
return len;
}

return 0;
return (mp_int_t)_handle_timed_transfer_callback(&xfer, timeout);
}

bool common_hal_usb_core_device_is_kernel_driver_active(usb_core_device_obj_t *self, mp_int_t interface) {
Expand Down