3
\$\begingroup\$

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.

\$\endgroup\$
6
  • \$\begingroup\$ What is your intention with the IDLE check? Is that your "If detects end of transmission before the buffer array is full" test? What does the STM32F746 ref manual say about what conditions set the IDLE flag? Same question for the RX timeout flag - what does the manual say about what condition sets that flag. What actually constitutes an 'RX timeout or IDLE' - not what do you expect, but what does the manual say? \$\endgroup\$
    – brhans
    Jan 18 at 15:50
  • \$\begingroup\$ It says that "if line is idle, there is no transmission, idle flag is set". "Receiver Timeout" flag is set if the line is not receiving anything for an amount of time set in the register Receiver timeout register (USART_RTOR) (must be 1 bit time without transmission with default 0x00 value) \$\endgroup\$
    – Ilya
    Jan 18 at 15:58
  • \$\begingroup\$ So following on from that - if there is even the tiniest delay between the individual bytes your PC is sending then one or both of those flags will get set and break your RX loop. Have you tried using the RX Timeout flag with USART_RTOR set to anything greater than 0x00? \$\endgroup\$
    – brhans
    Jan 18 at 17:02
  • 1
    \$\begingroup\$ @brhans no I haven't, and I haven't thought there can be a delay between multiple characters is I send them as a string all together. Good idea tho. Will keep in mind. Maybe I should scope my PC->MCU transmission out of curiosity and see if there are delays. Thx for the idea. \$\endgroup\$
    – Ilya
    Jan 19 at 10:10
  • 1
    \$\begingroup\$ Fixed a bug of array buffer overflow (receive transmission longer than buffer array), hanging the whole process. Optimized single byte receiver/transmitter. Seems to be working correctly with all sorts of lengths of messages. Updated code on github. \$\endgroup\$
    – Ilya
    Jan 21 at 12:18

1 Answer 1

3
\$\begingroup\$

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.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your insight. Given it's a relatively small piece of code, I will try to re-write it from scratch while taking your advice into account (specifically about flags not clearing automatically, which, I admit, I didn't think about, especially about IDLE and a single loop with checking ISR first). Will post if there is any progress. Given it's relatively small, I will write a blocking one first as a proof of concept that I understand how stuff works. Then I'll have interrupts, but they'll need some extra reference manual reading first. \$\endgroup\$
    – Ilya
    Jan 19 at 10:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.