Toggle LED from push button

I'm trying to toggle an LED when I push a button (which returns to a resting state after I push it) so that the LED stays on after one push (and release) , and turns off after another push (and release).

Here is my code. It doesn't seem to work. I cannot find my error. Any help would be greatly appreciated. I included the debugging monitoring, in case I am doing that wrong as well...

const int buttonPin = 2;    
const int ledPin =  13;      

// variables will change:
int buttonState = 0;         
int buttonStay = 0;
int buttonChange = 0;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT);

  Serial.begin(9600);
  // debug
}

void loop() {
  buttonState = digitalRead(buttonPin);

  if (buttonState == HIGH) {
    // switch is on
   Serial.println(buttonChange);
   Serial.println(buttonStay);
   
    if (buttonChange = 1) {
      //switch was just recently changed to on
      buttonChange = 0;
      
      if ( buttonStay = 0 ) {
        buttonStay = 1;
        digitalWrite(ledPin, HIGH);
      } else {
        buttonStay = 0;
        digitalWrite(ledPin, LOW);
      }
    }
    
  } else {
    // switch is off
    buttonChange = 1;
    Serial.println(buttonChange);
    Serial.println(buttonStay);
  }
  
}

I found the error: I have to use == for the if statement. Sorry, I don't usually program in C.

Now I've found that the program is stable when I use a delay (even of 5ms), but not when I don't use it. Any idea why?

const int buttonPin = 2;     
const int ledPin =  13;    

// variables will change:
int buttonState = 0;         
int buttonStay = 0;
int buttonChange = 0;

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);
}

void loop() {
  // read the state of the pushbutton value:
  buttonState = digitalRead(buttonPin);

  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (buttonState == HIGH) {
    // switch is on
       
    if (buttonChange == 1) {
      //switch was just recently changed to on
      buttonChange = 0;
      
      if ( buttonStay == 0 ) {
        buttonStay = 1;
        digitalWrite(ledPin, HIGH);
      } else {
        buttonStay = 0;
        digitalWrite(ledPin, LOW);
      }
    }
    
  } else {
    // switch is off
    buttonChange = 1;
  }
  
  // delay(5);
}

Should use the Code button </> to present your code.

Here's a different variation of your code that probably wouldn't need debounce or timing delays...

const int buttonPin = 2;
const int ledPin =  13;

// variables will change:
word buttonState = 0xAAAA;
byte ledState = 0;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);

  Serial.begin(9600);
  // debug
}

void loop() {
  buttonState = (buttonState << 1) | digitalRead(buttonPin); // logic analyzer

  // for viewing logic analyzer
  // delay(250);
  // Serial.println(buttonState, BIN);

  if (buttonState == 1) { // button just pressed
    ledState = !ledState;
    digitalWrite(ledPin, ledState);
    Serial.print("ledState: ");
    Serial.println(ledState);
  }
}

Feddar:
Now I've found that the program is stable when I use a delay (even of 5ms), but not when I don't use it. Any idea why?

Switch bounce. The code in reply #2 will also bounce, if there is not enough code in the loop to slow it down.

Yes, but that's the beauty of it. Its easy to add a 5ms "loopInterval" that would effectively be 80ms debounce time. Recording the past history makes it easy to detect any pattern desired. It inherently blocks noise or bounce, both on rising and falling state changes of the button. Also blocks intermittent noise. Also could get creative and detect long or short presses with unsigned long variable. Even combination long-short, short-long presses with unsigned long long variable.

dlloyd:
Recording the past history inherently blocks noise or bounce, both on rising and falling state changes of the button. Also blocks intermittent noise.

No, it can't. Without a delay, it will record every transition.

Yes, it records every transition at the loop speed ... the conditional statement doesn't become true until it sees binary 0000000000000001

Any noise within that prevents the if statement from becoming true.

A 5ms time interval will easily throttle down the speed, and provide 80ms debounce time.

dlloyd:
Yes, it records every transition at the loop speed ... the conditional statement doesn't become true until it sees binary 0000000000000001

Any noise within that prevents the if statement from becoming true.

A 5ms time interval will easily throttle down the speed, and provide 80ms debounce time.

Yes, I'm just clarifying that it still depends on an adequate debounce interval.

No response from Feddar ... added readInterval (give it a try).

const int buttonPin = 2;
const int ledPin =  13;

unsigned long lastReadTime;
byte readInterval = 12; // x 7 = 80 ms to detect button press
byte buttonState = 0xAA;
byte ledState = 0;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  Serial.begin(9600);
}

void loop() {
  buttonState = (buttonState << 1) | digitalRead(buttonPin); // shift register
  if ((buttonState == 1) && (millis() - lastReadTime) > readInterval) { // button just pressed
    lastReadTime = millis();
    ledState = !ledState;
    digitalWrite(ledPin, ledState);
    Serial.println(ledState);
  }
}

Sorry, i was out njoying a fantastic sunny day. The first in a week!

Your code is certainly a lot more efficient than mine!

I take if the interval needs to be greater than 12ms to avoid bounce by your example. Why do you comment that x8=96ms to detect button press? Isnt the interval 12ms as written? I'm missing something here...

Thanks for your help.

This is my button tester example sketch. It is a button debouncer modified to report the bounces actually given by the button (they vary). Those are shown as times in us (microseconds) with symbols _ for pressed and ^ for released (single char prints faster than word).

The sketch also shows the handler output with time in ms (milliseconds), don't get them confused.

// This is button_watcher, a test for button bounce --- free and open code Jul 31, 2015 by GFS
// Revised Nov 7, 2014 including many comments on operation of the sketch.
// Revised Dec 28, 2014 and Jul 31 - Aug 2, 2015 by GFS

// This sketch can be used to check buttons for needed debounce time.
// If you get multiple UP and DOWN messages for 1 press or 1 release
// then increase debounce time which is in microseconds.
// The goal is to have 1 and only 1 DOWN message for any press and
// 1 and only 1 UP message for any release.

/*
As you press and release your button/switch/jumper,

You will see are some lines that start with ^ or _ and a number.
Lines that start with ^ are button release and microseconds down before the release.
Lines that start with _ are button press and microseconds before the press.

Those lines are only information to show bounces the routine filtered out.
They are not considered stable UP or DOWN yet.

If the debounce time is too short, you will see multiple messages with UP or DOWN
in them for one press or release. Increase debounce time. Different buttons will need 
different debounce times.

You can always debounce extra. Remember debounce value here is in microseconds.

Lines that have the word UP or DOWN tell how long since last stable state. 
*/

byte ledPin = 13;
byte buttonPin = 7; // my button is a jumper wire grounded on the metal USB port box.

unsigned long btNow, btStart, btElapsed, btDebounce = 5000UL; // bt = button time. 10ms debounce

byte pinStates; // always the current state is in bit 0 and the previous is in bit 1
// value --- meaning with pin mode INPUT_PULLUP.
// 0 --- no change, pressed now and pressed last check
// 1 --- changed, released now, pressed last check
// 2 --- changes, pressed now, released last check
// 3 --- no change, released now and released last check

byte buttonState; // this only changes when the pin states are stable for some time.
byte buttonPrev; // this only changes when the pin states are stable for some time.

unsigned long checkCounter; // how many checks since last stable change


void setup( void )
{
  Serial.begin( 115200 );
  Serial.println( "\n  Button Bounce Check" );

  pinMode( ledPin, OUTPUT );  // default os LOW
  pinMode( buttonPin, INPUT_PULLUP );  // default is pullup HIGH
  pinStates = 1; // sets initial pin state as not pressed

  // buttonState logic will be 0=false for stable not pressed, 1=true for stable pressed
}

inline void buttonHandler( void ) // just so the cleanliness police don't freak out
{
  pinStates &= 1; // bit mask, only 0 or 1 are possible values
  pinStates *= 2; // current pin state becomes previous pin state as 0 or 2

  pinStates += digitalRead( buttonPin ); // read the current pin state into bit 0
  // now pinStates holds past and present states as 0 to 3

  btNow = micros(); // time at loop start and read pin state to within 4 usecs
  btElapsed = btNow - btStart;

  switch ( pinStates )
  {
    case  0 :  // 0 --- no change, pressed now and pressed last check
      if ( buttonState == 1 ) // already past debounce, added July 31, 2015
      {
        btStart = btNow; // reset the state change window start IF debounce micros is up
      }
      else if ( btElapsed >= btDebounce )
      {
        buttonState = 1; // button is pressed now long enough to be considered stable
        btStart = btNow; // reset the state change window start IF debounce micros is up
      }
      break;

    case  1 :  // 1 --- changed, released now, pressed last check
      Serial.print( '^ ' ); // show the bounce
      Serial.print( btElapsed ); // show micros between checks, the change detect window
      Serial.println( "us" );
      buttonState = 2; // button release detected
      btStart = btNow; // reset the state change window start
      break;

    case  2 :  // 2 --- changed, pressed now, released last check
      Serial.print( '_ ' ); // show the bounce
      Serial.print( btElapsed ); // show micros between checks, the change detect window
      Serial.println( "us" );
      buttonState = 3; // button press detected
      btStart = btNow; // reset the state change window start
      break;

    case  3 :  // 3 --- no change, released now and released last check
      if ( buttonState == 0 ) // already past debounce, added July 31, 2015
      {
        btStart = btNow; // reset the state change window start IF debounce micros is up
      }
      else if ( btElapsed >= btDebounce )
      {
        buttonState = 0; // button is released now long enough to be considered stable
        btStart = btNow; // reset the state change window start IF debounce micros is up
      }
      break;
  }
}

void loop( void )
{
  static unsigned long lastChange; // time between pin state change

  buttonHandler( ); // this code only watches the button, job never finished

  // this if() block only watches buttonState for change, job never finished
  if ( buttonState != buttonPrev ) // button stable state change detected
  {
    buttonPrev = buttonState;
    digitalWrite( ledPin, buttonState );
    if ( buttonState ) // if not zero, it is true
    {
      Serial.print( F( "\nDOWN " ));
    }
    else
    {
      Serial.print( F( "UP " ));
    }
    Serial.print( millis() - lastChange );
    Serial.println( F( " ms" ));
    lastChange = millis();
  }
}

And one other tip: Arduino RAM is small. Break the always use an int habit and you will save RAM (and have faster running code on these 8-bit machines), especially in arrays. If the range of values a variable may take is within 0 to 255 (pin number, for example) then use byte instead. A char variable is good for -128 to +127.

I take if the interval needs to be greater than 12ms to avoid bounce by your example. Why do you comment that x8=96ms to detect button press? Isnt the interval 12ms as written? I'm missing something here...

In the last example, I used a byte as a shift register to store the previous 8 samples (reads). 12ms is the sample period. When the button is pressed, the loop will already have iterated thousands of times, so the buttonState will go like this (if you have an external pull down resistor and the button switches the input high) ...

       00000000
   0ms 00000001  <-- button pressed (LED ON)
  12ms 00000011
  24ms 00000111
  36ms 00001111
  48ms 00011111
  60ms 00111110  <-- button released
  72ms 01111100
  80ms 11111000
  96ms 11110000
 108ms 11100000
 120ms 11000000
 132ms 10000000
 140ms 00000001  <-- button pressed (LED OFF)

As you can see, it will actually will take a minimum of 80ms after button release (x7 ... comment corrected)

My debounce typically takes until 2ms have passed without a bounce when using a jumper that I ground for a button.

Run the watcher and see what you get, it uses only an Uno and a jumper wire.

Complete coverage of switches and debounce.

Tip: use a "click" type push button.

Tip: use a "click" type push button.

Whaat?? 2-years trying to perfect debounce code gone down the drain??!

... More debounce stuff ...

This place uses a variation of a shift register approach.

Check the "Bounce Stats" here from The Ganssle Group (A Guide to Debouncing)

Or solder a cap across the button.

The reason that software debounce is so often used is that it's cheaper. In most devices (and projects) the extra cycles to run it are "free" and the little flash and RAM the code uses are insignificant.

If you go with capacitive sense buttons you don't need debounce but you do need cap sense code. Big upside there is buttons that don't wear out, unless you make them out of food or other perishable objects (the kids Make Camp bananna keyboard for example).

There are other kinds of sensors that make superior buttons, reflective IR comes to mind....

I debounce. It lets me use a jumper to test button apps. Just plug one end in the Arduino pin hole and touch the other to ground, the silver USB port box is grounded and easy to use.

Check the playground for debounce solutions. Everyone and his dog has had a crack at it.

dlloyd:
In the last example, I used a byte as a shift register to store the previous 8 samples (reads). 12ms is the sample period.

But your code states:

if ((buttonState == 1) && (millis() - lastReadTime) > readInterval) { // button just pressed
lastReadTime = millis();
ledState = !ledState;
digitalWrite(ledPin, ledState);
Serial.println(ledState);
}

So, if the buttonState is 00000001 (ie 1) at 13ms past lastReadTime, ledState will get changed. So isn't the interval still 12ms, rather than 80ms? I thnk this is great, but I'm still confused about the 80ms.

Also, I am wondering about how debounce works when using input shift registers like the 74HC165. I would guess things get quite muddled.

So, if the buttonState is 00000001 (ie 1) at 13ms past lastReadTime, ledState will get changed. So isn't the interval still 12ms, rather than 80ms? I thnk this is great, but I'm still confused about the 80ms.

Yes, the code detects a button press right after the first transition to 1 (without delay), providing the previous 7 reads were stable and all 0.

Yes, the read interval is always 12ms. The debounce time will be 80 milliseconds because that's how long it takes to left-shift any 1's to the most significant bit position. When all 8-bits are 10000000 or 00000000 (80ms after releasing the button) its ready to detect another button press. If the button is now pressed, the next buttonState becomes 00000001 and another press is detected.

Also, I am wondering about how debounce works when using input shift registers like the 74HC165. I would guess things get quite muddled.

Good question. You wouldn't have access to "software debounce" the 8 data inputs at the IC, however as you read the data into the MCU, you could compare each byte with previous byte readings and debounce the whole byte at once. The bitwise exclusive (^) can check for inequality (state-change of every bit). Using this with other bitwise operators should make it very easy to "debounce" a byte reading representing the 8 digital inputs.

A small byte array could store the readings as they come in and the data could be shifted through the array ... lots of possibilities.

You could read the pins of an input shift register many times a millisecond and use bitwise logic to detect state changes while doing it.

Whenever contacts with electrical potential (is voltage) between then are close enough, sparks will jump that gap. The gap jumped in dependent on the voltage and the medium (air) crossed.

Run the button watcher. It catches and reports spikes and gaps down to 12 microseconds -- that's as fast as I could get without using port reads and with serial printing going on. I tap a jumper on a metal ground and don't get more than about 1500 micros from one change to the next before the pin stabilizes. You can play with the contact and watch the results in serial monitor. You can tune your button response.

If you want more detail then do like Nick Gammon and use an oscilloscope to watch as his blog/tutorial shows.