PIC16F628A Global Variables and Flags. XC8

Thread Starter

odm4286

Joined Sep 20, 2009
265
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
C:
// Defines
#define _XTAL_FREQ 4000000         // running the chip at 4Mhz
#define ID_MAX 63                  // highest ID number possible. 6 bits base^n
#define ID 1                       // #DEFINE for hardcoded ID's. Safe route for presentation
#define BOOT_STALL 750             // Number of mS to delay upon receiving ACK from raspi
#define TMR_TWO_PERIOD 62500       // Remaing counts, 245 rolls rougly one second. 50% duty cycle
//#define TMR_TWO_HALF_PERIOD        // see above...
#define DISTRESS 0x11              // distress state
#define ALERT 0x02                 // alert state
#define BOOT_MASK 0x40             // boot packet mask
#define ALARM_MASK 0xC0            // alarm packet mask

char rx_buffer;                    // dumping UART data in here for processing
int timer_1_overflow_count = 0;    // Timer 1 overflow counter, keeps track of how often timer 1 overflows
int timer_2_overflow_count = 0;    // Timer 2 overflow counter, keeps track of how often timer 2(16bit) overflows
int id_ok = 0;                     // Set high when id is echoed(accepted) back by raspi.
int tmr0_rc = 0;                   // counts rolls of timer 0
int tmr1_rc = 0;                   // counts rolls of timer 1
int tmr2_rc = 0;                   // counts rolls of timer 2
char strobe_ok = 0x0;              // determines weather or not its ok for the strobe light to come on
char vibe_ok = 0x0;                // determines weather or not its ok for the vibration to come on
char packet = 0x0;                 // transmission packet
volatile char state = 0x00;

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

ErnieM

Joined Apr 24, 2011
8,377
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.
 

Thread Starter

odm4286

Joined Sep 20, 2009
265
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.
 

ErnieM

Joined Apr 24, 2011
8,377
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!
 

Picbuster

Joined Dec 2, 2013
1,047
  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
 

ErnieM

Joined Apr 24, 2011
8,377
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.
 

Thread Starter

odm4286

Joined Sep 20, 2009
265
  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
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:

Thread Starter

odm4286

Joined Sep 20, 2009
265
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.
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.

C:
void interrupt ISR (void)
{
    /* Receiving and External ISR */
if(INTF)
{
/* External RB0 rising edge interrupt */
    if(INTF)                             // RBO/INT interrupt, must clear flag in software. Disable Rx interrupt.
    {
        INTF = 0;
        state = DISTRESS;
        RA6 = 1;
    } else if(RCIF) {
        state = ALERT;
        rx_buffer = RCREG;
    }
    /* End Receiving ISR */

    /* Timer 0 ISR drives                  //  used for strobe light
     * strobe light and Vibration */
    /*if(T0IF)
    {
        if(strobe_ok || vibe_ok)           // we only want to keep track of rolls if its ok to strobe
        {
            tmr0_rc++;
            T0IF = 0;                          // reset flag
        }
    }*/
    /* Timer 1(16 bit) ISR transmits distress signal */
    if(TMR1IF)
    {
        TMR1IF = 0;
        tmr1_rc++;
        TMR1 = 0x7AE0;                   // dumping 31456 inside the register. Leaving roughly 1 second worth of counts remaining
    }
    /* end timer 1 ISR*/
    /* Timer 2 ISR drives piezo buzzer */
    if(TMR2IF)                           // timer 2 overflowed, used to control peizo
    {
        TMR2IF = 0;                      // reset flag
        tmr2_rc++;
    }
    /* End Timer 2 ISR */

 
}
 

ErnieM

Joined Apr 24, 2011
8,377
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.
 

Thread Starter

odm4286

Joined Sep 20, 2009
265
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.
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/
 
Top