Unreliable Interrupt for push button

I am relativley new to arduino adn electronics.
I am having problems with using a push button and interrupt to turn it into a toggle button.
here is the code

#include<Servo.h>

Servo motor1;

volatile bool wave = false;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);


  pinMode(3, INPUT);

  
  pinMode(5, OUTPUT);
  motor1.attach(5);
  

  attachInterrupt(digitalPinToInterrupt(3), setWave , RISING);
}

void loop() {

  if (wave) {
    motor1.write(90);
    delay(500);
    motor1.write(40);
    delay(500);
  } else {
    motor1.write(40);
  }
}

void setWave() {
  wave = !wave;
}

attached is the wiring.

It works unreliably, so I am guessing the button is sometimes sending out two pulses, does this sound right? how can i fix this?

The idea behind this ‘project’ was to learn both interrupts and servo control

thanks :smiley:

Try adding some bitton debouncing, for example :

void setWave() {
  static uint32_t lastInt = 0 ;
  if ( millis() - lastInt > 100 ) {  // debounce 100mS
    wave = !wave;
    lastInt = millis() ;
  }
}

GreenOlive:
The idea behind this 'project' was to learn both interrupts and servo control

Then you failed completely on the first. :astonished:

As a beginner, it is incredibly unlikely that interrupts will be useful to you.

A common "newbie" misunderstanding is that an interrupt is a mechanism for altering the flow of a program - to execute an alternate function. Nothing could be further from the truth! :astonished:

An interrupt is a mechanism for performing an action which can be executed in "no time at all" with an urgency that it must be performed immediately or else data - information - will be lost or some harm will occur. It then returns to the main task without disturbing that task in any way though the main task may well check at the appropriate point for a "flag" set by the interrupt.

Now these criteria are in a microprocessor time scale - microseconds. This must not be confused with a human time scale of tens or hundreds of milliseconds or indeed, a couple of seconds. A switch or pushbutton operation is in this latter category and a mechanical operation perhaps several milliseconds; the period of a 6000 RPM shaft rotation is ten milliseconds.

Unless it is a very complex procedure, you would expect the loop() to cycle many times per millisecond. If it does not, there is most likely an error in code planning; while the delay() function is provided for testing purposes, its action goes strictly against effective programming methods. The loop() will be successively testing a number of contingencies as to whether each requires action, only one of which may be whether a particular timing criteria has expired. Unless an action must be executed in the order of microseconds, it will be handled in the loop().

So what sort of actions do require such immediate attention? Well, generally those which result from the computer hardware itself, such as high speed transfer of data in UARTs(, USARTs) or disk controllers.

An alternate use of interrupts, for context switching in RTOSs, is rarely relevant to this category of microprocessors as it is more efficient to write cooperative code as described above.

In short, as a beginner, it is incredibly unlikely that interrupts will be useful to you and in particular, they have no place in connection with switches or pushbuttons. :astonished:

6v6gt:
Try adding some bitton debouncing, for example :

void setWave() {

static uint32_t lastInt = 0 ;
 if ( millis() - lastInt > 100 ) {  // debounce 100mS
   wave = !wave;
   lastInt = millis() ;
 }
}

thanks for that speedy reply, i had to double the debounce time but it works properly now. Obviosly if you clicked the button in that time it wouldnt respond, are there ways of fixing it on the hardware side, besides getting a better button? its not important for this but id just like to know for the future.

Paul__B:
In short, as a beginner, it is incredibly unlikely that interrupts will be useful to you and in particular, they have no place in connection with switches or pushbuttons. :astonished:

thanks for this whole thing, I will try rewrite the program without an interrupt or delay. At least I have an idea behind interrupts though and where to use them. :cold_sweat:

Are you using an external pullup on the button? If not the input will be floating and that could be causing spurious triggering of the interrupt.

@Paul__B
While I think your explanation of what interrupts are useful for and that they are not generally useful for detecting button presses is exactly right, I feel your comments are a little unhelpful to GreenOlive's stated purpose of learning about interrupts. For learning purposes (and probably only for learning purposes) using a push button to trigger an interrupt seems entirely reasonable. Understanding that interrupts would not normally be used for detecting a button press is also something useful to learn.

@GreenOlive
When it comes to reading other people's code I am not the best. Some observations which might well be corrected by others:

Above all else, learn to write code without using delay.

I think delay uses interrupts (I await correction), using delay and another interrupt is going to cause problems, especially if you are new to this.

GreenOlive:
thanks for that speedy reply, i had to double the debounce time but it works properly now. Obviosly if you clicked the button in that time it wouldnt respond, are there ways of fixing it on the hardware side, besides getting a better button? its not important for this but id just like to know for the future.

You could do a "Hardware Debounce", taking advantage of the Arduino's Schmidt Trigger inputs [Digital inputs, only].

Look for the heading "Hardware Setup," in the following article: https://www.allaboutcircuits.com/technical-articles/switch-bounce-how-to-deal-with-it/

PerryBebbington:
@Paul__B
While I think your explanation of what interrupts are useful for and that they are not generally useful for detecting button presses is exactly right, I feel your comments are a little unhelpful to GreenOlive's stated purpose of learning about interrupts. For learning purposes (and probably only for learning purposes) using a push button to trigger an interrupt seems entirely reasonable. Understanding that interrupts would not normally be used for detecting a button press is also something useful to learn.

I second this.

For a another hardware solution, simply solder a 1uF capacitor across the button contacts.
Interrupts for a button can be useful if, at the time of pressing, the MCU may be in sleep mode. Generally, though, and as has been pointed out with exemplary clarity, there are usually better solutions for handling button presses.

GreenOlive:
It works unreliably, so I am guessing the button is sometimes sending out two pulses, does this sound right? how can i fix this?

It may be not just two, but even dozens of pulses. That is why interrupts are inappropriate, apart from just being inappropriate. It's called contact bounce.

GreenOlive:
I had to double the debounce time but it works properly now. Obviously if you clicked the button in that time it wouldn't respond,

So it is not working properly, is it? You see, it not only trips when you press the button, but also when you release it, so you have had to make the delay so slow that you have released it within that time. :astonished:

GreenOlive:
are there ways of fixing it on the hardware side, besides getting a better button?

A "better button" would be something like a mercury wetted switch.

The thing is, you have the perfect solution for de-bouncing. It's called a microprocessor. As per my stock piece, the loop() can and always should cycle many times per millisecond so it has perfect opportunity to monitor switch bounce.

Your code does not properly handle "state change". Here if you care to study it, is some serious code:

// Blink without "delay()" - multi!  With button de-bounce.
// Note that this code is immediately extensible to *many* buttons.

const int led1Pin =  13;    // LED pin number
const int led2Pin =  10;
const int led3Pin =  11;
const int button1 =  4;     // buttons & switches always connect from input to ground

int led1State = LOW;        // initialise the LED
int led2State = LOW;
int led3State = LOW;
char bstate1 = 0;

unsigned long count1 = 0;   // will store last time LED was updated
unsigned long count2 = 0;
unsigned long count3 = 0;
unsigned long bcount1 = 0; // button debounce timer.  Replicate as necessary.

// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check 
boolean timeout(unsigned long *marker, unsigned long interval) {
  if (millis() - *marker >= interval) { 
    *marker += interval;    // move on ready for next interval
    return true;       
  } 
  else return false;
}

// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
  switch (*butnstate) {               // Odd states if was pressed, >= 2 if debounce in progress
  case 0: // Button up so far, 
    if (button == HIGH) return false; // Nothing happening!
    else { 
      *butnstate = 2;                 // record that is now pressed
      *marker = millis();             // note when was pressed
      return false;                   // and move on
    }

  case 1: // Button down so far, 
    if (button == LOW) return false; // Nothing happening!
    else { 
      *butnstate = 3;                 // record that is now released
      *marker = millis();             // note when was released
      return false;                   // and move on
    }

  case 2: // Button was up, now down.
    if (button == HIGH) {
      *butnstate = 0;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else { 
      if (millis() - *marker >= interval) {
        *butnstate = 1;               // jackpot!  update the state
        return true;                  // because we have the desired event!
      }
      else 
        return false;                 // not done yet; just move on
    }

  case 3: // Button was down, now up.
    if (button == LOW) {
      *butnstate = 1;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else { 
      if (millis() - *marker >= interval) {
        *butnstate = 0;               // Debounced; update the state
        return false;                 // but it is not the event we want
      }
      else 
        return false;                 // not done yet; just move on
    }
  default:                            // Error; recover anyway
    {  
      *butnstate = 0;
      return false;                   // Definitely false!
    }
  }
}

void setup() {
  pinMode(led1Pin, OUTPUT);      
  pinMode(led2Pin, OUTPUT);      
  pinMode(led3Pin, OUTPUT);      
  pinMode(button1, INPUT);      
  digitalWrite(button1,HIGH);        // internal pullup all versions
}

void loop() {
  // Toggle LED if button debounced
  if (butndown(digitalRead(button1), &bcount1, &bstate1, 10UL )) {
    if (led1State == LOW) {
      led1State = HIGH;
    }
    else {
      led1State = LOW; 
    } 
    digitalWrite(led1Pin, led1State);
  } 

  // Act if the latter time (ms) has now passed on this particular counter,
  if (timeout(&count2, 300UL )) {
    if (led2State == LOW) {
      led2State = HIGH;
    }
    else {
      led2State = LOW; 
    } 
    digitalWrite(led2Pin, led2State);
  } 

  if (timeout(&count3, 77UL )) {
    if (led3State == LOW) {
      led3State = HIGH;
    }
    else {
      led3State = LOW; 
    } 
    digitalWrite(led3Pin, led3State);
  } 
}

You can forget about interrupts!

And delay() too! You can use a variant of my code which flashes the LEDs, to perform your delays.

PerryBebbington:
I think delay uses interrupts (I await correction),

Indirectly.

"delay()" is essentially a "while()" loop on millis(). millis() does not itself use an interrupt, but reads a value which itself is incremented by a timer0 interrupt.

Indirectly.

"delay()" is essentially a "while()" loop on millis(). millis() does not itself use an interrupt, but reads a value which itself is incremented by a timer0 interrupt.

Thank you, makes sense.

Thanks for everyones responses when i have time again i will try to properly understand and rewrite the program