Skip to content

Commit

Permalink
usb-device-cdc: Fix lost data in read() path if short reads happened.
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
projectgus authored and dpgeorge committed Jul 3, 2024
1 parent b5aa5f0 commit 0a91a37
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
2 changes: 1 addition & 1 deletion micropython/usb/usb-device-cdc/manifest.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
metadata(version="0.1.0")
metadata(version="0.1.1")
require("usb-device")
package("usb")
10 changes: 7 additions & 3 deletions micropython/usb/usb-device-cdc/usb/device/cdc.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ def init(
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)
Expand Down Expand Up @@ -330,7 +330,11 @@ def _wr_cb(self, ep, res, num_bytes):
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)
Expand Down

0 comments on commit 0a91a37

Please sign in to comment.