From 9d4442e298f4e86fb0686d7986ba627231dd591b Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 19 Feb 2021 14:15:31 -0500 Subject: [PATCH] handle reads/writes larger than buffers; add .write_timeout --- shared-bindings/usb_cdc/Serial.c | 30 +++++++++++- shared-bindings/usb_cdc/Serial.h | 3 ++ shared-module/usb_cdc/Serial.c | 84 ++++++++++++++++++++++++++------ shared-module/usb_cdc/Serial.h | 5 +- shared-module/usb_cdc/__init__.c | 2 + 5 files changed, 106 insertions(+), 18 deletions(-) diff --git a/shared-bindings/usb_cdc/Serial.c b/shared-bindings/usb_cdc/Serial.c index 64f851a48e..c813dce5b3 100644 --- a/shared-bindings/usb_cdc/Serial.c +++ b/shared-bindings/usb_cdc/Serial.c @@ -56,7 +56,7 @@ //| :rtype: bytes""" //| ... //| -//| def readinto(self, buf: WriteableBuffer) -> bytes: +//| def readinto(self, buf: WriteableBuffer) -> int: //| """Read bytes into the ``buf``. If ``nbytes`` is specified then read at most //| that many bytes, subject to `timeout`. Otherwise, read at most ``len(buf)`` bytes. //| @@ -219,6 +219,33 @@ const mp_obj_property_t usb_cdc_serial_timeout_obj = { (mp_obj_t)&mp_const_none_obj}, }; +//| write_timeout: Optional[float] +//| """The initial value of `write_timeout` is ``None``. If ``None``, wait indefinitely to finish +//| writing all the bytes passed to ``write()``.If 0, do not wait. +//| If > 0, wait only ``write_timeout`` seconds.""" +//| +STATIC mp_obj_t usb_cdc_serial_get_write_timeout(mp_obj_t self_in) { + usb_cdc_serial_obj_t *self = MP_OBJ_TO_PTR(self_in); + mp_float_t write_timeout = common_hal_usb_cdc_serial_get_write_timeout(self); + return (write_timeout < 0.0f) ? mp_const_none : mp_obj_new_float(self->write_timeout); +} +MP_DEFINE_CONST_FUN_OBJ_1(usb_cdc_serial_get_write_timeout_obj, usb_cdc_serial_get_write_timeout); + +STATIC mp_obj_t usb_cdc_serial_set_write_timeout(mp_obj_t self_in, mp_obj_t write_timeout_in) { + usb_cdc_serial_obj_t *self = MP_OBJ_TO_PTR(self_in); + common_hal_usb_cdc_serial_set_write_timeout(self, + write_timeout_in == mp_const_none ? -1.0f : mp_obj_get_float(write_timeout_in)); + return mp_const_none; +} +MP_DEFINE_CONST_FUN_OBJ_2(usb_cdc_serial_set_write_timeout_obj, usb_cdc_serial_set_write_timeout); + +const mp_obj_property_t usb_cdc_serial_write_timeout_obj = { + .base.type = &mp_type_property, + .proxy = {(mp_obj_t)&usb_cdc_serial_get_write_timeout_obj, + (mp_obj_t)&usb_cdc_serial_set_write_timeout_obj, + (mp_obj_t)&mp_const_none_obj}, +}; + STATIC const mp_rom_map_elem_t usb_cdc_serial_locals_dict_table[] = { // Standard stream methods. @@ -235,6 +262,7 @@ STATIC const mp_rom_map_elem_t usb_cdc_serial_locals_dict_table[] = { { MP_OBJ_NEW_QSTR(MP_QSTR_reset_input_buffer), MP_ROM_PTR(&usb_cdc_serial_reset_input_buffer_obj) }, { MP_OBJ_NEW_QSTR(MP_QSTR_reset_output_buffer), MP_ROM_PTR(&usb_cdc_serial_reset_output_buffer_obj) }, { MP_OBJ_NEW_QSTR(MP_QSTR_timeout), MP_ROM_PTR(&usb_cdc_serial_timeout_obj) }, + { MP_OBJ_NEW_QSTR(MP_QSTR_write_timeout), MP_ROM_PTR(&usb_cdc_serial_write_timeout_obj) }, // Not in pyserial protocol. { MP_OBJ_NEW_QSTR(MP_QSTR_connected), MP_ROM_PTR(&usb_cdc_serial_connected_obj) }, diff --git a/shared-bindings/usb_cdc/Serial.h b/shared-bindings/usb_cdc/Serial.h index 149d2c2d84..cdf5c3a914 100644 --- a/shared-bindings/usb_cdc/Serial.h +++ b/shared-bindings/usb_cdc/Serial.h @@ -47,4 +47,7 @@ extern bool common_hal_usb_cdc_serial_get_connected(usb_cdc_serial_obj_t *self); extern mp_float_t common_hal_usb_cdc_serial_get_timeout(usb_cdc_serial_obj_t *self); extern void common_hal_usb_cdc_serial_set_timeout(usb_cdc_serial_obj_t *self, mp_float_t timeout); +extern mp_float_t common_hal_usb_cdc_serial_get_write_timeout(usb_cdc_serial_obj_t *self); +extern void common_hal_usb_cdc_serial_set_write_timeout(usb_cdc_serial_obj_t *self, mp_float_t write_timeout); + #endif // MICROPY_INCLUDED_SHARED_BINDINGS_USB_CDC_SERIAL_H diff --git a/shared-module/usb_cdc/Serial.c b/shared-module/usb_cdc/Serial.c index f569a19133..6037fddcc5 100644 --- a/shared-module/usb_cdc/Serial.c +++ b/shared-module/usb_cdc/Serial.c @@ -31,32 +31,78 @@ #include "tusb.h" size_t common_hal_usb_cdc_serial_read(usb_cdc_serial_obj_t *self, uint8_t *data, size_t len, int *errcode) { - if (self->timeout < 0.0f) { - while (tud_cdc_n_available(self->idx) < len) { - RUN_BACKGROUND_TASKS; - if (mp_hal_is_interrupted()) { - return 0; - } - } - } else if (self->timeout > 0.0f) { - uint64_t timeout_ms = self->timeout * 1000; + + const bool wait_forever = self->timeout < 0.0f; + const bool wait_for_timeout = self->timeout > 0.0f; + + // Read up to len bytes immediately. + // The number of bytes read will not be larger than what is already in the TinyUSB FIFO. + uint32_t total_num_read = tud_cdc_n_read(self->idx, data, len); + + if (wait_forever || wait_for_timeout) { + // Read more if we have time. + uint64_t timeout_ms = self->timeout * 1000; // Junk value if timeout < 0. uint64_t start_ticks = supervisor_ticks_ms64(); - while (tud_cdc_n_available(self->idx) < len && - supervisor_ticks_ms64() - start_ticks <= timeout_ms) { + + uint32_t num_read = 0; + while (total_num_read < len && + (wait_forever || supervisor_ticks_ms64() - start_ticks <= timeout_ms)) { + + // Wait for a bit, and check for ctrl-C. RUN_BACKGROUND_TASKS; if (mp_hal_is_interrupted()) { return 0; } + + // Advance buffer pointer and reduce number of bytes that need to be read. + len -= num_read; + data += num_read; + + // Try to read another batch of bytes. + num_read = tud_cdc_n_read(self->idx, data, len); + total_num_read += num_read; } } - // Timeout of 0.0f falls through to here with no waiting or unnecessary calculation. - return tud_cdc_n_read(self->idx, data, len); + + return total_num_read; } size_t common_hal_usb_cdc_serial_write(usb_cdc_serial_obj_t *self, const uint8_t *data, size_t len, int *errcode) { - uint32_t num_written = tud_cdc_n_write(self->idx, data, len); + const bool wait_forever = self->write_timeout < 0.0f; + const bool wait_for_timeout = self->write_timeout > 0.0f; + + // Write as many bytes as possible immediately. + // The number of bytes written at once will not be larger than what can fit in the TinyUSB FIFO. + uint32_t total_num_written = tud_cdc_n_write(self->idx, data, len); tud_cdc_n_write_flush(self->idx); - return num_written; + + if (wait_forever || wait_for_timeout) { + // Write more if we have time. + uint64_t timeout_ms = self->write_timeout * 1000; // Junk value if write_timeout < 0. + uint64_t start_ticks = supervisor_ticks_ms64(); + + uint32_t num_written = 0; + while (total_num_written < len && + (wait_forever || supervisor_ticks_ms64() - start_ticks <= timeout_ms)) { + + // Wait for a bit, and check for ctrl-C. + RUN_BACKGROUND_TASKS; + if (mp_hal_is_interrupted()) { + return 0; + } + + // Advance buffer pointer and reduce number of bytes that need to be written. + len -= num_written; + data += num_written; + + // Try to write another batch of bytes. + num_written = tud_cdc_n_write(self->idx, data, len); + tud_cdc_n_write_flush(self->idx); + total_num_written += num_written; + } + } + + return total_num_written; } uint32_t common_hal_usb_cdc_serial_get_in_waiting(usb_cdc_serial_obj_t *self) { @@ -91,3 +137,11 @@ mp_float_t common_hal_usb_cdc_serial_get_timeout(usb_cdc_serial_obj_t *self) { void common_hal_usb_cdc_serial_set_timeout(usb_cdc_serial_obj_t *self, mp_float_t timeout) { self->timeout = timeout; } + +mp_float_t common_hal_usb_cdc_serial_get_write_timeout(usb_cdc_serial_obj_t *self) { + return self->write_timeout; +} + +void common_hal_usb_cdc_serial_set_write_timeout(usb_cdc_serial_obj_t *self, mp_float_t write_timeout) { + self->write_timeout = write_timeout; +} diff --git a/shared-module/usb_cdc/Serial.h b/shared-module/usb_cdc/Serial.h index ad4bed1a32..ddf78eefa6 100644 --- a/shared-module/usb_cdc/Serial.h +++ b/shared-module/usb_cdc/Serial.h @@ -31,8 +31,9 @@ typedef struct { mp_obj_base_t base; - mp_float_t timeout; // if negative, wait forever. - uint8_t idx; // which CDC device? + mp_float_t timeout; // if negative, wait forever. + mp_float_t write_timeout; // if negative, wait forever. + uint8_t idx; // which CDC device? } usb_cdc_serial_obj_t; #endif // SHARED_MODULE_USB_CDC_SERIAL_H diff --git a/shared-module/usb_cdc/__init__.c b/shared-module/usb_cdc/__init__.c index 9eae188119..fe05cb1075 100644 --- a/shared-module/usb_cdc/__init__.c +++ b/shared-module/usb_cdc/__init__.c @@ -41,10 +41,12 @@ static usb_cdc_serial_obj_t serial_objs[CFG_TUD_CDC] = { { .base.type = &usb_cdc_serial_type, .timeout = -1.0f, + .write_timeout = -1.0f, .idx = 0, }, { .base.type = &usb_cdc_serial_type, .timeout = -1.0f, + .write_timeout = -1.0f, .idx = 1, } };