Unexpected float behaviour in C with AVR atmega8

regomodo picture regomodo · Feb 5, 2012 · Viewed 11.7k times · Source

I've been trying to figure out why I cannot get a sensible value from multiplying an unsigned int with a float value.

Doing something like 65535*0.1 works as expected but multiplying a float with a uint from memory creates mad values. I have a function that reads an ADC and returns an uin16_t. With this value I am printing it to a 4-digit led-display, which is working fine.
Multiplying the same value with 1.0 returns something different entirely (it's too large for my display so I don't really know what it is).

My code is below but the area of contention is at the bottom in main(). Any help would be great. Thanks

main.c:

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include <stdint.h>
#define BAUD 9600
#include <util/setbaud.h>
#define DISP_BRIGHT_CMD     'z'
#define DISP_RESET          'v'

#define ADC_AVG            3 

volatile uint8_t  hi,lo;
volatile uint16_t result; 

ISR(ADC_vect)
{ 
    lo = ADCL; 
    hi = ADCH; 
    MCUCR &= ~_BV(SE); //Clear enable sleep 
} 


void initSerial(void)
{
    // set baud rate
    UBRR0H = UBRRH_VALUE;
    UBRR0L = UBRRL_VALUE;
    // set frame format
    UCSR0C |= (0x3 << UCSZ00); // 8n1
    // set enable tx/rx
    UCSR0B = _BV(RXEN0) | _BV(TXEN0);
}

void initADC(void)
{
    // AVCC and ADC0
    ADMUX   = _BV(REFS0); 
    // Enable, div128, + 1st setup
    ADCSRA  |= _BV(ADEN)|_BV(ADSC)|_BV(ADPS2)|_BV(ADPS1)|_BV(ADPS0)|_BV(ADIE);
}

uint16_t readADC(void)
{
    uint16_t average=0;
    // Start Conversion
    ADCSRA |= _BV(ADSC);

    for (char i=0;i<ADC_AVG;i++) {
        MCUCR   |= _BV(SE);
        ADCSRA  |= _BV(ADSC);
        __asm volatile("sleep");
        MCUCR   &= ~_BV(SE);
        result  = (hi<<8);
        result  |= lo;
        average += result;
    }
    average /= ADC_AVG;
    return average;    
}

void sendByte(char val)
{
    while (! (UCSR0A & (1<<UDRE0)) ); //wait until tx is complete
    UDR0 = val;
}

/*
 * Convert voltage to temperature based on a negative coefficient for MAX6613
 */
uint16_t analogToTemp(uint16_t val)
{
  uint16_t temp;
  //v     = 5 * (val/1023.0);
  //temp  = (1.8455 - (5.0*(val/1023.0)))/0.01123;
  temp  = (1.8455 - (5.0*(val/1023.0)))*89;
  //temp = val * M_PI;
  //v     = 5 * ( val/1024);
  //temp  = (2 - v) * 89;

  return temp;
}

void initDisplay()
{
    sendByte(DISP_RESET);
    sendByte(DISP_BRIGHT_CMD);
    sendByte(0);
}

void serialSegments(uint16_t val) 
{  
  // 4 digit display
  sendByte(val / 1000);
  sendByte((val / 100) % 10);
  sendByte((val / 10) % 10);
  sendByte(val % 10);  
}

int main(void)
{
    uint16_t calc=0,sense=0;

    DDRB    |= _BV(DDB5);
    PORTB   |= _BV(PORTB5);
    initSerial();
    initADC();
    initDisplay();
    sei();
    MCUCR   |= (1 << SM0); // Setting sleep mode to "ADC Noise Reduction" 
    MCUCR   |= (1 << SE);  // Sleep enable 
    for(;;) {
        //PORTB   ^= _BV(PORTB5);
        if (calc>=9999){ // I can't see the real value. Max val on display is 9999
        //if (sense>=330){
            PORTB |= _BV(PORTB5);
        } else {
            PORTB &= ~_BV(PORTB5);
        }

        sense   = readADC();
        //calc    = sense*1.0;      // refuses to calculate properly
    calc    = analogToTemp(sense);  // a bunch of zeroes
        //calc = 65535*0.1;         // a-ok

        serialSegments(calc);
        _delay_ms(500);
        serialSegments(sense);
        _delay_ms(500);
    }
    return 0;
}

Makefile:

# AVR-GCC Makefile
PROJECT=Temp_Display
SOURCES=main.c
CC=avr-gcc
OBJCOPY=avr-objcopy
MMCU=atmega328p
OSC_HZ=16000000UL
OPTIMISATION=2
PORT=/dev/ttyUSB0

CFLAGS=-mmcu=${MMCU} -std=gnu99 -Wall -O${OPTIMISATION}  -DF_CPU=${OSC_HZ} -lm -lc 

${PROJECT}.hex: ${PROJECT}.out
    ${OBJCOPY} -j .text -O ihex ${PROJECT}.out ${PROJECT}.hex
    avr-size ${PROJECT}.out

$(PROJECT).out: $(SOURCES)
    ${CC} ${CFLAGS} -I./ -o ${PROJECT}.out ${SOURCES}

program: ${PROJECT}.hex
    stty -F ${PORT} hupcl
    avrdude -V -F -c arduino -p m168 -b 57600 -P ${PORT} -U flash:w:${PROJECT}.hex

clean:
    rm -f ${PROJECT}.out
    rm -f ${PROJECT}.hex

EDIT: OK, i've simplified the code somewhat

#include <avr/io.h>
#include <util/delay.h>
#include <stdint.h>
#define BAUD 9600
#include <util/setbaud.h>
#define DISP_BRIGHT_CMD     'z'
#define DISP_RESET          'v'


void initSerial(void)
{
    // set baud rate
    UBRR0H = UBRRH_VALUE;
    UBRR0L = UBRRL_VALUE;
    // set frame format
    UCSR0C |= (0x3 << UCSZ00); // 8n1
    // set enable tx/rx
    UCSR0B = _BV(TXEN0);
}


void sendByte(char val)
{
    while (! (UCSR0A & (1<<UDRE0)) ); //wait until tx is complete
    UDR0 = val;
}


void initDisplay()
{
    sendByte(DISP_RESET);
    sendByte(DISP_BRIGHT_CMD);
    sendByte(0);
}

void serialSegments(uint16_t val) {  
  // 4 digit display
  sendByte(val / 1000);
  sendByte((val / 100) % 10);
  sendByte((val / 10) % 10);
  sendByte(val % 10);  
}

int main(void)
{
    uint16_t i=0,val;

    DDRB    |= _BV(DDB5);
    initSerial();
    initDisplay();
    for(;;) {
        val = (uint16_t)(i++ * 1.5);
        serialSegments(i);
        _delay_ms(500);
        serialSegments(val);
        _delay_ms(500);
        if (val > 9999){
            PORTB |= _BV(PORTB5);
        } else {
            PORTB &= ~_BV(PORTB5);
        }
    }
    return 0;
}

Answer

ouah picture ouah · Feb 5, 2012

Unsuffixed floating point constant are of type double not float.

Use f suffix to have a float literal, e.g., 0.1f

This can make a huge overhead as MCUs like atmega8 don't have a floating point unit and all floating point operations have to be implemented in firmware by the implementation.

With small devices like atmega8 one usually try to avoid using float operations as without a FPU they are very expensive in CPU cycles.

Now there is no reason an implementation would no correctly translate an expression like:

calc = sense * 1.0;

when calc and sense are of uint16_t type.