ADC0 "Works" on ATmega328PA But All Values Zero

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

Hello,

 

I'm initializing the ADC and trying to use it as the documentation outlines, but all of my reads give me zero. I have a potentiometer set up as a voltage divider feeding the ADC0 pin. I have checked it with a voltmeter and it is about 1.8V, so the value should be something besides zero.

 

The value is captured from a single ADC comparison and then sent out over the USART. I wait for the ADSC bit to be cleared by hardware, so as far as I know the ADC is working. The code follows - some pieces are missing but everything that is being used should be there. If necessary I can create a smaller project that just tries to use the ADC.

 

Thanks in advance!

 

MCU=atmega328p
F_CPU=8000000
PORT=COM3
USE_2X=1
BAUD_TOL=3
BAUD=57600

CC=avr-gcc
CF=-mmcu=${MCU} -DF_CPU=${F_CPU} -DBAUD_TOL=${BAUD_TOL} -DUSE_2X=${USE_2X} -DBAUD=${BAUD} -Os -std=gnu99 -Wall -pedantic
LF=-mmcu=${MCU}
OC=avr-objcopy

SRC=$(wildcard *.c)
OBJ=${SRC:.c=.o}
BIN=lcbc

.phony: all clean

${BIN}.hex: ${BIN}.elf
	${OC} -O ihex $< $@

${BIN}.elf: ${OBJ}
	${CC} ${LF} -o $@ $^

%.o: %.c
	${CC} ${CF} -c -o $@ $<

program:
	avrdude -p m328p -P ${PORT} -b 57600 -c arduino -D -U flash:w:${BIN}.hex:i

clean:
	rm ${OBJ} ${BIN}.elf ${BIN}.hex
#include <avr/io.h>
#include <avr/power.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <util/setbaud.h>

#include <string.h>
#include <stdint.h>
#include <stdlib.h>

#include "io.h"
#include "adc.h"
#include "usart0.h"

int
main(void)
{
	cli();

	// Turn off all peripherals.
	PRR = //(1 << PRTWI0)	  |
		  (1 << PRTIM2)	  |
		  (1 << PRTIM0)	  |
		  (1 << PRTIM1)	  |
		  //(1 << PRSPI0)	  |
		  (1 << PRUSART0) |
		  (1 << PRADC);

	io_up();
	DDRB |= (1 << PB5);
	adc_up();
	rtc_up();
	usart0_up();

	sei();
	while (1) {
		uint16_t v = 0;
		char buffer[16];
		memset(buffer, 0, sizeof(buffer));

		adc_convert();
		v = adc_getv();
		itoa(v, buffer, 10);
		usart0_puts(buffer, 16);
		usart0_putc('\n');

		PORTB ^= (1 << PB5);
		_delay_ms(250);
	}
}

void
adc_up(void)
{
	// Ref. voltage is AVcc with decoupling on Aref.
	// Multiplexer reading ADC0, or pin 23.
	ADMUX = (0 << REFS1) |
			(1 << REFS0) |
			(0 << ADLAR) |
			(0 << MUX3)	 |
			(0 << MUX2)	 |
			(0 << MUX1)  |
			(0 << MUX0);

	// Prescaler of 64 for 125kHz clock from 8MHz.
	ADCSRA = (1 << ADEN)  |
			 (0 << ADSC)  |
			 (0 << ADATE) |
			 (0 << ADIF)  |
			 (0 << ADIE)  |
			 (1 << ADPS2) |
			 (1 << ADPS1) |
			 (0 << ADPS0);

	// Comparator multiplexing off, freerunning conversion mode.
	ADCSRB = (0 << ACME)  |
			 (0 << ADTS2) |
			 (0 << ADTS1) |
			 (0 << ADTS0);

	// Disable digital IO logic for all ADC pins.
	/*DIDR0 = (1 << ADC7D) |
			(1 << ADC6D) |
			(1 << ADC5D) |
			(1 << ADC4D) |
			(1 << ADC3D) |
			(1 << ADC2D) |
			(1 << ADC1D) |
			(1 << ADC0D);*/
}

void
adc_convert(void)
{
	ADCSRA |= (1 << ADSC);
	while(ADCSRA & (1 << ADSC));
}

uint16_t
adc_getv(void)
{
	uint16_t r = 0;
	r  = (ADCL);
	r |= (ADCH << 8);

	ADCSRA |= (1 << ADIF);
	return r;
}

void
usart0_putc(int c)
{
	while (!(UCSR0A & (1 << UDRE0)));
	UDR0 = c;
}

void
usart0_puts(int8_t *s, size_t n)
{
	for(n--; *s && n; s++, n--)
		usart0_putc(*s);
}

void
usart0_up(void)
{
	DDRD &= ~(1 << PD0);
	DDRD |=  (1 << PD1);

	// Enable usart 0.
	PRR &= ~(1 << PRUSART0);

	// Double speed, normal communication.
	UCSR0A = (1 << U2X0) |
			 (0 << MPCM0);

	// Disable usart 0 interrupts, enable transmitter and receiver.
	UCSR0B = (0 << RXCIE0) |
			 (0 << TXCIE0) |
			 (0 << UDRIE0) |
			 (1 << RXEN0)  |
			 (1 << TXEN0)  |
			 (0 << UCSZ02);

	// Enable asynchronous USART operation, no parity, one stop bit, 8 data
	// bits.
	UCSR0C = (0 << UMSEL01) |
			 (0 << UMSEL00) |
			 //(0 << UPMON01) |
			 //(0 << UPMON00) |
			 (0 << USBS0)	|
			 (1 << UCSZ01)	|
			 (1 << UCSZ00)	|
			 (0 << UCPOL0);

	// Set baudrate as calculated by setbaud.h.
	UBRR0 = UBRR_VALUE;
}

 

This topic has a solution.
Last Edited: Thu. Aug 3, 2017 - 07:11 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

R0b0t1 wrote:
// Turn off all peripherals. PRR = //(1 << PRTWI0) | (1 << PRTIM2) | (1 << PRTIM0) | (1 << PRTIM1) | //(1 << PRSPI0) | (1 << PRUSART0) | (1 << PRADC);
R0b0t1 wrote:
The code follows - some pieces are missing but everything that is being used should be there. If necessary I can create a smaller project that just tries to use the ADC.

Making and posting the smallest complete program that demonstrates the symptoms is often recommended here.  Many times the creation and testing of the "sanity check" program uncovers the problem area.

 

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

Thank you for pointing that out. Unfortunately, I am not a very smart person and likely would have needed that shown to me at some point in the process.

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

R0b0t1 wrote:
void usart0_putc(int c) { while (!(UCSR0A & (1 << UDRE0))); UDR0 = c; }

How will that code put a 16-bit "int" value into an 8-bit unsigned UDR0?

 

R0b0t1 wrote:
// Set baudrate as calculated by setbaud.h. UBRR0 = UBRR_VALUE;

OK, I'll bite:  where does UBRR_VALUE get set?  Aaah -- it is magic, with the global symbols and inclusion of setbaud.

 

8MHz, eh?  Internal oscillator?  Is it calibrated?  Even with U2Xn set, 8MHz and 57600 is 2.1% error, right?  General rule of thumb is 2%.

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. Aug 3, 2017 - 07:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:

R0b0t1 wrote:
void usart0_putc(int c) { while (!(UCSR0A & (1 << UDRE0))); UDR0 = c; }

How will that code put a 16-bit "int" value into an 8-bit unsigned UDR0?

 

uint16_t v = 0;
char buffer[16];
memset(buffer, 0, sizeof(buffer));

adc_convert();
v = adc_getv();
itoa(v, buffer, 10);
usart0_puts(buffer, 16);
usart0_putc('\n');

 

theusch wrote:

R0b0t1 wrote:
// Set baudrate as calculated by setbaud.h. UBRR0 = UBRR_VALUE;

OK, I'll bite:  where does UBRR_VALUE get set?  Aaah -- it is magic, with the global symbols and inclusion of setbaud.

 

8MHz, eh?  Internal oscillator?  Is it calibrated?  Even with U2Xn set, 8MHz and 57600 is 2.1% error, right?  General rule of thumb is 2%.

No, it is not calibrated. 2.1% is close enough for me to assume it will work while I test it, and it does work. I believe at some point I tested most baudrates given in the datasheet table and they worked on a few ATmega devices I had when using the internal oscillator. The exception would be the settings with 8.5% error.

 

I did have my own macro to define UBRR at one point, but I see no reason to use it when setbaud.h exists.

Last Edited: Thu. Aug 3, 2017 - 07:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
uint16_t
adc_getv(void)
{
	uint16_t r = 0;
	r  = (ADCL);
	r |= (ADCH << 8);

	ADCSRA |= (1 << ADIF);
	return r;
}

change to:

uint16_t
adc_getv(void)
{
	uint16_t r = 0;
	r  = ADC;


	ADCSRA |= (1 << ADIF);
	return r;
}

FYI:  The compiler knows the correct order to read 16 bit reg's!

 

Jim

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

Thanks for the tip! I didn't think to try it with registers besides UBRR.

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

Why do you bother clearing ADIF - you aren't using it and your blocking routine (sensibly) waits on ADSC not ADIF. So:

uint16_t
adc_getv(void)
{
	uint16_t r = 0;
	r  = ADC;


	ADCSRA |= (1 << ADIF);
	return r;
}

could actually be:

uint16_t
adc_getv(void)
{
	return ADC;
}

which seems a bit pointless because:

		adc_convert();
		v = adc_getv();

might as well be just:

		adc_convert();
		v = ADC;

int fact there's very little reason for 'v' either so:

		adc_convert();
		v = adc_getv();
		itoa(v, buffer, 10);

might as well be:

		adc_convert();
		itoa(ADC, buffer, 10);

In fact what you might then consider (because it makes more sense for it to do so) is:

void
adc_convert(void)
{
	ADCSRA |= (1 << ADSC);
	while(ADCSRA & (1 << ADSC));
}

becomes:

uint16_t
adc_convert(void)
{
	ADCSRA |= (1 << ADSC);
	while(ADCSRA & (1 << ADSC));
        return ADC;
}

in which case the code in main() now becomes the one line:

		itoa(adc_convert(), buffer, 10);

Just an idea.