STM32 UART Blocking Receiver Not Putting Data In Array, Overrun Flag

Question

STM32F746 Disco, UART1 connected to ST-Link, Win 10, STM32CubeIDE, C, no HAL no other libraries other than CMSIS register definitions. Putty on Windows side to send and receive UART messages to/from MCU.

I'm writing a basic UART driver. Nothing fancy, no noise checks or frame error checks or anything. Just send/receive. 9600 baud rate. System clock at defaults, didn't touch. 25MHz crystal, 16MHz UART clock (established experimentally from baud rate on the scope and example values in reference manual, adjusted BRR register accordingly, 9600bdrate from 16MHz matches the reference manual formula)

What works: I can send a single character, send an array of characters and even receive a single character totally OK. Receiving array doesn't work as intended. Thus, I believe hardware setup is OK (default values satisfy my needs).

Properties of my driver:

1. Receives pointer to array and array length as parameters, enables receiver and waits indefinitely until the beginning of the transmission.
2. After transmission begins, places (supposed to) data into array. If detects end of transmission before the buffer array is full, stop the receiver, return.
3. If buffer is full, but the transmission is going (transmission longer than buffer), wait out until transmission ends, then stop receiver and return.

Code:

void uart1_receiveArray(uint8_t *arraypointer, uint32_t length) {

USART1->CR1 |= USART_CR1_RE; //USART Receiver enabled, line idle
uint32_t pointer = 0;
while ((((USART1->ISR) >> USART_ISR_RXNE_Pos) & 1U) == 0);   //wait while first data comes from shift register
arraypointer[pointer] = USART1->RDR;
pointer++;
while ((((USART1->ISR >> USART_ISR_IDLE_Pos) & 1U) == 0) && (pointer < length)) { //if line not idle and buffer array not full
    while ((((USART1->ISR) >> USART_ISR_RXNE_Pos) & 1U) == 0);   //wait while data comes from shift register
    arraypointer[pointer] = USART1->RDR;
    pointer++;
}
while (((USART1->ISR >> USART_ISR_IDLE_Pos) & 1U) == 0); //if buffer is full, but transmission is still going, wait for it to end
USART1->CR1 &= ~USART_CR1_RE; //Receiver disabled
}

On the last line of the method, where I disable the receiver, I placed a breakpoint. My "Pointer" value is just at "1" as if I never entered the "if line is not idle" loop (test strings I send from PC are of length 3-6). Also, in ISR register I have "Overrun error" flag set, as well as Idle flag (ok that makes sense) and Read Data Register not empty set to 1.

Also, I tried to replace IDLE flag with Receiver timeout flag. Identical behavior.

From algorithmic point of view I can't find what I'm doing wrong, every line makes total sense to me. What am I missing? Something about timings?

Also, I've read HAL blocking receiver, there is no notion of transmission ending short, it expects strictly set amount of data, something I want to avoid to receive any amount of data in arbitrary buffer.

EDIT: as for ugly comparisons, I already got pointed I can make those a lot shorter and more readable.

EDIT2: I got it working!

I took advice from @Ocanath and, well, given my capabilities, produced the following thing, forgive the crudeness of the solution.

I based my blocker around Receiver Timeout. When I call receive method, the code waits indefinitely for the transmission start. Once it starts, the timeout event happens after one full bit without next start bit, which allows to process the last received byte (transmission ended, last byte received, we still have to process it). Established timeout experimentally, shorter timeout (0 bits) messes the entire string up. This timeout is always enough for data to end up in RDR and be processed.

Step 1: Enable receiver timeout. This is done in CR2, bit 23 RTOEN.

USART1->CR2 |= USART_CR2_RTOEN;

Step 2: Set timeout value in USART_RTOR register. From reference manual:

In standard mode, the RTOF flag is set if, after the last received character, no new start bit is detected for more than the RTO value.

USART1->RTOR = 0x01;

Step 3: receiver method itself (edited)

void uart1_receiveArray(uint8_t *arraypointer, uint32_t length) {
   uint8_t *currentpointer = arraypointer;
   for (uint32_t k = 0; k < length; k++) { //fill the buffer with nulls so that it doesn't retain data from previous transmissions
       *(arraypointer + k) = '\0';
   }
   while (!(USART1->ISR & USART_ISR_RXNE)); //wait indefinitely for the beginning of the transmission
   USART1->ICR |= USART_ICR_RTOCF; //make sure receiver timeout flag is cleared
   while (!(USART1->ISR & USART_ISR_RTOF)) { //while not receiver timeout
       if (((USART1->ISR & USART_ISR_RXNE)) && (currentpointer < arraypointer + length)) { //if buffer is not full yet and there is new data
           *currentpointer = USART1->RDR;
           currentpointer++;
       }
   }
   USART1->ICR |= USART_ICR_RTOCF; //when receiver timed out and we're done, clear the flag
}

Testing: buffer length 8, tried to send 1 character, 3-7 characters, 8 characters, everything was correct. If sent 10 characters, only the first 8 are retained, which was my intention from the beginning.

Answer 1

Your code hangs after reception of the first byte because the IDLE flag doesn't clear itself. You have to clear it yourself with the ICR. Same goes for all flags except RXNE, which is cleared after a read to RDR.

Some general tips for writing multi-byte UART handlers. I recommend skipping a blocking multi-byte UART receiver altogether in favor of an interrupt driven approach. The reasoning here is that UART is asynchronous, and so any event could happen at any time. Assuming things will happen in a set sequence is a recipe for getting a locked handler. If you must do a blocking multi-byte UART receiver, I highly recommend wrapping the entire thing in one while loop, and then treating the contents of that while loop like an interrupt handler. I also recommend doing only one read of the ISR per loop (should be the very first thing you do), so that you can be assured that there isn't a change in the ISR flags mid-handler. Finally, it's safe to always read RDR (after storing a copy of the ISR), and to clear all flags at the end of your handler. Writing a blocking handler this way will directly translate to a good interrupt handler.

You can do transmission and reception handling in the same handler 'simultaneously'. A single register read to RDR won't measurably preempt the transmission of a new byte in your TX sequence (and vice versa); register writes and reads happen much faster than clocking a new UART byte, even at high baud rates.

Your reasoning for using IDLE to frame sequences of UART bytes is sound. This is a good way to prevent framing errors for potentially unreliable incoming UART data.