PIC16F628A Global Variables and Flags. XC8

Discussion in 'Embedded Systems and Microcontrollers' started by odm4286, Apr 10, 2016.

  1. odm4286

    Thread Starter Active Member

    Sep 20, 2009
    155
    5
    Hello, still getting the hang of using interrupts properly and I'm having a hard time modifying a global variable in my ISR. It seems like no matter what I do state is never modified. Here are the relevant parts of the code. I know it is a bit much but any help it all is appreciated!

    defines and variables
    Code (C):
    1.  
    2. // Defines
    3. #define _XTAL_FREQ 4000000         // running the chip at 4Mhz
    4. #define ID_MAX 63                  // highest ID number possible. 6 bits base^n
    5. #define ID 1                       // #DEFINE for hardcoded ID's. Safe route for presentation
    6. #define BOOT_STALL 750             // Number of mS to delay upon receiving ACK from raspi
    7. #define TMR_TWO_PERIOD 62500       // Remaing counts, 245 rolls rougly one second. 50% duty cycle
    8. //#define TMR_TWO_HALF_PERIOD        // see above...
    9. #define DISTRESS 0x11              // distress state
    10. #define ALERT 0x02                 // alert state
    11. #define BOOT_MASK 0x40             // boot packet mask
    12. #define ALARM_MASK 0xC0            // alarm packet mask
    13.  
    14. char rx_buffer;                    // dumping UART data in here for processing
    15. int timer_1_overflow_count = 0;    // Timer 1 overflow counter, keeps track of how often timer 1 overflows
    16. int timer_2_overflow_count = 0;    // Timer 2 overflow counter, keeps track of how often timer 2(16bit) overflows
    17. int id_ok = 0;                     // Set high when id is echoed(accepted) back by raspi.
    18. int tmr0_rc = 0;                   // counts rolls of timer 0
    19. int tmr1_rc = 0;                   // counts rolls of timer 1
    20. int tmr2_rc = 0;                   // counts rolls of timer 2
    21. char strobe_ok = 0x0;              // determines weather or not its ok for the strobe light to come on
    22. char vibe_ok = 0x0;                // determines weather or not its ok for the vibration to come on
    23. char packet = 0x0;                 // transmission packet
    24. volatile char state = 0x00;              
    25.  

    ISRs
    Code (C):
    1.  
    2. void interrupt ISR (void)
    3. {
    4.     /* Receiving ISR */
    5.     if(RCIF)                             // If i receive a message from the network
    6.     {
    7.         state = ALERT;
    8.         rx_buffer = RCREG;
    9.     }
    10.     /* End Receiving ISR */
    11.  
    12.     /* Timer 0 ISR drives                  //  used for strobe light
    13.      * strobe light and Vibration */
    14.     /*if(T0IF)
    15.     {
    16.         if(strobe_ok || vibe_ok)           // we only want to keep track of rolls if its ok to strobe  
    17.         {
    18.             tmr0_rc++;
    19.             T0IF = 0;                          // reset flag
    20.         }
    21.     }*/
    22.     /* Timer 1(16 bit) ISR transmits distress signal */
    23.     if(TMR1IF)
    24.     {
    25.         TMR1IF = 0;
    26.         tmr1_rc++;
    27.         TMR1 = 0x7AE0;                   // dumping 31456 inside the register. Leaving roughly 1 second worth of counts remaining
    28.     }
    29.     /* end timer 1 ISR*/
    30.     /* Timer 2 ISR drives piezo buzzer */
    31.     if(TMR2IF)                           // timer 2 overflowed, used to control peizo
    32.     {
    33.         TMR2IF = 0;                      // reset flag
    34.         tmr2_rc++;
    35.     }
    36.     /* End Timer 2 ISR */
    37.  
    38.     /* External RB0 rising edge interrupt */
    39.     if(INTF)                             // RBO/INT interrupt, must clear flag in software. Disable Rx interrupt.
    40.     {
    41.         INTF = 0;
    42.         state = DISTRESS;
    43.         RA6 = 1;
    44.     }
    45. }
    46.  
    state machine
    Code (C):
    1.  
    2. while(1)                       // Loop forever...state machine
    3.     {
    4.         // state machine
    5.         switch(state)
    6.         {
    7.             case DISTRESS :
    8.                 RCIE = 0;                        // at this point we're ignoring all incoming messages
    9.                 RA1 = 0;                         // kill GIL
    10.                 /*    while(RCREG)                     // unlatch RCIF by...
    11.                     if(RCREG);                   // clearing the fifo buffer*/
    12.                 // tmr0 is constantly r
    13.                 T1CONbits.TMR1ON = 1;        // turn timer1 on Tx clock
    14.                 T2CONbits.TMR2ON = 1;        // turn timer2 on Strobe clock
    15.                 strobe_ok = 0x01;            // let tmr0 know its ok to turn on the strobe
    16.                 if(tmr1_rc >= 5)                 // 5 rolls..5 seconds.
    17.                 {
    18.                     packet = packet | ALARM_MASK;// turn it into a fall packet
    19.                     packet = packet | ID;        // attaching ID to last 6 bits
    20.                     TXREG = packet;              // transmit the distress signal                                
    21.                     tmr1_rc = 0;                 // reset roll counter
    22.                 }
    23.                 if(tmr2_rc >= 122)               // 122 rolls = half second
    24.                 {
    25.                     RB4 = !RB4;                  // drive/silence peizo
    26.                     RB7 = !RB7;                  // drive/silence strobe
    27.                     tmr2_rc = 0;
    28.                 }
    29.                 break;
    30.             case ALERT :
    31.                 if((rx_buffer & ALARM_MASK) == ALARM_MASK)   // someone transmitted, &'ing packet against fall mask 0xC0. Latching state, unlatched by INTF ISR
    32.                 {
    33.                     RCIE = 0;                    // disable Rx Interrupt, someone fell we're in alarm until the device is power cycled. Ignore all other incoming packets
    34.                     vibe_ok = 1;                 // putting us in "help" or "alert" mode
    35.                 } else if ((rx_buffer & BOOT_MASK) == BOOT_MASK) {  // we've received a boot packet
    36.                     if(rx_buffer == packet)      // boot packet matches what was sent out  
    37.                     {
    38.                         id_ok = 1;               // id accepted by network
    39.                         while(RCREG);            // clear the fifo buffer
    40.                     }
    41.                 }
    42.                 if(vibe_ok & tmr2_rc >= 122)     // we're ok...but someone else fell wake up and help! Vibe on
    43.                 {
    44.                     RB6 = !RB6;                // Vibe toggle
    45.                     tmr2_rc = 0;
    46.                 }
    47.             break;
    48.          
    49.             default:
    50.                 // do nothing
    51.                 break;
    52.         }
    53.     }
    54.  
    Moderators note: Please use code tags for the right language
     
    Last edited by a moderator: Apr 11, 2016
  2. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,387
    1,605
    You properly defined state as a global volatile, that is good, but where do you change it? You init it to 0, change that to ALERT inside the ISR, and that is it. It never gets changed anywhere else.

    Do you have a debugger like a PICkit? Run this in debug mode and trace out the variables and code path. If you can't do that fire up the simulator and see if that tells you something.
     
    odm4286 likes this.
  3. odm4286

    Thread Starter Active Member

    Sep 20, 2009
    155
    5
    Thanks for the reply, INTF is the interrupt I tested last night. I could never get state == distress, but I just noticed a mistake. For some reason I had volatile state = DISTRESS in my interrupt. After staring at the computer for 8 hours I think I missed the fact that I removed that part and forgot to recompile :eek: I'll try it tonight, hopefully that was the issue all along. Runs perfectly in the debugger now.
     
  4. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,387
    1,605
    Ah... C allows you to reuse variable names like that, so you get a local variable instead of a global variable. Sounds like it should work now.

    The moral is compile early and compile often.

    I'll compile when I am done with any routine just to make sure I didn't make a syntax error.

    Good luck!
     
  5. Picbuster

    Member

    Dec 2, 2013
    374
    50
    1. if(RCIF) // If i receive a message from the network
    2. {
    3. // I am missing RCIF=0; but maybe you did this purposely not.
    4. state = ALERT;
    5. rx_buffer = RCREG;
    6. }
    It is not needed to declare non volatile and why a char?
    int state=0; // will do the job I used the same type of mechanism
     
  6. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,387
    1,605
    It is proper to declare a variable as volatile where it may change for some reason outside of the flow of statements. This includes things like register values that may hold status changed in hardware, and also regular memory storage that may change due to an interrupt.

    It may also include variables that seem to have no effect outside of a block; that includes temporary variables used as a delay loop counter. If the compiler notices a value change that never gets used it may emit no code for that section.

    Basically I keeps the compiler from optimizing away code you intend to be run.
     
  7. odm4286

    Thread Starter Active Member

    Sep 20, 2009
    155
    5
    I don't think you can reset RCIF in software. Only clearing the RCREG buffer clears the flag. I chose char because I only need 1 byte, I believe ints are 2 bytes right?
     
    Last edited: Apr 12, 2016
  8. odm4286

    Thread Starter Active Member

    Sep 20, 2009
    155
    5
    This worked, somewhat. If I comment out my RCIF ISR it my code works perfectly, otherwise I'm having an issue were my RCIF interrupt routine is constantly called and doesn't give INT ISR a chance to do its thing. I've tried to change the order in which the if statements appear in the code but that hasn't helped.

    This makes sense, my device receives a constant stream of data so I can see why the RCIF flag would stay high. The issue is INT is the most critical interrupt, so is it valid to write something like this? My thinking here is that the RCIF flag is "ignored" if the INTF is set, which is exactly what I want.

    Code (C):
    1.  
    2. void interrupt ISR (void)
    3. {
    4.     /* Receiving and External ISR */
    5. if(INTF)
    6. {
    7. /* External RB0 rising edge interrupt */
    8.     if(INTF)                             // RBO/INT interrupt, must clear flag in software. Disable Rx interrupt.
    9.     {
    10.         INTF = 0;
    11.         state = DISTRESS;
    12.         RA6 = 1;
    13.     } else if(RCIF) {
    14.         state = ALERT;
    15.         rx_buffer = RCREG;
    16.     }
    17.     /* End Receiving ISR */
    18.  
    19.     /* Timer 0 ISR drives                  //  used for strobe light
    20.      * strobe light and Vibration */
    21.     /*if(T0IF)
    22.     {
    23.         if(strobe_ok || vibe_ok)           // we only want to keep track of rolls if its ok to strobe
    24.         {
    25.             tmr0_rc++;
    26.             T0IF = 0;                          // reset flag
    27.         }
    28.     }*/
    29.     /* Timer 1(16 bit) ISR transmits distress signal */
    30.     if(TMR1IF)
    31.     {
    32.         TMR1IF = 0;
    33.         tmr1_rc++;
    34.         TMR1 = 0x7AE0;                   // dumping 31456 inside the register. Leaving roughly 1 second worth of counts remaining
    35.     }
    36.     /* end timer 1 ISR*/
    37.     /* Timer 2 ISR drives piezo buzzer */
    38.     if(TMR2IF)                           // timer 2 overflowed, used to control peizo
    39.     {
    40.         TMR2IF = 0;                      // reset flag
    41.         tmr2_rc++;
    42.     }
    43.     /* End Timer 2 ISR */
    44.  
    45.  
    46. }
    47.  
    48.  
    49.  
     
  9. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,387
    1,605
    You are correct, reading RCREG clears RCIF, which is otherwise read only. Plus you chose your data types correctly.

    Constantly being stuck in the ISR for serial data seems strange, unless your pic is running slow and the serial data too fast to handle it as it comes in plus the daylight to handle other tasks.

    What is your baud rate? What s you (confirmed) clock speed?

    Do note I am spitballing here, while I do a lot of pic stuff I only used the serial port to send data out, not get I coming in.
     
  10. odm4286

    Thread Starter Active Member

    Sep 20, 2009
    155
    5
    Spitballing or not I appreciate the input! Baud rate is 9600 (really 9602 I believe, acceptable error % I believe) FOSC is 4mhz using internal oscillator. Would you mind taking a look at my other post? I go into my new problem in more detail. http://forum.allaboutcircuits.com/threads/pic16f628a-help-with-clarification-on-datasheet.122971/
     
Loading...