cancel
Showing results for 
Search instead for 
Did you mean: 

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

Thomas LB
Associate III

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 ?

1 ACCEPTED SOLUTION

Accepted Solutions

(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.

View solution in original post

10 REPLIES 10
Bob S
Principal

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 LB
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
Principal

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.

Thomas LB
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 LB
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
}

(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 LB
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 LB
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 !