ADC Resolution Problem & can't read ADCH

Go To Last Post
39 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi!,

I am learning ADC with a butterfly trying to get a 10 bit value, but the max i can get is a reading of 187.

I have 3 volts supplied from one of the output pins wired to a 10k pot. With no load with a 10 bit value shouldn't my readings be close to 1024?

My max values of 180-190 are always stored in ADCL. I can't figure out how to read ADCH, i'm ashamed to admit that i've been trying all day. Someone?
:?

here is my modified stolen code:

void ADC_init(char input)
{
    ADMUX = input;    // external AREF and ADCx

    ADCSRA = (1<<ADEN) | (1<<ADPS1) | (1<<ADPS0);    

    //  Take a dummy reading , which basically allows the ADC 
    // to hack up any hairballs before we take any real readings
    input = ADC_read();       
}

int ADC_read(void)
{
    char i;
    int ADC_temp;
	int ADCr = 0;
    
    sbi(PORTF, PF3); // mt sbi(PORTF, PORTF3);     // Enable the VCP (VC-peripheral)
    sbi(DDRF, DDF3); // sbi(DDRF, PORTF3);        

    sbi(ADCSRA, ADEN);     // Enable the ADC

    //do a dummy readout first
    ADCSRA |= (1<<ADSC);        // do single conversion
    while(!(ADCSRA & 0x10));    // wait for conversion done, ADIF flag active
        
    for(i=0;i<40;i++)            // do the ADC conversion 8 times for better accuracy 
    {
        ADCSRA |= (1<<ADSC);        // do single conversion
        while(!(ADCSRA & 0x10));    // wait for conversion done, ADIF flag active
        
        ADC_temp = ADCL;            // read out ADCL register		
		
    }
        
    cbi(PORTF,PF3); // mt cbi(PORTF, PORTF3);     // disable the VCP
    cbi(DDRF,DDF3); // mt cbi(DDRF, PORTF3);  
    
    cbi(ADCSRA, ADEN);      // disable the ADC

	return ADC_temp;

}
void getVolt1()
{
	char voltintpart[]= {'0','0','0','\0'};
	char voltfractpart[]= {'0','0','0','\0'};
	int intpart = 0;
	int fractpart = 0;
	int ADCresult = 0;
//	int test = 65555;
	ADCresult = ADC_read();	
	itoa(ADCresult, voltintpart, 10);	
	sendString("The reading is ");
	sendString(voltintpart);
	
	ADCresult=ADCH;
	itoa(ADCresult, voltintpart, 10);	
	sendString("The reading is ");
	sendString(voltintpart);
	

}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The first problem I see is this:

 for(i=0;i<40;i++)            // do the ADC conversion 8 times for better accuracy 

ADC_tmp is only an int and will easily overflow thus giving you a reading vastly different to what you would expect. 8 times was 8 times for a reason, at forty times, you've gone too far!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

yea. hehe.. i got frustrated and took a random guess. i will go back to 8 and keep trying in the meantime.

how does it overflow? it's a temp value that's rewritten each loop with no accumulation.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You're correct, you are not accumulating the result, so why read forty times? Can you not just read ADC and the compiler will take care of the rest?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How will the compiler solve my problems if i don't know what i am working with. in the future how do i approach things relatively? from pieces and wraps of other peoples code?

I fell into this problem because i was trying to figure out how the temperature sensor circuitry worked. i think it's worth the dig.

thanks,

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

in the future how do i approach things relatively?

Start with the simplest of C progams to flash an LED. When you totally understand that then add some button scanning to allow you to control the flash rate - work up from the simple end and don't move on until you totally understand the first steps.

Jumping into the middle of some complex existing code then trying to decipher it piecemeal is not a good way to learn either C or microcontrollers. You'll just get confused and disillusioned.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I feel as though i'm being flamed for doing what you wrote. I started slowly, learning the language just by itself, and am currently working smileys tutorial. I've read most of the good tutorials on this website and i got stuck at this point so i posted a question. What else do i do after i've read everything i could find?

It's discouraging to post on this forum where i see this happen occasionally, but i can understand the sentiment as people want things spoon fed all the time.

Moving on, thanks for the reply and i'd like to stay on the topic. Noone here has to help me, and i'll be a bit more cautious and respectful on this forum if I seemed like i was asking for it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm not flaming anyone. I'm simply responding to your call for advice. Whether you accept the advice or not is your choice entirely. It just seems to me that you are out of your depth so maybe you are trying to advance too fast?

Anyway advice is there to be ignored - so ignore it.

(PS as you may have noticed *any* traffic in a thread bumps it into life and often attracts more input so any response is better than no response and a thread that "dies")

BTW for ADC use I take you have read the articles in Tutorial Forum? As for reading the full ADC 10 bits. You have several options:

1) read the two halves and combine by <<8 shifting the ADCH value and adding it to the ADCL value.

2) set the ADLAR bit and just read 8 bits from ADCH only

3) use the fact that avr-gcc defines a 16 bit register called just "ADC" and use that - it reads both ADCH and ADCL and combined them for you so you don't have to - just use:

uint16_t reading = ADC;

and you are done.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Clawson:

Thank you very much for your post. I'm trying out your suggestions right now.

cheers,

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried #1 only to have my max value at 185 again. #2 gives me a max of 46, and #3 gives me a max of 185.

I have no idea why it's not working. off to bed, i'll try again tomm.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Use your voltmeter to check the voltage at the AREF pin. The comments in the code suggest it's using an external reference.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
You're correct, you are not accumulating the result, so why read forty times? Can you not just read ADC and the compiler will take care of the rest?
But even if he was accumulating it, it would not overflow since he is only reading an 8 bit value and putting it into an int. The real problem with that loop is that he is only reading ADCL which is useless. You MUST read ADCH (whether or not you read ADCL).

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Steve,

That wouldn't explain the result he got when using my option (3) to use "ADC" though would it?

Cliff

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I wasn't in any way trying to explain the results, only refuting the claim that the code would overflow the variable and stating an obvious flaw in the loop that made it useless anyways.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Something seems totally wrong with initialization.
ADMUX register defines the reference voltage,ADLAR bit and the channel selection bits.
ADMUX = input;///what value input has???

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Measuring across on C102 on ARef gives me 2.85V, after I switch on PF3(VCP).

I'm going to take another look soon. Thanks to all you guys for the resposes!

Last Edited: Wed. Sep 15, 2010 - 01:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A part of code for AD conversion.

AD initilization.
ADMUX=ADC_VREF_TYPE;//=0 connect external ref voltage.
ADCSRA=0x86;

// Read the AD conversion result
unsigned int read_adc(unsigned char adc_input)
{
ADMUX=adc_input|ADC_VREF_TYPE;
// Start the AD conversion
ADCSRA|=0x40;
// Wait for the AD conversion to complete
while ((ADCSRA & 0x10)==0);//Wait conversion to complete
ADCSRA|=0x10;//Clear ADIF flag writing one on it
return ADCW;
}

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

geoelec wrote:
Something seems totally wrong with initialization.
ADMUX register defines the reference voltage,ADLAR bit and the channel selection bits.
ADMUX = input;///what value input has???

No there's nothing wrong with what the code is doing. Look at table 88 in the mega169 datasheet. The 00 combination of the two REFS bits in ADMUX selects "AREF, internal Vref turned off". So doing:

ADMUX = input;

is no different to:

ADMUX = (0<<REFS1) | (0<<REFS0) | input;

and the comment in the code:

// external AREF and ADCx 

confirms this was the author's intention.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Talking about something wrong is because theres no
main() function and the unsigned char input has no initial value.
void ADC_init(char input)
{
ADMUX = input; // external AREF and ADCx

ADCSRA = (1<<ADEN) | (1<<ADPS1) | (1<<ADPS0);

// Take a dummy reading , which basically allows the ADC
// to hack up any hairballs before we take any real readings
input = ADC_read();
}
And in the end of ADC_init function input has the value of ADCL register and all this confuses me.

Also waiting for ADIF flag to set then must be cleared
writing one on it.

while(!(ADCSRA & 0x10)); // wait for conversion done, ADIF flag active
ADCSRA|=0x10;//clear ADIF flag.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The a/d gets its juice from the AVCC pin. You got 5V on that one?

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Talking about something wrong is because theres no
main() function
Well, presumably he is calling these functions from somewhere, he just didn't include the entire program.
Quote:
and the unsigned char input has no initial value.
Huh?? Surely it has a value from the call of the function. The compiler would complain bitterly if he called the function without a value.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Are you saying AVCC must be powered regardless, even if my VREF is selected as AREF?

Measured AVCC was 2.9V.

------
Koschi wrote:
"But even if he was accumulating it, it would not overflow since he is only reading an 8 bit value and putting it into an int. The real problem with that loop is that he is only reading ADCL which is useless. You MUST read ADCH (whether or not you read ADCL)."

ADCH is read through get volt who calls adc read, right after adcl is read. I tried doing it in the ADC-read loop as well, trying to display a combined sum of ADCL+ADCH with no success (there is no difference between the two methods?)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yo Cliff.... Time to add FAQ #6: All VCC pins and AVCC must be powered.

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Reading ADCL does nothing unless you also read ADCH. Once a conversion is complete and ADCL is read, the ADC registers are not updated until ADCH is read. Reading only ADCL for multiple conversions as you are doing will give you the same value every time since the registers still reflect the first conversion value. And as geoelec said, if you use the ADIF flag to detect when the conversion is finished, you must clear it manually before the next conversion. This I did not notice before since you used magic numbers in the flag test. Had you used bit names instead, I would have seen that you were checking ADIF instead of ADSC.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
Reading ADCL does nothing unless you also read ADCH. Once a conversion is complete and ADCL is read, the ADC registers are not updated until ADCH is read. Reading only ADCL for multiple conversions as you are doing will give you the same value every time since the registers still reflect the first conversion value. And as geoelec said, if you use the ADIF flag to detect when the conversion is finished, you must clear it manually before the next conversion. This I did not notice before since you used magic numbers in the flag test. Had you used bit names instead, I would have seen that you were checking ADIF instead of ADSC.

Using ADSC(along with a read of ADCH in the loop) instead of ADIF gives me a max value of around 210.

So far all of my results have come from a prescale of 4 with a 2MHZ clock, so an input of 500khz. Changing the input to 250k,125k or 62.5k gives me nothing.

I've lost a lot of hair at this point :shock:

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What do you mean by "Changing the input to 250k,125k or 62.5k gives me nothing"? As ADIF is never set, ADCL/H contain zeroes, ADCL/H do not change, or something else? Did you provide Vcc to AVCC? Keep the frequency low for now, so that the LSB's actually mean something (errors are significant past 200kHz).

3V is 3V/5V * 255 = 153 = 0x99

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Yo Cliff.... Time to add FAQ #6: All VCC pins and AVCC must be powered.

It's an idea though I'm not sure THAT many of the posts here are down to missing power rail (though I agree we see quite a few). Maybe he FAQ should just be one entry "read the manual" ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Maybe we need a 'using the A/D checklist' 1)AVCC has 5V? 2) A/D init looks like this? (typical init here) 3) read a/d channel looks like this? (typical read channel here)

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Maybe he FAQ should just be one entry "read the manual" ?

Stealing Proteus doesn't make you an engineer.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sweet and simple!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
As ADIF is never set, ADCL/H contain zeroes
ADIF will be set once (at the end of the first conversion). It will just never be cleared to be set again.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can you report to us the voltage on the AVCC pin? (that's the pin that runs porta and the a/d)

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

3) use the fact that avr-gcc defines a 16 bit register called just "ADC" and use that - it reads both ADCH and ADCL and combined them for you so you don't have to - just use:

uint16_t reading = ADC;

and you are done.

I have to thank you for that point, it just solved my problem.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Or better yet, use ADCW instead of ADC. In some circumstances ADC causes a name conflict (confused with ADd with Carry).

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Or better yet,

Steve's right (as expected ;-)) but just to note that the clash of "ADC" with the ADC opcode only occurs in .S files and for that reason the .h files have:

/* Combine ADCL and ADCH */
#ifndef __ASSEMBLER__
#define ADC     _SFR_IO16(0x04)
#endif
#define ADCW    _SFR_IO16(0x04)

which should prevent the name clash but if you might be using .S it's probably a good idea to get into the habit of using ADCW rather than ADC anyay.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I havent ever used .s, not sure if I ever will but thanks for the pointer, I will start using ADCW, but can I ask what it stands for and what does it do ?

I just had to thank you becuase I have been battling with
ADCH/L and trying to combine them unsuccessfully until I read this post.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I will start using ADCW, but can I ask what it stands for and what does it do ?

Just guessing but I imagine the author meant for the W to stand for Word access (i.e. 16bits). In a sense it makes ADCW more self documenting as readers might recognise that the W had this meaning - though (given the stdint naming scheme) if they'd called it ADC16 it might be even more self documenting.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Quote:

I will start using ADCW, but can I ask what it stands for and what does it do ?

Just guessing but I imagine the author meant for the W to stand for Word access (i.e. 16bits). In a sense it makes ADCW more self documenting as readers might recognise that the W had this meaning - though (given the stdint naming scheme) if they'd called it ADC16 it might be even more self documenting.

I have read through the ADC section quite a few time and I have never seen it. Probably just me, but It must be there because I tested it on my Attiny84 and it worked perfectly, thanks for the pointer.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I have read through the ADC section quite a few time and I have never seen it. Probably just me,

No it's not documented in the datasheet as it's not a single register called ADCW (/ADC). The AVR itself has two 8 bit registers called ADCL and ADCH. The provision of ADCW comes from the C compiler you choose to use and is just a "helper" that arranges to read/write ADCH/L in the right order.