prevent arithmetic overflow

Question

I want to calculate a voltage using ADC peripheral of PIC18F14K50. The result ranges between 0-1023 (10-bit). So I used this simple calculation:

uint16_t voltage = ADC_Result * 5000 / 1023;

However, the results are incorrect. I guess an arithmetic overflow happened. I tried many combination of parentheses, changing order of elements, etc.
The best result was 4088 when ADC_Result was 1023 using below code; which is really far from 5000.

uint16_t voltage = ADC_Result * (5000 / 1023);

What should I do to get better results in above calculation? Please don't suggest floating points as they cause disaster in MCUs! They use a lot of resources without any real benefit.

There's two ways to fix this. Use a wider integer, as has already been suggested, or, if you can sustain some loss of precision and resolution, scale the input values down by first dividing by 10 or 100. I'd give an example in an answer if I had more time, but you can probably do the math and work out the details on your own, or someone will spot this and write it up for us. — jwdonahue
– jwdonahue, Commented Jun 19, 2020 at 16:12

Ian Abbott · Accepted Answer · 2020-06-20 23:07:43Z

2

You could use a wider type for the intermediate multiplication, for example:

uint16_t voltage = (uint32_t)ADC_Result * 5000 / 1023;

EDIT

If division by 1023 is too slow, you can get an approximately equal conversion by changing 5000 / 1023 to 5005 / 1024 which can use a fast bit shift for the division:

uint16_t voltage = (uint32_t)ADC_Result * 5005 >> 10;

N.B. 1023 * 5005 / 1024 ≃ 5000.1123

edited Jun 20, 2020 at 23:07

answered Jun 19, 2020 at 16:05

Ian Abbott

18.1k1 gold badge22 silver badges39 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

AmirSina Mashayekh Over a year ago

When ADC_Result = 1023, result is 4995 by your code which is much better than 4088!

Ian Abbott Over a year ago

@AmirSinaMashayekh That's strange. The above code should convert an ADC_Result value of 1023 to a voltage value of exactly 5000. (Ah! I think you really meant ADC_Result = 1022.)

0___________ Over a year ago

remebber that 32 bit division is very slow on 8 bits microcontrollers. It you do not need it for humans just do not convert it at all.

AmirSina Mashayekh Over a year ago

@IanAbbott Sorry, you are right! That was my error!

Ian Abbott Over a year ago

@AmirSinaMashayekh If division by 1023 is too slow, you could divide by 1024 instead as that can be done with a simple bit shift. You can compensate by multiplying by 5005 instead of 5000. e.g. uint16_t voltage = (uint32_t)ADC_Result * 5005 >> 10;

|

ForceBru · Accepted Answer · 2020-06-19 16:06:01Z

0

You should use a wider integer type for this calculation, such as uint32_t.

In your case, 1023 * 5000 == 3192 (because the real result 5115000 doesn't fit), so that's not correct. 5000 / 1023 == 4, which is the expected result for integer division. Dividing ADC_Result by 1023 will result in the same behaviour.

You can calculate this into uint32_t and then check if it fits in uint16_t:

uint32_t result_tmp = ADC_Result * (5000 / 1023);
uint16_t result;

if (result > 0xffff) {
    // this won't fit
} else {
    result = (uint16_t) result_tmp;
}

answered Jun 19, 2020 at 16:06

ForceBru

45k10 gold badges71 silver badges104 bronze badges

Comments

chux · Accepted Answer · 2020-06-20 01:19:59Z

0

What should I do to get better results in above calculation?

OP's code overflow 1`6-bit math.

To get a correct and rounded result use wider math and offset.

// uint16_t voltage = ADC_Result * 5000 / 1023;
uint16_t voltage = (ADC_Result * 5000LU + 1024u/2) / 1024u;
// or
#include <stdint.h>
...
uint16_t voltage = (ADC_Result * UINT32_C(5000) + 1024u/2) / 1024u;

The L in 5000LU provides at least a 32-bit math.

Use U for potentially simpler/faster math and simpler rounding given ADC_Result is not negative.

+ 1024/2 effects a round to closest rather than a truncate.

Use 1024 instead of 1023 for correct scaling given the usual characteristics of A/D converters. Side benefit: faster division as 1024 is a power-of-2.

edited Jun 20, 2020 at 1:19

answered Jun 20, 2020 at 1:14

chux

158k17 gold badges160 silver badges311 bronze badges

Collectives™ on Stack Overflow

prevent arithmetic overflow

3 Answers 3

9 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

9 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related