Problem comparing ADC value in assembly

[azzammasood]

Hello all. I'm writing a code to compare incoming ADC value to check if it is greater than 2.5V (which maps to 1000 in ADC value and is equal to 0x03E8 in hex). I'm using 2.56V internal Vref, and using CPI instructions to compare ADCH with 0x03 and ADCL with 0xE8. But the code isn't working, it seems to be logically right.

    LDI R16, 0xFF    //To make PORTB as output for LED
    OUT DDRB, R16    //Make PORTB output       
    LDI R16, 0
    OUT DDRC, R16    //Make PORTC an input for ADC
    LDI R16, 0x87
    STS ADCSRA, R16
    LDI R16, 0xC0
    STS ADMUX, R16

READ_ADC:
    LDS R16, ADCSRA    // Read the status of ADCSRA to start conversion
    ORI R16, (1<<ADSC) // Modify to start conversion
    STS ADCSRA, R16    // Write to start conversion
KEEP_POLLING:
    LDS R17, ADCSRA
    SBRC R17, ADSC
    RJMP KEEP_POLLING
    LDS R19, ADCL
    LDS R18, ADCH
COMPARE_HIGHER_BYTE_OF_ADC:
    CPI R18, 0x03
    BRSH COMPARE_LOWER_BYTE_OF_ADC
    RJMP READ_ADC
COMPARE_LOWER_BYTE_OF_ADC:
    CPI R19, 0xE8
    BRSH SET_AND_UNSET_PB5
    RJMP READ_ADC

SET_AND_UNSET_PB5:
    SBI PORTB, 5
    LDI R20, 100
    LOOP1:
        LDI R21, 100
        LOOP2:
            LDI R22, 100
            LOOP3:
                DEC R22
                CPI R22, 0
                BRNE LOOP3
                DEC R21
                BRNE LOOP2
                DEC R20
                BRNE LOOP1
    CBI PORTB, 5
    RET

 

[Diver300]

You've got a return and without a call.

SET_AND_UNSET_PBS has RET at the end of it, so you should replace the RET with RJMP READ_ADC

Alternatively, you could call SET_AND_UNSET_PBS instead of jumping to it, and then have a jump to READ_ADC after that.

You code should be OK if the largest possible value of ADCH is 3. However, the code won't work if ADCH can be a larger value, or if you ever want to compare with a value of 0x2FF or less.

When performing a two-byte compare, it's normal to compare the high byte first. However there are really 3 outcomes to that.
high byte is smaller -> result is smaller
high byte is the same -> compare the next byte
high byte is larger -> result is larger

Your code is grouping the same and larger together, and treating them as though the high byte is the same. If the high byte can ever be larger than 3, or you want to compare with a smaller number, you code will only work for some results.

On a hardware point, if you are applying more than the reference voltage to the ADC pin, you need to check how the ADC behaves in that condition.