Changing ADC channel in free running mode always reads garbage.

Using Arduino Programming Questions
1

I've been having trouble getting an audio project to work so I've resorted to an empirical testing mode.

I want to read 2 ADC data channels in free running mode, changing channel only after a buffer is full. Each channel has a buffer of 256 bins. I have an ISR that triggers on each conversion, writes the 8-bit ADC value (ADCH only, with ADLAR = 1) to the buffer and increments a counter. Once the counter hits the end, it resets to zero and the channel is changed. Once both channel buffers are full, a flag to output the buffers is set. The buffer output is handled in the main loop and currently dumps the contents to the serial console.

The problem I have is that the second channel is always garbage. I've tried swapping the physical inputs, reordering the channels and using different input channels. The result is always the same. Random values in the 2nd channel, regardless of which channel I started with.

I've tried moving the channel change out of the ISR and into the main loop but it has no effect. Neither does increasing the prescaler.

I've run out of ideas. I've not used interrupts before and similar projects just serial poll the ADCs but I've read through the datasheet and can't figure out why it doesn't work. I've tried clearing and enabling the ADSC bit prior to and after the channel change, clearing and setting ADEN, but no joy.

Anyone else tackle this problem before?

The vars are:

#define SAMPLES   256 // Number of FHT input bins per channel.
#define CHANNELS    2 // Number of audio channels.

volatile int8_t   ADC_buffer[CHANNELS][SAMPLES];  // Ring buffers for ADC data.
volatile uint8_t  ADC_channel;                    // Keeps track of channel.
volatile uint16_t ADC_sample;                     // Keeps track of samples.
volatile boolean  ADC_buffer_full;                // Flags buffer is full.

const uint8_t admux[CHANNELS] = { ADC0D, ADC1D }; // ADC MUX channels.

The setup code is:

void setup( void )
{

  // ADCSRA.
  ADCSRA = B10101011;

  // ADEN  - 1  // Enable ADC.
  // ADSC  - 0  // Disable conversions.
  // ADATE - 1  // Enable ADC auto trigger.
  // ADIF  - 0  // Set by hardware
  // ADIE  - 1  // Enable interrupt.
  // ADPS2 - 0  // }
  // ADPS1 - 1  // }- ADC prescaler = 8.
  // ADPS0 - 1  // }

  // ADCSRB.
  ADCSRB = B00000000;

  // N/A        //
  // ACME  - 0  // Disable Analogue Comparator Mux.
  // N/A        //
  // N/A        //
  // N/A        //
  // ADTS2 - 0  // }
  // ADTS1 - 0  // }- Free Running Mode.
  // ADTS0 - 0  // }

  // ADMUX.
  ADMUX = B01100000;
  
  // REFS1 - 0  // }- Use Vcc as reference.
  // REFS0 - 1  // } 
  // ADLAR - 1  // Left adjust ADC results (8-bit mode).
  // N/A        //
  // MUX3  - 0  // } 
  // MUX2  - 0  // }- ADC input 0.
  // MUX1  - 0  // } 
  // MUX0  - 0  // } 

  //  Turning off some functions can reduce power consumption and reduce noise.
  //
  //  Timer0 is an 8-bit timer used for Arduino functions such as delay(),
  //  millis() and micros().
  //  DIDR0 and DIDR1 are digital input buffers that share circuitry with the
  //  ADC. Disabling them reduces noise.

  // TIMSK0.
  TIMSK0 - B00000000;

  // N/A        //
  // N/A        //
  // N/A        //
  // N/A        //
  // N/A        //
  // OCIE0B - 0 // Disable timer output compare B interrupt.
  // OCIE0A - 0 // Disable timer output compare A interrupt.
  // TOIE0  - 0 // Disable timer overflow interrupt.

  // DIDR0.
  DIDR0 = B00111111;

  // N/A        // }- Pins ADC7D & ADC6D do not have input buffers.
  // N/A        // }
  // ADC5D - 1  // Disable digital input ADC5.
  // ADC4D - 1  // Disable digital input ADC4.
  // ADC3D - 1  // Disable digital input ADC3.
  // ADC2D - 1  // Disable digital input ADC2.
  // ADC1D - 1  // Disable digital input ADC1.
  // ADC0D - 1  // Disable digital input ADC0.

  // DIDR1.
  DIDR1 = B00000011;

  // N/A        //
  // N/A        //
  // N/A        //
  // N/A        //
  // N/A        //
  // N/A        //
  // ACIS1 - 1  // Disable digital input AIN1D.
  // ACIS0 - 1  // Disable digital input AIN0D.

  // Clear buffer.
  memset( (void *) ADC_buffer, 0, sizeof( ADC_buffer ));

  // Set up counters and flags.
  ADC_sample = 0;
  ADC_channel = 0;
  ADC_buffer_full = false;
  ADC_change_channel=false;
  cycle = 0;

  Serial.begin( 115200 );

  startADC(); // Start conversions.

}

and the ISR is:

ISR( ADC_vect )
{
  
  // Fill next available buffer slot with ADC value.
  ADC_buffer[ADC_channel][ADC_sample] = ADCH - 0x80; // Read ADC value.

  // If the ADC buffer for the channel is full, switch channel.
  if ( ++ADC_sample >= SAMPLES )
  {
    ADC_sample = 0;

    if ( ++ADC_channel >= CHANNELS )
    {
      ADC_channel = 0;
      ADC_buffer_full = true; // ADC sample buffer is full.
    }

    // Switch to next channel.
    ADMUX = ( ADMUX & ~0x0f ) | ( admux[ADC_channel] & 0x0f );
    
  }
}
2

Anyone else tackle this problem before?

Yes see datasheet.

Mark

3

I've never found the datasheet to be very helpful with this issue. It does explain that if you change the channel in the interrupt triggered when the conversion is completed, your next conversion is still the previous channel.

I have got this to work in test code but never got it to the point where I wanted to rely on it for production code. Just lower your expectations for sample speed and use micros() and regular analogRead().

4

@holmes4 Radical!

@MorganS Thanks. It's a but of a weird one. I've gone over the datasheet and understand that the channel change won't be set until the previous conversion is finished. I've seen plenty of code that uses free running with no ISR and changes channels. I just don't get why it doesn't work with an ISR.

5

I've disabled free running mode and start a new conversion at the end of the ISR, but I'm still getting garbage on the next channel until it reverts back to the original channel.

Ugh!

6

You put "digital input disable register" as multiplex, should be MUX0 MUX1, I think.

#const uint8_t admux[CHANNELS] = { ADC0D, ADC1D }; // ADC MUX channels.
const uint8_t admux[CHANNELS] = { MUX1,  MUX0 }; // ADC MUX channels.
7

Thanks MasterT. I'll give that a go. The ADCxD values do work with polled reading but that may just be coincidental. MUXx looks more correct so I'll do some recapping and figure out where I got the info from. I'll do some serial prints too.

I have found the bug that was causing the random garbage on the second channel but have since changed the routine to single reads. I'll revert back to see if it makes any difference with free running mode and I'll close this thread once I have the ISR routine working either way, but in the meantime the error was actually in the memcpy...

I had

memcpy( ADC_copy, ADC_buffer, SAMPLES );

but should have had:

memcpy( ADC_copy, ADC_buffer, sizeof( ADC_buffer ));

I really should have spotted that one earlier because my init of the array was ok.

8

OK, free running mode does work but I can't see any real speed benefit when serial printing. After everything done here, and reading a lot of posts on the topic, I think that single conversion mode is easier to track. There may be some performance improvement if more of the ISR code was moved to the main loop.

Because I am reading a full sample before changing channels, there is an obvious lag between them when plotting the waveforms for a simple sine wave input. I don't think there will be much of a performance hit if I changed channels after every reading, but that's something I'll look into later.

In order to close this thread, the code works and is attached. It will eventually find it's way to my github repo, GitHub - alidaf/arduino: Teensy projects, but bear in mind it doesn't do much but print out the sampled inputs to the serial console. I'm also only using 8-bit mode to gain some speed and considering what I intend to do with it. It will eventually form the basis of some visual displays such as a spectrum analyser, VU meter or other mood type display on an LED strip so accuracy is not an issue. These projects will also appear at my github repo once they are done. I'm sure that plenty of improvements can be made but it's there for anyone to improve on.

For free running mode change:

ADCSRA = B10001011; to ADCSRA = B10101011;

and remove

startADC(); from the ISR.

ADC_tests.ino (8.5 KB)