attachInterrupt() seems to fire initially and incorrectly.

The code below does two things I do not understand...

  1. When the code runs, the very first thing is a serial output of

button PRESSED: 9

originally I didn't have the delay() in the setup() and it sensed a pressed @ 0 millis().

  1. I can end up with "button pressed" as the last thing in the serial output, despite the button NOT being pressed -- I can verify this by using an led or volt meter in line, but in simpliest mode
    Ground -> button -> arduino digital pin 2.
    So even tho the button isn't pressed, the code thinks otherwise. Next button press will result in a single "button RELEASED".

Can someone explain that please? I'm totally losted.

#define learn_PIN  2
volatile byte buttonState = 0;
 
void buttonDown() {
  attachInterrupt(digitalPinToInterrupt(learn_PIN), &buttonUp, RISING);
  Serial.print(F("button PRESSED: ")); Serial.println(millis());
  buttonState = 1;
}

void buttonUp() {
  attachInterrupt(digitalPinToInterrupt(learn_PIN), &buttonDown, FALLING);
  Serial.print(F("button RELEASED: ")); Serial.println(millis()); 
  buttonState = 0;
}

void setup() {
  Serial.begin(115200);
  
  pinMode(learn_PIN, INPUT_PULLUP);
  //digitalWrite(learn_PIN, HIGH);
delay(10);
  attachInterrupt(digitalPinToInterrupt(learn_PIN), &buttonDown, FALLING);
}

void loop() {
  delay(10);
}

I also tried this thinking maybe attaching interrupt was messingup..

#define learn_PIN  2
volatile byte buttonState = 0;
 
void buttonDown() {
  switch( buttonState ) {
    case 0:
      Serial.print(F("button PRESSED: ")); Serial.println(millis());
      buttonState = 1;
      break;
    case 1:  
      Serial.print(F("button RELEASED: ")); Serial.println(millis()); 
      buttonState = 0;
      break;
  }
}

void setup() {
  Serial.begin(115200);
  
  pinMode(learn_PIN, INPUT_PULLUP);
  digitalWrite(learn_PIN, HIGH);
  attachInterrupt(digitalPinToInterrupt(learn_PIN), &buttonDown, CHANGE);
}

void loop() {
}

Still, an initial "button pressed' message, and pressing/releasing the button can result in a "button pressed" message remaining (and again, pressing the button generates only a button released" message, no double "button pressed" so it doesn't seem to be a timing issue, athough it must be.

This is with an Arduino ProMini @ 16mhz

A wiring diagram and/or schematic would be helpful.
ESPECIALLY: How is learn_PIN wired?

https://www.arduino.cc/en/Reference/AttachInterrupt

attachInterrupt(digitalPinToInterrupt(learn_PIN), &buttonDown, CHANGE);
Where does it say to use '&' for the ISR (in re. the syntax)?

I was reviewing and they updated the Reference since the (only) time I ever did with an ISR [using the direct interrupt number instead of this newer 'digitalPinToInterrupt()' convention.]

You'll need to clear the interrupt flag in setup after you configure pin 2 as INPUT_PULLUP and write HIGH to the pin.

To do this, use:

EIFR |= bit(INTF0); // clear INT0 interrupt flag

Now you won't get the double "button PRESSED:" when you open the serial monitor.

How it's wired (@vaj4088): Sorry for not being clear: The button is between ground and pin #2 so when it's pushed, it brings pin#2 to ground.
See attached, both fritzing & image

Clear Interrupt flag (@dlloyd): Cool; but that doesn't explain why I an end up in a button-pushed state when it's released...
See attached, I am not 'holding' the button when I take the screen shot

"&" convension(@Runaway Pancake) -- like a LOT of things in arduino, it can be "Arduino" or "C"... that is distracting from the question... Take it out if it helps you... Result is the same.
This code is identical, and used in the above image.

#define learn_PIN  2
volatile byte buttonState = 0;

void buttonDown() {
  switch( buttonState ) {
    case 0:
      Serial.print(F("button PRESSED: ")); Serial.println(millis());
      buttonState = 1;
      break;
    case 1:  
      Serial.print(F("button RELEASED: ")); Serial.println(millis()); 
      buttonState = 0;
      break;
  }
}

void setup() {
  Serial.begin(115200);
  pinMode(learn_PIN, INPUT_PULLUP);
  digitalWrite(learn_PIN, HIGH);
  EIFR |= bit(INTF0); // clear INT0 interrupt flag
  attachInterrupt(digitalPinToInterrupt(learn_PIN), buttonDown, CHANGE);
}

void loop() {
}

buttonPressedWhenItIsNot.png

Yo do not read buttonState.

Do not use Serial.print in an ISR.

Also you do not denounce so your interrupts when functioning correctly will catch many up and down events while you think you just pressed the button once

dlloyd:
You'll need to clear the interrupt flag in setup after you configure pin 2 as INPUT_PULLUP and write HIGH to the pin.

There is no need to write HIGH to the pin - that's what INPUT_PULLUP does for you - sets the pin as INPUT And then set it HIGH to activate the built in PULLUP hence the mode INPUT_PULLUP :slight_smile:

The interrupt flag is probably not set at that time unless the button was already pressed but indeed no harm in clearing it properly unless the button is down in which case you might want to capture this

dlloyd:
You'll need to clear the interrupt flag...

EIFR = bit(INTF0); // clear INT0 interrupt flag

...is the correct way to clear the flag.

J-M-L:
The interrupt flag is probably not set at that time unless the button was already pressed...

Or the pin was floating, which it was, and it happened to change states, which it easily could.

...is the correct way to clear the flag.

I thought either way would only clear bit 0 in the register by writing logical 1 to it.
Sorry, I'm failing to see the difference in the net effect ... unless "|=" takes 2 cycles and "=" is a one cycle operation. Is that it?

Here to the end...
http://forum.arduino.cc/index.php?topic=410964.msg2829257#msg2829257

The main thing here is you don't read the actual state of the button.

The button will bounce. If you press it, it falls, but if it bounces up again before the ISR is ended to handle the falling you have an error. Because now you are waiting for it to rise but it's already high. And because you Serial.print() in the interrupt the interrupt is SLOW.

And it's a bit overkill to use the interrupt pins for a simple button... Just poll it :wink:

Also, using

const byte LearnPin = 2;

to avoid the macro madness is considered to be better. See

Out of curiosity, if he were to move the Serial.begin() after the pinMode()

void setup() {
  pinMode(learn_PIN, INPUT_PULLUP);
  Serial.begin(115200);
...

would the clearing of the flag be necessary?

The reason I ask is:

  • My assumption that setting the INPUT_PULLUP before attaching the interrupt would actually guarantee that the pin is not floating as we steer that pin to a known state.

  • in recent (a few years now?) versions of the library, the first call to attachInterrupt in the program clears all interrupt flags.

--> so if there is no code before pinMode in the setup() does anything calls attachInterrupt and thus does create a situation where the Interrupt flag would be set before we prevent the pin from floating?


for intellectual curiosity of the readers
(because this surprised me when I learnt about it)


EIFR = bit(INTF0); // clear INT0 interrupt flag

...is the correct way to clear the flag.

Indeed. while the other way with a [color=blue][b]|=[/b][/color] would seem reasonable and might work, it generates loads of unnecessary code to read the value and then writing the logical 1 which clears the flag. [EDIT after the good comment from Whandall below: You might also clear the other interrupt if it was set though which is possibly not what you want to do!]

if you read the ATMEGA reference for this register (look for 13.2.3 EIFR – External Interrupt Flag Register) , the flag is actually cleared by writing a logical one to it. One may worry about bits 7:2 but they are currently Read Only in hardware and Reserved for future use.

The ATMEL documentation also explains clearly why this is good to proceed this way (just one OUT instruction in assembly)

J-M-L:
One may worry about bits 7:2 but they are currently Read Only in hardware and Reserved for future use.

I would worry more about the other used bit.

Whandall:
I would worry more about the other used bit.

hum - the expression (bit(INTF0):wink: returns with only 1 bit set, right?

Here to the end...

Oh man, any other bits that are set in the register basically clear themselves. I guess with any register designed in this fashion (writing logical 1 clears the bit), you would need to do this.

Oh man x??!, in the Due there dozens and dozens of registers like this just for operations with I/O. Gulp.

+1 (thanks for that, I think).

Not if used in conjunction with |= .

indeed! good point.

dlloyd:
Oh man, any other bits that are set in the register basically clear themselves.

Exactly.

I guess with any register designed in this fashion (writing logical 1 clears the bit), you would need to do this.

Exactly.

J-M-L:
would the clearing of the flag be necessary?

Yes. After enabling the pull-up (and waiting five clock cycles which occurs naturally because of overhead) and before calling attachInterrupt. If spurious triggers should be ignored the sequence is...

• Enable pull-up
• Wait five clock cycles (not necessary when using Arduino API functions)
• Clear the appropriate flag
• Call attachInterrupt

The External Interrupt hardware is functional from power-up. If the appropriate trigger ever occurs the INTF0 flag is set.

thx