From ce508f47a9d22dd220a55e1705ea35dcd7ddbb67 Mon Sep 17 00:00:00 2001 From: Vladimir Smitka Date: Thu, 23 Jul 2026 14:38:48 +0000 Subject: [PATCH] usb.core: add raise_on_timeout=False option to Device.read Polling an interrupt IN endpoint every frame (a HID gamepad in a game loop) treats "no new report yet" as a normal outcome, but read() can only express it by raising USBTimeoutError. Each raise allocates an exception instance, which adds up to measurable GC pressure on small heaps: a 30 fps game polling one gamepad measured ~1-1.5 KB/s of transient garbage from these exceptions alone (RP2350 Fruit Jam). With raise_on_timeout=False (keyword-only, default True keeps the pyusb-compatible behavior), an elapsed timeout returns 0 instead of raising, so a per-frame poll allocates nothing. A timeout can be detected either by TinyUSB or by CircuitPython's own deadline; with the flag set, both paths return 0 after the usual transfer abort and buffer cleanup, instead of raising. --- shared-bindings/usb/core/Device.c | 18 +++++++++++++++--- shared-bindings/usb/core/Device.h | 2 +- shared-module/usb/core/Device.c | 31 ++++++++++++++++++++++--------- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/shared-bindings/usb/core/Device.c b/shared-bindings/usb/core/Device.c index 683115a8421..bc6c6296899 100644 --- a/shared-bindings/usb/core/Device.c +++ b/shared-bindings/usb/core/Device.c @@ -226,23 +226,35 @@ MP_DEFINE_CONST_FUN_OBJ_KW(usb_core_device_write_obj, 2, usb_core_device_write); //| def read( -//| self, endpoint: int, size_or_buffer: array.array, timeout: Optional[int] = None +//| self, +//| endpoint: int, +//| size_or_buffer: array.array, +//| timeout: Optional[int] = None, +//| *, +//| raise_on_timeout: bool = True, //| ) -> int: //| """Read data from the endpoint. //| //| :param int endpoint: the bEndpointAddress you want to communicate with. //| :param array.array size_or_buffer: the array to read data into. PyUSB also allows size but CircuitPython only support array to force deliberate memory use. //| :param int timeout: Time to wait specified in milliseconds. (Different from most CircuitPython!) +//| :param bool raise_on_timeout: when False, an elapsed timeout returns ``0`` +//| instead of raising `usb.core.USBTimeoutError`. Polling an interrupt +//| endpoint every frame (e.g. a HID gamepad in a game loop) treats "no +//| new report yet" as a normal outcome; the exception object each such +//| poll would otherwise allocate is measurable garbage-collector +//| pressure on small heaps. //| :returns: the number of bytes read //| """ //| ... //| static mp_obj_t usb_core_device_read(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { - enum { ARG_endpoint, ARG_size_or_buffer, ARG_timeout }; + enum { ARG_endpoint, ARG_size_or_buffer, ARG_timeout, ARG_raise_on_timeout }; static const mp_arg_t allowed_args[] = { { MP_QSTR_endpoint, MP_ARG_REQUIRED | MP_ARG_INT }, { MP_QSTR_size_or_buffer, MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_timeout, MP_ARG_INT, {.u_int = 0} }, + { MP_QSTR_raise_on_timeout, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = true} }, }; usb_core_device_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]); check_for_deinit(self); @@ -252,7 +264,7 @@ static mp_obj_t usb_core_device_read(size_t n_args, const mp_obj_t *pos_args, mp mp_buffer_info_t bufinfo; mp_get_buffer_raise(args[ARG_size_or_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE); - return MP_OBJ_NEW_SMALL_INT(common_hal_usb_core_device_read(self, args[ARG_endpoint].u_int, bufinfo.buf, bufinfo.len, args[ARG_timeout].u_int)); + return MP_OBJ_NEW_SMALL_INT(common_hal_usb_core_device_read(self, args[ARG_endpoint].u_int, bufinfo.buf, bufinfo.len, args[ARG_timeout].u_int, args[ARG_raise_on_timeout].u_bool)); } MP_DEFINE_CONST_FUN_OBJ_KW(usb_core_device_read_obj, 2, usb_core_device_read); diff --git a/shared-bindings/usb/core/Device.h b/shared-bindings/usb/core/Device.h index 28c2cfbe198..51338f61228 100644 --- a/shared-bindings/usb/core/Device.h +++ b/shared-bindings/usb/core/Device.h @@ -26,7 +26,7 @@ mp_int_t common_hal_usb_core_device_get_speed(usb_core_device_obj_t *self); void common_hal_usb_core_device_set_configuration(usb_core_device_obj_t *self, mp_int_t configuration); mp_int_t common_hal_usb_core_device_write(usb_core_device_obj_t *self, mp_int_t endpoint, const uint8_t *buffer, mp_int_t len, mp_int_t timeout); -mp_int_t common_hal_usb_core_device_read(usb_core_device_obj_t *self, mp_int_t endpoint, uint8_t *buffer, mp_int_t len, mp_int_t timeout); +mp_int_t common_hal_usb_core_device_read(usb_core_device_obj_t *self, mp_int_t endpoint, uint8_t *buffer, mp_int_t len, mp_int_t timeout, bool raise_on_timeout); mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self, mp_int_t bmRequestType, mp_int_t bRequest, mp_int_t wValue, mp_int_t wIndex, diff --git a/shared-module/usb/core/Device.c b/shared-module/usb/core/Device.c index 53c87de180a..a3bb449e87e 100644 --- a/shared-module/usb/core/Device.c +++ b/shared-module/usb/core/Device.c @@ -175,7 +175,7 @@ static void _abort_transfer(tuh_xfer_t *xfer) { } // Only frees the transfer buffer on error. -static size_t _handle_timed_transfer_callback(tuh_xfer_t *xfer, mp_int_t timeout, bool our_buffer) { +static size_t _handle_timed_transfer_callback(tuh_xfer_t *xfer, mp_int_t timeout, bool our_buffer, bool raise_on_timeout) { if (xfer == NULL) { mp_raise_usb_core_USBError(NULL); return 0; @@ -196,7 +196,14 @@ static size_t _handle_timed_transfer_callback(tuh_xfer_t *xfer, mp_int_t timeout // Handle transfer result code from TinyUSB xfer_result_t result = _xfer_result; _xfer_result = XFER_RESULT_INVALID; - if (our_buffer && result != XFER_RESULT_SUCCESS && result != XFER_RESULT_INVALID) { + // Free the bounce buffer only on paths that raise: raising unwinds past + // the caller's cleanup, so it must be freed here. Paths that return + // normally leave ownership with the caller, which copies out of and + // frees the buffer itself (freeing on both sides would be a double free). + bool will_raise = result == XFER_RESULT_ABORTED || result == XFER_RESULT_FAILED || + result == XFER_RESULT_STALLED || + (result == XFER_RESULT_TIMEOUT && raise_on_timeout); + if (our_buffer && will_raise) { port_free(xfer->buffer); } switch (result) { @@ -212,12 +219,18 @@ static size_t _handle_timed_transfer_callback(tuh_xfer_t *xfer, mp_int_t timeout 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) + if (!raise_on_timeout) { + return 0; + } mp_raise_usb_core_USBTimeoutError(); break; case XFER_RESULT_INVALID: // This timeout comes from CircuitPython, not TinyUSB, so tell TinyUSB - // to stop the transfer and then wait to free the buffer. + // to stop the transfer. _abort_transfer(xfer); + if (!raise_on_timeout) { + return 0; // the caller owns (and frees) the buffer + } if (our_buffer) { port_free(xfer->buffer); } @@ -385,7 +398,7 @@ void common_hal_usb_core_device_set_configuration(usb_core_device_obj_t *self, m } // Raises an exception on failure. Returns the number of bytes transferred (maybe zero) on success. -static size_t _xfer(tuh_xfer_t *xfer, mp_int_t timeout, bool our_buffer) { +static size_t _xfer(tuh_xfer_t *xfer, mp_int_t timeout, bool our_buffer, bool raise_on_timeout) { _prepare_for_transfer(); xfer->complete_cb = _transfer_done_cb; if (!tuh_edpt_xfer(xfer)) { @@ -395,7 +408,7 @@ static size_t _xfer(tuh_xfer_t *xfer, mp_int_t timeout, bool our_buffer) { mp_raise_usb_core_USBError(NULL); return 0; } - return _handle_timed_transfer_callback(xfer, timeout, our_buffer); + return _handle_timed_transfer_callback(xfer, timeout, our_buffer, raise_on_timeout); } static bool _open_endpoint(usb_core_device_obj_t *self, mp_int_t endpoint) { @@ -469,7 +482,7 @@ mp_int_t common_hal_usb_core_device_write(usb_core_device_obj_t *self, mp_int_t xfer.ep_addr = endpoint; xfer.buffer = dma_buffer; xfer.buflen = len; - size_t result = _xfer(&xfer, timeout, dma_buffer != buffer); + size_t result = _xfer(&xfer, timeout, dma_buffer != buffer, true); #if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE if (dma_buffer != buffer) { port_free(dma_buffer); @@ -478,7 +491,7 @@ mp_int_t common_hal_usb_core_device_write(usb_core_device_obj_t *self, mp_int_t return result; } -mp_int_t common_hal_usb_core_device_read(usb_core_device_obj_t *self, mp_int_t endpoint, uint8_t *buffer, mp_int_t len, mp_int_t timeout) { +mp_int_t common_hal_usb_core_device_read(usb_core_device_obj_t *self, mp_int_t endpoint, uint8_t *buffer, mp_int_t len, mp_int_t timeout, bool raise_on_timeout) { if (!_open_endpoint(self, endpoint)) { mp_raise_usb_core_USBError(NULL); return 0; @@ -500,7 +513,7 @@ mp_int_t common_hal_usb_core_device_read(usb_core_device_obj_t *self, mp_int_t e xfer.ep_addr = endpoint; xfer.buffer = dma_buffer; xfer.buflen = len; - mp_int_t result = _xfer(&xfer, timeout, dma_buffer != buffer); + mp_int_t result = _xfer(&xfer, timeout, dma_buffer != buffer, raise_on_timeout); #if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE // Copy data back to original buffer if needed @@ -556,7 +569,7 @@ mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self, mp_raise_usb_core_USBError(NULL); return 0; } - mp_int_t result = (mp_int_t)_handle_timed_transfer_callback(&xfer, timeout, dma_buffer != buffer); + mp_int_t result = (mp_int_t)_handle_timed_transfer_callback(&xfer, timeout, dma_buffer != buffer, true); #if !CIRCUITPY_ALL_MEMORY_DMA_CAPABLE if (dma_buffer != buffer) {