Problem with Interrupts - PIC18F1220

[Supercap2F]

Hello Everyone!

As most of you probably know, I have been trying to figure out PICs for a while now. Well I think I have made a lot of progress from googling around and reading a lot of data sheets. But right now I'm trying to figure out interrupts. I have read some articles and the section of the PIC18F1220 data sheet that explains it, and I think that I have an OK grasp on it (emphasis on 'think' ).

So, I decided to write up some code from what I understand about them, here it is:

/* This code was written on 1/21/15, and is made for
* leaning how to use an Interrupt. The author is
* Supercap2F.
*
* The code flashes an LED until a button on RB0 is
* pressed. Upon the press, the code interrupts and
* turns the LED on for two seconds, and then goes
* back to flashing the LED.
*/
// Main MCU Setup ********************************
#include <xc.h> //include main PIC header
#include <delays.h> //include delay file
/*                                  | Turns off the Watch Dog Timer
*                         | Turns on the Power Up Delay
*             | Turns the Internal RC Clock on.
*/
#pragma config OSC=INTIO2, PWRT=ON, WDT=OFF
/*                                   | Turns off the low Voltage Programing Mode
*                        | Turns off In Circuit Debugging
*             | Turns off the MCLRE function
*/
#pragma config MCLRE=OFF, DEBUG=OFF, LVP=OFF

// Interrupt code *********************************
void interrupt isr(void) //Interrupt service routine
{
    INTCONbits.INT0IF=0; //set flag back to orignal state
    PORTAbits.RA0=1;     //turn on RA0
    Delay10KTCYx(400);   //delay for 2s
}

// Main Program ***********************************
void main(void)
{
    // General Register Setup *********************
    TRISA=0x00; //All RAx I/O pins are set as outputs
    PORTA=0x00; //Turn off all PORTA outputs
    TRISB=0x01; //RB0 is set as an input, all others are outputs
    PORTB=0x00; //Turn off all PORTB outputs

    // Oscillator Mode *****************************
    OSCCONbits.IRCF0=1; //sets clock to 8MHz
    OSCCONbits.IRCF1=1;
    OSCCONbits.IRCF2=1;

    // Interrupt Setup ****************************
    RCONbits.IPEN=0;     //Turn off interrupt priority
    INTCONbits.INT0IE=1; //Enables interrupt on INT0 change
    INTCONbits.PEIE=0;   //Disable all peripheral interrupts
    INTCON2bits.INTEDG0=1; //Sets interrupt to trigger on the rising edge
    //INTCONbits.INT0IF INT0 flag bit

    INTCONbits.GIE=1; //enable interrupts
    /**** Main Program ****/
    while(1) //endless loop
    {
        PORTAbits.RA0=1;  //turn on RA0
        Delay10KTCYx(40); //delay for 0.1s
        PORTAbits.RA0=0;  //turn off RA0
        Delay10KTCYx(40); //delay for 0.1s
    }
}

What it's supposed to do is blink an LED on and off until a switch is pressed and an interrupt is created. The interrupt holds the LED on for two seconds and then returns to where the code was last. The LED is on pin RA0 and the switch is on RB0 (INT0). Here's my breadboard setup:

The LED blinks as expected, but when the switch is pressed nothing happens...

I wonder if one you guys could point me in the right direction? I really don't know what to do to fix it.
Thanks very much!!!
Dan

EDIT: Opps, forgot to say that the data sheet can be found here.

 

[Harald Kapp]

I'm not familiar with PICs, but I think you have an issue with the ISR:

void interrupt isr(void) //Interrupt service routine
{
    INTCONbits.INT0IF=0; //set flag back to orignal state
    PORTAbits.RA0=1;     //turn on RA0
    Delay10KTCYx(400);   //delay for 2s
}

1. How would the ISR know by which event it has to be triggered? I'd expect to have the source of the interrupt (the portpin where the pushbutton is connected) to be an argument of the ISR. Otherwise if there are multiple interrupt sources in a system (and there can be many!), how are you going to distinguish between the sources?
2. Never ever put a delay into an interrupt routine. Interrupt routines are menat to be short routines that handle the interrupt rather quickly and return to the main task of the CPU. In this case you better use a global boolean variable (e.g. button_pressed) which you set to FALSE before starting the main WHILE loop. The interrupt routine would then set button_pressed=TRUE instead of actually performing the delay. Within the WHILE loop you check for the state of button_pressed. If FLASE, continue with the 0.1s blink routine. If TRUE, turn on the LED for 2s.

 

[KrisBlueNZ]

I'm also not too familiar with PICs, but I can answer Harald's suggestions.

The PIC has only one interrupt vector (actually, the "high-end" 8-bit devices like this one have two: low-priority and high-priority, if this feature is enabled). The interrupt handler has to go through and test the interrupt request flags of each peripheral whose interrupt is (or could be) enabled, and handle each one, usually by calling the appropriate handler subfunction!

This is one of the worst of many poor choices that the PIC architecture designers made. Some compilers will try to make this kludge invisible to the user by generating the umbrella interrupt handler that tests for interrupt source flags, resets the flags explicitly (most peripherals require this), and calls user-written handlers, but the XC8 compiler Dan is using doesn't generate this umbrella interrupt handler. Since he has only one interrupt source enabled, he doesn't need to test the interrupt flag.

This program is just designed to demonstrate how interrupts work. In real code you would not normally put a two-second delay inside an interrupt handler - in most code architectures that would be a very bad thing to do. But for this purpose, it should at least work as described.

I've had a look through the code and the data sheet and there are several problems but I can't see anything that will definitely prevent the interrupt handler from firing the first time.

 

[BobK]

Any of the pins that can be analog inputs are configured as analog on reset.

You need to make B0 a digital input by turning off the analog feature.

Try:

ADCON1 = 0x70;

In your PORT setups.

(See example 10-2 in your datasheet)

Bob

 

[KrisBlueNZ]

Ah, the analog input "feature" that's ON by default. I remember making a mental note to watch out for that. Then obviously, I forgot! Good catch Bob.

 

[BobK]

I work with PICs enough that it is automatic to turn off analog first thing.

There is a good reason why they do it. If an input pin is connected to an analog signal which happens to be around Vdd/2 when the PIC comes up, it could cause shoot through it the pin were set to digital.

Bob

 

[Supercap2F]

Any of the pins that can be analog inputs are configured as analog on reset.
You need to make B0 a digital input by turning off the analog feature.

Awesome!! It works perfect now! Thanks so much Bob!! I probably would have never figured that out!

Never ever put a delay into an interrupt routine. Interrupt routines are menat to be short routines that handle the interrupt rather quickly and return to the main task of the CPU. In this case you better use a global boolean variable (e.g. button_pressed) which you set to FALSE before starting the main WHILE loop. The interrupt routine would then set button_pressed=TRUE instead of actually performing the delay. Within the WHILE loop you check for the state of button_pressed. If FLASE, continue with the 0.1s blink routine. If TRUE, turn on the LED for 2s.

Okay, I didn't know that! I did see some code examples of interrupts though and they didn't have a delay in the interrupt, they just set a variable like you said!

One thing that I'm still a little confused about though is on page 75 of the data sheet under the INTCON register:

And again on page 76 under the INTCON2 register:

These are the only two times I can find a reference to a interrupt on one of the RB7 through RB4 pins. There where no bits that where made to enable an interrupt on those pins... Am I missing something here?
Dan

 

[KrisBlueNZ]

The RBIE bit is in INTCON, along with RBIF.

Now you have the code working, how about changing it so the LED flash interval is controlled by a variable, and having the interrupt handler modify the variable somehow. The variable should be declared volatile.

 

[Supercap2F]

The RBIE bit is in INTCON, along with RBIF.

Lol! Sorry about that, I totally missed it!

Now you have the code working, how about changing it so the LED flash interval is controlled by a variable, and having the interrupt handler modify the variable somehow. The variable should be declared volatile.

Yeah, okay! I'll have to see if I can get the priority modes working too. But I don't understand why I should declare the variable as volatile? According to the book on C I have, it really doesn't do much nowadays...

Thanks for the help Bob, Kris, and Harald!!
Dan

 

[BobK]

volatile serves a couple of purposes and is very important in microcontroller programming.

What it means is either of 2 things:

1. Changing this variable has side effects which are not obvious via analysis of the program.

As an example, many hardware registers are used on PICs. Say the compiler puts a value in a hardware register variable, and never reads this value. If this were a normal variable, a compiler might say: "this variable is never read, so writing to it makes no difference". In reality, writing to it cause a change to an output port, for instance. Declaring that variable volatile forces the compiler to issue the write instruction even though it sees no other use of the variable.

2. The variable might change at an unexpected time, change independently from the program.

Reading an input port would be a good example of this. The compiler might think that once you have read the input port, no need to read it a second time, I can just keep track of what it read last time. But with a hardware port, the value is changed by external hardware that the compiler knows nothing about. In this case, marking it volatile tells the compiler that even though it sees no change to the variable, it must read it each time the programmer tells it to.

Variables used or changed in an interrupt routine are basically the same, the compiler can see a sequence of code that does not change a variable but reads it multiple times. It might decide to read it just once. But, if it is changed in an interrupt routine, this would not work.

For example, let's say STOP is a flag that you set in an interrupt routine to tell the program to exit a loop:

while (!STOP) do {/* no reference to STOP in here */};

if STOP is not marked volatile, the compiler is perfectly okay to assume that it cannot change in the loop, so it reads it once before the loop and then either never executes the loop or makes it an infinite loop. Marking the variable volatile prevents the compiler from making that assumption.

Bob

 

[Supercap2F]

Awesome Bob! Thanks for explaining that! I have made a copy of what you said and put in in my book on C where it talks about the volatile keyword.

I also did some more programming on the interrupts and figured out the priority modes! Here's the source code (I put it in a spoiler to keep everything nice and tidy):

Spoiler: Priority Mode

/* This code was written on 1/23/15, and is made for
* leaning how to use an Interrupt. The author is
* Supercap2F.
*
* this code has three mode of flashing an LED.
* The zero mode is the default state, it flashes
* the LED at 2.5Hz. The one mode is turned on when
* the button on RB0 is pressed. It holds the LED
* on for 3 seconds. The two mode is turned on when
* a button on RB1 is pressed. It flashes the LED
* at 1Hz for 3 seconds.
*
* If the button on RB0 is pressed while mode two
* is playing, it plays mode one and then goes back
* to playing mode two. But if the button on RB1 is
* pressed while mode one is playing, it does nothing
* until mode one is done.
*
*/
// Main MCU Setup ********************************
#include <xc.h> //include main PIC header
#include <delays.h> //include delay file
/*                                  | Turns off the Watch Dog Timer
*                         | Turns on the Power Up Delay
*             | Turns the Internal RC Clock on.
*/
#pragma config OSC=INTIO2, PWRT=ON, WDT=OFF
/*                                   | Turns off the low Voltage Programing Mode
*                        | Turns off In Circuit Debugging
*             | Turns off the MCLRE function
*/
#pragma config MCLRE=OFF, DEBUG=OFF, LVP=OFF

// Global Variables *******************************
int x;

// Interrupt code *********************************
void interrupt high_priority isr(void) // High Priority Interrupt Service Routine
{
    INTCONbits.INT0IF=0; // set flag back to original state
    PORTAbits.RA0=1;     //
    Delay10KTCYx(600);   // delay for 3 seconds
}                        // return

void interrupt low_priority isrtwo(void) // Low Priority Interrupt Service Routine
{
    INTCON3bits.INT1IF=0;    // reset INT1 flag bit
    int x;                   // define variable x
    for(x=0;x<3;x++)         //
    {                        //
          PORTAbits.RA0=1;   // turn on the LED on RA0
          Delay10KTCYx(200); // delay for 0.5s
          PORTAbits.RA0=0;   // turn off the LED on RA0
          Delay10KTCYx(200); // delay for 0.5s
    }                        //
}                            // return

// Main Program ***********************************
void main(void)
{
    // General Register Setup *********************
    ADCON1=0x70;
    TRISA=0x00; // All RAx I/O pins are set as outputs
    PORTA=0x00; // Turn off all PORTA outputs
    TRISB=0x03; // RB0 and RB1 are set as inputs, all others are outputs
    PORTB=0x00; // Turn off all PORTB outputs

    // Oscillator Mode *****************************
    OSCCONbits.IRCF0=1; // sets clock to 8MHz
    OSCCONbits.IRCF1=1;
    OSCCONbits.IRCF2=1;

    // Interrupt Setup ****************************
    RCONbits.IPEN=1;       // Turn on interrupt priority
    INTCONbits.PEIE=0;     // Disable all peripheral interrupts

    INTCON2bits.INTEDG0=1; // Sets INT0 to trigger on the rising edge
    INTCONbits.INT0IE=1;   // Enables interrupt on INT0 change
                           // INT0 is automatically set at high priority

    INTCON2bits.INTEDG1=1; // Sets INT1 to trigger on the rising edge
    INTCON3bits.INT1IP=0;  // Sets INT1 as a low priority interrupt
    INTCON3bits.INT1IE=1;  // Enables interrupt on INT1 change

    // INTCONbits.INT0IF INT0 flag bit
    // INTCONbits.INT1IF INT1 flag bit

    INTCONbits.GIEH=1; // enable interrupts
    INTCONbits.GIEL=1;
    /**** Main Program ****/
    while(1) //endless loop
    {
        PORTAbits.RA0=1;  //
        Delay10KTCYx(40); // delay for 0.1s
        PORTAbits.RA0=0;  //
        Delay10KTCYx(40); // delay for 0.1s
    }
}

And I also Changed the original source code to use variables instead of delays:

Spoiler: Variable Source

/* This code was written on 1/23/15, and is made for
* leaning how to use an Interrupt. The author is
* Supercap2F.
*
* The code flashes an LED until a button on RB0 is
* pressed. Upon the press, the code interrupts and
* turns the LED on for two seconds, and then goes
* back to flashing the LED.
*/
// Main MCU Setup ********************************
#include <xc.h> //include main PIC header
#include <delays.h> //include delay file
/*                                  | Turns off the Watch Dog Timer
*                         | Turns on the Power Up Delay
*             | Turns the Internal RC Clock on.
*/
#pragma config OSC=INTIO2, PWRT=ON, WDT=OFF
/*                                   | Turns off the low Voltage Programing Mode
*                        | Turns off In Circuit Debugging
*             | Turns off the MCLRE function
*/
#pragma config MCLRE=OFF, DEBUG=OFF, LVP=OFF

// Global Variables *******************************
volatile int LED_State;

// Interrupt code *********************************
void interrupt isr(void) //Interrupt service routine
{
    INTCONbits.INT0IF=0; //set flag back to original state
    LED_State=1;         //set LED_State to TRUE
}

// Main Program ***********************************
void main(void)
{
    // General Register Setup *********************
    ADCON1 = 0x70;
    TRISA=0x00; //All RAx I/O pins are set as outputs
    PORTA=0x00; //Turn off all PORTA outputs
    TRISB=0x01; //RB0 is set as an input, all others are outputs
    PORTB=0x00; //Turn off all PORTB outputs

    // Oscillator Mode *****************************
    OSCCONbits.IRCF0=1; //sets clock to 8MHz
    OSCCONbits.IRCF1=1;
    OSCCONbits.IRCF2=1;

    // Interrupt Setup ****************************
    RCONbits.IPEN=0;       //Turn off interrupt priority
    INTCONbits.INT0IE=1;   //Enables interrupt on INT0 change
    INTCONbits.PEIE=0;     //Disable all peripheral interrupts
    INTCON2bits.INTEDG0=1; //Sets interrupt to trigger on the rising edge
    //INTCONbits.INT0IF INT0 flag bit

    int pin_state=0;

    PORTAbits.RA0=0;  // turn the LED on RA0 off
    INTCONbits.GIE=1; //enable interrupts
    /**** Main Program ****/
    while(1) //endless loop
    {
        if(LED_State)
        {
            PORTAbits.RA0=1;   // turn on the LED on RA0
            Delay10KTCYx(400); // delay for two seconds
            PORTAbits.RA0=0;   // turn off the LED on RA0
            Delay10KTCYx(40);  // delay for 0.1 seconds
            LED_State=0;       // reset LED_State
        }
        else
        {
            pin_state=~pin_state;    // pin_state = bitwise NOT of pin_state
            PORTAbits.RA0=pin_state; // set RA0 to equal pin_state
            Delay10KTCYx(40);        // delay for 0.1s
        }
    }
}

Thanks to all you guys for the help! I guess the next challenge will be figuring out how to use the PICs EEPROM.
Dan

 

[Supercap2F]

Never ever put a delay into an interrupt routine. Interrupt routines are menat to be short routines that handle the interrupt rather quickly and return to the main task of the CPU. In this case you better use a global boolean variable (e.g. button_pressed) which you set to FALSE before starting the main WHILE loop. The interrupt routine would then set button_pressed=TRUE instead of actually performing the delay. Within the WHILE loop you check for the state of button_pressed. If FLASE, continue with the 0.1s blink routine. If TRUE, turn on the LED for 2s.

I have been thinking about this a bunch and I have another question...

Let's say you have a endless while loop in the void main(void) function. In the while loop you are controlling a complex pattern of LEDs blinking (let's say 200 instructions half of which are delays). You also have a button hooked to a port and a interrupt set up to trigger on the rising edge (when the button is pressed). What you want to happen is, when you push the button every thing stops and all the LEDs turn on for 3 seconds. But, your not supposed to have delays in interrupts... The only way I could think of around this would be to have the interrupt set a variable, and have a if(variable=1) --> delay 3 seconds thing every instruction! Is there a better way of doing this?

Thanks again for your time! I hope all that made since!
Dan

 

[Harald Kapp]

The only way I could think of around this would be to have the interrupt set a variable, and have a if(variable=1) --> delay 3 seconds thing every instruction

In terms of maintainabílity and extensibility of your code this is in my opinion the best way. You should have a close look at the while loop in main. As you describe it, you have a linear sequence of

set_LED_pattern
delay
set LED_pattern
delay ...

While this kind of coding is rather intuitive and can be seen often, you may consider using a second loop within the while loop. Arrange the LED patterns in an array, step through the array using a for loop. Then you will need to check for the global variable only once (once in your code, at runtime more often with each pass through the for loop. Here's some pseudo code:

while (1)
{
int index;
for (index=0;index<size_off_LED_array,index++)
{
set LED_pattern(LED_array[index]
delay LED_pattern_time
if global_variable_key_pressed
delay 3seconds
} //end for
] //end while

In your simple example you could of course also put the 3s delay into the interrupt routine - the CPU hasn't anything else to do anyway. But keep in mind later extensions of your program and don't get spoiled by bad coding style...

 

[Supercap2F]

Okay, thanks for explaining that Harald! I guess I will try and avoid using a delay in an interrupt as much as I can.
Dan

 

[BobK]

The problem with a delay in an interrupt is that, eventually, you will have the main program doing other time critical things, and if there is a delay in the interrupt routine, all that will stop.

For a more complex real-time system, you would use a timer interrupt that increments a variable, then all timing in the main program would check this time variable to know when to do something. It is not an easy concept to get, but once you start more complex programming, it will become necessary.

Bob