USB CDC ACM: RX data gets overwritten in app_usbd_cdc_acm_read_any() at end of data transmission

To handle RX of USB CDC ACM we are polling data using:

ret = app_usbd_cdc_acm_read_any(&m_app_cdc_acm, m_rx_buffer, MIN(BUFFER_SIZE,size));

On

case NRF_SUCCESS:

rx_size = app_usbd_cdc_acm_rx_size(&m_app_cdc_acm);
copy_rx_data(buffer, n, m_rx_buffer);

We assume data has been transferred to our use buffer `m_rx_buffer` and get the data directly.

On

case NRF_ERROR_IO_PENDING:

usb_txrx_state = USB_TXRX_STATE_RX;

We assume data has been NOT been transferred and assume to we get the data in the event handler instead.

void cdc_acm_user_ev_handler(app_usbd_class_inst_t const * p_inst,
app_usbd_cdc_acm_user_event_t event)
{
app_usbd_cdc_acm_t const * p_cdc_acm = app_usbd_cdc_acm_class_get(p_inst);
last_usb_cdc_event = event;
ret_code_t ret;
switch (event)
{
...
case APP_USBD_CDC_ACM_USER_EVT_RX_DONE:

rx_size = app_usbd_cdc_acm_rx_size(p_cdc_acm);
copy_rx_data(buffer, n, m_rx_buffer);

This works well until the end of the data (583 bytes in total) is transferred.

On the NEXT TO LAST call to app_usbd_cdc_acm_read_any() we get NRF_SUCCESS and get correct data.

On the last call to app_usbd_cdc_acm_read_any() we get NRF_ERROR_IO_PENDING, but we see that correct data anyway has still been transferred to m_rx_buffer.

But because we assume there are no data directly available (it should be fetched in event handler) we make an intermediate call to:

while (app_usbd_event_queue_process()) {
}

To finalize the transfer event handler.

Stepping through app_usbd_event_queue_process() we see the following sequence of calls ...

#if (APP_USBD_CONFIG_SOF_HANDLING_MODE == APP_USBD_SOF_HANDLING_COMPRESS_QUEUE)
if (p_event_item->sof_cnt > 0)
{
if (p_event_item->start_frame > USBD_FRAMECNTR_FRAMECNTR_Msk)
{
p_event_item->start_frame = 0;
}
sof_event.drv_evt.data.sof.framecnt = (p_event_item->start_frame)++;
--(p_event_item->sof_cnt);
app_usbd_event_execute(&sof_event);
return true;
}
#endif // (APP_USBD_CONFIG_SOF_HANDLING_MODE == APP_USBD_SOF_HANDLING_COMPRESS_QUEUE)

==> app_usbd_event_execute(&(p_event_item->evt));

...

/**
* @brief @ref app_usbd_class_methods_t::event_handler
*/
static ret_code_t cdc_acm_event_handler(app_usbd_class_inst_t const * p_inst,
app_usbd_complex_evt_t const * p_event)
{
ASSERT(p_inst != NULL);
ASSERT(p_event != NULL);

ret_code_t ret = NRF_SUCCESS;
switch (p_event->app_evt.type)
{
...
case APP_USBD_EVT_DRV_EPTRANSFER:
==> ret = cdc_acm_endpoint_ev(p_inst, p_event);
break;

...

static ret_code_t cdc_acm_endpoint_ev(app_usbd_class_inst_t const * p_inst,
app_usbd_complex_evt_t const * p_event)
{

==> ret = cdc_acm_rx_block_finished(p_inst);

...

static ret_code_t cdc_acm_rx_block_finished(app_usbd_class_inst_t const * p_inst)
{

...

==> memcpy(p_cdc_acm_ctx->rx_transfer[0].p_buf,
p_cdc_acm_ctx->internal_rx_buf,
bytes_to_cpy);

The final memcpy finally COPIES OLD DATA from p_cdc_acm_ctx into our user buffer (which contained good data).

So there seems to be some internal USB stack state machine problem here ...

To summarise:

1. Correct data is transferred from app_usbd_cdc_acm_read_any() even if we got NRF_ERROR_IO_PENDING.
2. Consective call app_usbd_event_queue_process() copies old data (i.e. overwrites good data) to user buffer. This should not be possible if USB state contained in p_cdc_acm_ctx is valid.
3. We dont get APP_USBD_CDC_ACM_USER_EVT_RX_DONE event as expected until user buffer is overwritten by app_usbd_event_queue_process()


We are using SDK 17.0.2.

We have enabled APP_USBD_CDC_ACM_CONFIG_LOG_ENABLED, but it does not give any good indication.

One addional note: This is size dependent as well. So if last call returns data pending (which seems to be size dependent) internal buffer seems to be corrupted.

615 OK
593 - 599 OK
581-592 NOK


  • Hello PeterLjung,

    Our sincere apologies for the long wait. We have been unexpectedly understaffed.

    Please be informed that I will support you with this case.

    From your description here:

    On the NEXT TO LAST call to app_usbd_cdc_acm_read_any() we get NRF_SUCCESS and get correct data.

    On the last call to app_usbd_cdc_acm_read_any() we get NRF_ERROR_IO_PENDING, but we see that correct data anyway has still been transferred to m_rx_buffer.

    But because we assume there are no data directly available (it should be fetched in event handler) we make an intermediate call to:

    while (app_usbd_event_queue_process()) {
    }

    To finalize the transfer event handler.

    Am I right to understand that you have an immediate workaround, where you can stop receiving at the last call to app_usbd_cdc_acm_read_any()?

    I haven't been able to figure out what is wrong today. However, I will keep looking into it. It would be very helpful if you can give me some step to reproduce the issue with our examples.

    One addional note: This is size dependent as well. So if last call returns data pending (which seems to be size dependent) internal buffer seems to be corrupted.

    615 OK
    593 - 599 OK
    581-592 NOK

    This seems very odd to me. There is no commonly seen pattern to issues here.

    How consistent are your observations regarding which size of data will produce the issue?
    I.e.: Are you able to consistently reproduce the issue or get things to work correctly with particular size of data?

    Hieu

  • Actually I have not found any way to detect that this issue has or will happen in runtime except during debugging where I noticed this specific behaviour by looking at the specific data buffers.

    I am using a workaround currently where we have split up the message in several smaller parts instead. This seems to work ok for now. When I am using the same size sent from PC host the error always occur so its consistent.

    I will try to extract a smaller sample from our codebase which reproduce this error.

    Thanks / Peter

  • Hi Peter,

    Firstly, I would like to update you that I have been looking into the USB CDC ACM modules and a reproduction setup base on the USB CDC ACM example. 

    Next, I would like to ask about a few details in your code:

    ret = app_usbd_cdc_acm_read_any(&m_app_cdc_acm, m_rx_buffer, MIN(BUFFER_SIZE,size));

    How are you calling this function?

    rx_size = app_usbd_cdc_acm_rx_size(p_cdc_acm);
    copy_rx_data(buffer, n, m_rx_buffer);

    Are buffer and n variables for another level of buffer where you just recombine the fragments of data from received in m_rx_buffer across multiple the app_usbd_cdc_acm_read_any() calls?

    If that's not correct, could you give more details about them?

    Also, it would be great if you can recommend me a terminal program that goes well with this kind of test. I have been using PuTTy but I don't feel very confident that it's suitable for sending a large number of bytes at once. 

    Finally, I understand that you have waited a long time before receiving the first reply. However, I am out of office for the rest of the week. I will try to follow up if possible. My apology for the inconvenience. 

    Regards,

    Hieu

  • Size is accessed both directly after app_usbd_cdc_acm_read_any() on success. 
    And from event handler in pending response.

    The following is a complete example including firmware and a small Ruby script on PC side to reproduce the error.
    That should hopefully make it clear how we are using the API.
    usb-test-packed-no-sdk.tgz

    See README.md for intro of how to use the code. 

    Regards 

  • Hi Peter,

    This seems to be some thing in the design of the state machine.I haven't been able to replicate it yet as both of the working and non_working seems be stuck at my end. I am running ubuntu in the virtual machine but running VM should not be an issue.

    I will try to see if I can include the developer into helping us into this issue. 

Related