Pic 18F4550: local variable problem:

cmicrochipmicrocontroller

I'm using the pic18F4550 with microchip v8.63 and with the C 18 compiler. I'm using a LDR that retrieve the value of the led (not on my picdem board) (red, green and blue) these values are stored in a variable after each conversion. Afther that when I press the button S2, I come into the method ISR: this part works.

But now: I try to compare the variable red, green and blue in the if's: but I think that it not happen, he just go to my 'else' (led RB3 on my picdem board burns).

#include <p18f4550.h>  

/** V E C T O R  R E M A P P I N G *******************************************/

extern void _startup (void);        // See c018i.c in your C18 compiler dir
#pragma code _RESET_INTERRUPT_VECTOR = 0x001000
void _reset (void)
{
    _asm goto _startup _endasm
}
#pragma code
void ISR (void);
#pragma code _HIGH_INTERRUPT_VECTOR = 0x001008
void _high_ISR (void)
{
      _asm goto ISR _endasm
}

#pragma code _LOW_INTERRUPT_VECTOR = 0x001018
void _low_ISR (void)
{
    ;
}
#pragma code
/******************************************************************************/

// global variable, value off LDR.
unsigned int var1ADRESH = 0x00;
unsigned int color_red = 0;
unsigned int color_green = 0;
unsigned int color_blue = 0;

void main (void)
{   
    TRISD = 0x00;               // PORTD  als uitgang

    RCONbits.IPEN = 0;          // prioriteit uit
    INTCONbits.GIE = 1;         // enable interrupt
    INTCONbits.RBIE = 1;        // interrupt portB aan

    //= set up port =
    TRISAbits.TRISA0 = 1;           // Set RA0/AN0 to input
    //leds
    TRISAbits.TRISA3 = 0;
    TRISAbits.TRISA4 = 0;
    TRISAbits.TRISA5 = 0;
    LATAbits.LATA3 = 1;
    LATAbits.LATA4 = 1;
    LATAbits.LATA5 = 1;

    ADCON0 = 0b00000000;            // Set channel select to AN0
    ADCON1 = 0b00001110;            // Configure RA0/AN0 as analogue
    ADCON2 = 0b10101010;            // Right justified result
                                    // TAD 12 and FOSC 32 - may need to adjust this
                                    // depending on your clock frequency (see datasheet)

    while(1)
    {   
        _asm sleep _endasm  
    }
}

#pragma interrupt ISR
void ISR (void)
{
    if (INTCONbits.RBIF==1) {

        //conversie blauw
        LATAbits.LATA3 = 0;
        ADCON0bits.ADON = 1;            // Enable ADC   
        // read LDR value.
        ADCON0bits.GO = 1;              // Set the GO bit of the ADCON0 register to start the conversion.
        while (ADCON0bits.GO);          // Wait until the conversion is complete.
        ADCON2bits.ADFM = 0;            // read result as 8-bit. (dus data in ADRESH) ! 
        //= read data in ADRESH =
        var1ADRESH = ADRESH;    // reading value from LDR

        color_blue = ADRESH; //waarde in blauw

        //conversie rood
        LATAbits.LATA3 = 1;
        LATAbits.LATA4 = 0;

        ADCON0bits.ADON = 1;            // Enable ADC   
        // read LDR value.
        ADCON0bits.GO = 1;              // Set the GO bit of the ADCON0 register to start the conversion.
        while (ADCON0bits.GO);          // Wait until the conversion is complete.
        ADCON2bits.ADFM = 0;            // read result as 8-bit. (dus data in ADRESH) ! 
        //= read data in ADRESH =
        var1ADRESH = ADRESH;    // reading value from LDR

        color_red = ADRESH; //waarde in blauwe steken

        //conversie groen
        LATAbits.LATA4 = 1;
        LATAbits.LATA5 = 0;

        ADCON0bits.ADON = 1;            // Enable ADC   
        // read LDR value.
        ADCON0bits.GO = 1;              // Set the GO bit of the ADCON0 register to start the conversion.
        while (ADCON0bits.GO);          // Wait until the conversion is complete.
        ADCON2bits.ADFM = 0;            // read result as 8-bit. (dus data in ADRESH) ! 
        //= read data in ADRESH =
        var1ADRESH = ADRESH;    // reading value from LDR

        color_green = ADRESH; //waarde in blauwe steken

        // alles uitzetten
        //PORTB = 0b1111111;
        LATAbits.LATA5 = 1;

        if(color_blue > color_red && color_blue > color_green){
            //blauw
            LATDbits.LATD0 = 1;
        }
        if(color_red > color_blue && color_red > color_green){
            //rood
            LATDbits.LATD1 = 1;
        }
        if(color_green > color_red && color_green > color_blue){
            //groen
            LATDbits.LATD2 = 1;
        } 
        else {
            LATDbits.LATD3 = 1;
        }   
    }
    INTCONbits.RBIF = 0;
}

Best Answer

Just to mak Martin's response clearer:

if(color_blue > color_red && color_blue > color_green){
    //blauw
    LATDbits.LATD0 = 1;
}
if(color_red > color_blue && color_red > color_green){
    //rood
    LATDbits.LATD1 = 1;
}
if(color_green > color_red && color_green > color_blue){
    //groen
    LATDbits.LATD2 = 1;
} 
else {
    LATDbits.LATD3 = 1;
}   

should become

    if(color_blue > color_red && color_blue > color_green){
        //blauw
        LATDbits.LATD0 = 1;
    }
    else if(color_red > color_blue && color_red > color_green){
        //rood
        LATDbits.LATD1 = 1;
    }
    else if(color_green > color_red && color_green > color_blue){
        //groen
        LATDbits.LATD2 = 1;
    } 
    else {
        LATDbits.LATD3 = 1;
    }