Atmega1280 - cannot read analog channel beyond ADC7

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

Hi there,

 

 

With the code below compiled for WinAVR, I can read the channels 0-7 correctly, however channels 8-11 are returning Zero. I'm in the Proteus simulation environment. I'm sure there is something obvious, but I could not realize so far:

 

At the Header file:

#define adc0  ((1<<ADLAR) | 0     )
#define adc1  ((1<<ADLAR) | 1     )
#define adc2  ((1<<ADLAR) | 2     )
#define adc3  ((1<<ADLAR) | 3     )
#define adc4  ((1<<ADLAR) | 4     )
#define adc5  ((1<<ADLAR) | 5     )
#define adc6  ((1<<ADLAR) | 6     )
#define adc7  ((1<<ADLAR) | 7     )
#define adc8  ((1<<ADLAR) | 8     )
#define adc9  ((1<<ADLAR) | 9     )
#define adc10 ((1<<ADLAR) | 10    )
#define adc11 ((1<<ADLAR) | 11    )

At ADC ISR:

ISR(ADC_vect)
   {
   static int8_t val;

   val = ADCH;
   switch (ADMUX)
      {
      case adc0 : ADMUX = adc1; analoginput=0   ; break ;
      case adc1 : ADMUX = adc2; analoginput=1   ; break ;
      case adc2 : ADMUX = adc3; analoginput=2   ; break ;
      case adc3 : ADMUX = adc4; analoginput=3   ; break ;
      case adc4 : ADMUX = adc5; analoginput=4   ; break ;
      case adc5 : ADMUX = adc6; analoginput=5   ; break ;
      case adc6 : ADMUX = adc7; analoginput=6   ; break ;
      case adc7 : ADMUX = adc8; analoginput=7   ; break ;
      case adc8 : ADMUX = adc9; analoginput=8   ; break ;
      case adc9 : ADMUX = adc10; analoginput=9  ; break ;
      case adc10: ADMUX = adc11; analoginput=10 ; break ;
      case adc11: ADMUX = adc0 ; analoginput=11 ; break ;
      }
   adcresult[analoginput] = val ;
   }

At init:

void init_ADC (void)
{
ADMUX =
      (1 << ADLAR)|                                                           // left shift result
      (0 << MUX0) | (0 << MUX1) | (0 << MUX2) | (0 << MUX3) | (0 << MUX4) ;   // Select channel 0

ADCSRA =
        (1 << ADEN)  |                                                     // Enable ADC
        (1 << ADIE)  |                                                     // enable ADC interrupt
        (1 << ADPS0) | (1 << ADPS1) | (1 << ADPS2) ;                       // Prescaler = 128  | - 125KHz with 16MHz clock

ADCSRB = 0;                                                         // free running mode isabled
ADCSRA |= (1 << ADSC);                                              // start conversions

// Turn off digital inputs on ADC0-7
   DIDR0  = (1<<ADC7D) | (1<<ADC6D)| (1<<ADC5D)| (1<<ADC4D)| (1<<ADC3D)| (1<<ADC2D)| (1<<ADC1D)| (1<<ADC0D)  ;
// Turn off digital inputs on ADC8-12
   DIDR2  = (1<<ADC8D) | (1<<ADC9D)| (1<<ADC10D)| (1<<ADC11D);

    DDRF = 0;
    DDRK = 0;
}

At main.c:

ADCSRA |= (1 << ADSC); // start new conversion 

Did someone see something I could have overlooked ?

 

Attachment(s): 

This topic has a solution.
Last Edited: Wed. Sep 6, 2017 - 06:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andre_teprom wrote:
however channels 8-11 are returning Zero
Because you never really select any of these channels. Instead you are selecting other channels in differential mode.

Stefan Ernst

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

sternst wrote:
Because you never really select any of these channels. Instead you are selecting other channels in differential mode.

OP, do you see that point?  I'll paste the ADMUX chart below; 0-7 not shown...

 

At ADC ISR:

ISR(ADC_vect)
   {
   static int8_t val;

   val = ADCH;
   switch (ADMUX)
      {
      case adc0 : ADMUX = adc1; analoginput=0   ; break ;
      case adc1 : ADMUX = adc2; analoginput=1   ; break ;
      case adc2 : ADMUX = adc3; analoginput=2   ; break ;
      case adc3 : ADMUX = adc4; analoginput=3   ; break ;
      case adc4 : ADMUX = adc5; analoginput=4   ; break ;
      case adc5 : ADMUX = adc6; analoginput=5   ; break ;
      case adc6 : ADMUX = adc7; analoginput=6   ; break ;
      case adc7 : ADMUX = adc8; analoginput=7   ; break ;
      case adc8 : ADMUX = adc9; analoginput=8   ; break ;
      case adc9 : ADMUX = adc10; analoginput=9  ; break ;
      case adc10: ADMUX = adc11; analoginput=10 ; break ;
      case adc11: ADMUX = adc0 ; analoginput=11 ; break ;
      }
   adcresult[analoginput] = val ;
   }

Now, you can suit yourself.  But I wouldn't set up my ISR and conversion handling that way.

 

-- Yes, I very commonly use an array to handle "raw" results, just as with your adcresult[].

-- You already have analoginput as the index into that.  Good.

-- But use that index as your "master".  Eliminate that switch/case.  At best, it will be non-deterministic.  And you see from the chart the ADMUX as the switch variable is going to cause you problems, as MUX5 is in a different register.

 

Generally, in your type of situation, I have a table of ADMUX (and/or MUXn) values in a separate array.  The master is usually in EEPROM, but I make an SRAM copy because I don't want to do EEPROM work in an ISR.  Your case will need an extra step from the below to handle MUX5, and set/clear MUX5 in ADCSRB.

 

// **************************************************************************
// *
// *		A D C _ I S R
// *
// **************************************************************************
//
// ADC interrupt service routine
// with auto input scanning
interrupt [ADC_INT] void adc_isr(void)
{
// Read the AD conversion result
	adc_raw[ad_index] = ADCW;

// Select next ad_indexADC input
	if (++ad_index >= ad_count)
		{
		ad_index=0;
		}

	ADMUX = ADC_VREF_TYPE + adc_chan[ad_index];

// Start the AD conversion
	ADCSRA |= (1 << ADSC);

}

-- My adc_raw[] is your adcresult[].

-- My ad_index is your analoginput.

-- I start the next conversion in the ISR, so continuous round-robin conversions.

-- For good result, we need to know the drive of your signals.  There is another current thread with links to prior "bleed through" discussions.

 

Now, this particular app that I grabbed the code from doesn't use many channels, so kind of a trivial example.  It does a "double convert" on the two used channels; again see the bleed-through discussions.

 

// Double-convert the two channels and use the second conversion result
// (indices into adc_raw[])
#define	ADC_CONDX			0	// dummy conversion
#define	ADC_COND			1
#define	ADC_PRESSX			2	// dummy conversion
#define	ADC_PRESS			3

#define	ADC_CHANNELS		4 // "logical" channels; double-converted (high impedance)
#define FIRST_ADC_INPUT		1 // used when starting up the ADC

// Mapping of data channels 0,1,2,3 to ADC1, ADC1, ADC3, ADC3
__flash	unsigned char	adc_chan[ADC_CHANNELS] = {ADCH1, ADCH1, ADCH3, ADCH3};

// Bandgap channel
#define	ADC_BG		0x1e
// Ground channel
#define	ADC_GND		0x1f

I lied about the EEPROM above; in this app I used __flash. ;)

 

As mentioned, you will need to do extra steps to pull the value from the table; mask off the low 5 bits and apply to ADMUX; examine bit 6/MUX5 and apply the value to ADCSRB.

 

 

 

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 there,

 

 

But use that index as your "master"

Yes, I agree, and I'll do a further clean on this code, thanks.

 

I start the next conversion in the ISR, so continuous round-robin conversions.

Much better your approach than mine, now I can account a substantial increase on the amount of conversions made per second.

 

And you see from the chart the ADMUX as the switch variable is going to cause you problems, as MUX5 is in a different register.

But all the other bits at ADMUX are set to Zero, so this should not interfere on the initial #define ADMUX I made for each channel.

In fact, I forgot to handle MUX5 at ADCSRB register, but even doing it now it is still not working:

// From 0-7 channels
ADCSRB &= ~(1<<MUX5)
// From 8-11 channels
ADCSRB |=  (1<<MUX5) 

 

Note that MUX4 bit is allways set to 0.

 

I reviewed all your above inputs, focusing on the difference from what I made ( I mean, what I have overlooked ), but even taking in consideration the key parts, there are more things which have not covered, I guess. Anyway, thank all you once again.

 

 

Last Edited: Wed. Sep 6, 2017 - 05:28 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andre_teprom wrote:
but even doing it now it is still not working:

How are we supposed to respond to that without seeing any code?

 

And you need to tell more about "not working".  What voltage levels are on the pins?  What ADC counts do you expect?  What ADC counts are you getting?

 

Are you sure that Proteus knows how to handle all ADMUX combinations?

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.

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

andre_teprom wrote:
But all the other bits at ADMUX are set to Zero, so this should not interfere on the initial #define ADMUX I made for each channel.

 

Not true, if I understand your comment correctly.

 

#define adc8  ((1<<ADLAR) | 8     )

That should be MUX4:0 of 0, not 8.  (along with the MUX5 handling)

 

Did you study the Channel Selection chart in the datasheet?

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

That should be MUX4:0 of 0, not 8.  (along with the MUX5 handling)

You are absolutely right, despite the clear explanations that you provided above, I still had not been able to realize that the MUX3 bit was responsible for the problem.

Thank you.

 

Last Edited: Wed. Sep 6, 2017 - 06:07 PM