My ADC does not work!

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

Plz help! My ADC does not work!1!!!11 What is wrong???
:)

So I have an ATmega16. I have set up the ADC as single conversion with interrupt enabled. The interrupt reads the value from the register and then disables the ADC. After reset the conversion run once and the value returned is correct. But all the following runs the conversion returns the same value as the first time, even if the input is changed. I suspect that the interrupt is executing before the conversion is finished, causing the conversion to be aborted (because of ADC disable) and then no new value is written in the ADC register.

I noticed when a ran the code in the AVRStudio4 simulator that the time from when I started the conversion to when the interrupt was executed were not what I expected considering that the first conversion after ADEN=1 should take 25 cc. The time was more like 13 cc... Have not measured on target.

I did a test on the target without the interrupt, starting the conversion and then just finish everything else (~1.5 seconds of work), after that read the ADC value and disable the ADC. It is working all the time.

So what am I missing here? Why is the interrupt version not working? It seems like the interrupt comes after 13 cc instead of 25 cc which would be expected for the first conversion after ADEN.

/COLA

#include 
#include 
#include 
#include 

/************************************
	Defines
 ************************************/
 
#define ADC_PIN	PA0
 
#define INTERNAL_VREF	_BV(REFS1) | _BV(REFS0)
 
#define ENABLE_ADC		ADCSRA |= _BV(ADEN) | _BV(ADIE)
#define DISABLE_ADC		ADCSRA &= ~(_BV(ADEN) | _BV(ADIE))
#define START_ADC		ADCSRA |= _BV(ADSC)

/************************************
	Private variables
 ************************************/

static volatile uint16_t ADC_Value;

/************************************
	Private functions
 ************************************/

ISR(ADC_vect)
{
	ADC_Value = (uint16_t)(ADCH << 8) | (uint16_t)ADCL;
	
	DISABLE_ADC;
}

/************************************
	Public functions
 ************************************/

void ADC_Init(void)
{
	DDRA &= ~(_BV(ADC_PIN));			// pin as input
	ADMUX |= INTERNAL_VREF;				// Select internal Vref
	ADCSRA = _BV(ADPS2) | _BV(ADPS0);	// Prescaler = /32 => 125kHz @ 4MHz

	DISABLE_ADC;
}

void ADC_StartConversion(void)
{
	ENABLE_ADC;
	// wait 70µs for Vref to stabilize
	// each iteration take 3cc = 0.750µs @ 4MHz
	// 100 iterations takes 75µs
	_delay_loop_1(100);

	// the conversion takes 25 cycles. 25*(1/125kHz)=0.2ms
	START_ADC;
}

uint16_t ADC_GetValue(void)
{
	return ADC_Value;
}
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
So what am I missing here?

Showing the code that actually calls these functions would be nice.

I'm not sure why you are disabling the ADC between readings. Enable it once and leave it on.

Regards,
Steve A.

The Board helps those that help themselves.

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

Quote:

ADC_Value = (uint16_t)(ADCH << 8) | (uint16_t)ADCL;

Depending on your compiler, the options set, the phase of the moon, and the rules of C this may or may not work. If ADCH is not promoted to 16 bits then the x8 shift ends up with 0 which is then cast to a uint. As I mentioned, YMMV.

What does "does not work" mean?

It appears that you accesses are in the wrong order:

Quote:
ADCL must be read first, then ADCH, to ensure that the content of the Data Registers belongs to the same conversion. Once ADCL is read, ADC access to Data Registers is blocked. This means that if ADCL has been read, and a conversion completes before ADCH is
read, neither register is updated and the result from the conversion is lost. When ADCH is read, ADC access to the ADCH and ADCL Registers is re-enabled.

I think that your compiler gives you a ADCW or ADC that you can use for the simple single assignment.

Quote:

ADMUX |= INTERNAL_VREF; // Select internal Vref

This is kinda scary as it never really sets ADMUX, but rather ORs bits. Your code will only work for ADC0 assuming that ADMUX is in fact clear at startup. For real work you want to pass a channel number to the StartConversion routine, and it then becomes

ADMUX = INTERNAL_VREF | chan;

Lee

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

Thanks for the fast responses!

theusch wrote:
Quote:

ADC_Value = (uint16_t)(ADCH << 8) | (uint16_t)ADCL;

It appears that you accesses are in the wrong order

Ahh you might be onto something there! The ADCL read will block the entire ADC register from writes, and since I do that last maybe the register is blocked even after I toggle the ADEN off->on. The only time when my ADC register is not blocked is between the ADCH and ADCL read in that line of code... This would explain why my first conversion works, but all the subsequent conversions return the same value as the first; the register is simply blocked so it can not be updated with the new value.

Since I only did single conversions and disabled the ADC between conversions I never bothered to change the order, I thought that disabling the ADC would remove the block in the ADC register :(

Tested on target with the correct reading order, working as intended :) Thanks!