Hi Everyone,

I am trying to implement Interrupt on change into my system and I am having problems with leaving the ISR once it has been completed.

I am using PIC16F1459 with MPLAB v8.92.

A secondary problem is that I can only trigger the interrupt using keys 7,8,9,*,0,# on the keypad but the other keys work once the ISR has been triggered.

I am displaying an ADC value in the main loop and once the ISR is triggered the ADC value does not update, telling me that the code is stuck in the ISR.

Code for the whole system is below. but most will be irrelevant to this problem...

//Author: JSwale
//July 2015
// PIC16F1459 Configuration Bit Settings
// 'C' source line config statements
#include <xc.h>
#include<stdlib.h>
// #pragma config statements should precede project file includes.
// Use project enums instead of #define for ON and OFF.
// CONFIG1
#pragma config FOSC = INTOSC    // Oscillator Selection Bits (INTOSC oscillator: I/O function on CLKIN pin)
#pragma config WDTE = OFF       // Watchdog Timer Enable (WDT disabled)
#pragma config PWRTE = ON       // Power-up Timer Enable (PWRT enabled)
#pragma config MCLRE = OFF       // MCLR Pin Function Select (MCLR/VPP pin function is Digital Input)
#pragma config CP = OFF         // Flash Program Memory Code Protection (Program memory code protection is disabled)
#pragma config BOREN = OFF      // Brown-out Reset Enable (Brown-out Reset disabled)
#pragma config CLKOUTEN = OFF   // Clock Out Enable (CLKOUT function is disabled. I/O or oscillator function on the CLKOUT pin)
#pragma config IESO = ON        // Internal/External Switchover Mode (Internal/External Switchover Mode is enabled)
#pragma config FCMEN = ON       // Fail-Safe Clock Monitor Enable (Fail-Safe Clock Monitor is enabled)
// CONFIG2
#pragma config WRT = OFF        // Flash Memory Self-Write Protection (Write protection off)
#pragma config CPUDIV = NOCLKDIV// CPU System Clock Selection Bit (NO CPU system divide)
#pragma config USBLSCLK = 48MHz // USB Low SPeed Clock Selection bit (System clock expects 48 MHz, FS/LS USB CLKENs divide-by is set to 8.)
#pragma config PLLMULT = 3x     // PLL Multipler Selection Bit (3x Output Frequency Selected)
#pragma config PLLEN = ENABLED  // PLL Enable Bit (3x or 4x PLL Enabled)
#pragma config STVREN = ON      // Stack Overflow/Underflow Reset Enable (Stack Overflow or Underflow will cause a Reset)
#pragma config BORV = LO        // Brown-out Reset Voltage Selection (Brown-out Reset Voltage (Vbor), low trip point selected.)
#pragma config LPBOR = OFF      // Low-Power Brown Out Reset (Low-Power BOR is disabled)
#pragma config LVP = OFF        // Low-Voltage Programming Enable (High-voltage on MCLR/VPP must be used for programming)
//#pragma interrupt isr            //declares isr as service interrupt routine
//#pragma code isr = 0x08            //puts isr in memory location 0x08
 
#define _XTAL_FREQ 4000000      // 4MHz for now
//---------------------INIT IO -----------------------------------
void initIO(void)
{
    OSCCON = 0b00110100;  // Internal oscillator = 4MHz (for now)
    TRISC=0b00000001;            //RC0 is input
    PORTC=0b00000000;
    ANSELC=0b00000001;            //RC0 is analog
    TRISB=0b00000000;
    PORTB=0b00000000;
    ANSELB=0b00000000;
    TRISA=0b11111111;            //PORTA all inputs from keypad
    PORTA=0b00000000;
    ANSELA=0b00000000;
   
}
 
//-------------------------Interrupt DEFINITIONS------------------
void int_init ()
{
INTCON=0x88;                //Sets GIE & IOCIE
IOCAN=0x18;                    //Sets positive edge on RA0,1,3,4
}
 
 
//---------------------ADC DEFINITIONS---------------------------
void adc_init()
{
    ADCON0= 0b00010001;            //RC0 for ADC use
    ADCON1= 0b10010000;            //Right justified and use of Vsupply + clock conversion 1/4MHz*8        8Fosc = 2us
    ADCON2= 0b00000000;           
}
//---------------------IO DEFINITIONS  --------------------------
#define lcd_port  LATC        // write to LAT, read from PORT
// RC7-4 is the LCD 4 bit databus
#define LCD_RSout LATC2     // moved from ICSP
#define LCD_ENout LATC3
 
#define col1    PORTBbits.RB4
#define col2    PORTBbits.RB5
#define col3    PORTBbits.RB6
#define row1    PORTAbits.RA0
#define row2    PORTAbits.RA1
#define row3    PORTAbits.RA3
#define row4    PORTAbits.RA4
 
int numberpressed;
const char transTable[] = {'0','1','2','3','4','5','6','7','8','9','*','#','?'};
unsigned char ASCIIchar;
unsigned int ADC_result;
unsigned char temp[5];
int PORTCtemp;
//-------------------- DISPLAY SETTINGS  ---------------------
// Define some display settings. 
#define lcdLINE1 0x00       // where line 1 begins
#define lcdLINE2 0x40       // where line 2 begins
//--------------------- STROBE LCD ---------------------------
// Pulses E line on LCD to write
int strobeLCD(void)
{
LCD_ENout = 1;
__delay_us(2);     // Added a little here
LCD_ENout = 0;
}
//--------------------- WRITE 8 BIT DATA TO LCD  -----------------
// Assumes LCD is ready and RS is set to correct value
// LCD data bus is RC4-RC7
void writeLCD(unsigned char dat)
{
    lcd_port &= 0x0f;               // get current port, clear upper bits
    lcd_port |= (dat & 0xf0);       // combine w/upper nibble, leave lower same
    strobeLCD();
    lcd_port &= 0x0f;               // get current port, clear upper bits
    lcd_port |= ((dat <<4) & (0xf0)); // combine w/lower nibble, leave lower port same
    strobeLCD();
    __delay_ms(2);                // wait for display to process
}
//-------------------- WRITE LCD COMMAND  -------------------------
// Write cmd to LCD with RS=0
// Assumes E is low and display is NOT busy
void lcd_cmd (unsigned char cmd)
{
    LCD_RSout = 0;       // select command register
    writeLCD(cmd);
}
//---------------------- WRITE LCD DATA  --------------------------
// Write dat to LCD with RS=1
// Assumes E is low and display is NOT busy
void lcd_data (unsigned char dat)
{
    LCD_RSout = 1;       // select data register
    writeLCD(dat);
}
//-------------------- RESET/CONFIGURE LCD  -------------------------
void lcd_init(void)
{
    lcd_port &= 0x0f;   // clear upper bits of LCD port
    lcd_port |= 0x30;   // direct data to LCD DB7-4
    LCD_RSout = 0;
    strobeLCD();        // write 3h, wait 10ms
    __delay_ms(10);
    strobeLCD();        // write 3h, wait..
    __delay_ms(10);
     strobeLCD();       // write 3h
    __delay_ms(10);
   lcd_port &= 0x0f;    // clear upper bits of LCD port
   lcd_port |= 0x20;    // direct data to LCD DB7-4
   strobeLCD();         // write 2h
   __delay_ms(10);
    lcd_cmd(0x28);       // Funciton Set: 4-bit mode - 2 line - 5x7 font.
    lcd_cmd(0x0c);       // DisplayON, cursor ON, blink OFF
    lcd_cmd(0x06);       // Automatic Increment - No Display shift.
    lcd_cmd(0x80);       // Address DDRAM with 0 offset 80h.
}
//----------------------- WRITE STRING TO LCD  ---------------------
// Writes null terminated string to LCD from ROM
void lcd_WriteStr(const unsigned char *c)
{
    while(*c != '\0'){
        lcd_data(*c);
        c++;
    }
}
//--------------------  SETS CURSOR ANYWHERE IN DISPLAY MEMORY  ---------
// Valid locations are 0-79 decimal.  This doesn't check for valid location
void lcd_SetCursor(unsigned char loc)
{
    lcd_cmd(loc | 0x80);        // form and send 'set DDRAM address' cmd
}
//----------------- CANNED LINE COMMANDS  -------------------------
// For a 2 line display
void lcd_LINE1(void)
{
    lcd_SetCursor(lcdLINE1);
}
void lcd_LINE2(void)
{
    lcd_SetCursor(lcdLINE2);
}
 
//--------------------KEYPAD SCAN------------------------------------
void Scan(void)
{
 
    col1=0;
    col2=1;
    col3=1;                                //B4 is 0, scans for 1,4,7,*
       
    if (row1==0)   
        {
        numberpressed=1;
        return;                            //'return' leaves current subroutine
        }
   
    if (row2==0)               
        {
        numberpressed=4;
        return;
        }
           
    if (row3==0)
        {
        numberpressed=7;
        return;
        }
       
    if (row4==0)
        {
        numberpressed=10;
        return;
        }
   
 
    col1=1;                                //B5 is 0, scans for 2,5,8,0
    col2=0;
    col3=1;                   
       
    if (row1==0)
        {
        numberpressed=2;
        return;
        }
   
    if (row2==0)
        {
        numberpressed=5;
        return;
        }       
   
    if (row3==0)
        {
        numberpressed=8;
        return;
        }
       
    if (row4==0)
        {
        numberpressed=0;
        return;
        }
 
   
    col1=1;                                    //B6 is 0, scans for 3,6,9,#
    col2=1;
    col3=0;               
       
    if (row1==0)
        {
        numberpressed=3;
        return;
        }
   
    if (row2==0)
        {
        numberpressed=6;
        return;
        }
           
    if (row3==0)
        {
        numberpressed=9;
        return;
        }
       
    if (row4==0)
        {
        numberpressed=11;
        return;
        }
}
 
//-------------------------INTERRUPT SERVICE ROUTINE------------------
void interrupt isr (void)
{
if (IOCIF && IOCIE)
{
PORTCtemp=PORTC;
IOCIF=0x00;
 
Scan();
 
if(numberpressed>11) numberpressed = 12;  //qualify your index- will print '?' if unknown
 
ASCIIchar = transTable[numberpressed]; // translate to ASCII numbers and symbols
lcd_cmd(0xC0);                            //Starts Line 2
lcd_WriteStr("Keypad value = ");
lcd_data(ASCIIchar); //write it
   
    if (numberpressed==1)
    {
    lcd_SetCursor(lcdLINE1);        //sets cursor to line1
    lcd_WriteStr("UNLOCKED        ");
    lcd_SetCursor(lcdLINE2);           //sets cursor to line2
    lcd_WriteStr("UNLOCKED        ");
 
    while(1);    // loop forever
    }
    PORTC=PORTCtemp;
}   
}
 
//-----------------------VERIFICATION OF LCD + KEYPAD--------------------------
void Verify (void)
{
numberpressed=50;        
lcd_WriteStr("Press 0 to cont");
while(numberpressed!=0)
{
Scan();
}
lcd_cmd(0x01);
}
//======================= MAIN =====================================
void main (void)
{
    initIO();               // init the chip IO
    __delay_ms(300);        // power up delay for LCD - adjust as necessary
    lcd_init();             // init the LCD
    adc_init();                // ADC init
    //Verify();                // Demonstrates LCD powering up
    __delay_ms(10);
    int_init();                // interrupt initilization
   
   
while(1)
    {   
       
        ADCON0bits.GO=1;                        //Sets Go_Done bit
        while(ADCON0bits.GO==1);                //Waits for conversion to finish
        __delay_us(10);
        ADC_result=ADRESL+(ADRESH*256);            //gets 10 bit result
       
        itoa(temp, ADC_result, 10);                //integer to ASCII,  base 10
 
        lcd_cmd(0x80);                            //Start of Line 1
        if (ADC_result < 0x03E8)
        {lcd_WriteStr("ADC value =  ");}
        else if (ADC_result <0x000A)
        {lcd_WriteStr("ADC value =   ");}
        else
        {lcd_WriteStr("ADC value = ");}   
        lcd_WriteStr(temp);                        //writes the value (0-1023)
 
        }//while   
}    //main

The main points of interest are the Main loop, the ISR sub routine and the Interrupt initilization (int init).


Regards
JSwale

 

You have a fundamental problem in that you have a function [Scan()], which is called from the ISR and from another routine. This is a big Bozo no-no. In addition you have succumbed to the temptation to do a large amount of processing inside the interrupt routine. This is also a big bright red warning flag.

 

Yep... basically you want to do as little as possible and basically never call another routine from the ISR.

The way to do heavy lifting is to set a flag inside the ISR to tell another routine (that polls the flag in a loop) their is data that needs to be processed.

As is, you get stuck inside the ISR since you put an infinite loop (while(1)) inside there!

 

The while(1) loop only comes into play when number pressed = 1. That is not the case in the tests I've been doing.

In terms of the large amount of processing and the subroutine being called, I'll change that tomorrow.

Thanks.

 

you get into devils kitchen with these long testing constructs for key input.

Better write a key handler, pass key variables to it, test these later,
execute the task and reset them.

As pointed out, leave the ISR again immediately.

 

The while(1) loop only comes into play when number pressed = 1. That is not the case in the tests I've been doing.

You are misunderstanding what the interrupt is for. Its not to break away from one big chunk of code (the main routines) to spend forever doing something else. Interrupts, especially in PIC, are best used for short - very short - tasks that interleave with the main task, consuming as little time as possible - so little that the main task doesn't really know anything has happened. Your first cut at this commits the cardinal sin of waiting in the interrupt routine (the while(1)) and as others pointed out, attempts too much processing even without the 'while'. Remember, ISRs are like a drive-by-shooting, just do what has to be done, remember what you have done as applicable then exit, ready for the next time. NO delays, ever.

IIRC, in an earlier thread, we got scan() to do one pass through the key matrix each time it was called and return a raw result. A quick look at your code seems to agree. Since you haven't much interrupt experience, I would do this..
First -
disable all interrupts.
Just scan the keypad by repeated scans as described in your thread here.

When that works you can schedule the scans with an interrupt like this:
Configure TIMER 2 to interrupt the PIC at 1ms intervals (at 4MHz its PR2=100, Prescaler=1, Postscaler=10 but do the math to confirm).
Set up the ISR (interrupt service routine) to examine TMR2IF and IE and when set, simply set a flag that says 'Its been 1ms, time for another scan'. (We'll call that a SysTik). Clear TMR2IF and exit the ISR. Note how fast that is. Note also that since its called at 1ms intervals, you have a built in timer.. each call is 1ms, so you can accumulate debounce counts as 1ms intervals.
Redo your calls to scan() to be scheduled by the flag. As you loop through your code, whenever the flag is set, call scan() and clear the flag.
Note that the ISR 'produces' the flag and your main code 'consumes' the flag. This producer->consumer construct is pulled from multitasking methods.
SO.. after some calls, timed by the 1ms flag, if scan() has decided that it has a good key, it emits that key code - for now, just sets (produces) another flag 'NewKey' with the detected code.
Your main routine, as it loops around, detects NewFlag, reads the keycode, resets NewKey (consumes the flag) and then gets on with processing the key.

Note that scan() has to be smart enough to only emit new keys and set the flag. Main has to be smart and fast enough to consume the keys as they are pressed. We are OK for now..

To get the TMR2 interrupt going, I would do this:
Ignore the keypad and everything else.
Cobb together some code that counts the 1ms SysTiks and flashes an LED on some uncommitted output. If you count 500, turn LED on, count 500, turn LED off it will flash at 1sec intervals. Don't call scan() or anything, just flash the LED. When that is working, you know that the SysTik is working and you can use it to schedule scan().

There's lot more to it all but this will keep you busy for awhile. I'm going to the coast (SFO) for a week of cool.

Good luck.

 

When the Timer is using the instruction cycle to count before overflowing and the code comes across a delay, does this halt the timer also or is the timer unaffected?

I have set the flag and Timer0 to 1ms. Once the interrupt triggers and the flag is set I delay for take RC0 high, delay 10ms and then take RC0 low again. But this gives a 80ms gap between pulses when the timer0 is set to 1ms, which is what I don't understand.
I also shortened the time period of the pulse to 10us which doubled the time period between pulses to 156ms, which is clearly not correct.

The PROBLEM is that the Interrupt does trigger after every 1ms and then return to where it left but because there are so many millisecond delays in the LCD program for read/write it means the program never reaches the point of the main program where the flag has to be dealt with (causing the pulse). I'll have to have a think about how to get around that, if possible.

 

The PROBLEM is that the Interrupt does trigger after every 1ms and then return to where it left but because there are so many millisecond delays in the LCD program for read/write it means the program never reaches the point of the main program where the flag has to be dealt with (causing the pulse). I'll have to have a think about how to get around that, if possible.

Yeah, dumb delays will eat your sack lunch. That's why I said to use some test code to verify the timer. By that I mean let the LCD init but after that, comment out all of the writing to it. Then just have a simple routine with NO delays that counts and clears the 1ms flag say 10 times then tiks the IO port. You should see tiks at 10ms intervals if the timing is right.

Then, use your known tik time to schedule calls to scan() which likewise has NO delays and emits the scan code, again no active LCD writes and no delays. Debouncing in scan() will require static variables for the debounce timer and keycode being debounced. Each call to scan() will indicate 1ms (or whatever tik time is set to) of debounce. When the counter hits some debounce limit (30ms maybe), emit the code. Subsequent calls to scan() look for no keys pressed only. When no keys are found, start looking for the next press. When scan() emits a code to main, translate it and THEN write to the LCD for test.

Once the scanner is working perfectly, emitting key codes for each key pressed, move the call to scan() to the interrupt service routine and restore your working code. Now, the kbd scanner works in the background, even during LCD delays, debouncing keys and emitting codes. I would still do the key translation to ASCII or whatever in main, not the ISR.

Note that in scan(), since detecting and debouncing a key requires many calls, you'll need some way of remembering where you are in the detect, debounce, emit key-pressed, wait-for-clear process. A couple of flags is sufficient to know where you are. Frequently, this sort of thing is done with a state machine but you can get away without it.

Since you're not using a debugger, I'd do something like this, step by step.

Later, we can talk about how to get rid of dumb delays (think scheduling with a timer tik like we did with the IO port test)

Have fun