ADC Help Atmega 328P

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

Hey guys ive written a simple code to read an adc value calculate a voltage and switch a relay accorrding to the value however it is not working Ive tried examples online same issue, I eventually tried using the AVR simulator in atmel studio and found the code gets stuck at the while loop waiting for the conversion to complete.

please note adc_init() and adc_read() are located in a seperate c file but heres what i have

 

heres main.c

#include <avr/io.h>
#include <util/delay.h>

#define F_CPU 16000000

void main(void)
{
	int adcraw; //Raw ADC data
	float sysv; //System Voltage
	int vbl; //Voltage Batt Low
	int vdiscon; //Voltage Batt Disconnect
	
	DDRD = 0xBF; //Port D as OP except PIND6
	DDRC = 0x00; //Port C as IP
	
	adc_init(); //Initilize ADC
	adcraw = adc_read(); //Read ADC value
	sysv = 5*(adcraw/255); //Calculate System Voltage
	
    while(1)
    {
        if (sysv <= 3)
		{
			PORTD = 0b00010000;
		}		
		else
		{
			PORTD = 0b00001000;
		}		
    }
}

and heres adc_init() which enables ADC and sets to ADC 0 Prescaler to 128 and Ref to AVcc

/*
 * ADCinitR10.c
 *
 * Created: 02,12,15 7:51:34 PM
 *  Author: Ethan
 */ 
#include <avr/io.h>

void adc_init(void)
{
	ADCSRA |= (1<<ADEN) | (1<<ADPS2) | (1<<ADPS1) | (1<<ADPS0);
	ADMUX |= (1<<REFS0);
}

 

Heres my actual file which reads this is were I believe the code is getting stuck

/*
 * ADCinitR10.c
 *
 * Created: 02,12,15 7:51:34 PM
 *  Author: Ethan
 */ 
#include <avr/io.h>

int adc_read(void)
{
	ADCSRA |= (1<<ADSC); //Start Conversion
	while (ADCSRA & (1<<ADSC)); // Wait for conversion to complete
	return ADCL;
}

 

 

ANy help would be appreciated, this has given me many late nights and for something so simple i don't understand whats going wrong

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

Your code isn't going to do much useful work. You only read the adc value once then sit in a loop testing a value that will never change.

#include
#include

#define F_CPU 16000000

void main(void)
{
int adcraw; //Raw ADC data
float sysv; //System Voltage
int vbl; //Voltage Batt Low
int vdiscon; //Voltage Batt Disconnect

DDRD = 0xBF; //Port D as OP except PIND6
DDRC = 0x00; //Port C as IP

adc_init(); //Initilize ADC

while(1)
{
adcraw = adc_read(); //Read ADC value
sysv = 5.05*(adcraw/255.0); //Calculate System Voltage

if (sysv <= 3)
{
PORTD |=(1<<PD4);
}
else
{
PORTD &= ~(1<<PD4);
}
}
}

Last Edited: Wed. Dec 2, 2015 - 08:29 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you I have moved the adc_read(); within the while(1) loop, In regards to calculating the voltage is my maths right sorry im new and got confused with some of the maths and just need a breif explanation,

 

Basically theres a voltage divider with a small pot to allow for tuning in front that scales 0-30V to the 0-5V the adc can read, Currently my ADC 0 pin has 1.2V on it, Would i be right to think that by using floating point values this is playing with the formula i have, Would i be better using int values and making making the values higher by 10 times e.g 1.2V would equal an integer of 12

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

Note that the 5.05 should be 5.0 but this forum does not play well with mobile devices. Similarly with the code formatting.

Float vs integer? If you can afford the extra code space and speed penalty, then using float is the easy way out. Sure, you can use scaled integers but you have to think a little harder! You also need to multiply before you divide.

I don't think your adc calcs are correct - you only read adcl which gives the lower 8 bits but if your reference is 5V, then you still need to divide by 1023 rather than 255. It also means that currently you only read correctly for inputs 0 to 1.25V, over that, the values 'wrap around'. You would also need to factor in the scaling from 30 down to 5V, so a factor of 6 needs to be introduced.

Last Edited: Wed. Dec 2, 2015 - 12:44 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can never read just ADCL anyway. Either you want a 16 bit result (10 bits significant) in which case you "return ADC;" or you want an 8 bit result so you set the ADLAR bit in the ADMUX register then "return ADCH;" but your "return ADCL;" is never going to work.

 

Also why is the return type of adc_read() and "int"? When would the ADC ever give you a negative reading?

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

Hi Kartman,

 

Thanks for that that makes some sense, 

I will amend the line to become

sysv = ((5*(adcraw/1023))*6); //Calculate System Voltage this should give me my 30V range

 

I will also ammend ADMUX to 

 

ADMUX |= (1<<REFS0) | (1<<ADLAR); //To read the upper 8 bits for accuracy

Then I will run through complete testing again, 

 

Clawson is there a better way to define the return type for adc_read(),  My understanding was the best way was to return the ADCx value as an integer is this not the case, What would be better practice for this?

 

 

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

Is there a particular reason you only want 8 bit resolution from the ADC? Why not just use the full 10 bits? In which case, don't set ADLAR. Then return ADC rather than ADCL.

I put the .0 in the constants for good reason. As an integer calculation, you'll lose bits of precision. To force the compiler to do the calculation as floating point, you need to guide it. Thus a value of 1023.0 suggests to the compiler you want a float calculation. You may need to do a cast to help it. Google c language cast for an explanation.

The return type would be unsigned int.

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

I was just lookinga t 8 bit because it seems a nice simple way to do it, I only need accuracy to 1 dp, essentially I just need work out the supply voltage is it 12v system or 24v system, set alarm thresholds based on power up voltage and then monitor battery for low volt and disconnect when it gets to low whilst alarming via relay outputs. so 8 bit should give sufficient accuracy for 1 dp at 0-30v, 

 

Would changing to 10 bit be as easy as replacing "return ADCH;" with "return ADC;"?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
/*
 * ADCinitR10.c
 *
 * Created: 02,12,15 7:51:34 PM
 *  Author: Ethan
 */
#include <avr/io.h>

unsigned int adc_read(char input)
{

        ADMUX &= 0xf8;
        input &= 0x07;
        ADMUX |= input;
	ADCSRA |= (1<<ADSC); //Start Conversion
	while (ADCSRA & (1<<ADSC)); // Wait for conversion to complete
	return ADC;
}
/*
 * ADCinitR10.c
 *
 * Created: 02,12,15 7:51:34 PM
 *  Author: Ethan
 */
#include <avr/io.h>

void adc_init(void)
{
	ADCSRA = (1<<ADEN) | (1<<ADPS2) | (1<<ADPS1) | (1<<ADPS0);
	ADMUX = (1<<REFS0);
}
#include <avr/io.h>
#include <util/delay.h>

#define F_CPU 16000000

void main(void)
{
	unsigned int adcraw; //Raw ADC data
	unsigned long voltage;
	float sysv; //System Voltage
	int vbl; //Voltage Batt Low
	int vdiscon; //Voltage Batt Disconnect

	DDRD = 0xBF; //Port D as OP except PIND6
	DDRC = 0x00; //Port C as IP

	adc_init(); //Initilize ADC

    while(1)
    {
	adcraw = adc_read(0); //Read ADC value
	sysv = 30.0 * (adcraw/1023.0); //Calculate System Voltage
	//or
	voltage = ((unsigned long)adcraw*300) / 1023;  //voltage in units of 100mV
        if (sysv <= 3)
		{
			PORTD |= (1<<PD4);
		}
		else
		{
			PORTD &= ~(1<<PD4);
		}
    }
}

 

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

I see the changes your made to adc_read to allow for which adc it is to read and that's a great change I would never have thought of which will enable that file to be included in many projects. I kind of understand how it works but im confused a little to understanding 

ADMUX &= 0xf8;
        input &= 0x07;
        ADMUX |= input;

Could you please briefly explain this part or point me in the direction of some good articles of these operators?

is the line input &= 0x07 telling the compiler we only want input to change the lower 3 bits of ADMUX?

 

 

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

There's a tutorial name bit manipulation in the tutorial section.

Your summation is correct. I clear the first three bits in admux, i clear the last 5 bits of input then 'or' input into admux.
Net result i can select adc inputs 0..7

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

Not to be pedantic, but I would propose:

ADMUX = (ADMUX & 0xF8) | (input & 0x07);

While this crams more into one line of code, and perhaps makes it slightly more difficult to understand (although I personally don't think it does), it also has the effect of writing to ADMUX only once.

 

The benefit is a bit faster/smaller code, but more importantly the input mux is only changed once.  With the separate writes to ADMUX, the channel is first changed to 0, then to the desired channel.

 

The time spent on channel zero may be very short (perhaps under a microsecond, depending upon F_CPU), but other threads teach us that for input sources with somewhat high impedances this can make a difference.

"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."

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

 

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

joeymorin wrote:

Not to be pedantic, but I would propose:

ADMUX = (ADMUX & 0xF8) | (input & 0x07);

While this crams more into one line of code, and perhaps makes it slightly more difficult to understand (although I personally don't think it does), it also has the effect of writing to ADMUX only once.

 

The benefit is a bit faster/smaller code, but more importantly the input mux is only changed once.  With the separate writes to ADMUX, the channel is first changed to 0, then to the desired channel.

 

The time spent on channel zero may be very short (perhaps under a microsecond, depending upon F_CPU), but other threads teach us that for input sources with somewhat high impedances this can make a difference.

...but still a RMW.  I've [almost] always just rebuilt ADMUX from "component parts" each time.  In most apps, the "other bits" for reference selection and ADLAR stay constant throughout operation.  But even if different phases of the app use different settings, ADMUX can be built from those settings.

 

A similar thread:

https://www.avrfreaks.net/comment...

where I linked back to

https://www.avrfreaks.net/comment...

 

https://www.avrfreaks.net/comment...

https://www.avrfreaks.net/comment...

...

theusch wrote:
Quote:
The better option is an AND then an OR. The AND preserves the upper bits but clear the channel bits then the OR puts the new channel selection in place.
That's why I always just write it to "rebuild" all of ADMUX each time. In nearly all apps the REFSn and ADLAR bits stay constant anyway. As I do this in typically the ADC-complete ISR it saves a few cycles and the ADMUX read. E.g.
 // VREF // ==== // Note that in ADMUX, bit 7 is REFS1; bit 6 is REFS0 // REFS1:0 ADMUX Mask Description // ------- ---------- ----------- // 0 0x00 External AREF // 1 0x40 Use Vcc as Aref // 3 0xc0 Internal 1.1V bandgap #define ADC_VREF_VCC 0x40 #define ADC_VREF_AREF 0x00 #define ADC_VREF_BG 0xC0 #define ADC_VREF_TYPE ADC_VREF_VCC

... ADMUX = ADC_VREF_TYPE | input_index;

 ; 0000 0A57 ADMUX = ADC_VREF_TYPE | input_index; 000660 64e0 ORI R30,0x40 000661 93e0 007c STS 124,R30 

 

 

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.

Last Edited: Thu. Dec 3, 2015 - 02:54 PM