Associate III

Solved

HAL_UART_Transmit() with HAL_UART_Receive_IT() fails if RX when TX not completed.

Forum|Forum|7 years ago
April 16, 2019
8 replies
14694 views

I used to send AT command to a module with HAL_UART_Transmit():

void send_uart(char * buffer)
{
	uint16_t buffer_size = strlen(buffer);	
	uint8_t CRLFbuff[] = "\r\n";
	HAL_UART_Transmit(&hlpuart1, CRLFbuff, 2, 0xFF); // ok
	HAL_UART_Transmit(&hlpuart1, (uint8_t *)buffer, buffer_size, 0xFF); // ok
	HAL_UART_Transmit(&hlpuart1, CRLFbuff, 2, 0xFF); // ok
}

and receive answer with HAL_UART_Receive_IT() byte per byte:

HAL_UART_Receive_IT(&hlpuart1, &Rx_data, 1);
 
void HAL_UART_RxCpltCallback(UART_HandleTypeDef *huart) 
{
 if (huart->Instance == LPUART1){ // Current UART
		if (Rx_indx + 1 > rx_buffer_size){
			Rx_indx = 0;
		}
 if (Rx_data != 0x0A){ // If received data different from LF
 Rx_Buffer[Rx_indx++] = Rx_data; // Add data to Rx_Buffer
		}
		else {
			Rx_indx = 0;
			Transfer_cplt++;//transfer complete, data is ready to read
		}
		HAL_UART_Receive_IT(&hlpuart1, &Rx_data, 1); //activate UART receive interrupt every time 
	}
}

And this is working well at 9600 in nominal TX/RX. But if I send bad data to the module it sends immediately "ERROR: parse error" while my STM32L031 is still transmitting. This as the effect of killing my RX interrupt. Instead of being HAL_UART_STATE_BUSY_RX the UART->RxState switches to HAL_UART_STATE_READY and RX interrupt is never triggered (transmit part still works good).

I changed my transmit code to HAL_UART_Transmit_IT (with little rework) and this behavior is not observed (works well).

Is this normal ?

This topic has been closed for replies.

Best answer by Bob S

(1) your 2nd param to snprintf() is not correct and may overflow your msg[] array. Unless you can guarantee that buffer always contains a string that is "tx_buffer_size" bytes or less. Likewise, your size param in HAL_UART_Transmit_IT() is needlessly complicated. And why not include the CR/LF in your format string? Better would be:

void send_uart(char * buffer)
{
 char msg[2+tx_buffer_size+2+1] = "";	// 'CR' 'LF' "data" 'CR' 'LF' '\0'
 // The "-1" here guarantees that there is room for snprintf() to append
 // the trailing NULL
 snprintf(msg, sizeof(msg)-1, "\r\n%s\r\n", buffer);
 HAL_UART_Transmit_IT(&hlpuart1, (uint8_t *)msg, strlen(msg)); // No need to transmit the null terminator
}

(2) To know when the transmit has actually completed you need to implement the HAL_UART_TxCpltCallback() function (which is called from the UART interrupt handler) and set a flag telling your main code loop that the TX is done.

Bob S

Super User

Yes. The command parsers in some devices will generate an error message as soon as they detect an error in the command string. Others may wait until they get the complete command before they generate an error message. So you need to be ready to receive data as soon as you start transmitting (which really means *before* you start transmitting).

Thomas LBAuthor

Associate III

Thank you for answer.

The problem is that I don't understand why the RX interrupt is stopped in this situation ?

Each time a character is received the IT is re-armed with HAL_UART_Receive_IT(&hlpuart1, &Rx_data, 1); So I don't understand how this could be stopped (even if something is transmitting on TX, I doesn't expect this to be a issue for RX side ?

Bob S

Super User

I think the problem is mixing polled and interrupt HAL functions and the __HAL_LOCK() macro. If you look in HAL_UART_Receive_IT(), at least in the STM32L4xx library, one of the first things is a call to __HAL_LOCK(). This seemingly harmless looking macro will, SUPRISE!!!, EXIT THE CALLING FUNCTION if the huart instance is already locked (look at the definition of __HAL_LOCK). In my opinion this is really poor code design to hide a conditional function return inside a macro.

So when you were using HAL_UART_Transmit(), each call would lock that uart, send the data, then unlock the uart. If you ever got a receive interrupt while HAL_UART_Transmit() was active, the call to HAL_UART_Receive_IT() in the interrupt callback would fail (and return) because the uart is locked by HAL_UART_Transmit(). So no more receive interrupts.

So I guess the lesson is don't mix polled and interrupt UART functions.

stm32_source

Visitor II

I know, I'm late!

Thanks a lot for your explanations. I'm using the STM32Cube_FW_G4_V1.5.1. In the functions HAL_UART_Transmit_IT and the HAL_UART_Receive_IT containt the macro __HAL_LOCK().

If I wand to process TX and RX with variable length. I would implement the RxCallback like this.

void HAL_UART_RxCpltCallback(UART_HandleTypeDef *huart) {
	// Store the recieve character to the next layer
 
 // Restart Receive
	HAL_StatusTypeDef status = HAL_UART_Receive_IT(&huart1, UART1_rxBuffer, 1);
	if (status != HAL_OK) {
		__NOP(); // Only for setting breakpoint
	}
}

And somewhere in the main loop (for example) I would like to trigger a TX-Message like this.

...
HAL_StatusTypeDef status = HAL_UART_Transmit_IT(&huart1, UART1_txBuffer, length);
if(status!=HAL_OK) {
 __NOP(); // Only for setting breakpoint
}
...

Due the asynchronous processing there is a race condition by the __HAL_LOCK().

What did I miss? Thank you for your help.

Thomas LBAuthor

Associate III

Thank you so much for explanation !

Even with debugger can't find the reason (no error so not easy !) !

My code is still not working as HAL_UART_Transmit_IT is not easy to use with variable length buffer :s.

Got this working keeping my pooling TX and byte per byte IT RX and reseting the entire LPUART1 every loop (fail in case of error but recover next loop)...

But I'd like to do something better.

For this use case : UART communication with module (variable length commande TX and variable length answer RX) what is the best solution ?

Thank you again for nice help :)

S.Ma

Principal

For USART LL might be easier to manage coding. Might even be easier to control.

I would change the reasoning of the code here

// pseudo code
RX_ISR() {
Data = USART.DR
if(index<0) return; // or breakpoint here with a NOP, loss of data
Buffer[index++] = Data
if(index>sizeof(Buffer)) return; // or breakpoint, buffer overflow
if(Data == 0x0A) index = -1;
}
 
main loop
index = 0;
while() {
if(index == -1) { process buffer; index = 0; // ready for next message }

Hope this make sense. It's just idea.

Thomas LBAuthor

Associate III

Hey,

I've seen a lot of suggestion of using LL instead of HAL but I never used this so for the moment I focus on HAL.

Thanks also for RX_ISR suggestion but I didn't process every message and got too many other function based on this Transfer_cplt counter variable.

My last issue was not enough delay between last HAL_UART_Transmit_IT() and going into stop mode so the last message was never sent.

Not sure this is the right way to do it but here is my TX function:

Note: buffer has to be not NULL !

void send_uart(char * buffer)
{
	char msg[2+tx_buffer_size+2+1] = "";	// 'CR' 'LF' "data" 'CR' 'LF' '\0'
	snprintf(msg, 2+strlen(buffer)+2+1, "%s%s%s", "\r\n", buffer, "\r\n");
	HAL_UART_Transmit_IT(&hlpuart1, (uint8_t *)msg, 2+strlen(buffer)+2); // No need to transmit the null terminator
}

Bob SBest answer

Super User

(1) your 2nd param to snprintf() is not correct and may overflow your msg[] array. Unless you can guarantee that buffer always contains a string that is "tx_buffer_size" bytes or less. Likewise, your size param in HAL_UART_Transmit_IT() is needlessly complicated. And why not include the CR/LF in your format string? Better would be:

void send_uart(char * buffer)
{
 char msg[2+tx_buffer_size+2+1] = "";	// 'CR' 'LF' "data" 'CR' 'LF' '\0'
 // The "-1" here guarantees that there is room for snprintf() to append
 // the trailing NULL
 snprintf(msg, sizeof(msg)-1, "\r\n%s\r\n", buffer);
 HAL_UART_Transmit_IT(&hlpuart1, (uint8_t *)msg, strlen(msg)); // No need to transmit the null terminator
}

(2) To know when the transmit has actually completed you need to implement the HAL_UART_TxCpltCallback() function (which is called from the UART interrupt handler) and set a flag telling your main code loop that the TX is done.

Thomas LBAuthor

Associate III

Once again very nice answer Bob (If you pass by Toulouse (FR) I owe you a beer !)

(1) OK

(2) OK, That was the plan but very useful for future readers

Have a good day :)

Thomas LBAuthor

Associate III

After last HAL drivers update it was not working anymore for long data transmission (first 32 characters were OK) to fix this I add "static" in msg declaration:

void send_uart(char * buffer)
{
	static char msg[2+tx_buffer_size+2+1] = "";	// 'CR' 'LF' "data" 'CR' 'LF' '\0'
	snprintf(msg, sizeof(msg)-1, "\r\n%s\r\n", buffer);
	HAL_UART_Transmit_IT(&hlpuart1, (uint8_t *)msg, strlen(msg)); // No need to transmit the null terminator
}

back on track !

Sign up

Login with SSO

Login to the community

Login with SSO

Scanning file for viruses.

This file cannot be downloaded