problem with reading ADC value - avr

[m.m.m]

hi everybody, I recently started working with atmel avr and like most of people my first project was reading temperature from a lm35 temperature sensor. this is my code, written in C and compiled by Atmel Studio:

Code: [Select]

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#define F_CPU 8000000;
#define ADC_VREF_TYPE 0xff;

unsigned int read_adc(unsigned char adc_input)
{
ADMUX=adc_input|ADC_VREF_TYPE;
// Delay needed for the stabilization of the ADC input voltage
_delay_us(10);
// Start the AD conversion
ADCSRA|=0x40;
// Wait for the AD conversion to complete
while ((ADCSRA & 0x10)==0);
ADCSRA|=0x10;
return ADCW;
}

int main(void)
{
int temp;
ADMUX=ADC_VREF_TYPE;
ADCSRA=0x83;
char segment[10]={0x3F,0x06,0x5B,0x4F,0x66,0x6D,0x7D,0x07,0x7F,0x6F};
DDRB=0xFF;//output
DDRD=0xFF;//output
sei();
    while(1)
{
temp = read_adc(0)/4;
if (temp>=150)
{
PORTD=segment[5];
PORTB=0x00;
} else {
PORTD=segment[temp%10];//last digit
PORTB=segment[1];
_delay_ms(1500);
}
}
}at first, I used the code to write the read value and divide it by four (because of the internal 2.56v ref voltage) and show it on the seven segment display, and then it showed some random symbols on the seven segment display, and they changed very quickly. then I wrote the code like above so it tells my the read value divided by four is in which range. then it showed it's above 150! then I disconnected the adc0 pin of the ATMega 8 MCU which was connected to the output of the sensor, then nothing changed!!!  it means the code's completely wrong! I asked for the solution in another forum, and no one helped my.
can someone please help my with this?
thanks a lot.

[dannyf]

then nothing changed!!! 

Slow down. Swearing doesn't help you think logically.

it means the code's completely wrong!

It depends on what you meant by "the code".

I asked for the solution in another forum, and no one helped my.

Maybe because you didn't help others help you.

Others can point out issues for you but in the end, it is better if you learn to think logically so you can spot issues all by yourself in the future.

Take your code for example. You have to figure out if it is a display problem (the right temp isn't being display'd correctly), or it is a measurement problem (the right temp isn't obtained by the adc routine.

One way to ascertain that is to provide temp a known value ("temp = 180;" or "temp = 120;"), right after the adc function.

Depending on the code's behavior after that, you can track down the issues.

Two more advises for you:

1) get a decent C book and make sure you really understand it - your code now fails a C 101 class assignment;
2) learn a good coding habit. For example, comment your code as much as you can and write modular code blocks.

[Jeroen3]

When writing 0xFF to ADMUX you are:
- Setting a reserved bit. Never do that! rtfm .
- Setting left adjust.
- Setting input to gnd mode (0xFF | 0 = 0xFF)

Then, when programming ADCSRA you set Div to 8, make sure you end up with less than 200kHz.

Also clear old ADMUX channel bits before setting the new channel. Use y &= ~(x)

And make sure all (a)Vcc's and Ref's are powered, with stabilizer capacitors.

[adam1213]

Improvements to make:

• Its great that you use #defines. Though don't use semicolons after #defines as the semicolon will become part of the value of the define. e.g. "#define a 42;" "int result = a + 2;"  will translate to "int result = 42; + 2;
• "#define ADC_VREF_TYPE 0xff;"   "ADMUX=adc_input|ADC_VREF_TYPE;"  will be equivalent to "ADMUX=adc_input|0xff" As 0xff for a char means all bits are set, or'ing this value with a char will have no effect on the value. This is unlikely to be the behaviour that you want. This code needs to be changed. 

[m.m.m]

thank you both Jeroen and Adam, so is this gonna work? :

Code: [Select]

ADMUX &= ~(1<<REFS0)|(1<<REFS1);// AREF=internal 2.56vand display and other stuff work well, tested them before.

[adam1213]

thank you both Jeroen and Adam, so is this gonna work? :

Code: [Select]

ADMUX &= ~(1<<REFS0)|(1<<REFS1);// AREF=internal 2.56vand display and other stuff work well, tested them before.

The code won't work as expected due to an issue with the order of operations.
e.g. instead of
"~((1<<REFS0)|(1<<REFS1))" you will get "(~(1<<REFS0))|(1<<REFS1)"

A better way to implement this would be to modify the code as to split things across lines. e.g.

Code: [Select]

char bitsToClearToUseInternalReference = (1<<REFS0)|(1<<REFS1)
ADMUX &= ~bitsToClearToUseInternalReference
The is likely a better way of implementing this.
I wonder if you can easily set REFS0 and REFS1 using a union. e.g. for a different brand of microcontroller the compiler / its libraries would allow one to write something similar to "ADMUXbits.REFS0 = 0". I wonder if something similar is supported by the default environment for AVRs.

[dannyf]

The is likely a better way of implementing this.

I would dispute that,

I wonder if something similar is supported by the default environment for AVRs.

Not sure what your default environment is but typically it can be done via a pointer to a struct / union.

I prefer masks over bit fields.

[Jeroen3]

Using bitfields to manipulate hardware bits is not recommended since there is no standardisation in the implementation of bitfields.
An 8 element bitfield might use 8 bytes or even 8 words, or shuffle the bits.
It is better to play it safe and don't use them.

unions can be used, but keep an eye on the structure packing and alignment issues. How slim they might be in 8 bit architectures.
Remember you have 16 bit flash.

[m.m.m]

that all solved by adding a damn cap on the Aref pin!! 
didn't know that a small cap can make a huge difference on the result.
sorry guys.