ATMEGA328p ADC keeps returning 1023

Question

I'm trying to configure my ATMEGA328p to measure temperature using the TMP36GZ temperature sensor. However, the 10-bit ADC keeps returning 1023. I've tried disconnecting the temperature sensor (it should return 0 at this point for the ADC reading), but it still returns 1023. I'm not exactly sure what I'm doing wrong in my code:

volatile uint16_t TMP36_VoutADCReading = 0U;

void TMP36_Init( void )
{
   // Set the ADC prescaler to 128 (i.e., 16MHz/128 = 125KHz)
   ADCSRA |= ( 1 << ADPS2 ) | ( 1 << ADPS1 ) | ( 1 << ADPS0 );

   // Set the voltage reference from AVcc (i.e., 5V).
   ADMUX |= ( 1 << REFS0 );

   // Turn on the ADC.
   ADCSRA |= ( 1 << ADEN );

   // Do the initial conversion (i.e., the slowest conversion)
   // to ensure that everything is up and running.
   ADCSRA |= ( 1 << ADSC );
}

void TMP36_ADCRead( uint8_t channel )
{
   // Clear the previously read channel.
   // ADCSRA &= 0xf0;                      <-- This was causing the bug.
   ADMUX &= 0xf0;                          // <-- This is the fix. Thanks IgnacioVazquez-Abrams!

   // Select the ADC channel to be read.
   ADMUX |= channel;

   // Start a new conversion. By default, this conversion will
   // be performed in single conversion mode.
   ADCSRA |= ( 1 << ADSC );

   // Wait until the conversion is complete.
   // while( ADCSRA & ( 1 << ADSC ) );        <-- This is probably also correct.
   while( ADCSRA & ( 1 << ADIF ) );           // <-- Added. Thanks Adithya!

   // Obtain the ADC reading from Vout.
   TMP36_VoutADCReading = ADC;

   // Clear the ADIF flag.
   ADCSRA |= ( 1 << ADIF );                    // <-- Just added. Thanks Adithya!
}

int main( void )
{
   // Disable all interrupts for the time being.
   cli();

   // Initialize the TMP36GZ temperature sensor.
   TMP36_Init();

   // Enable all interrupts.
   sei();

   while( 1 )
   {
      // Compute the distance.
      uint8_t channel = 1;
      TMP36_ADCRead( channel ); // At this point, TMP36_VoutADCReading = 1023
   }
}

In terms of connections, I'm trying to read the output voltage from pin PC1 on the ATMEGA328p and I have AVcc connected to VCC and pin 22 (i.e., GND) connected to pin 8's GND. I have disabled setting the voltage reference from AREF, so AREF isn't connected.

Any help would be greatly appreciated.

UPDATE: IgnacioVazquez-Abrams pointed out that ADCSRA &= 0xf0; should have been ADMUX &= 0xf0;. I've since modified the code above. Also, thanks Adithya for the suggestion you made about checking the ADIF flag! I'll take it into consideration.

Answer 1

The solution provided by IgnacioVazquez-Abrams was correct. Apparently, I was incorrectly clearing the lower 4 bits in ADCSRA when I should have been doing it to ADMUX. I've updated the code above to reflect this and the errors I've made in the code have been commented out (they are still there for reference).

Answer 2

// Wait until the conversion is complete. while( ADCSRA & ( 1 << ADSC ) )

This is wrong. You should be using ADIF to check if the conversion is complete. So,the code would be

while(!(ADCSRA&(1<<ADIF)); 
 or
while(ADCSRA&(1<<ADIF)==0);

This waits for the ADIF flag to be set once the conversion is complete. Then you have to clear the flag manually by setting the ADIF flag in order for the next conversion to start:

ADCSRA|=(1<<ADIF);

That said,why don't you stick on to ISRs? That would make life a hell lot easier.