My code on ATtiny13A:

#include <avr/io.h>
#include <stdint.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <stdlib.h>

#include "dbg_putchar.h"

void ADC_init()
    // Set the ADC input to PB4/ADC2
    ADMUX |= (1 << MUX1);
    //ADMUX |= (1 << ADLAR);
    ADMUX |= (1 << REFS0);

    // Set the prescaler to clock/128 & enable ADC
    // At 9.6 MHz this is 75 kHz.
    // See ATtiny13 datasheet, Table 14.4.
    ADCSRA |= (1 << ADPS1) | (1 << ADPS0) | (1 << ADEN);

int adc_read (void)
    // Start the conversion
    ADCSRA |= (1 << ADSC);

    // Wait for it to finish
    while (ADCSRA & (1 << ADSC));

    uint8_t low  = ADCL; // must read ADCL first - it then locks ADCH
    uint8_t high = ADCH; // unlocks both

    int ADC_val = (high<<8) | low;

    return ADC_val;

void wyslij_wynik_pomiaru(int wynik)
    char str[4];
    itoa(wynik, str, 10);
    if (wynik > 9) dbg_putchar(str[1]);
    if (wynik > 99) dbg_putchar(str[2]);
    if (wynik > 999) dbg_putchar(str[3]);

int main(void)
    DDRB = _BV(1);
    int wynik_poczatkowy = adc_read();
    int wynik_nowy;
        wynik_nowy = adc_read();
        if (abs(wynik_poczatkowy-wynik_nowy) > 150) {
            wynik_poczatkowy = wynik_nowy;
            PORTB |= _BV(1); //turn on ESP8266
            wyslij_wynik_pomiaru(wynik_nowy); //send data
            PORTB  &= ~_BV(1); //turn off ESP8266

When you read the voltage level of the ADC and it is less than 1023, but steady, then everything works correctly: IF loop executes only once, however when the voltage level read on the ADC is equal to 1023, then the IF is executed continuously (condition is still met, but it should not).

Why is this happening and how to fix it?

  • \$\begingroup\$ I've looked carefully and I cannot find "1023" in your code anywhere. Please post the correct code. \$\endgroup\$
    – DoxyLover
    Nov 20, 2015 at 1:24
  • \$\begingroup\$ @DoxyLover ATTiny's have 10bit ADCs built in. The maximum value that can be returned from adc_read() is 1023. The "ADC=1023" bit is referring to the AVRs ADC(L/H) 16bit register which with the current settings is right aligned so the upper 6 bits will always be 0. \$\endgroup\$ Nov 20, 2015 at 1:26
  • \$\begingroup\$ Ok, I misread the question. \$\endgroup\$
    – DoxyLover
    Nov 20, 2015 at 1:29
  • \$\begingroup\$ Would you be able to upload the .elf file (produced when you compiled the code) to somewhere like dropbox. I can't see anything obvious which suggests there is an odd compiler optimisation happening somewhere. \$\endgroup\$ Nov 20, 2015 at 1:35
  • \$\begingroup\$ What signal are you feeding into the ADC? Please show the circuit with all relevant information (component values, voltages etc.) \$\endgroup\$ Nov 20, 2015 at 2:11

2 Answers 2


If ADMUX:ADLAR is set your code will crash & burn when you left shift high << 8, because you are shifting data into the sign bit of "high-promoted-to-int" = undefined behavior. Are you sure ADLAR is not set? You have commented it out. If ADLAR is not set, your code will probably work, by luck.

You need to read up on implicit integer promotion. In this shift operation, the uint8_t variable high will get implicitly promoted to an int, which is a signed int16 (assuming int is 16 bit), which is not what you want or need.

Also, in order to write deterministic, portable programs, get rid of the int type, it should never be used in embedded systems programming. Use uint16_t instead.

Also, (if ADLAR is not set) what makes you think bits 7 to 2 in ADCH contain jolly-good-data? The manual doesn't mention what reading those bits will give you.

To turn your code into production quality, change it in the following ways:

uint16_t adc_read (void)
    // Start the conversion
    ADCSRA |= 1 << ADSC;

    // Wait for it to finish
    while (ADCSRA & (1 << ADSC))
        ; // semicolon on separate line to show that this was intentional

    uint8_t low  = ADCL; // must read ADCL first - it then locks ADCH
    uint8_t high = ADCH; // unlocks both

    const uint8_t ADCH_DATA_MASK = 0x03; // the two MSB of 10 bit ADC value
    high = high & ADCH_DATA_MASK; // mask out data, discard junk

    return (uint16_t)high << 8 | (uint16_t)low;

Your main() code needs to be adapted to support the above changes. In main(), you may have to convert the uint16_t to int16_t for your algorithm to work.

  • \$\begingroup\$ Why a const instead of a pre-processor macro? \$\endgroup\$
    – David
    Nov 20, 2015 at 10:53
  • \$\begingroup\$ @David The mask is only needed locally. No need to clutter up the global namespace. The variable is really just there for self-documenting code purposes, to avoid code with "magic numbers" such as high = high & 0x03. \$\endgroup\$
    – Lundin
    Nov 20, 2015 at 11:51

This is just a hunch, but have you tried using unsigned ints for your ADC values and doing the comparison in a different way? Since 1023 is a power of two minus one, I suspect there's an arithmetic issue. Try something like this:

if (wynik_poczatkowy > wynik_nowy + 150 || wynik_nowy > wynik_poczatkowy + 150)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.