What’s the easiest way in C to assemble two bytes received from a UART to make a (signed) int? (an ARM int is 32 bits).
In assembler, it’s a complete doddle:
with the two bytes from the UART in R1 (LSB) and R2 (MSB)
ORRS R0, R1, R2, LSL #8
SXTH R0,R0

I can only think of long-winded ways of doing it in C.
Or is it best just to write the above as a function and call it from C?

 

What’s the easiest way in C to assemble two bytes received from a UART to make a (signed) int? (an ARM int is 32 bits).
In assembler, it’s a complete doddle:
with the two bytes from the UART in R1 (LSB) and R2 (MSB)
ORRS R0, R1, R2, LSL #8
SXTH R0,R0

I can only think of long-winded ways of doing it in C.
Or is it best just to write the above as a function and call it from C?

Given the simplicity of the assembler function, calling it from C seems the best way (save the lack of portability). But if this is pretty much a single-architecture project, I don’t see how you could improve on the assembler by relying on the compiler…

 

I’m surprised that C doesn’t have facilities to deal with this sort of situation. In assembler the signed/unsigned nature of a number is a bit like Schrödinger’s cat, whereas C wants it to be well defined from the outset. Each has its own problems. . . .

 

union {
    int x;
    char c[4];
} BtoI;

for (i=0; i < 4; i++) BtoI.c[i] = read_byte();

// BtoI.x is now the int.
// May need to adjust for endianness

 

union {
    int x;
    char c[4];
} BtoI;

for (i=0; i < 4; i++) BtoI.c[i] = read_byte();

// BtoI.x is now the int.
// May need to adjust for endianness

That’s OK, but I’m reading 16 bits into a 32-bit int. Your “int” would be my “int16_t” so I could do (int32_t)Btol to make it sign-extend from 16 to32 bit.

 

Try something like

x = (((int)(R2<<24))>>16) | R1

However, this assumed the compiler will do an arithmetic right shift on a signed int, which is implementation-specified.

Another possibility would be do casts and hope the compiler does them efficiently (which it probably does)

x = (((int) ((signed char) R2)) << 8) | R1

 

What's wrong with

v = lo + (hi << 8);

 

What's wrong with

v = lo + (hi << 8);

That won't deal with negative values properly.

 

Thanks, everyone, I have to admit that I was surprised by your answers. I was expecting C to have evolved a facility to do exactly what I wanted, and that I simply didn’t know what it was.
I’ll go with the assembler function, even though it is processor-specific. Any other processor will have different UART registers as well.

 

I have done this in Java:

int lo, hi, v;

if (lo < 0) lo += 256;
if (hi < 0) hi += 256;
v = lo + (hi << 8);

 

My C solution that led me to think “There must be a better way” was:

variable = data0 | (data1<<8);
if (data1>128) variable-=65536;

 

My C solution that led me to think “There must be a better way” was:

variable = data0 | (data1<<8);
if (data1>128) variable-=65536;

It would be interesting to see that that complies to.

 

It would be interesting to see that that complies to.

Any good compiler should crunch that C into decent asm if you give it proper limits on variable sizes and types.

IMO This is one of those things that you just let the compiler find the optimized assembly code. While you, the programmer write the most understandable source code. It's munging data from a UART, unlikely to be a very time and resource critical part of the program.

 

It would be interesting to see that that complies to.

It’s very messy code, but it loads the two bytes into R0 and R3, then does
ORR R3, R3, R0, LSL#8
then does something else unrelated
then
LSLS R0,R0,#24 // this instruction sets the flags
LDR R0,[PC,#504] // this line . . .
STRB R12,[R4,#7] // . . .and this one are part of a different calculation
IT MI // but they don’t set the flags
SUBMI R3,R3,#65536 // subtract if negative flag set by LSLS instruction

 

IMO This is one of those things that you just let the compiler find the optimized assembly code. While you, the programmer write the most understandable source code. It's munging data from a UART, unlikely to be a very time and resource critical part of the program.

Yes, that’s why I was wondering. @Ian0’s implementation in assembler is quite compact—comparing instruction count/clock count would be instersting.

 

It’s very messy code, but it loads the two bytes into R0 and R3, then does
ORR R3, R3, R0, LSL#8
then does something else unrelated
then
LSLS R0,R0,#24 // this instruction sets the flags
LDR R0,[PC,#504] // this line . . .
STRB R12,[R4,#7] // . . .and this one are part of a different calculation
IT MI // but they don’t set the flags
SUBMI R3,R3,#65536 // subtract if negative flag set by LSLS instruction

Seems it wants quite a few more cycles than yours!

 

Readability should be paramount. If I call my assembmler function "Sixteenbit" , it's a question of whether:
BatteryCurrent = Sixteenbit(uart_lo_byte,uart_hi_byte);
is more or less readable than
BatteryCurrent = uart_lo_byte |(uart_hi_byte<<8);
if (BatteryCurrent>32768) BatteryCurrent-=65536;
or even
BatteryCurrent = uart_lo_byte |(uart_hi_byte<<8);
if (uart_hi_byte>128) BatteryCurrent-=65536;

My personal opinion is that the first is more readable, because the second and third beg the question "why does the battery current need to be reduced by 65536?".

 

Readability should be paramount. If I call my assembmler function "Sixteenbit" , it's a question of whether:
BatteryCurrent = Sixteenbit(uart_lo_byte,uart_hi_byte);
is more or less readable than
BatteryCurrent = uart_lo_byte |(uart_hi_byte<<8);
if (BatteryCurrent>32768) BatteryCurrent-=65536;
or even
BatteryCurrent = uart_lo_byte |(uart_hi_byte<<8);
if (uart_hi_byte>128) BatteryCurrent-=65536;

My personal opinion is that the first is more readable, because the second and third beg the question "why does the battery current need to be reduced by 65536?".

What? I would assume you would also have the equivalent C function called Sixteenbit. Readability is what's inside the implementation of Sixteenbit.

 

Readability should be paramount. If I call my assembmler function "Sixteenbit" , it's a question of whether:
BatteryCurrent = Sixteenbit(uart_lo_byte,uart_hi_byte);
is more or less readable than
BatteryCurrent = uart_lo_byte |(uart_hi_byte<<8);
if (BatteryCurrent>32768) BatteryCurrent-=65536;
or even
BatteryCurrent = uart_lo_byte |(uart_hi_byte<<8);
if (uart_hi_byte>128) BatteryCurrent-=65536;

My personal opinion is that the first is more readable, because the second and third beg the question "why does the battery current need to be reduced by 65536?".

If readability is paramount, then why are you concerned with trying to match a particular hardware-specific assembly-level implementation?

That implies that performance is paramount.

These are generally two competing goals, so you have to decide which is truly important and, at times, just how much you are willing to exchange one for the other.

If readability is truly paramount, then perhaps the following:

int32_t Sixteenbit(uint8_t uart_hi_byte, uint8_t uart_lo_byte)
{
   int16_t value = 256 * uart_hi_byte + uart_lo_byte;

   return (int32_t) value;
}

The final type cast isn't necessary, but aids readability.

Most compilers will optimize this pretty well, though compilers for MCUs often don't do as good a job.

Since you are getting the data over a UART, the speed is likely to be dictated by the time it takes to transfer the 16 bits across the interface.

 

Is it entirely predictable how this line will behave when uart_hi_byte > 128?

   int16_t value = 256 * uart_hi_byte + uart_lo_byte;

I've not tried it yet, but I think it will generate some compiler warnings.
One thing I'm not clear about is how the compiler instructs the processor to do the intermediate calculations before outputting the int16_t.
As it has 32-bit registers, I would expect it to do the calculation in 32-bit then lose the top 16 bits to give the answer. (Am I right? Is it compiler-dependent?)
If the number is positive (in 32-bit) then does it really output a negative 16-bit integer? Because it really is a numeric overflow.

If it is acceptable to the complier then:

int16_t i;
i=256 * uart_hi_byte + uart_lo_byte;
value =(int32_t)i;

might be clearer still.
Is

256 * uart_hi_byte + uart_lo_byte;

better than

(uart_hi_byte<<8)| uart_lo_byte;