ADC function that always return first readed value

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

Hi guys,

 

A question about ADC use.

 

I have this function supposed to initialize and read value from ADC channel 0 :

 

uint16_t adc_measpres(void)
{
 ADCSRC = 10<<ADSUT0;   // set start-up time
 ADCSRB = 0<<MUX5;    // set MUX5 first
 ADMUX = (3<<REFS0) + (0<<MUX0); // 1.6V AREF - channel 0
 
 // switch ADC on, set prescaler (32 -> 500kHz), start conversion
 ADCSRA = (1<<ADEN) + (1<<ADSC) + (5<<ADPS0);
 do
 {} while( (ADCSRA & (1<<ADSC))); // wait for conversion end
 ADCSRA = 0; // disable the ADC
 
 return (ADC); 
}

 

It works good first time I call it in the main(), but any subsequent call to this function return always the same value as the first reading, till reset of the MCU.

 

Do you see the problem?

 

Thanks!

 

Br

 

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

First, tell us which AVR model you are using, and its clock speed.

 

Next, do you realize that the first conversion after starting up the ADC probably does not give a "good" value?  Generally, one would start up the ADC once, and run a dummy conversion.  Then call the read routine as desired.

 

All that said, nothing jumps out at me from the posted code.

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Hey,

 

Using Atmega1284RFR2 clocked at 16MHz (External oscillator used for RF module).

 

Strangely here, only the first reading is correct, I checked and it match the applied voltage on channel 0 pin. But if I modify this volage, subsequent call to this function will return same value as first one !

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

Look in the datasheet and answer a couple of questions:

How long, after turning on an internal reference does it take the reference to stabilize?    Do you have an external cap on the AREF pin, if so how what size?

Do you think this will require some time to charge?

Does turning the ADC on/off have any effect on this?

 

Jim

 

Click Link: Get Free Stock: Retire early!

share.robinhood.com/jamesc3274

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
ADCSRB = 0<<MUX5;    // set MUX5 first

This does not "set" anything, it clears all bits of ADCSRB.

 ADCSRA = 0; // disable the ADC

Why are you disabling the ADC? This is almost never done. As Lee said, the first reading after enabling the ADC is not reliable. Usually one has a function to set up the ADC which is called at the start of the program, and a separate function that does a conversion and returns a value.

Regards,
Steve A.

The Board helps those that help themselves.

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

Koshchi wrote:

ADCSRB = 0<<MUX5;    // set MUX5 first

This does not "set" anything, it clears all bits of ADCSRB.

The OP is referring to the two-step access to the MUX bits.  When writing them, the MUX bits in ADCSRB must be written first.  Once they are written they are buffered.  The buffered bits are commited when ADMUX is written.  This is similar to the way in which 16-bit registers like TCNT1L/H are accessed.  I'll agree that the OP's  code comment is misleading.  Should read // write MUX5 first

Quote:

 ADCSRA = 0; // disable the ADC

Why are you disabling the ADC? This is almost never done. As Lee said, the first reading after enabling the ADC is not reliable. Usually one has a function to set up the ADC which is called at the start of the program, and a separate function that does a conversion and returns a value.

The ADC in the ATmegaxxxxRF family is a bit different.  Startup time is managed internally and is somewhat more sophisticated than with other AVR.  ADCSRC has the bits ADSUT[4:0] which configures the startup-time of the ADC.  The OP has configured a startup time of 88 us.

 

I'll agree that it's unusual for the ADC to be disabled after every conversion, but this could be useful in cases where the ADC is relatively infrequently, such as to check the voltage of a battery.  Furthermore, changes to the REF bits are ignored until the ADC is disabled and re-enabled again so the use of more than one reference in an app requires that the ADC be disabled between conversions using different references.  It's unclear why the OP is doing it here.

 

Although the datasheet suggests that the OP's posted code should work, I would recommend:

1) configure and enable ADC but don't start conversion yet

2) poll and wait until REFOK and AVDDOK in ADCSRB are set

3) start conversion

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

The ADC in the ATmegaxxxxRF family is a bit different ... and is somewhat more sophisticated than with other AVR. 

Indeed.  Another  very useful feature is "stretching" of the sample-and-hold time.  I wish all AVR8 had that.  (The poor-man's alternative is double-convert on the same channel.)

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Quote:
Indeed.  Another  very useful feature is "stretching" of the sample-and-hold time.  I wish all AVR8 had that.
Would that it were so ;)

 

Quote:
(The poor-man's alternative is double-convert on the same channel.)
That's one way.  However since the ADC 'aperture' is open (i.e. the selected ADC channel is connected to the S/H cap) so long as the ADC is enabled via ADEN, you can control the sample time simply by waiting before initiating a conversion with ADSC (or arranging an appropriate interval via ADATE and ADTS[2:0]).  The aperture closes at the moment of the hold (1.5 ADC clock cycles after the conversion is initiated [2 cycles when using ADATE]), and opens again at the end of every conversion.

 

If you're using free-running mode, then of course you have no option to lengthen the sample time.  A new conversion will be initiated 1.5 ADC clock cycles after the end of the previous one, so the aperture will remain open only 1.5 cycles during each 13 cycle conversion.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

The OP is referring to the two-step access to the MUX bits. 

It still does not change the fact that he is writing to the entire register, not just one bit.

Regards,
Steve A.

The Board helps those that help themselves.

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

Koshchi wrote:
It still does not change the fact that he is writing to the entire register, not just one bit.
Indeed.  However I don't believe the OP intended to write just one bit.  Yes the code and comment are misleading (as I have already mentioned), however since:

 

- ADCSRA has a default value of 0x00 anyway

- the OP is just writing it to 0x00

- the channel selected by this action is consistent with the OP's desired channel (channel 0)

- the remaining writeable bits are ACME, ACCH, and ADTS[2:0], none of which have when written to zero an impact on the ADC (without ADATE)

 

... I can't see how this is related to the problem.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

... I can't see how this is related to the problem.

Indeed; the posted fragment looks OK.  I guess we'll need to see the calling sequence--perhaps  some conditional logic?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

Hi,

 

I tried to modify as advised but it didn't change anything :

 

uint16_t adc_measpres(void)
{
 adc_result = 0;
 ADCSRC = 10<<ADSUT0;   // set start-up time
 ADCSRB = 0<<MUX5;    // set MUX5 first
 ADMUX = (3<<REFS0) + (0<<MUX0); // 1.6V AREF - channel 0

 // switch ADC on, set prescaler (32 -> 500kHz), start conversion
// ADCSRA = (1<<ADEN) + (1<<ADSC) + (5<<ADPS0);
 ADCSRA = (1<<ADEN) + (5<<ADPS0);
 while (!(ADCSRB & (1<<REFOK)) | !(ADCSRB & (1<<AVDDOK)));
 ADCSRA |= (1<<ADSC); 
 do
 {} while( (ADCSRA & (1<<ADSC))); // wait for conversion end
 adc_result = ADCL;
 adc_result |= (ADCH << 8);
 ADCSRA = 0; // disable the ADC
 
 return (adc_result); 
}

Indeed, I deleted everything else in my code and the value is updated properly if I call this function every second and do nothing else.

It is great mistery to me why it doesn't work in original code. Will have to delete other function one by one and check what's wrong ...

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

I would approach this issue two ways.  First, I would try to separate the ADC initialisation from the reading.  Second, if the above runs perfectly if you call it every second then what happens if you call it every half second, quarter second etc.. 

 

Then I would also check how many times the while and do/while loops are looping - it might be falling through 'too quickly' for whatever reason.

 

David  

Last Edited: Wed. Feb 4, 2015 - 10:37 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

 

It seems I found the mistake. I had left by mistake these two line between two call to the function :

 

YYY = adc_measpres();

...

XXX1 = ADCH;

XXX2 = ADCL;

 

Instead doing :

 

YYY = adc_measpres();

...

XXX1 = YYY >> 8;

XXX2 = YYY;

 

Don't know why it was making next calls to the function giving wrong results, but seems working

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

So the problem wasn't in the routine at all but in the code calling the routine.  I have to ask, why are you creating a 10bit value in the routine and then splitting it outside the routine?

 

David

Last Edited: Wed. Feb 4, 2015 - 10:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Also why bother with ADCH and ADCL separately anyway? As your very first code showed you just use "ADC" and the compiler sorts out the access order and amalgamation of the two 8bit values anyway.

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

Mmm could not justify this ;)

 

Anyway I don't think any of the irregularities pointed out was the reason of the problem. As I understand, problem arised when accessing ADCH and ADCL when outside of the function. Maybe because ADC was disabled don't know.

 

Thanks anyway

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

It is likely to have been due to how/where you declared your variables.  When you cut out most of your code they were being preserved rather than overwritten.  Just a guess of course.

 

David