cancel
Showing results for 
Search instead for 
Did you mean: 

STM32F3Discovery USB CDC Receive loosing data

Torben Hansen
Associate II
Posted on October 18, 2017 at 14:54

I'm trying to make my STM32F3Dsicovery just loop back some data over USB however with code below I loosing approximate every 2nd character. If I enable the &sharpif clause the missing characters drop to approximately 2%.

I'm using minicom as terminal

$ minicom -D /dev/ttyACM0

and the linux kernel 4.4.87 stock cdc_acm driver

I'm pasting 50 characters into the terminal. They seems to be sent as 50 single character packages.

Apart from the code below it's as generated from CubeMX, imported in eclipse as Makefile-project.

Any idea what I'm doing wrong?

I have the following in usb_cdc_if.c

/* USER CODE BEGIN PRIVATE_VARIABLES */

 

&sharpdefine USER_DATA_SIZE (1<<8)

 

uint8_t ReadBuffer[USER_DATA_SIZE];

 

uint32_t ptrIn=0,ptrOut=0;

 

/* USER CODE END PRIVATE_VARIABLES */

 

 ...........snip ....

static int8_t CDC_Receive_FS (uint8_t* Buf, uint32_t *Len)

{

  /* USER CODE BEGIN 6 */

    uint32_t ptrCDC=0;

    while (ptrCDC<(*Len))

    {

        if (((ptrIn+1) & (USER_DATA_SIZE-1))!=ptrOut)

        {

            ReadBuffer[ptrIn++]=Buf[ptrCDC++];

            ptrIn&=(USER_DATA_SIZE-1);

        }

        else

        {

            return(USBD_FAIL);

        }

    }

&sharpif 0

    for (int i=0;i<50;i++)

        ;

&sharpendif

  USBD_CDC_SetRxBuffer(&hUsbDeviceFS, &Buf[0]);

  USBD_CDC_ReceivePacket(&hUsbDeviceFS);

  return (USBD_OK);

  /* USER CODE END 6 */

}

and in main()

  /* USER CODE BEGIN WHILE */

  while (1)

  {

  /* USER CODE END WHILE */

  /* USER CODE BEGIN 3 */

      uint8_t tBuffer[100];

      uint32_t rLen = CDC_Read(tBuffer,100);

      if (rLen>0)

          CDC_Transmit_FS(tBuffer, rLen);

  }

  /* USER CODE END 3 */

#cdc-stm32f3discovery
1 ACCEPTED SOLUTION

Accepted Solutions
Daniel Glasser_2
Associate II
Posted on October 18, 2017 at 16:06

torhans

: Can you post the 'CDD_Read()' code?

You shouldn't pass tBuffer to CDC_Read() until the transfer initiated byCDC_Transmit_FS()has completed. The reason the loop you have commented out improves the loss rate is that it allows the hardware more time to complete the previous transmit before allowing the main thread to resume execution.

'There is no RTOS, only one thread' I can imagine someone asserting, but 'CDC_Receive_FS()' is running in handler state which preempts the main thread, RTOS or no RTOS. When the inbound packet arrives, the USB CDC device middleware is called by the IRQ handler at the IRQ level set for the USB FS in CubeMX, likely 0. The main loop and 'CDD_read()' are executing in thread level.

There is another function generated by CubeMx that is called when a transmit is complete, set a flag before you send a buffer, clear it in the tx complete routine, and don't reuse 'tBuffer' until the flag is clear.

View solution in original post

6 REPLIES 6
Torben Hansen
Associate II
Posted on October 18, 2017 at 15:46

Hmmmm

Seems to be a problem with minicom. The problem appear neither using miniterm nor a python script.

Sorry for the

inconvenience

Daniel Glasser_2
Associate II
Posted on October 18, 2017 at 16:06

torhans

: Can you post the 'CDD_Read()' code?

You shouldn't pass tBuffer to CDC_Read() until the transfer initiated byCDC_Transmit_FS()has completed. The reason the loop you have commented out improves the loss rate is that it allows the hardware more time to complete the previous transmit before allowing the main thread to resume execution.

'There is no RTOS, only one thread' I can imagine someone asserting, but 'CDC_Receive_FS()' is running in handler state which preempts the main thread, RTOS or no RTOS. When the inbound packet arrives, the USB CDC device middleware is called by the IRQ handler at the IRQ level set for the USB FS in CubeMX, likely 0. The main loop and 'CDD_read()' are executing in thread level.

There is another function generated by CubeMx that is called when a transmit is complete, set a flag before you send a buffer, clear it in the tx complete routine, and don't reuse 'tBuffer' until the flag is clear.

Posted on October 18, 2017 at 16:55

Yes, of cause!

This fixes the problem (maybe not elegantly)

  /* USER CODE BEGIN WHILE */

  while (1)

  {

  /* USER CODE END WHILE */

  /* USER CODE BEGIN 3 */

      uint8_t tBuffer[100];

      uint32_t rLen = CDC_Read(tBuffer,100);

      if (rLen>0)

      {

          CDC_Transmit_FS(tBuffer, rLen);

          USBD_CDC_HandleTypeDef   *hcdc = (USBD_CDC_HandleTypeDef*) hUsbDeviceFS.pClassData;

          while (hcdc->TxState !=0)

              ;

      }

  }

  /* USER CODE END 3 */

and as requested

uint32_t CDC_Read(uint8_t* Buffer,uint32_t bLength)

{

    uint32_t i=0;

    while ((i<bLength) && (ptrOut!=ptrIn))

    {

        Buffer[i++]=ReadBuffer[ptrOut++];

        ptrOut&=(USER_DATA_SIZE-1);

    }

    return(i);

}

Thanks

Daniel Glasser
Associate III
Posted on October 20, 2017 at 02:37

torhans

I'm glad that helped. I entered the reply on a 7' tablet, and I'm not very good at typing on a virtual keyboard.

I also seem to have become two separate users. Oh, well...

Overwritingbuffers that have data you're not done with yet is a fairly common multithreading error, and I've encountered many programmers who don't realize that their program becomes multithreaded as soon as interrupt handlers enter the picture.

I think there's one other problem with your code, though, and that is a possible unintended interraction between 'CDC_Read()' and 'CDC_Receive_FS()'. There may be a race condition because the 'ptrOut++', which is not atomic itself but isn't the problem, may update 'ptrOut' before the copy of the byte from 'ReadBuffer[ptrOut]' to 'Buffer[i]'; the compiler is free to write machine code that acts like you had written:

uint32_t ptrTemp = ptrOut;

ptrOut += 1;

/* what happens if an interrupt occurs right here */

Buffer[i++] = ReadBuffer[ptrTemp];

I'm not saying the compiler is doing this, but there's nothing saying that it's not allowed to. There are two things I suggest to make the code less dependent on the compiler for running correctly:

  1. add 'volatile' to the global definitions of both 'ptrOut' and 'ptrIn'
  2. In 'CDC_Read()', the variable'i' is going to be in a register, and isn't shared between the main and handler threads, so it's not a problem, but 'ptrOut' is a global. The compiler might, adding 'volatile' will make the compiler load the value each time through the loop and store it after it's modified rather than doing it once at the beginning or end of the loop, rather than add the post-increment in the array reference, do it on the next line in the loop. The code becomes:

Buffer[i++] = ReadBuffer[ptrOut];

ptrOut++;

This may seem a bit silly, but I've been bitten by this sort of thing in the past. None of the compound operations you use are, by definition, atomic. The stuff running in the handler thread cannot be preempted by the main thread, so what that's doing is pretty safe, but the main thread can be preempted between any two instructions if you've not disabled or otherwise masked the interrupts.

Good luck!

dhaselwood
Associate II
Posted on December 05, 2017 at 19:13

I have a similar problem and it appears that ttyACM0 either itself, or in conjunction with

ST's CDC routine (and maybe hardware), is part of the problem.  In my case I check the USBD_BUSY flag before sending some test lines repeatedly--

sprintf(b,'%7d ',ix++);

while((CDC_Transmit_FS((uint8_t*)b, strlen( b))) == USBD_BUSY);

while((CDC_Transmit_FS((uint8_t*)cdc0, strlen(cdc0))) == USBD_BUSY);

If the PC terminal is Ubuntu w minicom the first 15-30 seconds will show many missed lines. 

If I put a program delay loop (ranging from nil to about 3/4 second) in between each CDC_Transmit the errors still occur, so the rate of calling CDC_Transmit doesn't appear to be an issue.  (The baud rate settings at each end make no difference, whether they are slow or fast, or the same or different.)

If the terminal is a Windows 10 w PuTTY nothing is missed.  Works correctly every time.

If the terminal is Ubuntu w minicom, with Ubuntu running under 'virtualbox' on the Windows 10 machine the result is the same as the Ubuntu-only  machine.

Using 'cat /dev/ttyACM0' instead of minicom results in the same errors.  This indicates that it is not associated with minicom, but it is a linux ACM driver issue, either alone, or in conjunction with the ST CDC driver and/or hardware. 

As 12/5/2017 that is as far as I have gotten in sorting this out.  Any ideas of how to proceed would be appreciated.

dhaselwood
Associate II
Posted on December 06, 2017 at 05:26

Solution to my problem.  Linux ACM driver is attempting to communicate with a modem and that blocks the data transfer.  Apparently it gives up after 15 or so seconds and everything works fine afterwards.  Disabling or getting rid of 'modemmanager' takes care of the problem.  The following link describes it in more detail--

https://hackingmajenkoblog.wordpress.com/2016/08/24/diagnosing-arduino-problems-in-linux/