Skip to content

Instantly share code, notes, and snippets.

@milesfrain
Last active February 3, 2021 20:45
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save milesfrain/cf779c28a531bb5a389eced5bee35649 to your computer and use it in GitHub Desktop.
Save milesfrain/cf779c28a531bb5a389eced5bee35649 to your computer and use it in GitHub Desktop.
Nicer formatting for forum question

Why is USBD CDC Data OUT Receive callback passed RxLength as a pointer instead of as a value?

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)

https://github.com/STMicroelectronics/STM32CubeF4/blob/master/Middlewares/ST/STM32_USB_Device_Library/Class/CDC/Src/usbd_cdc_if_template.c#L190-L212

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);
}

https://github.com/STMicroelectronics/STM32CubeF4/blob/e084518f363e04344dc37822210a75e87377b200/Projects/STM324x9I_EVAL/Applications/USB_Device/CDC_Standalone/Src/usbd_cdc_interface.c#L304-L308

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment