Skip to content

Commit 77bd2db

Browse files
tannewttannewt
authored
Merge pull request #10616 from samblenny/usbh-err-handling-v2
Improve usb.core.Device error handling REV 2
2 parents a96f112 + ad745c2 commit 77bd2db

File tree

1 file changed

+137
-77
lines changed

1 file changed

+137
-77
lines changed

‎shared-module/usb/core/Device.c‎

Lines changed: 137 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// This file is part of the CircuitPython project: https://circuitpython.org
22
//
33
// SPDX-FileCopyrightText: Copyright (c) 2022 Scott Shawcroft for Adafruit Industries
4+
// SPDX-FileCopyrightText: Copyright (c) 2025 Sam Blenny
45
//
56
// SPDX-License-Identifier: MIT
67

@@ -45,7 +46,7 @@ bool common_hal_usb_core_device_construct(usb_core_device_obj_t *self, uint8_t d
4546
}
4647
self->device_address = device_address;
4748
self->first_langid = 0;
48-
_xfer_result = 0xff;
49+
_xfer_result = XFER_RESULT_INVALID;
4950
return true;
5051
}
5152

@@ -70,14 +71,18 @@ void common_hal_usb_core_device_deinit(usb_core_device_obj_t *self) {
7071
uint16_t common_hal_usb_core_device_get_idVendor(usb_core_device_obj_t *self) {
7172
uint16_t vid;
7273
uint16_t pid;
73-
tuh_vid_pid_get(self->device_address, &vid, &pid);
74+
if (!tuh_vid_pid_get(self->device_address, &vid, &pid)) {
75+
mp_raise_usb_core_USBError(NULL);
76+
}
7477
return vid;
7578
}
7679

7780
uint16_t common_hal_usb_core_device_get_idProduct(usb_core_device_obj_t *self) {
7881
uint16_t vid;
7982
uint16_t pid;
80-
tuh_vid_pid_get(self->device_address, &vid, &pid);
83+
if (!tuh_vid_pid_get(self->device_address, &vid, &pid)) {
84+
mp_raise_usb_core_USBError(NULL);
85+
}
8186
return pid;
8287
}
8388

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

9297
static bool _wait_for_callback(void) {
9398
while (!mp_hal_is_interrupted() &&
94-
_xfer_result == 0xff) {
99+
_xfer_result == XFER_RESULT_INVALID) {
100+
// The background tasks include TinyUSB which will call the function
101+
// we provided above. In other words, the callback isn't in an interrupt.
102+
RUN_BACKGROUND_TASKS;
103+
}
104+
if (mp_hal_is_interrupted()) {
105+
// Handle case of VM being interrupted by Ctrl-C or autoreload
106+
return false;
107+
}
108+
// Handle callback result code from TinyUSB
109+
xfer_result_t result = _xfer_result;
110+
_xfer_result = XFER_RESULT_INVALID;
111+
switch (result) {
112+
case XFER_RESULT_SUCCESS:
113+
return true;
114+
case XFER_RESULT_FAILED:
115+
mp_raise_usb_core_USBError(NULL);
116+
break;
117+
case XFER_RESULT_STALLED:
118+
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
119+
break;
120+
case XFER_RESULT_TIMEOUT:
121+
case XFER_RESULT_INVALID:
122+
mp_raise_usb_core_USBTimeoutError();
123+
break;
124+
}
125+
return false;
126+
}
127+
128+
static void _prepare_for_transfer(void) {
129+
// Prepare for transfer. Unless there is a timeout, these static globals will
130+
// get modified by the _transfer_done_cb() callback when tinyusb finishes the
131+
// transfer or encounters an error condition.
132+
_xfer_result = XFER_RESULT_INVALID;
133+
_actual_len = 0;
134+
}
135+
136+
static size_t _handle_timed_transfer_callback(tuh_xfer_t *xfer, mp_int_t timeout) {
137+
if (xfer == NULL) {
138+
mp_raise_usb_core_USBError(NULL);
139+
return 0;
140+
}
141+
uint32_t start_time = supervisor_ticks_ms32();
142+
while ((timeout == 0 || supervisor_ticks_ms32() - start_time < (uint32_t)timeout) &&
143+
!mp_hal_is_interrupted() &&
144+
_xfer_result == XFER_RESULT_INVALID) {
95145
// The background tasks include TinyUSB which will call the function
96146
// we provided above. In other words, the callback isn't in an interrupt.
97147
RUN_BACKGROUND_TASKS;
98148
}
149+
if (mp_hal_is_interrupted()) {
150+
// Handle case of VM being interrupted by Ctrl-C or autoreload
151+
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
152+
return 0;
153+
}
154+
// Handle transfer result code from TinyUSB
99155
xfer_result_t result = _xfer_result;
100-
_xfer_result = 0xff;
101-
return result == XFER_RESULT_SUCCESS;
156+
_xfer_result = XFER_RESULT_INVALID;
157+
switch (result) {
158+
case XFER_RESULT_SUCCESS:
159+
return _actual_len;
160+
case XFER_RESULT_FAILED:
161+
mp_raise_usb_core_USBError(NULL);
162+
break;
163+
case XFER_RESULT_STALLED:
164+
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
165+
break;
166+
case XFER_RESULT_TIMEOUT:
167+
// This timeout comes from TinyUSB, so assume that it has stopped the
168+
// transfer (note: timeout logic may be unimplemented on TinyUSB side)
169+
mp_raise_usb_core_USBTimeoutError();
170+
break;
171+
case XFER_RESULT_INVALID:
172+
// This timeout comes from CircuitPython, not TinyUSB, so tell TinyUSB
173+
// to stop the transfer
174+
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
175+
mp_raise_usb_core_USBTimeoutError();
176+
break;
177+
}
178+
return 0;
102179
}
103180

104181
static mp_obj_t _get_string(const uint16_t *temp_buf) {
@@ -115,41 +192,75 @@ static void _get_langid(usb_core_device_obj_t *self) {
115192
}
116193
// Two control bytes and one uint16_t language code.
117194
uint16_t temp_buf[2];
118-
if (!tuh_descriptor_get_string(self->device_address, 0, 0, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
119-
!_wait_for_callback()) {
120-
return;
195+
_prepare_for_transfer();
196+
if (!tuh_descriptor_get_string(self->device_address, 0, 0, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
197+
mp_raise_usb_core_USBError(NULL);
198+
} else if (_wait_for_callback()) {
199+
self->first_langid = temp_buf[1];
121200
}
122-
self->first_langid = temp_buf[1];
123201
}
124202

125203
mp_obj_t common_hal_usb_core_device_get_serial_number(usb_core_device_obj_t *self) {
126204
uint16_t temp_buf[127];
127-
_get_langid(self);
128-
if (!tuh_descriptor_get_serial_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
129-
!_wait_for_callback()) {
205+
tusb_desc_device_t descriptor;
206+
// First, be sure not to ask TinyUSB for a non-existent string (avoid error)
207+
if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) {
130208
return mp_const_none;
131209
}
132-
return _get_string(temp_buf);
210+
if (descriptor.iSerialNumber == 0) {
211+
return mp_const_none;
212+
}
213+
// Device does provide this string, so continue
214+
_get_langid(self);
215+
_prepare_for_transfer();
216+
if (!tuh_descriptor_get_serial_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
217+
mp_raise_usb_core_USBError(NULL);
218+
} else if (_wait_for_callback()) {
219+
return _get_string(temp_buf);
220+
}
221+
return mp_const_none;
133222
}
134223

135224
mp_obj_t common_hal_usb_core_device_get_product(usb_core_device_obj_t *self) {
136225
uint16_t temp_buf[127];
137-
_get_langid(self);
138-
if (!tuh_descriptor_get_product_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
139-
!_wait_for_callback()) {
226+
tusb_desc_device_t descriptor;
227+
// First, be sure not to ask TinyUSB for a non-existent string (avoid error)
228+
if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) {
229+
return mp_const_none;
230+
}
231+
if (descriptor.iProduct == 0) {
140232
return mp_const_none;
141233
}
142-
return _get_string(temp_buf);
234+
// Device does provide this string, so continue
235+
_get_langid(self);
236+
_prepare_for_transfer();
237+
if (!tuh_descriptor_get_product_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
238+
mp_raise_usb_core_USBError(NULL);
239+
} else if (_wait_for_callback()) {
240+
return _get_string(temp_buf);
241+
}
242+
return mp_const_none;
143243
}
144244

145245
mp_obj_t common_hal_usb_core_device_get_manufacturer(usb_core_device_obj_t *self) {
146246
uint16_t temp_buf[127];
147-
_get_langid(self);
148-
if (!tuh_descriptor_get_manufacturer_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
149-
!_wait_for_callback()) {
247+
tusb_desc_device_t descriptor;
248+
// First, be sure not to ask TinyUSB for a non-existent string (avoid error)
249+
if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) {
250+
return mp_const_none;
251+
}
252+
if (descriptor.iManufacturer == 0) {
150253
return mp_const_none;
151254
}
152-
return _get_string(temp_buf);
255+
// Device does provide this string, so continue
256+
_get_langid(self);
257+
_prepare_for_transfer();
258+
if (!tuh_descriptor_get_manufacturer_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0)) {
259+
mp_raise_usb_core_USBError(NULL);
260+
} else if (_wait_for_callback()) {
261+
return _get_string(temp_buf);
262+
}
263+
return mp_const_none;
153264
}
154265

155266

@@ -225,38 +336,13 @@ void common_hal_usb_core_device_set_configuration(usb_core_device_obj_t *self, m
225336
}
226337

227338
static size_t _xfer(tuh_xfer_t *xfer, mp_int_t timeout) {
228-
_xfer_result = 0xff;
339+
_prepare_for_transfer();
229340
xfer->complete_cb = _transfer_done_cb;
230341
if (!tuh_edpt_xfer(xfer)) {
231342
mp_raise_usb_core_USBError(NULL);
232343
return 0;
233344
}
234-
uint32_t start_time = supervisor_ticks_ms32();
235-
while ((timeout == 0 || supervisor_ticks_ms32() - start_time < (uint32_t)timeout) &&
236-
!mp_hal_is_interrupted() &&
237-
_xfer_result == 0xff) {
238-
// The background tasks include TinyUSB which will call the function
239-
// we provided above. In other words, the callback isn't in an interrupt.
240-
RUN_BACKGROUND_TASKS;
241-
}
242-
if (mp_hal_is_interrupted()) {
243-
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
244-
return 0;
245-
}
246-
xfer_result_t result = _xfer_result;
247-
_xfer_result = 0xff;
248-
if (result == XFER_RESULT_STALLED) {
249-
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
250-
}
251-
if (result == 0xff) {
252-
tuh_edpt_abort_xfer(xfer->daddr, xfer->ep_addr);
253-
mp_raise_usb_core_USBTimeoutError();
254-
}
255-
if (result == XFER_RESULT_SUCCESS) {
256-
return _actual_len;
257-
}
258-
259-
return 0;
345+
return _handle_timed_transfer_callback(xfer, timeout);
260346
}
261347

262348
static bool _open_endpoint(usb_core_device_obj_t *self, mp_int_t endpoint) {
@@ -355,38 +441,12 @@ mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self,
355441
.complete_cb = _transfer_done_cb,
356442
};
357443

358-
_xfer_result = 0xff;
359-
444+
_prepare_for_transfer();
360445
if (!tuh_control_xfer(&xfer)) {
361446
mp_raise_usb_core_USBError(NULL);
362447
return 0;
363448
}
364-
uint32_t start_time = supervisor_ticks_ms32();
365-
while ((timeout == 0 || supervisor_ticks_ms32() - start_time < (uint32_t)timeout) &&
366-
!mp_hal_is_interrupted() &&
367-
_xfer_result == 0xff) {
368-
// The background tasks include TinyUSB which will call the function
369-
// we provided above. In other words, the callback isn't in an interrupt.
370-
RUN_BACKGROUND_TASKS;
371-
}
372-
if (mp_hal_is_interrupted()) {
373-
tuh_edpt_abort_xfer(xfer.daddr, xfer.ep_addr);
374-
return 0;
375-
}
376-
xfer_result_t result = _xfer_result;
377-
_xfer_result = 0xff;
378-
if (result == XFER_RESULT_STALLED) {
379-
mp_raise_usb_core_USBError(MP_ERROR_TEXT("Pipe error"));
380-
}
381-
if (result == 0xff) {
382-
tuh_edpt_abort_xfer(xfer.daddr, xfer.ep_addr);
383-
mp_raise_usb_core_USBTimeoutError();
384-
}
385-
if (result == XFER_RESULT_SUCCESS) {
386-
return len;
387-
}
388-
389-
return 0;
449+
return (mp_int_t)_handle_timed_transfer_callback(&xfer, timeout);
390450
}
391451

392452
bool common_hal_usb_core_device_is_kernel_driver_active(usb_core_device_obj_t *self, mp_int_t interface) {

0 commit comments

Comments
 (0)