PIC: Global variable not modified inside interrupt handler

Question

I have an interrupt handler for my PIC 16 that should modify a global variable. The global variable is then read by main() in an infinite loop.

For some reason, it seems that the global variable is being modified inside the interrupt handler, but that the modifications do not persist out of the interrupt handler. I am sure that:

• The interrupt handler is being called properly
• The handler modifies the global variable inside the handler
• The code in main() that reads the global variable is correct

Finally, I have been careful to declare the global variable as volatile as recommended here.

What could explain that the global variable change in the interrupt handler does not persist to main()?

[Note: My C code is compiled by XC8. Below is the full code of my reduced test case.]

#include <pic16f1824.h>
#include <xc.h>
volatile unsigned char buttonPress = 0b0;
void interrupt InterruptServiceRoutine(void) {
 // Update the button press value
 buttonPress = 0b1;
}
void main(void) {
 // Make RA4 an output
 TRISAbits.TRISA4 = 0b0;
 // Turn off status LED
 LATAbits.LATA4 = 0b0;
 // Enable interrupts
 INTCONbits.GIE = 0b1;
 // Enable interrupt-on-change to wake from sleep
 INTCONbits.IOCIE = 0b1;
 // Make the RA2 pin a digital input
 TRISAbits.TRISA2 = 0b1;
 ANSELAbits.ANSA2 = 0b0;
 // Interrupt on RA2 negedges
 IOCANbits.IOCAN2 = 0b1;
 while(1) {
 if(buttonPress == 0b1) {
 // Turn on LED <-- Never turns on
 LATAbits.LATA4 = 0b1;
 }
 }
}

• Garrett Fogerlie: 99% of the time this is because volatile is not used. But since your sure you have, it may be an optimization with XC8 (that's the whole reason to use volatile, to prevent optimization.) You need to add your code or some example of it.
• Garrett Fogerlie: Actually on second take I would guess it has to do with your debounce method button_debounce()
• Anindo Ghosh: @Randomblue Adding code piece by piece when seeking debug help is not effective use of others' time. The DEFINEs could be wrong, the initialization could be bad, the button_debounce could be the problem, and so on.
• apalopohapa: Try assigning the value you are checking for directly. Post button_debounce code. It may be that buttonPress gets checked value only briefly, so main loop never sees it.
• akohlsmith: Stop revealing your code an edit at a time. You're asking us for help, not performing a strip tease.

Answer 1

Now that more code is available, here's my theory:

The interrupt flag does not seem to be reset, so the interrupt gets called over and over after the first time, therefore the code in the main loop never progresses.

• Garrett Fogerlie: I'm not too familiar with PIC, but I take it that the interrupt InterruptServiceRoutine does not automatically clear the flag? Or is it just being reset because of debounce?
• Randomblue: @GarrettFogerlie: It's not cleared automatically. The one line IOCAFbits.IOCAF2 = 0b0; fixed everything.
• apalopohapa: @Garret With PICs, isr flags need to be explicitly reset, usually clearing a bit in a register.
• akohlsmith: very well done; clearing the flags is the first thing I write in an ISR and that "muscle memory" prevented me from even seeing that as a possibility. The other thing I try to do is to make sure my main loop does something so that I can verify that the code is running normally and the ISR is failing (or whatever the problem happens to be). +1 from me at any rate.
• Lundin: Clearing interrupt flags improperly is a very common bug in embedded systems, so a good practice is to always disassemble the ISR code clearing the flag, to ensure that it actually does what you expect it to: actually clearing the specific flag, and only that. Make sure no other adjacent flag is cleared by mistake. No matter which MCU you use, some peripheral hardware wants you to write 0 to clear, some wants you to write 1, some want you to read the register etc etc.

Answer 2

Have you identified the global variable as a volatile?

To ensure that the compiler does not optimize out any variable updates expected to be done within an interrupt handler, the variable declaration must be thus:

volatile unsigned int theVariableThatTheIsrChanges;

---

Edit to original question invalidates above solution. Other possible issues:

• Global variable is not in scope within ISR: This should throw an error, but some compilers and compile flags have automatic declaration of undeclared variables on first access, making such bugs notoriously difficult to track down.
• Disable all compiler optimizations for debugging: It is possible that for some reason the compiler does not see the actual variable update, and thus optimizes that code out of existence.
• The LED does turn on, but for too short a duration: button_debounce() code is not in listing, not sure how it behaves. Try changing the code to use booleans, and set the variable on a positive return from button_debounce, and never resetting it (OR the return value) for debugging - the LED should now turn on but never turn off.

More possibilities:

• initialize_stuff() has some error causing ISR to never be set up
• ISR is not being called at all
• Interrupt has not been enabled
• The while() loop is blocking interrupts (why?)
• Interrupt flag is not being reset, hence next interrupt never gets generated

---

Final update: The last point above was the solution.

• Garrett Fogerlie: You beat me too it! I bet it's the button_debounce(), possibly optimization related because method is being called from a isr; even though the OP says the value is returned properly.
• apalopohapa: When debugging, check the memory location directly, and make sure the variable referred to in both places point to the same memory location.. Also, how do you know that the interrupt is being called? Have you set breakpoints? Last but not least, does the LED work?
• Garrett Fogerlie: +1 for every possible answer and narrowing it down every time a bit of code was 'strip teased'! Don't delete or update this with only the answer when it finally comes to it since it will probably help a lot of ppl trouble shoot a similar issue.
• Anindo Ghosh: @GarrettFogerlie Yes, it's funny how one spends effort, identifies the issue, and yet another answer gets accepted.
• Garrett Fogerlie: Yep, the worst case of that for me was this answer and I didn't put any time into it, except that I first edited the OP's post then answered the question 1 minute after the OP's posted time. Then hours later someone posted almost the exact same thing (looking at the question now, the selected answer was removed... Now I feel a bit better.)

Answer 3

in "MPLAB XC8 C Compiler User’s Guide for PIC" it says " Do not re-enable interrupts inside the ISR body. This is performed automatically when the ISR returns." But don't forget the keyword "return;", else it won't reenable GIE bit. Also in ASSEMBLY when you return with "RETFIE" it will activate GIE. And as far as I know, PIC16F doesn't use LAT but PORT?

• Velvet: 1. "Do not re-enable interrupts inside the ISR body.": The OP didn't re-enable interrupts within the ISR.
• Velvet: 2. "But don't forget the keyword "return;", else it won't reenable GIE bit." If I understand you correctly, and you're implying that the OP should use the return keyword to issue a RETFIE, then I can assure you that this isn't true. Please clarify if you meant something else.
• Justme: As written, this does not try to address the issue from the correct point of view as it was already correctly solved.
• ggrig: indeed velvet, return doesn't take care of flags, that must be handled manually