next topic »

[Reply #15 on:]

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.

[Reply #16 on:]

In fact it should probably be more like this:

Code:

  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.

[Reply #17 on:]

...you should "or" it in (or you have the effect of clearing the other interrupt flag...

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.

In fact it should probably be more like this:

Code:

 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.

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

[Reply #18 on:]

Good point. Well that simplifies things.

[Reply #19 on:]

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

[Reply #20 on:]

// 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!

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

[Reply #21 on:]

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?

[Reply #22 on:]

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

Why not write an "ATTACH_INTERRUPT" macro that hides it?

[Reply #23 on:]

I would think, eventually, the fix should be included in the attachInterrupt() routine itself, unless there is some unforseen down-side.  (As Nick mentioned, as unlikely as it may seem, maybe some sketches out there are using the pre-triggered interrupts?)

[Reply #24 on:]

...these methods work

The compiler evaluates constant expressions at compile time and just sticks the value into the code appropriately.  (No run-time operations are required to do the bit shifting in the statements we are talking about here.)

The code generated by the statement EIFR = 1; is exactly the same as that generated by the statement EIFR = (1 << INTF0);

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

Use symbolic identifiers, not "magic numbers."

In other words don't use EIFR = 1;

Another possibility (with the exact same results) is to use the avr gcc/libc macro _BV  (stands for Bit Value);

_BV(INTF0) is the same as (1 << INTF0)
_BV(INTF1) is the same as (1 << INTF1)

Etc.

Since the shift operation is standard C (and C++) and the _BV requires a vendor-supplied macro, many programmers will prefer the shift stuff.  (Believe it or not, some people actually program things other than Arduino boards and with compilers other than avr-gcc, and they like to keep things vendor-independent wherever it's practical.  Really.)



Regards,

Dave

[Reply #25 on:]

(Believe it or not, some people actually program things other than Arduino boards and with compilers other than avr-gcc, and they like to keep things vendor-independent wherever it's practical.  Really.)

Thanks for the clarification.   

[Reply #26 on:]

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:

Code:

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:

Code:

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:

Code:

EIFR = 2;

Thanks Nick.

I've been pulling my hair out thinking  I had some electrical issue on my project.
I had suspected it was something to do with attach/detach but couldn't nail it down.

I'm using an UNO- REV2 (does the rev make a difference for this problem)?
Which Rev did you use to duplicate this?

Should this be treated as a software bug and implemented in the compiler, so the user doesn't have to remember to clear the interrupt manually?

[Reply #27 on:]

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.

Agreed. Has this issue been formally reported to the maintainers of the IDE / compiler?
Perhaps it's already been fixed in 1.0, but that release apparently has enough problems to avoid using it for a while.

[Reply #28 on:]

Perhaps it's already been fixed in 1.0, but that release apparently has enough problems to avoid using it for a while.

My copy of version 1.0 RC2 does not appear to have any mention of EIFR in attachInterrupt.

[Reply #29 on:]

Perhaps it's already been fixed in 1.0, but that release apparently has enough problems to avoid using it for a while.

My copy of version 1.0 RC2 does not appear to have any mention of EIFR in attachInterrupt.

Thanks. Do we know if this issue has been officially reported to the maintainers?