Skip to content

Commit ad745c2

Browse files
committed
error checks for usb device string properties
Improve the error handling for usb.core.Device string properties: .serial_number, .product, .manufacturer Previously, the property getters didn't check the device descriptor to see if the device actually had the requested string. Instead, they relied on TinyUSB to return a failure result if the string was not available. That made it impossible to distinguish missing strings from other more serious USB errors (e.g. unplugged device). These changes make it possible to return None for a missing string or raise a USBError exception in case of a more serious problem.
1 parent 99f244e commit ad745c2

File tree

1 file changed

+70
-17
lines changed

1 file changed

+70
-17
lines changed

shared-module/usb/core/Device.c

Lines changed: 70 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,28 @@ static bool _wait_for_callback(void) {
101101
// we provided above. In other words, the callback isn't in an interrupt.
102102
RUN_BACKGROUND_TASKS;
103103
}
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
104109
xfer_result_t result = _xfer_result;
105110
_xfer_result = XFER_RESULT_INVALID;
106-
return result == XFER_RESULT_SUCCESS;
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;
107126
}
108127

109128
static void _prepare_for_transfer(void) {
@@ -173,41 +192,75 @@ static void _get_langid(usb_core_device_obj_t *self) {
173192
}
174193
// Two control bytes and one uint16_t language code.
175194
uint16_t temp_buf[2];
176-
if (!tuh_descriptor_get_string(self->device_address, 0, 0, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
177-
!_wait_for_callback()) {
178-
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];
179200
}
180-
self->first_langid = temp_buf[1];
181201
}
182202

183203
mp_obj_t common_hal_usb_core_device_get_serial_number(usb_core_device_obj_t *self) {
184204
uint16_t temp_buf[127];
185-
_get_langid(self);
186-
if (!tuh_descriptor_get_serial_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
187-
!_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)) {
208+
return mp_const_none;
209+
}
210+
if (descriptor.iSerialNumber == 0) {
188211
return mp_const_none;
189212
}
190-
return _get_string(temp_buf);
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;
191222
}
192223

193224
mp_obj_t common_hal_usb_core_device_get_product(usb_core_device_obj_t *self) {
194225
uint16_t temp_buf[127];
195-
_get_langid(self);
196-
if (!tuh_descriptor_get_product_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
197-
!_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)) {
198229
return mp_const_none;
199230
}
200-
return _get_string(temp_buf);
231+
if (descriptor.iProduct == 0) {
232+
return mp_const_none;
233+
}
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;
201243
}
202244

203245
mp_obj_t common_hal_usb_core_device_get_manufacturer(usb_core_device_obj_t *self) {
204246
uint16_t temp_buf[127];
205-
_get_langid(self);
206-
if (!tuh_descriptor_get_manufacturer_string(self->device_address, self->first_langid, temp_buf, sizeof(temp_buf), _transfer_done_cb, 0) ||
207-
!_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)) {
208250
return mp_const_none;
209251
}
210-
return _get_string(temp_buf);
252+
if (descriptor.iManufacturer == 0) {
253+
return mp_const_none;
254+
}
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;
211264
}
212265

213266

0 commit comments

Comments
 (0)