How to use HAL_UART_Receive_IT to receive uart data?

[E-John]

Hi all,

What is the correct flow to use HAL_UART_Receive_IT() to receive uart data?

The first package is lost first its first char,

Here is the flow used now, 

From the gif, you can see "H" is missing for 1st package, and all normal for others.

The problem is that the we call "HAL_UART_Receive_IT(&huart3, &data, 1);" outside the endless while(1), which lost the first char "H"

Maybe it can solved by LL lib to solve it, I just want to know if it is possible to use all HAL function to implement a normal flow to receive data correctly,

Thanks,

E-John

typedef struct {
    uint8_t buffer[RX_BUFFER_SIZE];
    size_t head;
    size_t tail;
} RingBuffer;

RingBuffer rx_buffer;

void HAL_UART_RxCpltCallback(UART_HandleTypeDef *huart)
{
    if (huart->Instance == USART3)
    {
      PC_Uart_RXCpltCallback(huart);
    }
}

void PC_Uart_RXCpltCallback(UART_HandleTypeDef *huart)
{
  uint8_t rx_data;
  HAL_StatusTypeDef status;

  status = HAL_UART_Receive_IT(&huart3, &rx_data, 1);

  if (status == HAL_OK) {
    taskENTER_CRITICAL();
    /* Add the data to the ring buffer */
    rx_buffer.buffer[rx_buffer.head] = rx_data;
    rx_buffer.head = (rx_buffer.head + 1) % RX_BUFFER_SIZE;
    taskEXIT_CRITICAL();
  }

  if (rx_data == '\r') {
  	cmd_terminated = 1;
  }
}
void PCUartTask(void *pvParameters)
{
    uint8_t data;
    uint16_t len = 0;

    HAL_UART_Receive_IT(&huart3, &data, 1);
    while (1) {
        if (cmd_terminated == 1) {
            taskENTER_CRITICAL();
            len = rx_buffer.head;
            if (len > 0) {
              HAL_UART_Transmit_IT(&huart3, rx_buffer.buffer, len);
            }

            cmd_terminated = 0;
            rx_buffer.head = 0;
            taskEXIT_CRITICAL();
        }
        vTaskDelay(pdMS_TO_TICKS(100)); // Delay for 100ms
    }
}

 

[AScha.3]

Hi,

>I just want to know if it is possible to use all HAL function to implement a normal flow to receive data correctly

Sure !

Even on a kiddies-cpu (G0 = M0+ series )  the serial/UART (even at 115k ) is super slow , compared to the cpu/core .

So its only up to you, to think about : when start receive, what to do then, if received a byte ,  etc. 

What i see in the little part of your code, you showed :

1. you start HAL_UART_Receive_IT(..)  again, before you get/use the last received thing - wrong timing sequence.

First use the received char, then start IT again .

2. why call from uart callback -> next function, just to read the received byte ? Anything you do here, delays the INT service, so think about every instruction here - really needed here ?

[TDK · Accepted solution]

Code is mostly okay except:

• Use a global variable to store the received byte. You're using a local variable which goes out of scope immediately. This will lead to stack corruption.
• Your first call to HAL_UART_Receive_IT uses a different variable than the subsequent calls. This is the reason for the lost "H".
• Read the variable before you do the next HAL_UART_Receive_IT call. Since UART is slow, probably isn't making a difference here but it's bad practice since it introduces a race condition.

[E-John]

Hi TDK,

Thanks for your guide, I have modified the code as you mentioned, it works.

• Use a global variable to store the received byte. You're using a local variable which goes out of scope immediately. This will lead to stack corruption.

         ---> modify code to use global variable "g_rx3_data" to store the received byte.

• Your first call to HAL_UART_Receive_IT uses a different variable than the subsequent calls. This is the reason for the lost "H".

        ---> yes.

• Read the variable before you do the next HAL_UART_Receive_IT call. Since UART is slow, probably isn't making a difference here but it's bad practice since it introduces a race condition.

       ---> Understood.

#define RX_BUFFER_SIZE 128

extern UART_HandleTypeDef huart3;

typedef struct {
    uint8_t buffer[RX_BUFFER_SIZE];
    size_t head;
    size_t tail;
} RingBuffer;
xSemaphoreHandle xSemaphore = NULL;
volatile uint8_t cmd_terminated = 0;
volatile char ring_buffer[BUFFER_SIZE];
static uint8_t g_rx3_data; // put this in the global scope

void PC_Uart_RXCpltCallback(UART_HandleTypeDef *huart)
{
  taskENTER_CRITICAL();
  /* Add the data to the ring buffer */
  rx_buffer.buffer[rx_buffer.head] = g_rx3_data;  // it is read from the uart3 rx interrupt 
  rx_buffer.head = (rx_buffer.head + 1) % RX_BUFFER_SIZE;
  taskEXIT_CRITICAL();

  if (g_rx3_data == '\r') {
  	cmd_terminated = 1;
  }
  HAL_UART_Receive_IT(&huart3, &g_rx3_data, 1);
}

void PCUartTask(void *pvParameters)
{
    uint16_t len = 0;

    HAL_UART_Receive_IT(&huart3, &g_rx3_data, 1);

    while (1) {
        if (cmd_terminated == 1) {
            taskENTER_CRITICAL();
            len = rx_buffer.head;
            if (len > 0) {
              HAL_UART_Transmit_IT(&huart3, rx_buffer.buffer, len);
            }

            cmd_terminated = 0;
            rx_buffer.head = 0;
            taskEXIT_CRITICAL();
        }

        vTaskDelay(pdMS_TO_TICKS(100)); // Delay for 100ms
    }
}

 

[Karl Yamashita]

HAL_UART_Receive_IT should only be called from the callback and not in your PCUartTask. 

Instead of using HAL_UART_Receive_IT and receiving 1 byte at a time, use HAL_UARTEx_ReceiveToIdle_DMA to interrupt on each packet. If you don't want to use DMA, you can use HAL_UARTEx_ReceiveToIdle_IT instead.

You can replace the circular buffer you have with a queue buffer to hold each packet.

See this project https://github.com/karlyamashita/Nucleo-G431RB_Three_UART/wiki

[E-John]

Dear Karl,

1. HAL_UART_Receive_IT should only be called from the callback and not in your PCUartTask. 

---> It is needed to call HAL_UART_Receive_IT() outside the callback once.

      Because there is a state machine controlled by HAL_UART_xxx API, 

      The purpose of call HAL_UART_Receive_IT outside the callback is to "start the HAL UART mechanism", trace the code inside HAL_UART_Receive_IT(), finally it call "return (UART_Start_Receive_IT(huart, pData, Size));" which start the UART HAL state machine to work.

I have removed it from outside callback, it does not work.

2. HAL_UARTEx_ReceiveToIdle_IT instead.

---> OK, I will try it, thanks.

3. You can replace the circular buffer you have with a queue buffer to hold each packet.

---> Also thanks for the link, also try it, thanks.

Thanks,

E-John

[Karl Yamashita]

You would call HAL_UART_Receive_IT before your main while loop just to initialize it. After that it is only called from the callback. See my project and you'll see how it's done.