ADC Mux Value switching issue - Atmega2560

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

Hey all!
I'm currently doing some experimenting with the internal multiplexer in the Atmega 2560. 
I'm essentially switching between 2 potentiometers, reading their values and printing them on the screen. 

Caveats I think I avoided:
-After the Internal mux on the ADC is switched, there needs to be 125 micros seconds before that next read happens

-The very next read after this is garbage.

At first I was getting a reading of 1 knob for both values, since I wasn't waiting for the sample and hold circuitry to catch up.
I was still getting the same  result not taking a garbage read, so that addition fixed almost everything.

My only problem now, is that the reads are switched! The knob connected to O0 is changing the value for P0, and vice versa.

Things seem to work fine when de-bugging through J-tag, but when running, the values are switched. 

 

 

I haven't tried with additional knobs yet, but does anyone have any ideas?

I'd like to not use interrupts for this, since I've got a bunch of other stuff also going on that is a bit more timing dependent. (knobs can kindof be read whenever)

Here is the code:
 

#define F_CPU 16000000UL
#include <avr/io.h>
#include <util/delay.h>
#include "OLEDLib.h"

char adcO0[] = "O0: xxx             ";
char adcP0[] = "P0: xxx             ";
int readKnobO0 = 0;
int readKnobP0 = 0;

void startRead()
{
    ADCSRA |= (1 << ADSC); //this moves the read instruction bit to the ADC Register.
}
void initADC()
{
    DDRE = 0B00111000; //external MUX pins are E 5, 4, and 3. We'll not be using these for this test.
    PORTE = 0; //we make sure that all pins on this port are low, so mux is for sure at 0.
    
    ADCSRA = (1 <<  ADEN) | (1 <<ADPS2) | (1 << ADPS1) | (1 << ADPS0); //changing the pre-scaler from 128 to 0 doesn't seem to have any effect.
    ADCSRB = (1 << MUX5); // we're only reading the bottom registers for now.
    DIDR0 = 0xff; // we should set this register to all 1s, so there is no digital input triggering.
    DIDR2 = 0xff;
    ADMUX = 0B00000000;//we want to start at input 1, and use external reference reading ADC 8.
    startRead(); // do first read, should take 25 clock cycles.
}

int main(void)
{
    initADC();
    initScreen();
    
    /* Replace with your application code */
    while (1) 
    {
        //ADMUX = 0B00000000;
        _delay_us(125);
        startRead();
        readKnobO0 = (ADC >> 2);
        numPrinter(adcO0, 4, 3, readKnobO0); //function to convert an int to characters
        outputS(adcO0, 0); //printing the character array on the screen, at a specific line. (20x4 character display)
        
         ADMUX = 0B00000001;
         startRead();
         uint8_t trash = ADC;
        _delay_us(125);
         startRead();
         readKnobP0 = (ADC >> 2);
         numPrinter(adcP0, 4, 3, readKnobP0);
         outputS(adcP0, 1);
         
         ADMUX = 0B00000000;
         startRead();
         trash = ADC;
        
    }
}

Thanks for reading!

This topic has a solution.
Last Edited: Wed. Aug 14, 2019 - 08:13 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

After you start the conversion, you need to wait for completion before accessing ADC.

 

I'd suggest a "standard" routine, called with the desired channel, which sets the MUX, starts the conversion, waits for completion, and returns unsigned int.

 

Beyond that, show schematic and tell the voltages on all the pertinent pins -- right at the pin.

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

halfordC wrote:

 ADCSRB = (1 << MUX5); // we're only reading the bottom registers for now.

Huh?

 

 

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

theusch wrote:

After you start the conversion, you need to wait for completion before accessing ADC.

 

I'd suggest a "standard" routine, called with the desired channel, which sets the MUX, starts the conversion, waits for completion, and returns unsigned int.

 

Beyond that, show schematic and tell the voltages on all the pertinent pins -- right at the pin.

 

Throwing an really short delay between startConversion(); and the ADC read seemed to do the trick.
This doesn't seem like the optimal solution, is there a better way to do this involving the ADIF bit, or something else?
Either way, a delay for 13 clock cycles might not be that big of an issue, with all the timing specific stuff being driven by interrupts.
 

theusch wrote:

halfordC wrote:

 ADCSRB = (1 << MUX5); // we're only reading the bottom registers for now.

Huh?

 

 

I have 9 analog inputs, but I'm just using ADC 8-15 at the moment, so I'm not worrying about the 1 input I have in ADC 0.

But yeah, thanks for the pointer!

Last Edited: Wed. Aug 14, 2019 - 02:08 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Those are ADC clock cycles, not MCU clock cycles. 

 

Yes, just wait for ADIF to be set.You can do that in a wait loop or by polling in your main application loop.

 

Jim

Jim Wagner Oregon Research Electronics, Consulting Div. Tangent, OR, USA http://www.orelectronics.net

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

ka7ehk wrote:

Yes, just wait for ADIF to be set.You can do that in a wait loop or by polling in your main application loop.

Or wait for ADSC to be cleared.  The benefit is that no other action is required.  ADIF does not self-clear.

"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

Correct.

 

Jim

Jim Wagner Oregon Research Electronics, Consulting Div. Tangent, OR, USA http://www.orelectronics.net

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

Rather than "startRead()" I'd suggest you simply have something like:

uint8_t readADCChannel(uint8_t channel) {
    ADMUX = (REF_BITS_IF_USED) | (1 << ADLAR) | channel;
    ADCSRA |= (1 << ADSC);
    while (ADCSRA & (1 << ADSC));
    return ADCH; // only need ADCH when ADLAR used
}

then main() becomes:

    while (1) 
    {
        readKnobO0 = readADCChannel(0);
        numPrinter(adcO0, 4, 3, readKnobO0); //function to convert an int to characters
        outputS(adcO0, 0); //printing the character array on the screen, at a specific line. (20x4 character display)
        
         uint8_t trash = readADCChannel(1);
         readKnobP0 = readADCChannel(1);
         numPrinter(adcP0, 4, 3, readKnobP0);
         outputS(adcP0, 1);
         
         trash = readADCChannel(0);
    }

 

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

clawson wrote:

// only need ADCH when ADLAR used

OP also said "bottom registers", and you picked up on it, but there is no hint of ADLAR in the posted code, is there?

 

halfordC wrote:
This doesn't seem like the optimal solution, is there a better way to do this involving the ADIF bit, or something else?

Check out the Tutorials forum for pertinent articles. 

 

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

theusch wrote:
, but there is no hint of ADLAR in the posted code, is there?

readKnobO0 = (ADC >> 2);

In fact it was when I copy/pasted his original while(1) stuff to convert it to use the function I showed that I hit this >>2 and thought "Surely if he's doing a >>2 to get 8 bits then ADLAR is the more obvious way to do this?".

 

In fact the very raison d'etre for ADLAR is to prevent the need for this very >>2 ;-)

 

My original readADCChannel() had a uint16_t return but I changed this to uint8_t once I spotted and implemented the ADLAR thing.

Last Edited: Wed. Aug 14, 2019 - 01:14 PM