0

I want to combine two bytes into one unsigned long variable, my current code does not work. I am using MPLAB C18 compiler, this is my code.

    unsigned long red = 0;
    BYTE t[2];


    t[0]  = 0x12;
    t[1]  = 0x33;

    red = 100 * t[0] + t[1];
    printf("%lu", red);

Please let me know why I am not getting 1233 as my output.

2
  • You don't seem to be understanding the difference between decimal and hexadecimal... Commented Jun 11, 2013 at 19:38
  • 2^8 is 256 or 0x100 and not 100! Commented Jun 11, 2013 at 19:49

3 Answers 3

5

You are multiplying t[0] by 100 when you should be multiplying it by 256. A better way would be to shift t[0] by 8 bits to the left and add t[1].

red = ( t[0] << 8 ) | t[1];
Sign up to request clarification or add additional context in comments.

4 Comments

Neither of these ways worked. I am getting 51 for red using the better way?
@cookiemonster Hexadecimal is still not decimal.
You need to modify your printf to print hexadecimal values. You should say printf ( "%x", red );
you need to typecast t[0] to something larger before doing a shift like that, the standard does not insure that works (implementation defined), some compilers happen to allow that to work (gcc). Note this solution is much much safer than the pointer solution as the pointer solution can fail with optimization.
1

You'll note that the values in your array are specified with 0x prefixes. 100 is not equal to 0x100.

Your compiler is non-compliant. Additionally, binary operators have poorly defined aspects for signed integer types, of which BYTE might be. Here's how I'd write that code, to be on the safe side:

unsigned long red = 0;
BYTE t[2];

t[0]  = 0x12;
t[1]  = 0x33;

red = (unsigned char) t[0];
red = red * 0x100;
red = red + (unsigned char) t[1];

printf("Decimal: %lu\n"
       "Hexadecimal: 0x%lx\n", red, red);

12 Comments

... if (red == 0x1233UL) { ... } WTF? Why haven't you learnt that 1233 IS NOT 0x1233, yet?
@unxnut According to the C standard, the format specifier %x corresponds to an unsigned int typed argument. Which type does red have? Comment before you edit, please.
@JoshuaTaylor By approving that edit, you allowed undefined behaviour to be introduced into my answer! WTF?
@Jean-BernardPellerin ^----- Please ensure you approve less undefined behaviour!
@greeness That was sort of like saying "hey, hey... Die in a fire, would you?". Look at my screen-name! Please don't approve edits for code written in programming languages you don't know; You might cause some actual damage.
|
0

First of all note that MPLAB C18 has some divergences to the C standard. Read it's user manual, especially section 2.7 labeled "ISO divergences".

So pay attention to that C18 does char promotion instead of integer promotion which also applies to literals. So if you add 0x80 and 0x80 together you get 0x00. The way to avoid this is the same as how it can be worked around normally if you need larger constants Use 'L' or 'UL' postfixes to force calculating in 32 bits, maybe also casts where needed.

The compiler's optimizer may be somewhat imperfect, you will need to check the disassembly. If you work with 32bit literals, it may emit 32bit code even if, say, only 8bit would be needed. You may work this around by casting the result of a calculation such as:

#define C8BIT ((unsigned char)(((0x12UL * 0x100UL) + 0xC0UL)>>6))

(Just an example arbitrary calculation which would overflow otherwise)

In the example you posted the problem is similar, you essentially work with unsigned chars, the compiler promotes to unsigned char, and you except it to be calculated with at least 16bit depth. A proper and clean solution for the problematic line might be:

red = (0x100UL * (unsigned long)(t[0])) + ((unsigned long)(t[1]));

If performance matters however (assuming t[0] and t[1] there just got some example values, so may get arbitrary contents in the real program) you may want to check disassembly and finally resort to this:

red = (0x100U * t[0]) + t[1];

or

red = (((unsigned short)(t[0]))<<8) + t[1];

A sane compiler (even including C18) should give proper result on these. I also showed a version with shifting as C18 is not too strong on optimization and might include a library call to a multiply routine there. I omitted casting t[0] and t[1] as the compiler should properly promote them (the 0x100U demands at least 16bit promotion), and the cast might confuse the compiler to add extra 16bit logic (C18 is very weak in this term!) with literal '0' operands. If performance matters, check disassembly, otherwise just go with the clean solution!

You may also want to check that BYTE type. It should be unsigned char, but not necessarily. You might want to define types with fixed sizes (such as typedef unsigned char uint8; and so) for a consistent use.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.