Leaving an ISR and returning to main loop problem

Discussion in 'Embedded Systems and Microcontrollers' started by Jswale, Aug 3, 2015.

  1. Jswale

    Thread Starter Member

    Jun 30, 2015
    121
    6
    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...

    Code (C):
    1. //Author: JSwale
    2. //July 2015
    3. // PIC16F1459 Configuration Bit Settings
    4. // 'C' source line config statements
    5. #include <xc.h>
    6. #include<stdlib.h>
    7. // #pragma config statements should precede project file includes.
    8. // Use project enums instead of #define for ON and OFF.
    9. // CONFIG1
    10. #pragma config FOSC = INTOSC    // Oscillator Selection Bits (INTOSC oscillator: I/O function on CLKIN pin)
    11. #pragma config WDTE = OFF       // Watchdog Timer Enable (WDT disabled)
    12. #pragma config PWRTE = ON       // Power-up Timer Enable (PWRT enabled)
    13. #pragma config MCLRE = OFF       // MCLR Pin Function Select (MCLR/VPP pin function is Digital Input)
    14. #pragma config CP = OFF         // Flash Program Memory Code Protection (Program memory code protection is disabled)
    15. #pragma config BOREN = OFF      // Brown-out Reset Enable (Brown-out Reset disabled)
    16. #pragma config CLKOUTEN = OFF   // Clock Out Enable (CLKOUT function is disabled. I/O or oscillator function on the CLKOUT pin)
    17. #pragma config IESO = ON        // Internal/External Switchover Mode (Internal/External Switchover Mode is enabled)
    18. #pragma config FCMEN = ON       // Fail-Safe Clock Monitor Enable (Fail-Safe Clock Monitor is enabled)
    19. // CONFIG2
    20. #pragma config WRT = OFF        // Flash Memory Self-Write Protection (Write protection off)
    21. #pragma config CPUDIV = NOCLKDIV// CPU System Clock Selection Bit (NO CPU system divide)
    22. #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.)
    23. #pragma config PLLMULT = 3x     // PLL Multipler Selection Bit (3x Output Frequency Selected)
    24. #pragma config PLLEN = ENABLED  // PLL Enable Bit (3x or 4x PLL Enabled)
    25. #pragma config STVREN = ON      // Stack Overflow/Underflow Reset Enable (Stack Overflow or Underflow will cause a Reset)
    26. #pragma config BORV = LO        // Brown-out Reset Voltage Selection (Brown-out Reset Voltage (Vbor), low trip point selected.)
    27. #pragma config LPBOR = OFF      // Low-Power Brown Out Reset (Low-Power BOR is disabled)
    28. #pragma config LVP = OFF        // Low-Voltage Programming Enable (High-voltage on MCLR/VPP must be used for programming)
    29. //#pragma interrupt isr            //declares isr as service interrupt routine
    30. //#pragma code isr = 0x08            //puts isr in memory location 0x08
    31.  
    32. #define _XTAL_FREQ 4000000      // 4MHz for now
    33. //---------------------INIT IO -----------------------------------
    34. void initIO(void)
    35. {
    36.     OSCCON = 0b00110100;  // Internal oscillator = 4MHz (for now)
    37.     TRISC=0b00000001;            //RC0 is input
    38.     PORTC=0b00000000;
    39.     ANSELC=0b00000001;            //RC0 is analog
    40.     TRISB=0b00000000;
    41.     PORTB=0b00000000;
    42.     ANSELB=0b00000000;
    43.     TRISA=0b11111111;            //PORTA all inputs from keypad
    44.     PORTA=0b00000000;
    45.     ANSELA=0b00000000;
    46.    
    47. }
    48.  
    49. //-------------------------Interrupt DEFINITIONS------------------
    50. void int_init ()
    51. {
    52. INTCON=0x88;                //Sets GIE & IOCIE
    53. IOCAN=0x18;                    //Sets positive edge on RA0,1,3,4
    54. }
    55.  
    56.  
    57. //---------------------ADC DEFINITIONS---------------------------
    58. void adc_init()
    59. {
    60.     ADCON0= 0b00010001;            //RC0 for ADC use
    61.     ADCON1= 0b10010000;            //Right justified and use of Vsupply + clock conversion 1/4MHz*8        8Fosc = 2us
    62.     ADCON2= 0b00000000;          
    63. }
    64. //---------------------IO DEFINITIONS  --------------------------
    65. #define lcd_port  LATC        // write to LAT, read from PORT
    66. // RC7-4 is the LCD 4 bit databus
    67. #define LCD_RSout LATC2     // moved from ICSP
    68. #define LCD_ENout LATC3
    69.  
    70. #define col1    PORTBbits.RB4
    71. #define col2    PORTBbits.RB5
    72. #define col3    PORTBbits.RB6
    73. #define row1    PORTAbits.RA0
    74. #define row2    PORTAbits.RA1
    75. #define row3    PORTAbits.RA3
    76. #define row4    PORTAbits.RA4
    77.  
    78. int numberpressed;
    79. const char transTable[] = {'0','1','2','3','4','5','6','7','8','9','*','#','?'};
    80. unsigned char ASCIIchar;
    81. unsigned int ADC_result;
    82. unsigned char temp[5];
    83. int PORTCtemp;
    84. //-------------------- DISPLAY SETTINGS  ---------------------
    85. // Define some display settings.
    86. #define lcdLINE1 0x00       // where line 1 begins
    87. #define lcdLINE2 0x40       // where line 2 begins
    88. //--------------------- STROBE LCD ---------------------------
    89. // Pulses E line on LCD to write
    90. int strobeLCD(void)
    91. {
    92. LCD_ENout = 1;
    93. __delay_us(2);     // Added a little here
    94. LCD_ENout = 0;
    95. }
    96. //--------------------- WRITE 8 BIT DATA TO LCD  -----------------
    97. // Assumes LCD is ready and RS is set to correct value
    98. // LCD data bus is RC4-RC7
    99. void writeLCD(unsigned char dat)
    100. {
    101.     lcd_port &= 0x0f;               // get current port, clear upper bits
    102.     lcd_port |= (dat & 0xf0);       // combine w/upper nibble, leave lower same
    103.     strobeLCD();
    104.     lcd_port &= 0x0f;               // get current port, clear upper bits
    105.     lcd_port |= ((dat <<4) & (0xf0)); // combine w/lower nibble, leave lower port same
    106.     strobeLCD();
    107.     __delay_ms(2);                // wait for display to process
    108. }
    109. //-------------------- WRITE LCD COMMAND  -------------------------
    110. // Write cmd to LCD with RS=0
    111. // Assumes E is low and display is NOT busy
    112. void lcd_cmd (unsigned char cmd)
    113. {
    114.     LCD_RSout = 0;       // select command register
    115.     writeLCD(cmd);
    116. }
    117. //---------------------- WRITE LCD DATA  --------------------------
    118. // Write dat to LCD with RS=1
    119. // Assumes E is low and display is NOT busy
    120. void lcd_data (unsigned char dat)
    121. {
    122.     LCD_RSout = 1;       // select data register
    123.     writeLCD(dat);
    124. }
    125. //-------------------- RESET/CONFIGURE LCD  -------------------------
    126. void lcd_init(void)
    127. {
    128.     lcd_port &= 0x0f;   // clear upper bits of LCD port
    129.     lcd_port |= 0x30;   // direct data to LCD DB7-4
    130.     LCD_RSout = 0;
    131.     strobeLCD();        // write 3h, wait 10ms
    132.     __delay_ms(10);
    133.     strobeLCD();        // write 3h, wait..
    134.     __delay_ms(10);
    135.      strobeLCD();       // write 3h
    136.     __delay_ms(10);
    137.    lcd_port &= 0x0f;    // clear upper bits of LCD port
    138.    lcd_port |= 0x20;    // direct data to LCD DB7-4
    139.    strobeLCD();         // write 2h
    140.    __delay_ms(10);
    141.     lcd_cmd(0x28);       // Funciton Set: 4-bit mode - 2 line - 5x7 font.
    142.     lcd_cmd(0x0c);       // DisplayON, cursor ON, blink OFF
    143.     lcd_cmd(0x06);       // Automatic Increment - No Display shift.
    144.     lcd_cmd(0x80);       // Address DDRAM with 0 offset 80h.
    145. }
    146. //----------------------- WRITE STRING TO LCD  ---------------------
    147. // Writes null terminated string to LCD from ROM
    148. void lcd_WriteStr(const unsigned char *c)
    149. {
    150.     while(*c != '\0'){
    151.         lcd_data(*c);
    152.         c++;
    153.     }
    154. }
    155. //--------------------  SETS CURSOR ANYWHERE IN DISPLAY MEMORY  ---------
    156. // Valid locations are 0-79 decimal.  This doesn't check for valid location
    157. void lcd_SetCursor(unsigned char loc)
    158. {
    159.     lcd_cmd(loc | 0x80);        // form and send 'set DDRAM address' cmd
    160. }
    161. //----------------- CANNED LINE COMMANDS  -------------------------
    162. // For a 2 line display
    163. void lcd_LINE1(void)
    164. {
    165.     lcd_SetCursor(lcdLINE1);
    166. }
    167. void lcd_LINE2(void)
    168. {
    169.     lcd_SetCursor(lcdLINE2);
    170. }
    171.  
    172. //--------------------KEYPAD SCAN------------------------------------
    173. void Scan(void)
    174. {
    175.  
    176.     col1=0;
    177.     col2=1;
    178.     col3=1;                                //B4 is 0, scans for 1,4,7,*
    179.        
    180.     if (row1==0)  
    181.         {
    182.         numberpressed=1;
    183.         return;                            //'return' leaves current subroutine
    184.         }
    185.    
    186.     if (row2==0)              
    187.         {
    188.         numberpressed=4;
    189.         return;
    190.         }
    191.            
    192.     if (row3==0)
    193.         {
    194.         numberpressed=7;
    195.         return;
    196.         }
    197.        
    198.     if (row4==0)
    199.         {
    200.         numberpressed=10;
    201.         return;
    202.         }
    203.    
    204.  
    205.     col1=1;                                //B5 is 0, scans for 2,5,8,0
    206.     col2=0;
    207.     col3=1;                  
    208.        
    209.     if (row1==0)
    210.         {
    211.         numberpressed=2;
    212.         return;
    213.         }
    214.    
    215.     if (row2==0)
    216.         {
    217.         numberpressed=5;
    218.         return;
    219.         }      
    220.    
    221.     if (row3==0)
    222.         {
    223.         numberpressed=8;
    224.         return;
    225.         }
    226.        
    227.     if (row4==0)
    228.         {
    229.         numberpressed=0;
    230.         return;
    231.         }
    232.  
    233.    
    234.     col1=1;                                    //B6 is 0, scans for 3,6,9,#
    235.     col2=1;
    236.     col3=0;              
    237.        
    238.     if (row1==0)
    239.         {
    240.         numberpressed=3;
    241.         return;
    242.         }
    243.    
    244.     if (row2==0)
    245.         {
    246.         numberpressed=6;
    247.         return;
    248.         }
    249.            
    250.     if (row3==0)
    251.         {
    252.         numberpressed=9;
    253.         return;
    254.         }
    255.        
    256.     if (row4==0)
    257.         {
    258.         numberpressed=11;
    259.         return;
    260.         }
    261. }
    262.  
    263. //-------------------------INTERRUPT SERVICE ROUTINE------------------
    264. void interrupt isr (void)
    265. {
    266. if (IOCIF && IOCIE)
    267. {
    268. PORTCtemp=PORTC;
    269. IOCIF=0x00;
    270.  
    271. Scan();
    272.  
    273. if(numberpressed>11) numberpressed = 12;  //qualify your index- will print '?' if unknown
    274.  
    275. ASCIIchar = transTable[numberpressed]; // translate to ASCII numbers and symbols
    276. lcd_cmd(0xC0);                            //Starts Line 2
    277. lcd_WriteStr("Keypad value = ");
    278. lcd_data(ASCIIchar); //write it
    279.    
    280.     if (numberpressed==1)
    281.     {
    282.     lcd_SetCursor(lcdLINE1);        //sets cursor to line1
    283.     lcd_WriteStr("UNLOCKED        ");
    284.     lcd_SetCursor(lcdLINE2);           //sets cursor to line2
    285.     lcd_WriteStr("UNLOCKED        ");
    286.  
    287.     while(1);    // loop forever
    288.     }
    289.     PORTC=PORTCtemp;
    290. }  
    291. }
    292.  
    293. //-----------------------VERIFICATION OF LCD + KEYPAD--------------------------
    294. void Verify (void)
    295. {
    296. numberpressed=50;        
    297. lcd_WriteStr("Press 0 to cont");
    298. while(numberpressed!=0)
    299. {
    300. Scan();
    301. }
    302. lcd_cmd(0x01);
    303. }
    304. //======================= MAIN =====================================
    305. void main (void)
    306. {
    307.     initIO();               // init the chip IO
    308.     __delay_ms(300);        // power up delay for LCD - adjust as necessary
    309.     lcd_init();             // init the LCD
    310.     adc_init();                // ADC init
    311.     //Verify();                // Demonstrates LCD powering up
    312.     __delay_ms(10);
    313.     int_init();                // interrupt initilization
    314.    
    315.    
    316. while(1)
    317.     {  
    318.        
    319.         ADCON0bits.GO=1;                        //Sets Go_Done bit
    320.         while(ADCON0bits.GO==1);                //Waits for conversion to finish
    321.         __delay_us(10);
    322.         ADC_result=ADRESL+(ADRESH*256);            //gets 10 bit result
    323.        
    324.         itoa(temp, ADC_result, 10);                //integer to ASCII,  base 10
    325.  
    326.         lcd_cmd(0x80);                            //Start of Line 1
    327.         if (ADC_result < 0x03E8)
    328.         {lcd_WriteStr("ADC value =  ");}
    329.         else if (ADC_result <0x000A)
    330.         {lcd_WriteStr("ADC value =   ");}
    331.         else
    332.         {lcd_WriteStr("ADC value = ");}  
    333.         lcd_WriteStr(temp);                        //writes the value (0-1023)
    334.  
    335.         }//while  
    336. }    //main
    337.  
    338.  
    The main points of interest are the Main loop, the ISR sub routine and the Interrupt initilization (int init).


    Regards
    JSwale
     
  2. Papabravo

    Expert

    Feb 24, 2006
    10,144
    1,790
    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.
     
  3. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,388
    1,605
    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!
     
  4. Jswale

    Thread Starter Member

    Jun 30, 2015
    121
    6
    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.
     
  5. takao21203

    Distinguished Member

    Apr 28, 2012
    3,577
    463
    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.
     
  6. JohnInTX

    Moderator

    Jun 26, 2012
    2,345
    1,028
    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.
     
    Last edited: Aug 3, 2015
  7. Jswale

    Thread Starter Member

    Jun 30, 2015
    121
    6
    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.
     
    Last edited: Aug 4, 2015
  8. JohnInTX

    Moderator

    Jun 26, 2012
    2,345
    1,028
    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
     
Loading...