attachInterrupt(...) should ignore pending interrupts (clear interrupt flag)

[GoogleCodeExporter]

What steps will reproduce the problem?
Use this code:

/*

Attach your interrupt signaling button to DPIN2
Attach you attachInterrupt button to DPIN13

Fire up the serial monitor:

The first number shows if the interrupt is currently attached.
The second number shows if the interrupt is in a signaled state, i.e. if it is 
prepared to fire off the ISR.
  Note that in normal operation, this interrupt ready flag is unset immediately upon call of the ISR.
The third number shows a count of the calls to isrSignal, within which it is 
incremented by 1.

Follow these steps:
Trigger your interrupt signaling button once. 
You will see that the interrupt is detached by (EIMSK & 0x1) being 0.
(EIFR & 1) will be 0, as the trigger flag is not seen at this point in the 
code: The interrupt triggers and the flag is cleared.
The count will be 1.

Trigger the interrupt signal button again. 
EIMSK & 1 will not change: The interrupt was already not attached.
EIFR & 1 will now be 1, because the interrupt flag signals regardless of 
whether an interrupt is attached.
The count will still be 1, as the ISR was not called due to the interrupt not 
being attached.

Now press your attachInterrupt button to cause the interrupt to be reattached.
EIMSK & 1 will be 1. The ISR is reattached.
EIFR & 1 will be 0: The flag has been cleared.
The count will be 2. This is wrong, it should be 1 as the ISR should only have 
been called 
  once: The first time the interrupt button was pressed. The second time the button was pressed,
  the interrupt was not attached, so the user does not expect intervening triggers of the interrupt
  to cause an immediate call to the ISR upon reattachment.

*/

#define INTERRUPT_SIGNAL_PIN 2
#define ATTACH_INTERRUPT_PIN 13

volatile boolean signaled = false;
volatile long signalCounter = 0;

void setup () {
  Serial.begin(115200);
  pinMode(INTERRUPT_SIGNAL_PIN,INPUT);
  pinMode(ATTACH_INTERRUPT_PIN,INPUT);
  attachInterrupt(0,isrSignal,RISING);
}

void loop () {
  if (digitalRead(ATTACH_INTERRUPT_PIN) == HIGH) {
    attachInterrupt(0,isrSignal,RISING);
  }
  Serial.print( EIMSK, BIN );
  Serial.print(' ');
  Serial.print( EIFR, BIN );
  Serial.print(' ');
  Serial.println( signalCounter );
}

void isrSignal () {
  detachInterrupt(0);
  signalCounter++;
  signaled = true;
}

What is the expected output? What do you see instead?
Explained in the comment in the code.

What version of the Arduino software are you using? On what operating
system?  Which Arduino board are you using?
This issue is seen in version 0022 of the IDE on Windows XP using a 
Duemilanove. However, this issue has nothing to do with any of this: See below.

Please provide any additional information below.

The solution to this issue is to read the datasheet for certain Atmegas, for 
example the 1284P, which on page 69 says:

When changing the ISCn bit, an interrupt can occur. Therefore, it is 
recommended to first disable INTn by clearing its Interrupt Enable bit in the 
EIMSK Register. Then, the ISCn bit can be changed. Finally, the INTn interrupt 
flag should be cleared by writing a logical one to its Interrupt Flag bit 
(INTFn) in the EIFR Register before the interrupt is re-enabled.

This is in reference to attaching an interrupt and is in the EICRA subsection 
of the External Interrupts section.

The short version: The interrupt flag should be unset when an interrupt is 
attached, otherwise the ISR can be triggered immediately upon attachment.

The fix involves a modification to WInterrupts.c. Currently, there is a 
switch() full of #ifs, each condition containing a variation of the following 
two lines of code:

EICRA = (EICRA & ~((1 << ISC00) | (1 << ISC01))) | (mode << ISC00);
EIMSK |= (1 << INT0);

The first line sets EICRA, which determines under which interrupt triggers 
conditions an ISR will be triggered (RISING, FALLING, etc.). The second line 
activates the correct bit in EIMSK high, which enables the external interrupt 
pin.

The fix is the addition of two more lines of code:

EIMSK &= ~(1 << INT0); //Added
EICRA = (EICRA & ~((1 << ISC00) | (1 << ISC01))) | (mode << ISC00);
EIFR |= (1 << INTF0); //Added
EIMSK |= (1 << INT0);

The first added line makes sure that the external interrupt pin is disabled 
while changing the value of EICRA, as recommended in the datasheet. The second 
added line clears the interrupt triggered flag. It looks like the flag is being 
set by use of |=, but this is the correct method. See page 69: "Alternatively, 
the flag can be cleared by writing a logical one to it."

This fix is tested and fully working.

The first added line of code, which disables the external interrupt pin, is not 
required for this specific fix. However, it is recommended in the datasheet and 
seems to be able to help in certain fringe conditions.

Original issue reported on code.google.com by kylehard...@gmail.com on 23 Mar 2011 at 5:33

[GoogleCodeExporter]

A note: The line

EIFR |= (1 << INTF0);

must, as written, come before the activation of the external interrupt pin by 
use of

EIMSK |= (1 << INT0);

This is because if the flag is set at the point when the external interrupt is 
activated, the interrupt will be called before the flag is unset.

Original comment by kylehard...@gmail.com on 23 Mar 2011 at 5:44

[GoogleCodeExporter]

Attached is a copy of WInterrupts.c that I'm currently using which is updated 
with the fix for all conditions, so no one has to waste time duplicating effort.

Original comment by kylehard...@gmail.com on 26 Mar 2011 at 5:49

Attachments:

• WInterrupts.c

[GoogleCodeExporter]

I had the same problem in my sketch and fixed it by setting INTF0 in EIFR, as 
you did.

I hope your fix is soon incorporated.

Original comment by sakud...@gmail.com on 28 Dec 2011 at 3:21

[GoogleCodeExporter]

Original comment by dmel...@gmail.com on 11 Mar 2012 at 5:16

• Changed title: attachInterrupt(...) should ignore pending interrupts (clear interrupt flag)
• Added labels: Component-Core

[GoogleCodeExporter]

Issue 852 has been merged into this issue.

Original comment by dmel...@gmail.com on 11 Mar 2012 at 5:17

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I have also verified this issue.

Further, we should add a note in the documentation about interrupts indicating 
that while interrupts are disabled with cli(), they will still be tracked in a 
pending interrupts register.  So when you re-enable interrupts, the handler 
will be called.  Maybe this is my own fault for not remembering the "joy" of 
interrupt programming on x86 platforms 20 years ago..  :)  But docs would be 
good.  Heck, we should even write a simple "clear pending interrupts" function 
for folks.  Just my $0.02.

Thanks

Original comment by digi...@gmail.com on 5 Apr 2013 at 4:36

[GoogleCodeExporter]

I've encountered this issue as well and am trying to incorporate the changes 
here, can anyone direct me to some information as to how I can use the modified 
Winterrupts.c file? I've changed the files in my Arduino installation and this 
seems to have no effect. I also changed the suffix on these file and this 
(surprisingly) didn't result in any complaints from the compiler. Any ideas? 

Thanks!

Original comment by yish...@gmail.com on 24 Aug 2013 at 8:45