Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USBD CDC package maybe splited when using SerialUSB.write() #2372

Open
yp05327 opened this issue May 21, 2024 · 2 comments
Open

USBD CDC package maybe splited when using SerialUSB.write() #2372

yp05327 opened this issue May 21, 2024 · 2 comments

Comments

@yp05327
Copy link
Contributor

yp05327 commented May 21, 2024

As the title, I noticed that sometimes the output buffer will be splited into two bulk packages.
e.g. if I send [1, 2, 3, 4, 5, 6, 7] by using SerialUSB.write(), sometimes the output package will be [1, 2, 3, 4] and [5, 6, 7], not [1, 2, 3, 4, 5, 6, 7].

Then I found that, in cdc_queue.c, we are using TransmitQueue to handle the output buffer, maybe for avoiding overflow? Not sure about this.

And for TransmitQueue, actually, it is a ring buffer. So when write pointer < read pointer, which means something like this:
[x4 x5 x6 x7 0 0 ...... 0 0 x0 x1 x2 x3]
x(n) means the output buffer we provided, 0 means no-related bytes.

Then the current approach will run USBD_LL_Transmit twice to send the whole output buffer.
first time, we send [x0 x1 x2 x3]
then, we send [x4 x5 x6 x7]
(it seems that each of them are called block in the code, I will call them block bellow)

Function call:
USBSerial::write
|-> CDC_TransmitQueue_Enqueue
|-> CDC_continue_transmit
|-> |-> CDC_TransmitQueue_ReadBlock
|-> |-> USBD_CDC_SetTxBuffer
|-> |-> USBD_CDC_TransmitPacket
|-> |-> |-> USBD_LL_Transmit

Before we send the data, as the comment in L980, the following code will set the total length of the packet.

/* Update the packet total length */
pdev->ep_in[CDCInEpAdd & 0xFU].total_length = hcdc->TxLength;

But it seems that the total length is incorrect. It is same to the length of the block.
So two packets will be sent instead of one as expected.

For the solusion, I'm using USBD_LL_Transmit directly, instead of using the ring buffer TransmitQueue.
And it works as expected now, so I think there's a bug in the output buffer handling approach.

Desktop (please complete the following information):

  • OS: Windows
  • Arduino Cli: 0.34.2
  • STM32 core version: 2.7.1

Board (please complete the following information):

  • Name: Nucleo H753ZI
@fpistm
Copy link
Member

fpistm commented May 28, 2024

Hi @yp05327
Thanks for this detailed report.
I will not have time soon to check this, do not hesitate to provide a fix if you have it. 😉

@yp05327
Copy link
Contributor Author

yp05327 commented May 28, 2024

Actually, I don't have. I'm not a pro developer of embedded system, so I have no knowledge about what embedded system engineers will do to fix it.
Just a discussion, maybe we can provide a new buffer when write pointer < read pointer, but this may increase the memory usage. 😕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants