Dividing with/without using floats in C

Question

Below is the main function I wrote in C (for PIC18F8722 microprocessor) attempting to drive 2 multiplexing 7 segments displays at a specific frequency set by the unsigned int function get_ADC_value(). The displays also display the current multiplexing frequency. This frequency range is set by #define to be in the range LAB_Fmin and LAB_Fmax and must scale as the get_ADC_value() increases or decreases from 0 to 255.

This code however does not work as I think there is implicit conversion from int to float at freq =.

The challenge is to fix this error with floats and to find an alternative using only integer types (int, char...).

 while (1) {

   unsigned int x, y, z;
   float freq, delay;

    x = get_ADC_value(); 
    y = x & 0b00001111;
    z = (x & 0b11110000) >> 4 ;

    freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin))/ 255)*x ;
    delay = 1/(freq*1000); // convert hZ to ms delay accurately

    LATF = int_to_SSD(y); 
    LATH = 0b11111110; //enable 7seg U1
    for (unsigned int i = 0; i<(delay) ; i++){ 
        Delay10TCYx(250); //1ms delay
    }

    LATF = int_to_SSD(z); 
    LATH = 0b11111101; //enable 7seg U2
    for (unsigned int j = 0; j<(delay) ; j++){
        Delay10TCYx(250); //1ms delay
    }
}

Answer 1

C is defined to divide ints using integer division, and only when there is a float does it "promote" other ints to floats first. Note that this even happens if it will be assigned to a float - if the right-hand side is all ints, then the division will all be integer, and only for the final assignment will C convert the int result to float.

So, with your line:

freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin))/ 255)*x ;

it all depends on what LAB_Fmax and LAB_Fmin are. It doesn't matter what freq or x are, because the "damage" will already have been done due to the parentheses forcing the division to be first.

If those LAB_F variables are ints, the easiest way to use floating point division is to simply tell C that you want that by making the constant 255 a floating point number rather than an integer, by using a decimal point: 255. (or 255.0 to be less subtle).

If you want to use integer arithmetic only, then the usual suggestion is to do all of your multiplications before any divisions. Of course, this runs the risk of overflowing the intermediate result - to help that, you can use the type long. Define your LAB_F or x variables as long, and do the division last:

freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin)) * x / 255);

Answer 2

Code review:

• unsigned int x, y, z; Avoid using raw integer types on embedded systems. Exact-width types from stdint.h should always be used, so you know exactly what size you use. If you don't have access to stdint.h then typedef those types yourself.

• float freq, delay; Floating point numbers should generally be avoided on most embedded systems. Particularly on 8 bit MCUs with no FPU! This will result in software-defined floating point numbers that are incredibly slow and memory-consuming. There seem to be no reason for you to use floats in this program, it would seem that you should be able to write this algorithm with uint16_t or smaller, unless you have extreme accuracy requirements.

• x = get_ADC_value(); Since you only seem interested in 8 bits of the ADC read, why not use an 8 bit type?

• Please note that binary number literals are not standard C.

• ((LAB_Fmax) - (LAB_Fmin))/ 255 This looks fishy. First of all, are these integers or floats? What's their size? The answer to your question depends on that. By swapping the literal to 255.0f you can force a conversion to float. But are you sure the division should be by 255? And not 256?

• i<(delay). You should always avoid using floating point expressions inside loop conditions, since it makes the loop needlessly slow and can potentially lead to floating point inaccuracy bugs. Also, the parenthesis fills no purpose.

Overall, your program suffers from "sloppy typing", meaning that the programmer has not given any thought about what types that are used in each expression. Note that literals have types too. Implicit conversions might cause a lot of these expressions to be calculated on too large types, which is very bad news for the PIC. I'd recommend reading up on "balancing", aka the usual arithmetic conversions.

This "sloppy typing" will cause your program to get very bloated and slow, for nothing gained. You must keep in mind that PIC is perhaps the least code-efficient MCU still manufactured. When writing C code for any 8-bit MCU, you should avoid types larger than 8 bit. In particular, you should avoid 32 bit integers and floating point numbers like the plague.

Your program re-scales all data to types that ease the thinking for the programmer. This is a common design mistake - instead your program should use types that are easy to use for the processor. For example, instead of milliseconds, you could use timer ticks as the unit.

Answer 3

You are correct about integer division. Change to

freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin)) / 255.0)*x;
                                                  ^^ 

Answer 4

 freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin))/ 255)*x ;

This is indeed an implicit conversion to integer, and you're doing integer division to do that.

That is because 255 is an Integer literal.

Change it to 255.0 to be a double literal, which should play nicely with your calculation.

If you want to be more precise, you can even use a float literal, like 255.0f or an explicit cast like (float)255.

Your code could look like this then:

freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin))/ 255.0)*x ;

Or this:

freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin))/ (float)255)*x ;

Answer 5

Math operations with integers will by default result into an integer too, so you need to either express one of the literals as double/float

freq = LAB_Fmin + (((LAB_Fmax) - (LAB_Fmin))/ 255.0)*x ;

or cast (float)

as many other state the first option is the most commonly implemented.