0
\$\begingroup\$

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;
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I was going to go edit your code block to make it easier to read, but boy is it messed up. Might be easier for you to just do it yourself. Edit your question, copy your code in fresh, and then highlight it, and select the code button. \$\endgroup\$ – Kellenjb Apr 21 '11 at 15:03
  • 3
    \$\begingroup\$ Generally, an interrupt should be handled 'as quickly as possible'. Doing things like an A to D conversion in an interrupt is not considered good design. Have your interrupt change a state variable so that your processing can be done in your main loop. \$\endgroup\$ – Erik Apr 21 '11 at 20:29
2
\$\begingroup\$

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;
    }   
\$\endgroup\$
0
1
\$\begingroup\$

The else only ties to the last if, not to all the preceeding ifs. If you want that it ties to all the preceeding ifs, use an if/else if/else if/else structure.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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