From 0a91a37563f6783d3913577705218fcea479edc7 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 18 Jun 2024 17:17:46 +1000 Subject: [PATCH] usb-device-cdc: Fix lost data in read() path if short reads happened. If the CDC receive buffer was full and some code read less than 64 bytes (wMaxTransferSize), the CDC code would submit an OUT transfer with N<64 bytes length to fill the buffer back up. However if the host had more than N bytes to send then it would still send the full 64 bytes (correctly) in the transfer. The remaining (64-N) bytes would be lost. Adds the restriction that CDCInterface rxbuf has to be at least 64 bytes. Fixes issue #885. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton --- micropython/usb/usb-device-cdc/manifest.py | 2 +- micropython/usb/usb-device-cdc/usb/device/cdc.py | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/micropython/usb/usb-device-cdc/manifest.py b/micropython/usb/usb-device-cdc/manifest.py index af9b8cb8..4520325e 100644 --- a/micropython/usb/usb-device-cdc/manifest.py +++ b/micropython/usb/usb-device-cdc/manifest.py @@ -1,3 +1,3 @@ -metadata(version="0.1.0") +metadata(version="0.1.1") require("usb-device") package("usb") diff --git a/micropython/usb/usb-device-cdc/usb/device/cdc.py b/micropython/usb/usb-device-cdc/usb/device/cdc.py index 741eaafb..28bfb065 100644 --- a/micropython/usb/usb-device-cdc/usb/device/cdc.py +++ b/micropython/usb/usb-device-cdc/usb/device/cdc.py @@ -144,8 +144,8 @@ class CDCInterface(io.IOBase, Interface): if flow != 0: raise NotImplementedError # UART flow control currently not supported - if not (txbuf and rxbuf): - raise ValueError # Buffer sizes are required + if not (txbuf and rxbuf >= _BULK_EP_LEN): + raise ValueError # Buffer sizes are required, rxbuf must be at least one EP self._timeout = timeout self._wb = Buffer(txbuf) @@ -330,7 +330,11 @@ class CDCInterface(io.IOBase, Interface): def _rd_xfer(self): # Keep an active data OUT transfer to read data from the host, # whenever the receive buffer has room for new data - if self.is_open() and not self.xfer_pending(self.ep_d_out) and self._rb.writable(): + if ( + self.is_open() + and not self.xfer_pending(self.ep_d_out) + and self._rb.writable() >= _BULK_EP_LEN + ): # Can only submit up to the endpoint length per transaction, otherwise we won't # get any transfer callback until the full transaction completes. self.submit_xfer(self.ep_d_out, self._rb.pend_write(_BULK_EP_LEN), self._rd_cb)