Consider the definition of the Receive
callback in usbd_cdc.h:
typedef struct _USBD_CDC_Itf
{
// ...
int8_t (* Receive)(uint8_t *Buf, uint32_t *Len);
// ...
} USBD_CDC_ItfTypeDef;
Passing length as a pointer implies that the callback is supposed to edit this value, but I don't see where the USB library makes use of a modified RxLength
(maybe the ECM and RNDIS USB libraries expect this to be edited in the callback, but that's still unclear). For basic CDC operation:
- Are we even supposed to edit
RxLength
in the callback? - If not, why does the library pass this as a pointer? Passing it as a value would be clearer.
For context, here's where USBD_CDC_DataOut
calls the user-provided Receive
callback:
https://github.com/STMicroelectronics/STM32CubeF4/blob/b95577991ef8891c62c41e0845903e182cdf60d3/Middlewares/ST/STM32_USB_Device_Library/Class/CDC/Src/usbd_cdc.c#L710-L728
I didn't find any clarification about this in UM1734 Rev 4. Search for CDC_Itf_Receive
.
The comments in the provided Receive template callback are confusing, specifically "data to be received" vs "data received" in the @param
lines. It's unclear when the receive happened (or will happen) and who did (or will do) the receiving. Are we talking about what happened before this callback is invoked (e.g. "receiving" data into Buf
), or what's going to happen inside this callback (e.g. copying data out of Buf
to another location that's ultimately "receiving" the data).
/*
* @param Buf: Buffer of data to be received
* @param Len: Number of data received (in bytes)
*/
static int8_t TEMPLATE_Receive(uint8_t *Buf, uint32_t *Len)
Also, some of the other comments could be clearer. For example, I assume "transfer is complete on CDC interface" refers to your custom copy operation.
Original:
"If you exit this function before transfer is complete on CDC interface (ie. using DMA controller) it will result in receiving more data while previous ones are still not sent."
My Interpretation:
"If you exit this function before copying all data out of Buf (ie. using DMA controller) you risk losing or corrupting data due to Buf being overwritten with newly-received data."
So I believe the appropriate way to implement this callback (assuming you want to read a continuous stream of data) is something like this:
static int8_t CDC_Itf_Receive(uint8_t * Buf, uint32_t * Len)
{
// Copy *Len bytes from Buf
// e.g. memcpy(some_other_buf, Buf, *Len);
// Todo error handling if there's not enough free space in some_other_buf
// Enable next transfer
USBD_CDC_ReceivePacket(&USBD_Device);
return (USBD_OK);
}
In contrast, the provided example appears to do a bunch of stuff incorrectly:
- Initiates DMA transfer using
Buf
, which invites corruption as the USB library could overwrite this same buffer that's being used for UART DMA. A better version would make a temporary buffer copy for use with UART DMA. - Doesn't call
USBD_CDC_ReceivePacket(&USBD_Device)
to allow for the next transfer.
static int8_t CDC_Itf_Receive(uint8_t * Buf, uint32_t * Len)
{
HAL_UART_Transmit_DMA(&UartHandle, Buf, *Len);
return (USBD_OK);
}
In summary:
- I don't think length should be passed as pointer to the receive callback. If using a pointer is correct, then it's unclear how we should and in what situations we should edit that value in the callback.
- The documentation, code comments, and examples are not as helpful as they could be.