# Strange behaviour of Attiny's ADC

#include <avr/io.h>
#include <util/delay.h>

void init(){
// LED output.
DDRB |= 1<<4;
// Turn on ADC on third pin, continuous mode, prescaler 128.
}

int main(){
init();

while(1){
// _delay_ms(1);

PORTB |= 1<<4;
}
else{
// PORTB = 0;
PORTB &= ~(1<<4);
}
}
}


This is the code I have running on my ATtiny13A. There is an LED connected to pin B4, and potentiometer to B3. The code is supposed to turn on the LED if the analog value is greater than 512 (half of maximum 1024). It doesn't work though, and LED seems to be disabled all the time.

Normally I would assume there is something wrong in configuration of peripherals, but there are some very strange things happening. For example, if I change PORTB &= ~(1<<4); to PORTB = 0 (which in this case should do exactly the same thing), the code magically starts working just as expected. Another thing I tried is changing LED status using simple counter by uncommenting the two lines near _delay_ms - and again, LED starts blinking as expected.

The only time when this code does not work is when it's in exactly this form - so there must be something wrong with combination of ADC reading and output port bit writing.

Why does this happen?

• One more thing - even with the code as shown, it seems as though the LED could be turned on ONE time - so if the potentiometer started low (with LED off), I could turn LED on, then off, but the next time I tried turning potentiometer, it fails. This is so weird... – akrasuski1 Nov 27 '17 at 1:57
• OK, I have a good lead - if I use PORTB = 0 (the line that worked), but then add a single nop afterwards, it gets broken again. It appears to be a timing issue for some reason - adding 4 nops fixes it again... – akrasuski1 Nov 27 '17 at 2:27

1. Incorrect 16 bit register access pattern

In C, the evaluation order of the | operator is undefined, so this code is could be reading ADCH first, and then reading ADCL. Once you read ADCL then you have locked the data register until the next pass though the loop, which is certainly not what you want.

Try changing this line to...

adc = ADC;


(the C compiler should generate the correct access order coding).

This diagram strongly suggests that the ADC register is updating up until ADIF goes high...

I'd expect that new bits are shifted into place with each successive approximation, and the extra delay of the nop could be enough so that the ADC is ready by the time you hit it just by chance.

Try replacing...

adc = (ADCH << 8) | ADCL;


...with...

while (! ADCSRA & _BV(ADIF) );

• "In C, [...] the | operator is evaluated left to right" - this is not correct. The evaluation order is, in fact, unspecified (unlike, say, for ||, where it is indeed left-to-right to permit short-circuit evaluation). – NPE Nov 27 '17 at 9:21