'Best' way to load UART data to ring buffer with STM32/HAL

[zzattack]

I'm new to development for this platform so I apologize in advance if there is a simpler solution to my problem than what I'm thinking of. In particular I want to confirm my tedious proposed approach is indeed a good one, or whether I should go with something much simpler.

It seems to me that the HAL 'way' of loading UART data is particularly useless, especially so if you're unsure of how much data you'll expect to receive, or if you want to do so at any somewhat high baudrate. This is mostly because with the HAL_UART_Receive_IT function takes a size parameter indicating how much data should be received before the callback is invoked. And since the overhead in the HAL's way of packing data in the interrupt handler is quite large, overrun errors may happen if we specify size=1 and re-issue the HAL_UART_Receive_IT call from the callback.

One solution that I thought of was to use a DMA channel in circular mode to continuously load data from the peripheral. Then the DMA can take care of a first layer of buffering, and I can use the regular USARTx_IRQHandler to load data from the DMA buffer into my own ring buffer by looking at however much has been transferred into it so far.

I haven't found mention of this approach online so far, so I was wondering whether or not this is a viable approach. Advantages would be the ability to handle bursts at incredibly high baudrates as there cannot be a faster way to free the receive buffer when incoming data arrives, so no risk of overrun. Disadvantages would be the sacrifice of a DMA channel, perhaps inability to handle framing/parity errors and I suppose it's a somewhat convoluted method.

[Psi]

It seems to me that the HAL 'way' of loading UART data is particularly useless, especially so if you're unsure of how much data you'll expect to receive, or if you want to do so at any somewhat high baudrate. This is mostly because with the HAL_UART_Receive_IT function takes a size parameter indicating how much data should be received before the callback is invoked. And since the overhead in the HAL's way of packing data in the interrupt handler is quite large, overrun errors may happen if we specify size=1 and re-issue the HAL_UART_Receive_IT call from the callback.

If you need to be notified of RX bytes 1 at a time then it defeats the point of having a UART HAL layer.

I'm not sure about the STM32 UART HAL but it should also fire the callback if a specific time has elapsed since the last byte (or if data in the buffer is older than x) This way you can have a large buffer but still respond to incoming data quick enough if the data happens to stop with the buffer not full.

[voltsandjolts]

Yes, you can use DMA in circular mode to continuously receive UART chars directly into a RAM circular buffer with no CPU time required, you choose how large the buffer is.
I just poll that DMA buffer from the background routine and process incoming chars there (maybe polling not acceptable for your scenario).
You can use hdmarx->Instance->CNDTR to determine where the DMA write pointer is at (and hence the number of chars received)

Code: [Select]


/*
 * The STM32 makes receiving chars into a large circular buffer simple
 * and requires no CPU time. The UART receiver DMA must be setup as CIRCULAR.
 */
#define CIRC_BUF_SZ       64  /* must be power of two */
static uint8_t rx_dma_circ_buf[CIRC_BUF_SZ];
static UART_HandleTypeDef *huart_cobs;
static uint32_t rd_ptr;

#define DMA_WRITE_PTR ( (CIRC_BUF_SZ - huart_cobs->hdmarx->Instance->CNDTR) & (CIRC_BUF_SZ - 1) )

void msgrx_init(UART_HandleTypeDef *huart)
{
huart_cobs = huart;
HAL_UART_Receive_DMA(huart_cobs, rx_dma_circ_buf, CIRC_BUF_SZ);
rd_ptr = 0;
}

static bool msgrx_circ_buf_is_empty(void) {
if(rd_ptr == DMA_WRITE_PTR) {
return true;
}
return false;
}

static uint8_t msgrx_circ_buf_get(void) {
uint8_t c = 0;
if(rd_ptr != DMA_WRITE_PTR) {
c = rx_dma_circ_buf[rd_ptr++];
rd_ptr &= (CIRC_BUF_SZ - 1);
}
return c;
}


You mentioned parity/framing errors: the default HAL handlers just disable the uart reception. I disable those fault interrupts and use checksummed messages to detect errors.

Code: [Select]

/* These uart interrupts halt any ongoing transfer if an error occurs, disable them */
/* Disable the UART Parity Error Interrupt */
 __HAL_UART_DISABLE_IT(&huart1, UART_IT_PE);
/* Disable the UART Error Interrupt: (Frame error, noise error, overrun error) */
__HAL_UART_DISABLE_IT(&huart1, UART_IT_ERR);


[zzattack]

You mentioned parity/framing errors: the default HAL handlers just disable the uart reception. I disable those fault interrupts and use checksummed messages to detect errors.

Code: [Select]

/* These uart interrupts halt any ongoing transfer if an error occurs, disable them */
/* Disable the UART Parity Error Interrupt */
 __HAL_UART_DISABLE_IT(&huart1, UART_IT_PE);
/* Disable the UART Error Interrupt: (Frame error, noise error, overrun error) */
__HAL_UART_DISABLE_IT(&huart1, UART_IT_ERR);


This appears to have changed with the HAL 1.5.0 UART driver. The HAL_UART_IRQHandler function seems to unconditionally disable ongoing DMA's. I'm having a hard time finding precisely how to handle this. Did you encounter this as well?

[voltsandjolts]

Are you trying to use DMA transfers and IRQ transfers on the same UART at the same time?

[Jeroen3]

ST's DMA is not very suitable of being used as ringbuffer for receiving char based interfaces due to the lack of a current pointer when disabled, eg paused when reading.
When DMA is active, CNDTR reads the current index, but when turned off, it is restored to config register.

Best option is to run the interrupt and read everything in a software ringbuffer. Just pay attention that when reading the buffer from outside the ISR, you'd want to disable the uart interrupt temporarily.

[voltsandjolts]

The above DMA ringbuffer implementation does not need to be "paused when reading" so CNDTR is always valid.
I don't care what the CNDTR value when DMA is disabled, read it before disabling DMA if you need it.

[Jeroen3]

I guess it could be possible to use the CNDTR in runtime to compute the amount of data used. You would have to keep track of overflows though, which is a challenge when the tails isn't aligned at 0 or 0.5*N.

I never tried this before, though of it, but didn't implement it. The ISR of UART is negligible short most of the time. Only short commands.

[voltsandjolts]

I don't keep track of any overflows, not sure if you mean ring buffer or linear buffer overflow or something else.

If the reader doesn't keep up and allows the ringbuffer to overflow, it loses a whole buffer of data.
But that's a data flow issue, the buffer should be large enough to cope with reception bursts while the reader must keep up with the average data rate.

The other option is to stop reception if a ringbuffer overflows but either way you lose data.

[Jeroen3]

The other option is to stop reception if a ringbuffer overflows but either way you lose data.

You do. But you should know. If you don't check, you don't know.

[voltsandjolts]

I use checksummed messages and timeout on packets so lost data is handled graciously.
The app doesn't know if the error was due to buffer overflow / noise corrupted transmission / physical disconnect / etc.. but for me it doesn't matter.

[aandrew]

I've not used DMA with the STM32 U(S)ARTs yet, it's on my todo list though.

My typical use is to have an ISR that reads data from the USART into a ring buffer if a char is available. I ignore errored characters (e.g. drop them in the ISR, they never make it into the ring buffer). If the transmitter empty interrupt fired, I check to see if there are outgoing characters in the tx ring buffer and send them out. If there are no more chars in the TX ring buffer, I disable the transmitter empty interrupt. My uart_write() routine writes chars into the transmitter ring buffer and enables the transmitter empty interrupt. This way both directions of the hardware are handled efficiently. I think that DMA would be slightly more efficient, which is why I mention it's on my todo list, although realistically I've been using the ISR method reliably even at 2Mbps without issue.

Working example lifted from my own code below. I just noticed I don't loop read/write until there are no more available characters (rx) or no more space (tx). That's an area for improvement, as is cleaning up the generic ISR itself, as it's more or less lifted directly from HAL code and isn't up to my usual standards.

Code: [Select]

struct uart {
UART_HandleTypeDef *huart; /* pointer to the HAL_UART struct */
struct fifo *tx, *rx; /* transmit (out of STM32) and receive (in to STM32) FIFOs */
};


static struct uart uart3;
static struct uart uart6;


static void generic_usart_handler(struct uart *u)
{
UART_HandleTypeDef *huart;
uint32_t tmp1, tmp2;
bool err;

err = false;
huart = u->huart;

/* UART parity error interrupt occurred ------------------------------------*/
tmp1 = __HAL_UART_GET_FLAG(huart, UART_FLAG_PE);
tmp2 = __HAL_UART_GET_IT_SOURCE(huart, UART_IT_PE);
if ((tmp1 != RESET) && (tmp2 != RESET)) {
__HAL_UART_CLEAR_PEFLAG(huart);
err = true;
}

/* UART frame error interrupt occurred -------------------------------------*/
tmp1 = __HAL_UART_GET_FLAG(huart, UART_FLAG_FE);
tmp2 = __HAL_UART_GET_IT_SOURCE(huart, UART_IT_ERR);
if ((tmp1 != RESET) && (tmp2 != RESET)) {
__HAL_UART_CLEAR_FEFLAG(huart);
err = true;
}

/* UART noise error interrupt occurred -------------------------------------*/
tmp1 = __HAL_UART_GET_FLAG(huart, UART_FLAG_NE);
tmp2 = __HAL_UART_GET_IT_SOURCE(huart, UART_IT_ERR);
if ((tmp1 != RESET) && (tmp2 != RESET)) {
__HAL_UART_CLEAR_NEFLAG(huart);
err = true;
}

/* UART Over-Run interrupt occurred ----------------------------------------*/
tmp1 = __HAL_UART_GET_FLAG(huart, UART_FLAG_ORE);
tmp2 = __HAL_UART_GET_IT_SOURCE(huart, UART_IT_ERR);
if ((tmp1 != RESET) && (tmp2 != RESET)) {
__HAL_UART_CLEAR_OREFLAG(huart);
err = true;
}

/* UART in mode Receiver ---------------------------------------------------*/
tmp1 = __HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE);
tmp2 = __HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE);
if((tmp1 != RESET) && (tmp2 != RESET)) {
uint16_t val;

val = (uint16_t)(huart->Instance->DR);

/* don't put errored data into the FIFO */
if (!err) {
_fifo_put(u->rx, val);
}
}

/* UART in mode Transmitter ------------------------------------------------*/
tmp1 = __HAL_UART_GET_FLAG(huart, UART_FLAG_TXE);
tmp2 = __HAL_UART_GET_IT_SOURCE(huart, UART_IT_TXE);
if((tmp1 != RESET) && (tmp2 != RESET)) {
char val;

/*
* if there's data to send, send it.
* otherwise disable the transmit empty interrupt.
*/
if (_fifo_get(u->tx, &val) == 0) {
huart->Instance->DR = val;
} else {
__HAL_UART_DISABLE_IT(huart, UART_IT_TXE);
}
}
}


/* HW specific IRQ handlers (routes to the generic handler */
void USART3_IRQHandler(void)
{
generic_usart_handler(&uart3);
}


void USART6_IRQHandler(void)
{
generic_usart_handler(&uart6);
}


/*
 * HW-specific UART init, returns augmented uart struct or NULL
 * NOTE: should probably do the low level stuff in the MSP functions instead
 */
struct uart *uart_hw_init(UART_HandleTypeDef *huart)
{
GPIO_InitTypeDef io;
struct uart *u;

switch ((int)huart->Instance) {
case (int)USART3:
/* enable GPIO and UART clocks. */
__HAL_RCC_GPIOD_CLK_ENABLE(); /* TX/RX on PORTD bit 8/9 */
__HAL_RCC_USART3_CLK_ENABLE();

/* UART TX GPIO pin configuration  */
io.Pin = GPIO_PIN_8;
io.Mode = GPIO_MODE_AF_PP;
io.Pull = GPIO_NOPULL;
io.Speed = GPIO_SPEED_FAST;
io.Alternate = GPIO_AF7_USART3;
HAL_GPIO_Init(GPIOD, &io);

/* UART RX GPIO pin configuration  */
io.Pin = GPIO_PIN_9;
io.Pull = GPIO_PULLUP;
io.Alternate = GPIO_AF7_USART3;
HAL_GPIO_Init(GPIOD, &io);

huart->Init.BaudRate = 115200;
huart->Init.WordLength = UART_WORDLENGTH_8B;
huart->Init.StopBits = UART_STOPBITS_1;
huart->Init.Parity = UART_PARITY_NONE;
huart->Init.HwFlowCtl = UART_HWCONTROL_NONE;
huart->Init.Mode = UART_MODE_TX_RX;
huart->Init.OverSampling = UART_OVERSAMPLING_16;
HAL_UART_Init(huart);

/* NVIC setup */
HAL_NVIC_SetPriority(USART3_IRQn, configMAX_SYSCALL_INTERRUPT_PRIORITY, 0);
HAL_NVIC_EnableIRQ(USART3_IRQn);

u = &uart3;
break;

case (int)USART6:
/* enable GPIO and UART clocks. */
__HAL_RCC_GPIOC_CLK_ENABLE(); /* TX/RX on PORTC bit 6/7 */
__HAL_RCC_USART6_CLK_ENABLE();

/* UART TX GPIO pin configuration  */
io.Pin = GPIO_PIN_6;
io.Mode = GPIO_MODE_AF_PP;
io.Pull = GPIO_NOPULL;
io.Speed = GPIO_SPEED_FAST;
io.Alternate = GPIO_AF8_USART6;
HAL_GPIO_Init(GPIOC, &io);

/* UART RX GPIO pin configuration  */
io.Pin = GPIO_PIN_7;
io.Pull = GPIO_PULLUP;
io.Alternate = GPIO_AF8_USART6;
HAL_GPIO_Init(GPIOC, &io);

huart->Init.BaudRate = 115200;
huart->Init.WordLength = UART_WORDLENGTH_8B;
huart->Init.StopBits = UART_STOPBITS_1;
huart->Init.Parity = UART_PARITY_NONE;
huart->Init.HwFlowCtl = UART_HWCONTROL_NONE;
huart->Init.Mode = UART_MODE_TX_RX;
huart->Init.OverSampling = UART_OVERSAMPLING_16;
HAL_UART_Init(huart);

/* NVIC setup */
HAL_NVIC_SetPriority(USART6_IRQn, configMAX_SYSCALL_INTERRUPT_PRIORITY, 0);
HAL_NVIC_EnableIRQ(USART6_IRQn);

u = &uart6;
break;

default:
/* do nothing */
u = NULL;
break;
};

/* initialize the uart's FIFO subsystem */
if (u) {
u->huart = huart;
u->tx = fifo_init(128);
u->rx = fifo_init(128);

/* enable UART RX interrupt. Disable the TX interrupt since the FIFO's empty right now */
//__HAL_UART_ENABLE_IT(huart, UART_IT_RXNE); /* receiver not empty */
__HAL_UART_DISABLE_IT(huart, UART_IT_TXE); /* transmit empty */
}

return u;
}


/* writes to the transmitter FIFO. Returns the number of bytes written */
int uart_write(struct uart *u, const char *buf, int len)
{
int nwritten;

nwritten = 0;
while (len) {
if (fifo_put(u->tx, *buf) == 0) {
++buf;
++nwritten;
--len;
} else {
break;
}

/* enable the transmitter empty interrupt to start things flowing */
__HAL_UART_ENABLE_IT(u->huart, UART_IT_TXE);
};

return nwritten;
}


/* reads from the receiver FIFO. Returns the number of bytes read */
int uart_read(struct uart *u, char *buf, int maxlen)
{
int nread, left;

nread = 0;
left = maxlen;
while (left) {
if (fifo_get(u->rx, buf)) {
++buf;
++nread;
--left;

/* no character available, drop out immediately */
} else {
break;
}
};

return nread;
}


[AlexandreGuimaraes]

Hi Andrew,
 
Would it be possible for you to post the "fifo" functions and a working TX/RX example ? I am new to using the STM32's and am trying to get up to speed on the new environment. I am using a "variation" of your implementation but yours seem to be much cleaner than my take at it and also the fact that you have used the solution up to 2 mbps makes me more comfortable with a tested implementation.
 
Thanks a lot for sharing the knowledge !
 
Best Regards,
Alexandre

 

[alexthehun]

Yes, you can use DMA in circular mode to continuously receive UART chars directly into a RAM circular buffer with no CPU time required, you choose how large the buffer is.
I just poll that DMA buffer from the background routine and process incoming chars there (maybe polling not acceptable for your scenario).
You can use hdmarx->Instance->CNDTR to determine where the DMA write pointer is at (and hence the number of chars received)

Code: [Select]


/*
 * The STM32 makes receiving chars into a large circular buffer simple
 * and requires no CPU time. The UART receiver DMA must be setup as CIRCULAR.
 */
#define CIRC_BUF_SZ       64  /* must be power of two */
static uint8_t rx_dma_circ_buf[CIRC_BUF_SZ];
static UART_HandleTypeDef *huart_cobs;
static uint32_t rd_ptr;

#define DMA_WRITE_PTR ( (CIRC_BUF_SZ - huart_cobs->hdmarx->Instance->CNDTR) & (CIRC_BUF_SZ - 1) )

void msgrx_init(UART_HandleTypeDef *huart)
{
huart_cobs = huart;
HAL_UART_Receive_DMA(huart_cobs, rx_dma_circ_buf, CIRC_BUF_SZ);
rd_ptr = 0;
}

static bool msgrx_circ_buf_is_empty(void) {
if(rd_ptr == DMA_WRITE_PTR) {
return true;
}
return false;
}

static uint8_t msgrx_circ_buf_get(void) {
uint8_t c = 0;
if(rd_ptr != DMA_WRITE_PTR) {
c = rx_dma_circ_buf[rd_ptr++];
rd_ptr &= (CIRC_BUF_SZ - 1);
}
return c;
}


You mentioned parity/framing errors: the default HAL handlers just disable the uart reception. I disable those fault interrupts and use checksummed messages to detect errors.

Code: [Select]

/* These uart interrupts halt any ongoing transfer if an error occurs, disable them */
/* Disable the UART Parity Error Interrupt */
 __HAL_UART_DISABLE_IT(&huart1, UART_IT_PE);
/* Disable the UART Error Interrupt: (Frame error, noise error, overrun error) */
__HAL_UART_DISABLE_IT(&huart1, UART_IT_ERR);


_______________________________________________________________________
Excellent solution! After a small modification it works as intended! Good work.
Only change required because STM has changed the register's name from
huart_cobs->hdmarx->Instance->CNDTR to
huart_cobs->hdmarx->Instance->NDTR.
_______________________________________________________________________