3

I have sensor data in a uint8_t array which are signed (when concatenated to a 16 Bit value). Now, I want to average the values over N cycles, such that I have to add them into a larger than int16_t value, i.e. a int32_t value.

int32_t val_x = 0 

//...

//do this n times when data is received, dataReceived is a uint8_t array
val_x += ((int16_t)dataReceived[3] << 8) | dataReceived[4];

But I have the feeling that this might cause errors. Assume I have a value of -1 (0xFFFF) and add it to a 32Bit signed value, it will interpret it as a positive value, right? Or does the compiler know, that when I cast to int16_t that this is a negative value and will represent it in the int32_t variable val_x as 0xFFFFFFFF? Do I have to do on more cast on the total value like

val_x += (int16_t)(((int16_t)dataReceived[3] << 8) | dataReceived[4]);

I program on a 16Bit (or possibly 24Bit with some always-zero-bits) (PIC24F from Microchip). I hope you get the problem I am having. I'm a little lost by such a seemingly trivial problem...

Improve this question
4
  • 2
    Doing bitwise operations on signed numbers is generally a bad idea. And converting an unsigned value outside the signed range to signed causes implementation-defined behavior. Commented Aug 24, 2021 at 22:51
  • 1
    If you're lost, it's because you have mixed up a lot of different, unrelated and irrelevant parts. Start by isolating the parts you don't understand. Don't mix in the val_x +=-part, remove the | dataReceived[4]-part, replace dataReceived[3] with a scalar variable of the type you actually care about. Can you understand your problem then? Commented Aug 24, 2021 at 22:53
  • Yes, but this is the way the sensor puts out data. I have to work with it as is... Commented Aug 24, 2021 at 22:53
  • @EOF I am not sure what you mean. I think about it and hope you could walk me through it... Commented Aug 24, 2021 at 22:59

2 Answers 2

Reset to default
2

In (int16_t)dataReceived[3] << 8, the uint8_t dataReceived[3] is converted to int16_t. This conversion operates by value; the bits representing it are irrelevant. Since all uint8_t values are representable in int16_t, the result is a value from 0 to 255, inclusive. In the case you ask about, the value is 255.

Then the integer promotions are performed on the left operand of <<. If int is 16 bits in this C implementation, then either int16_t is an alias for int or it is converted to int. (Exception: If the C implementation does not use two’s complement for int, the behavior is more complicated. However, this answer will not go into that.) Then the shift operation is performed. Shifting 255 left 8 bits mathematically yields 65,280. However, this is not representable in a 16-bit int, so the behavior is undefined, per C 2018 6.5.7 4.

If int is wider than 16 bits, then the integer promotions promote int16_t to int, the shift does not overflow, and the result is some value in 0 to 65,280, inclusive, that is a multiple of 256.

Then ORing in dataReceived[4] merges its bits into this value. The result is a value from 0 to 65,535, inclusive.

Then the cast to (int16_t) is applied. If the value is 0 to 32,767, it is the result. Otherwise, the value cannot be represented in int16_t. Then the result is implementation-defined or an implementation-defined signal is raised, per C 2018 6.3.13. GCC and Clang define the result to wrap modulo 216, so this will produce the result you desire. Other compilers might not produce the result you desire.

If int is 16 bits, you could avoid the overflow in shifting by casting dataReceived[3] to int32_t instead of to int16_t. Then the integer promotions will not change it, and the int32_t value will be shifted.

However, since we still need to deal with the overflow in the final conversion to int16_t, there is another method:

uint16_t tu = (uint16_t) dataReceived[3] << 8 | dataReceived[4];
int16_t  ti;
memcpy(&ti, &tu, sizeof ti);
val_x += ti;

This:

  • Converts dataReceived[3] to uint16_t so the shift will not overflow.
  • Completes the shift and merge and stores the result in a temporary uint16_t object.
  • Copies the bits of the uint16_t object into an int16_t object to reinterpret them as a 16-bit two’s complement integer.
  • Adds the integer to val_x.

The following code is effectively the same and avoids the named temporaries but may be less familiar and harder to read for novice C programmers:

val_x += (union { uint16_t u; int16_t i; }) { (uint16_t) dataReceived[3] | dataReceived[4] } .i;
Improve this answer
Sign up to request clarification or add additional context in comments.

11 Comments

Did you type the answer before I asked the question, like, just in case? :) Ok, give me a moment
Ok, thanks, I think I got it now (had to google integer promotion). So it depends on architecture and compiler, right? And, as I use int32_t as final type I want my data in, the two's complement is "padded as needed" in the new, larger type: 0xFFFF -> 0xFFFFFFF in case I add int16_t to the int32_t variable? If I added 0xFFFF typed as a uint16_t, I would get 0x0000FFFF in the int32_t variable?
@jjstcool: You do not need to think about “padding bits” when an int16_t is added to an int32_t. Arithmetic in expressions is done by values. The bits are representations in memory, and they are irrelevant. If val_x is 93 and the thing you are adding to it is −1, then the sum is 92, period. It does not matter what bits are used to represent val_x or what bits are used to represent −1. The types only matter if the arithmetic overflows the representable values in the result type.
@jjstcool: If the bits 0xFFFF are in a uint16_t, then its value is 65,535, and that is what would be used in arithmetic. You do not want that, which is why it must be converted to int16_t (in a compiler that defines this conversion to wrap) or its bits must be reinterpreted as an int16_t (which can be done with the memcpy or the union methods I showed). The value those bits represent in an int16_t type is −1. So adding that int16_t to the sum adds −1 to it.
You mention the memcpy thing, wouldn't a simple cast do the job too? Or wait, no, because the value 65535 would not fit into the int16 type. I think I never had a problem doing this but now I wonder why.
|
1

Do not be afraid to use inline functions to make code clearer:

#ifdef __GNUC__
#define ALWAYS_INLINE __attribute__((always_inline))
#else
#define ALWAYS_INLINE
#endif

static inline ALWAYS_INLINE int16_t combineInt16(uint8_t *d)
{
    int16_t i16;
    memcpy(&i16, d, sizeof(i16));

    return i16;
}

int16_t average(uint8_t *data, size_t size)
{
    int32_t average = 0;

    for(size_t index = 0; index < size / 2; index ++)
    {
        average += combineInt16(data);
        data += 2;
    }
    return average / size;
}

https://godbolt.org/z/GcEKoGeWM

Improve this answer

5 Comments

I will read this tomorrow, need sleep now. Boy, I am learning more about C on my 5 day hobby project, then in 3years of work...
Ok, I looked at it now and at the core you do the same thing as the answer from Eric. You do a bitwise concatenation of the input bytes and interpret it as int16_t afterwards. Correct so far? Now, you do the inline thing, but I don't get the context in which this is relevant. I guess this might be faster? Could you elaborate on that a little? Thanks.
(In my context this is not applicable however, as I don't have an array of [Highbyte1,Lowbyte1, Highbyte2,Lowbyte2,...] as my original post might misleadingly suggest. I get an array of sensor data and this adding-procedure is done on every new input, but only two bytes in the array are actually x-vals, next to y and z vals and status data. The averaging is done in a global variable added with new data everytime an interrupt occurs)
@jjstcool it makes almost no difference. BTW for that running average is better - as you want simple low pass filter.
ok, so the main point in your post is the memcpy as a means of re-interpretation of data? The inline is for speed-reasons only? I am just trying to figure out what your point was to make it all inline

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.