Interrupt still happening after detached. ( fix now available)

pin 2 is pulled up. I'm pulling down by touching a ground wire to it while the led is off (3 second interval).

The pin should be stabilized in that amount of time.

Board?

UNO board with Duemilanove bootloaded atmega328.

Further test show evidence this may be a bug (if it's not a programming issue on my part).

It is not a hardware issue (ie floating pin). I added a counter to the isr that increments every time the isr is called. I also added a flag to indicate attached or detached, and another counter to increment every time the isr is called while interrupt is detached. These would be invalid interrupts.

While the interrupt is attached, the interrupt counter increments each time I ground the pin (many times actually, due to contact bounce). This is expected.

However, if I ground the pin multiple times while the interrupt is detached, the interrupt counter only increments 1 time, as does the invalid-interrupt counter. (If I don't ground the pin at all, there is no increment.)

This would imply there is at least one interrupt that will occur after the interrupt is detached (if the interrupting action occurs). This would be a bug, no?

I wonder if this would explain some of the interrupt headaches so many seem to have. (me included)

Loudhvx:
...a bug, no?

Well, there is a bug in the program, not a bug in Arduino or ATmega.

Here's the deal:

If an edge-triggered interrupt condition occurs while the interrupt is enabled, the interrupt flag for that particular condition is set and the the Interrupt Service Routine is executed. The interrupt flag is automatically reset when the program returns from the ISR.

However...

If an edge-triggered interrupt condition occurs while the interrupt is disabled, the interrupt flag is set but the Interrupt Service Routine is not executed. That is, the ATmega remembers that an interrupt condition has occurred. Then, when you re-enable the interrupt, the interrupt flag causes the ISR to be executed immediately.

Bottom line: Make sure the interrupt flag is reset before re-enabling the interrupt.

Maybe something like the following:

.
.
.
void loop()
{
    digitalWrite(13, HIGH);               //led on
    //
    // Write 1 to Interrupt Flag 0 to make sure it is reset
    //
    EIFR = (1 << INTF0);
    attachInterrupt(0, isr0, FALLING);     //enable interrupt
.
.
.

Regards,

Dave

The interrupt flag is automatically reset when the program returns from the ISR.

As far as I can tell from the 328 datasheet, the flag is cleared when the ISR is called not when it returns.

[quote author=Coding Badly link=topic=59217.msg426299#msg426299 date=1303499961]
...flag is cleared when the ISR is called not when it returns.

[/quote]I don't disagree.

Actual wording is
"The flag is cleared when the interrupt routine is executed."

Now, if people are using un-debounced inputs for edge-triggered interrupts, maybe it could make a difference whether the flag is cleared going-into or coming-out-of, but for me all bets would be off anyhow. I mean, the mind boggles and the flabber is gasted...

Regards,

Dave

Having played with interrupts some, and while not an expert by far, my advice is to simply not use detachInterrupt(0);, as there are better ways to structure your program then rely on it.

It's not unlike using Serial.flush, while it may have a rare useful purpose once and a while, it almost always is either mistake to use or comes with side effects.

Lefty

Bug? Feature? Someone else will have to decide. In my opinion it's a bug because the behaviour deviates from what is expected.

@Loudhvx: What do you want? Are you looking for a solution to this problem? Or are you trying to report a bug?

In my opinion it's a bug because the behaviour deviates from what is expected.

But if that is what is documented in the processor datasheet, the behaviour is exactly as expected.

I have reproduced it on a Uno.

Extensive reading and testing seems to indicate what the problem is. Rather bizarrely the processor, when attachInterrupt is called, can "remember" whether the interrupt condition has already been met. Hence, if you ground the pin, and then call attachInterrupt, the interrupt fires.

This proves it:

volatile int flag = 0;

void setup() {
  Serial.begin (115200);
  digitalWrite(2, HIGH);      //pullup 2

  pinMode(13, OUTPUT);       //LED
  Serial.println ("go!");
  digitalWrite(13, HIGH);                 //time to go!
  delay (5000);
  digitalWrite(13, LOW);                 //time is up!
  
   // interrupt on
  attachInterrupt(0, isr0, FALLING);     //enable interrupt

  // did we get an interrupt?
  Serial.println (flag, DEC);
  }

void loop() {}

void isr0 () {
  flag = 1;
}

There is no detachInterrupt there. After the sketch starts and displays "go" you have 5 seconds to ground (and release) pin 2. If you do, and only if, then it displays 1, otherwise it displays 0. And this is despite the fact that you generate the "interrupt" before interrupts are enabled on that pin.

Now, add this line just before attaching the interrupt:

EIFR = 1;

That clears the interrupt flag (for interrupt 0). According to the datasheet:

The flag is cleared when the interrupt routine is executed. Alternatively, the flag can be cleared by writing a logical one to it.

So this is manually clearing the "I saw an interrupt" flag, before enabling interrupts.

Arguably, attachInterrupt should do this. And note that detachInterrupt has no effect on the flag at all.

I don't know whether there would be programs around that rely on interrupts being "pre-triggered" before you even attach the interrupt. Perhaps there are.

Note that if you are using interrupt 1, it would be:

EIFR = 2;

I agree, as far as I'm concerned, it's a bug in the Arduino system. Not an Atmega bug, but as far as anyone programming an Arduino using the Arduino Reference documentation, it's a bug.

I don't think it matters if the input is a de-bounced switch or not. The same bug would happen with any pulsing input were applied. I produced the input pulse several seconds before the false interrupt happened. The same could happen with an encoder etc.

I think there should be some sort of note in the Attach/Detach Reference if an interrupt can be triggered by some event that happened before the attach statement. It may save someone a lot of frustration. I chose Arduino because it's aimed at non-programmers like me.

...maybe just a note to clear the flag or whatever before doing the attach statement. Dave's and Nick Gammon's notes will surely be in my personal notes on the Arduino. Thanks for that. I agree, I would have assumed that should have been taken care of in the Attach routine.

EIFR = 1; // for interrupt 0
EIFR = 2; // for interrupt 1

What would be better, or different etc. using Dave's bit-shift statement (if that's what it is) or just Nick's simple assignment?

Dave's is more correctly documented. However more accurately than what either of us suggested you should "or" it in (or you have the effect of clearing the other interrupt flag, almost certainly what you don't want). That is (one of):

    EIFR |= (1 << INTF0);    // clear any outstanding interrupt 0
    EIFR |= (1 << INTF1);    // clear any outstanding interrupt 1

You will find (depending on the processor however) INTF0 and INTF1 defined like this:

#define INTF0 0
#define INTF1 1

So 1 << 0 is going to be 1, and 1 << 1 is going to be 2, so we are using the same numbers. Dave's method just follows the mnemonics in the datasheet better, and probably translates better into different processors, which might have them defined as different values.

I think the documentation for attachInterrupt should be amended to strongly suggest doing the lines above. I agree that you don't expect an interrupt to immediately register from some action that you might have taken an hour ago, just because you attach interrupts. You reasonably expect them to be effective "now" not in the past.

There is also a reasonably strong case for amending attachInterrupt to do it for you. After all, attachInterrupt is supposed to be shielding you from having to know all that low-level stuff.

In fact it should probably be more like this:

  uint8_t oldSREG = SREG;
  cli();
  EIFR |= (1 << INTF0);    // clear any outstanding interrupt 0
  SREG = oldSREG;

Just in case there is an interrupt between reading EIFR and or'ing in the new value.

[quote author=Nick Gammon link=topic=59217.msg426522#msg426522 date=1303533435]
...you should "or" it in (or you have the effect of clearing the other interrupt flag...[/quote]
No you don't. Here's how it works:

If you write a '0' to one of the interrupt flag bits it doesn't affect that bit. If you write a '1' to one of the interrupt flags it makes sure that that bit is cleared to '0'.

There is absolutely no reason to "or" the bit into the interrupt flag register. Just write the '1' to whatever bit position corresponds to the flag that you want to clear, and the other bits are not changed. That's why the ATmega designers did it this way and that's why I suggested the code that I showed.

Well, if you just write the bit instead of "oring" into EIFR you don't need to do the SREG thing. Period. Full stop.

Regards,

Dave

Good point. Well that simplifies things.

Thanks for the replies.
So then would this be the final code then? :

// put these 2 lines before Setup? or in Setup?
#define INTF0 0
#define INTF1 1

EIFR = (1 << INTF0); //use before attachInterrupt(0,isr,xxxx) to clear interrupt 0 flag
EIFR = (1 << INTF1); //use before attachInterrupt(1,isr,xxxx) to clear interrupt 1 flag

Loudhvx:
// put these 2 lines before Setup? or in Setup?
#define INTF0 0
#define INTF1 1

No. They are already defined. Do not re-define them!

Loudhvx:
EIFR = (1 << INTF0); //use before attachInterrupt(0,isr,xxxx) to clear interrupt 0 flag
EIFR = (1 << INTF1); //use before attachInterrupt(1,isr,xxxx) to clear interrupt 1 flag

That would work.

Just take the code you have been testing with and put the "EIFR = (1<< INTF0); " statement before the call to attachInterrupt().

Let us know if it works out.

Regards,

Dave

I ran 12 permutations of the 4 clearing methods against each interrupt and both interrupts simultaneously. (Just to make sure nothing unexpected happened) (Of course, there are probably more conditions I didn't think of.)

Both of these methods work, and work exclusively on the interrupt flag they are meant to clear.

EIFR = 1; //use before attachInterrupt(0,isr,xxxx) to clear interrupt 0 flag
EIFR = 2; //use before attachInterrupt(1,isr,xxxx) to clear interrupt 1 flag

or

EIFR = (1 << INTF0); //use before attachInterrupt(0,isr,xxxx) to clear interrupt 0 flag
EIFR = (1 << INTF1); //use before attachInterrupt(1,isr,xxxx) to clear interrupt 1 flag

Which method would be better to advise users in the Arduino's attachInterrupt() documentation?

In my opinion, even though the first method looks more straight forward, the latter keeps the 0 and 1 aligned with the interrupt numbers, so would be less prone to an accidental mismatch. There are probably other advantages to the latter method as well?