Checking and reading bits using 'while' loops

Question

I am struggling to understand how reading registers using while loops work in embedded C.

MCU: ATmega328P

For example AVR-C USART Protocol:

 while (!(UCSRnA & (1<<UDREn)))
 ;
 /* Put data into buffer, sends the data */
 UDRn = data;

Equally:

 while ((UCSR0A & (1<<UDRE0)) == 0){}
 UDR0 = data;

In this case as when UDRE0 bit is equal to 1 the data should be loaded into the buffer and sent. The logic seems to say the opposite, it seems to say that when UDR0 is full load the data which contradicts the datasheets explanation.

Answer 1

Both versions of the code do the same. There is an empty while loop waiting for the USART Data Register Empty bit of the USART Control and Status Register to become 1.

The while loop

For the compiler, it doesn't matter how you write an empty loop body.

while (/* any condition */)
 ;

and

while (/* any condition */) {}

are equivalent.

You can even write

while
 (
 /* any condition */
 )
{
}

or

while (/* any condition */);

or

while (/* any condition */) {
}

or

while(/*any condition*/);

or

while(/*any condition*/){}

C is a language that accepts line breaks as whitespace, just as blanks and tabs. Language elements as { or } don't even need whitespace around them.

Since an empty loop body "smells" for some people, you have several options.

while (/* any condition */) {
 // empty, nothing to do here
}

or (but I never saw this in professional production code)

while (/* any condition */) {
 continue;
}

The condition

Both conditions will be compiled to the same machine code, most probably. It depends on the specific compiler, but nowadays compilers are really smart.

(UCSR0A & (1 << UDRE0)) == 0 is in my eyes the better expression, because it shows the intention:

1. Use the mask 1 << UDRE0 to obtain just the bit UDRE0.
2. Obtain this bit from the register UCSR0A.
3. Compare the result with 0. If the bit is 1, the result will be non-zero.

!(UCSR0A & (1 << UDRE0)) works because C interprets any non-zero value as true. The operator ! inverts the boolean value. The complete expression is true, if bit UDRE0 is 0.

Answer 2

Coding style goes a long way to making it readable. I hate "clever" programmers who do that kind of thing and put complex expressions into one line of code.

The code should be expanded to multiple, clear, lines where each expression is obvious.

Maybe they think they are being "efficient", but the days are long gone when people writing firmware had to save every byte because of speed or memory constraints.

Additionally, compilers do a better job of optimization than the vast majority of programmers. Having the latter do it is a waste of time.

• Dirk Bruere: @Justme Something like this is more readable, and it can be made even simpler: bool flag = false while (flag == false) { flag = UCSRnA & (1<<UDREn)); }
• Justme: That line is a direct copy from e.g ATMega64 data sheet, from the example how to send data with UART in C language. I do not see anything clever or out of the ordinary about it. It equals two lines of assembler code, one which checks the bit and other to jump back to check the bit if it wasn't set. Please explain what is wrong with the code in Atmel's official data sheet?
• Dirk Bruere: @Justme What is wrong with it is that it is not "at a glance" understandable. Spreading it across multiple lines makes it so
• the busybee: The original way is common in the whole embedded control world that I enjoy since decades. Needing to read 3 lines and to find the condition in another line than the while makes it harder to grasp.
• Dirk Bruere: @thebusybee Opinions vary. I have been doing embedded for 40 years

Answer 3

while (( UCSR0A & (1<<UDRE0)) == 0) {} // NOTE THE CLOSED BRACKETS BREAK THE LOOP
UDR0 = data;

Equally:

while (!(UCSRnA & (1<<UDREn))) 
 ; // <---- NOTE THE SEMI-COLON BREAKS THE LOOP
UDR0 = data;` 
/* Put data into buffer, sends the data */
UDRn = data;

Clearer version of the code: The while loops runs with no code block in the body until the !(UCSRnA & (1<<UDREn) condition is violated which stops the looping and runs the next line UDRn = data.

while (!(UCSRnA & (1<<UDREn))) {
} 
UDRn = data;

• Ron Beyer: You don't need a semicolon after the brackets... while (condition) {} is the same as while condition();, but while(condition){}; the semicolon is redundant since the brackets denote the end of the code block. In your first example, the ; does not break the code loop, the } of the bracket set does.
• The Photon: Your examples would be more clear if you fixed the indentation.
• jsotola: UDRn = data; is not inside the while loop, so it should not be indented ... it should be at the same indent level as while (...... ... this applies to the second and third listing in your answer .... you had it indented correctly in your question, but you messed it up in your answer ... let me guess, you copied the code in your question from somewere
• user13174343: Correct, when I've seen this code it usually appears indented on the datasheet and other places so I thought I would keep it the same, as part of the problem is that the indentation makes it look part of the while loop
• user57037: A while loop like that might not be a bad place to use "continue."