2

I want to have a LED turn on when I push a button and go off when I push the button a second time.

This code doesn't work:

const int buttonPin = 4;    
const int motorPin =  10;  
const int ledPin = 6;
 int x = 1;
// variables will change:
int buttonState = 0;         

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

void loop(){
  buttonState = digitalRead(buttonPin);
  if (buttonState == HIGH && x==1) { 
     digitalWrite(motorPin , 1);
     x=0;
    } 
  if (buttonState == HIGH && x==0){
    digitalWrite(motorPin, 0);
    x=1;
  } 
  delay(1);
}

What could be wrong?

  • 2
    So is your LED connected to pin 6 or pin 10? You have pin 6 named ledPin but you are changing motorPin in your loop. – Greg Hewgill Jul 28 '14 at 20:46
  • 2
    What's your circuit? Can you provide a schematic? Also, what is "doesn't work?" Is it lit up for any amount of time? – Anonymous Penguin Jul 28 '14 at 20:54
  • Reading your question I also suspect you don't have a pull-down resistor on the input pin. – Gerben Jul 29 '14 at 9:56
6

At the moment, you've got two main problems. Firstly, both your if statements are being triggered one after the other. This is because the first if statement sets x to 0, which is part of the condition the second if statement looks for.

The second problem is that you're not monitoring the previous state of the button. buttonState will appear HIGH every time round loop, even if the button has been held down for several seconds. The result is that the output pin will be quickly getting turned on and off all the time.

What you need to do is store the last known state of the button. On every iteration of the main loop, only respond to the button if it's currently HIGH, and if it was LOW last time round the loop. You also need to make sure your two if statements are mutually exclusive; i.e. if you trigger one then don't trigger the other by mistake too, or it will cancel it out.

Something like this should work better:

// This will store the last known state of the button
int oldButtonState = LOW;

void loop()
{
  // Get the current state of the button
  int newButtonState = digitalRead(buttonPin);

  // Has the button gone high since we last read it?
  if (newButtonState == HIGH && oldButtonState == LOW) {

    if (x == 0) {
      // Toggle on
      digitalWrite(ledPin, HIGH);
      x = 1;

    } else {
      // Toggle off
      digitalWrite(ledPin, LOW);
      x = 0;
    }
  }

  // Store the button's state so we can tell if it's changed next time round
  oldButtonState = newButtonState;
}

As noted elsewhere, you still need to debounce the button, otherwise you're likely to get some false-positives. That can be done in hardware or software. A really simple way to get around it temporarily is to put in a delay of several milliseconds every time you detect the button changing state. It's not perfect, but it might be enough to get you started.

3

Don't forget to turn ON the LED:

digitalWrite(ledPin, HIGH);   

And OFF:

digitalWrite(ledPin, LOW);  
2

You don't need a separate variable to store the state of the LED. The output pin register does that already. So you can toggle the LED simply with

    digitalWrite(ledPin, !digitalRead(ledPin)); 

or if you prefer

    if(digitalRead(ledPin))
       digitalWrite(ledPin, 0);
    else
       digitalWrite(ledPin, 1);

In you code the loop executes many times while the key is bouncing. If you delay by at least 50mS after the key press is first detected, and you have toggled the led and turned on the motor, then you have effectively debounced the button. If you hold the key down continously the led will toggle every 50mS (plus a little overhead)

1

You are not handling debouncing at all. Mechanical buttons typically bounce for a few milliseconds after an "on" press. Arduino software (or some specialist hardware) needs to handle this. This really matters when you expect a button to toggle some output pin.

1

I just grabbed the version from the ones above here and made is somewhat smaller in terms of code. Feel free to use it in your Arduino app.

// This will store the last known state of the button
int lastButtonState = LOW;

void loop()
{
  // Get the current state of the button
  int currentButtonState = digitalRead(buttonPin);

  // Has the button gone high since we last read it?
  if (currentButtonState == HIGH && lastButtonState == LOW) {
     // Switch the state of the output
     digitalWrite(ledPin, !digitalRead(ledPin));
  }

  // Store the button's state so we can tell if it's changed next time round
  lastButtonState = currentButtonState;
}
0

Pinouts, pullups and logic errors have been addressed elsewhere, so I will focus on switch debouncing. Debouncing, in my opinion, is best handled in software (rather than hardware) for large volume production costs and lazy programmers (like me). For small projects a simple pullup / cap circuit offers better reliability and improved speed, but I'm just lazy in my old age. Also keep in mind that switch bounce worsens significantly over the life of a switch. I've seen new PB's with bounce < 20 ms worsen to over 300 ms with reasonable usage, so keep that in mind with projects that become semi-permanent.

Here's what I use for a software solution on a fairly bouncy SPST NO pushbutton. In this case, I'm using NO PB to GND with 1K external pullups, since I personally don't want to depend on whether or not the internal pullup still works on older devices I may have abused in a former life.... Of course, arguments could be made for various areas still needing cleanup, improved encapsulation, quicker response, higher quality, etc., but this works for me for now.

inSw.h:

class inSw
{
private:
    volatile boolean swState;           // current state ( 0 / 1 ) of switch

public:
    uint8_t  pinNum;
    volatile unsigned long lastPush;    // Time (millis() ) when last switch push detected
    unsigned long debTime = 30;         // Time (micros() ) required to debounce switch

    void init(uint8_t sw);
    boolean readState(void);
} ;

inSw.cpp:

void inSw::init(uint8_t sw) {
    pinNum = sw;
    pinMode(sw, INPUT);
    swState = 0;
    lastPush = millis();
};

boolean inSw::readState() {
//  swState = digitalRead(pinNum);
return swState;
}
void setupSwitches(void) {
    swS1.init(I2C_SOA);     // pin D20
    attachInterrupt(digitalPinToInterrupt(swS1.pinNum), swS1Press, FALLING);
}

Interrupt Service Routines are placed outside Setup() and Loop(), with the attachInterrupt (above) called from inside Setup(). This allows the switch to break long-running routines, such as Stepper.step(10000) which bangs against the limit switch on step 14....

void swS1Press() {
    unsigned long now = millis();
    if (swS1.lastPush < (now - swS1.debTime)) { // if newly pushed
        RunStop.toggle();
        swS1.lastPush = millis();       // sw last pushed now

        if (RunStop.val == RS_RUN)
            digitalWrite(LED_PIN, HIGH);   // set the LED on
        else
            digitalWrite(LED_PIN, LOW);   // set the LED off
    }
}

Lastly, just remember to check the switch state at various points inside loop(). I use this switch as an emergency stop switch, and the onboard LED to ensure it triggers.

0
// When first time program launches it confused to press push button twice. Well replace it with this

if (x == 1) {
    // Toggle on
    digitalWrite(ledPin, HIGH);
    x = 0;
} else {
    // Toggle off
    digitalWrite(ledPin, LOW);
    x = 1;
}
  • What happens if I hold the button? Think about that. – Avamander Aug 19 '16 at 19:19
0

success case tested OK,modified from arduino examples.UNO r3,atmega 328 try it !!!!!

/*  The circuit:
  - pushbutton attached to pin 2 from +5V
  - 10 kilohm resistor attached to pin 2 from ground
  - LED attached from pin 13 to ground (or use the built-in LED on most
    Arduino boards)
*/

// this constant won't change:
const int  buttonPin = 2;    // the pin that the pushbutton is attached to
const int ledPin = 8;       // the pin that the LED is attached to

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button

void setup() {
  // initialize the button pin as a input:
  pinMode(buttonPin, INPUT);
  // initialize the LED as an output:
  pinMode(ledPin, OUTPUT);
  // initialize serial communication:
  Serial.begin(9600);
}

void loop() {
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button went from off to on:
      buttonPushCounter++;
      Serial.println("on");
      Serial.print("number of button pushes: ");
      Serial.println(buttonPushCounter);
    } else {
      // if the current state is LOW then the button went from on to off:
      Serial.println("off");
    }
    // Delay a little bit to avoid bouncing
    delay(10);
  }
  // save the current state as the last state, for next time through the loop
  lastButtonState = buttonState;


  // turns on the LED every four button pushes by checking the modulo of the
  // button push counter. the modulo function gives you the remainder of the
  // division of two numbers:
  if (buttonPushCounter % 2 == 0) {
    digitalWrite(ledPin, LOW);
  } else {
    digitalWrite(ledPin, HIGH);
  }
}
-1

hello sachleen try this code it working I edited from your code

// This will store the last known state of the button
const int buttonPin = A15;   // change as per your button attached. 
const int motorPin =  12;   // 
const int ledPin = 13;
 int x = 1;
// variables will change:
int buttonState = 0;         
int oldButtonState = LOW;


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




void loop()
{
  // Get the current state of the button
  int newButtonState = digitalRead(buttonPin);

  // Has the button gone high since we last read it?
  if (newButtonState == HIGH && oldButtonState == LOW) {

    if (x == 0) {
      // Toggle on
      digitalWrite(ledPin, HIGH);
      x = 1;

    } else {
      // Toggle off
      digitalWrite(ledPin, LOW);
      x = 0;
    }
  }

  // Store the button's state so we can tell if it's changed next time round
  oldButtonState = newButtonState;
}
  • Rahul, next time please select your code, and press Ctrl+K to indent it 4 characters. That way it displays better on the page. – Nick Gammon Jul 29 '15 at 20:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.