int to byte to int doesn't return same value

Hi everyone,

I am stuck with a data conversion problem. I am trying to convert an uint16 to a byte array and then back into an uint16.
Unfortunately, I don't get the same number back. I found a couple of postings on stackoverflow but none of them helped.

Here is my code:

byte bint[2];
unsigned int numberbefore = 12345;
unsigned int numberafter;

void setup()
{
  Serial.begin(19200);
}

void loop()
{
  Serial.println();
  Serial.print("Before conversion: ");
  Serial.println(numberbefore);

  bint[0] = (byte) numberbefore;
  bint[1] = (byte) numberbefore >> 8;
  
  numberafter = (bint[0] << 8) | bint[1];
  //numberafter = (int)bint;
  
  Serial.print("After conversion: ");
  Serial.println(numberafter);
  
  delay(5000);
}

For me the code returns the following:
Before conversion: 12345
After conversion: 14592

I am using Arduino 1.04 and an Arduino Uno.
If anyone has working code that does it (and I am sure a lot of people solved it) then I would really appreciate it. Thanks!

Looking at the hex or binary representations of those decimals numbers should clue you in as to where you are going wrong:

12345 = 0x3039
14592 = 0x3900

Looks like the lower order byte is being put into the wrong place.

Look up lowByte(), highByte() and word() in the Arduino Reference.

http://arduino.cc/en/Reference/LowByte

I would probably suggest looking into unions:
http://www.cplusplus.com/doc/tutorial/other_data_types/
to be able to access the same memory location two different ways. As an int and as a byte array. (Remember, because we are working little-endian, byteArray[0] == lowByte(integer).)

Decide what makes the most sense to you and fits your logic: word(), lowByte(), and highByte() functions; or unions.

Two bugs:

1. you are casting to byte before shifting the high byte thus always resulting 0
2. you are merging the bytes in wrong order
   So to fix it do:

  bint[0] = (byte) numberbefore;
  bint[1] = (byte) (numberbefore >> 8);
  numberafter = (bint[1] << 8) | bint[0];

  bint[1] = (byte) numberbefore >> 8;

was probably meant to be:

  bint[1] = (byte) (numberbefore >> 8) ;

casting binds tightly...

I normally use structs and unions

typedef union {
	unsigned int value;
	struct {
		byte lowByte;
		byte highByte;
	} lh;
} conv;

conv conversion;
conv newNumber;

void setup()
{
	Serial.begin(9600);
	conversion.value = 12345;
	//
	// print as decimal
	//
	Serial.print("decimal value = ");
	Serial.println(conversion.value, DEC);
	//
	// as hex value
	//
	Serial.print("hexadecimal value = ");
	Serial.println(conversion.value, HEX);
	//
	// as low / high order bytes
	//
	Serial.print("hexadecimal value L H = ");
	Serial.print(conversion.lh.lowByte, HEX);
	Serial.print(" ");
	Serial.println(conversion.lh.highByte, HEX);
	//
	// now move the bytes
	//
	newNumber.lh.lowByte = conversion.lh.lowByte;
	newNumber.lh.highByte = conversion.lh.highByte;
	//
	// and display the decimal
	//
	Serial.print("decimal value = ");
	Serial.println(conversion.value, DEC);

}

void loop()
{

}

I normally use structs and unions

...which need a little more care and attention than shifts if your code is to be portable.

AWOL:

I normally use structs and unions

...which need a little more care and attention than shifts if your code is to be portable.

I know but that's only for the 32 bit ints....

I know but that's only for the 32 bit ints....

Nope.
It's an endian issue, so affects anything bigger than a byte.

And strictly speaking casting between types using unions isn't well-defined by the C++ standard

  bint[0] = (byte) numberbefore;
  bint[1] = (byte) numberbefore >> 8;

is not the inverse of

  numberafter = (bint[0] << 8) | bint[1];

Your number is being reconstructed incorrectly.

Not only that, take this example:

unsigned int numberbefore = 12345;

void setup()
{
  Serial.begin(115200);
  Serial.println();
  byte foo = (byte) numberbefore >> 8;
  Serial.println(foo);
}

void loop() { }

Output:

0

The "byte" cast has higher precedence than the shift right, so it is truncating 12345 (0x3039) to 57 (0x39) before the shift. Then the shift shifts all the non-zero bits away, leaving nothing (unless you count zero as something).

Do people read the previous replies before commenting? I had the complete answer posted like 10 posts ago

Absolutely. And I also suggested lowByte and highByte which saves you having to get the shifts correct.

Thanks everyone for the fast and valuable help. Indeed, JarkkoL's suggestions fixed the problem.
Just in case someone else is looking for a solution to convert uint to byte and back, here my working sample code (including using lowByte and highByte like Nick Gammon recommended). I hope it helps others.

byte bint[2];
unsigned int numberbefore = 12345;
unsigned int numberafter;

void byteToUint16(byte *bytearray, unsigned int *intvalue)
{
  *intvalue = (bytearray[1] << 8) | bytearray[0];
}

void uint16ToByte(unsigned int intvalue, byte *bytearray)
{
  bytearray[0] = lowByte(intvalue);
  bytearray[1] = highByte(intvalue);  
}

void setup()
{
  Serial.begin(19200);
}

void loop()
{
  Serial.println();
  Serial.print("Before conversion: ");
  Serial.println(numberbefore);

  byte *pointerToByteArray = &bint[0];
  uint16ToByte(numberbefore, pointerToByteArray);
  
  unsigned int *pointerToInteger = &numberafter;
  byteToUint16(pointerToByteArray, pointerToInteger);
  
  Serial.print("After conversion: ");
  Serial.println(numberafter);
  
  delay(5000);
}

If you're going to do the typecasting and pointer thing, why don't you just do it directly:

void loop()
{
  Serial.println();
  Serial.print("Before conversion: ");
  Serial.println(numberbefore);

  *(unsigned int *)bint = numberbefore;
  numberafter = *(unsigned int *)bint;

  Serial.print("After conversion: ");
  Serial.println(numberafter);
  
  delay(5000);
}