Repository navigation
USBD CDC package maybe splited when using SerialUSB.write() #2372
Description
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.
Arduino_Core_STM32/cores/arduino/stm32/usb/cdc/usbd_cdc.c
Lines 980 to 981 in f31d070
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
Activity
fpistm commentedon 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 commentedon 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. 😕catbraincell commentedon Nov 3, 2024
The behavior is CORRECT nothing need to be fixed. CDC is emulating serial port behavior which is a bytestream with correct ordering, no lost byte or duplication. packet boundary does not exist in serial port nor CDC.
yp05327 commentedon Nov 4, 2024
I also have Arduino Due, but the behavior is different.
Maybe you are right, I'm not sure which is correct, as I said above I'm not a pro developer of embedded system.
But I can image this:
If it will be split into 2 packets (or maybe more?) randomly, then it will increase the usage of transport?
As there will be more header bytes for these additional packages.
So will this decrease the transport speed?
catbraincell commentedon Nov 5, 2024
If u write the cdc serial fast enough there's no splitting.
yp05327 commentedon Nov 5, 2024
No, IIRC, the splitting is caused by
TransmitQueue. It is a ring buffer, so:It will always transform all bytes from
read pointerto the end of the array defined inTransmitQueue(of cause, your data should be long enough, or it will only simply send all of them)
Then, in the next turn (package), send the rest of your data from the start of the array to
write pointer.Why I said
randomlybecause it is depends on the length of your data and the value ofread pointerand the length ofTransmitQueue.Actually, it is not real randomly happen.
Unless you send bytes one by one, I think you can reproduce it, no matter how
fastyou send the data by usingSerialUSB.write()