Is my Arduino skipping a statement?

Question

So, I made this code (I'm rather new to this). The only thing that the Arduino should do, is when I press the button on pin 2, turn on the LED on pin 4 and emulate CTRL + R on the keyboard for the computer. Then, if the LED is on and I hit the button again, turn it off and hit the spacebar on my computer. But as soon as I hit the button for the first time, it does indeed press CTRL + R and the LED turns on, but after the delay, it immediately skips checking if I am pressing the button and assumes that I am pressing the button. Thereby it only sees that the LED is on and it turns it off while hitting the spacebar. Any suggestions?

#include <Keyboard.h>

int buttonPin = 2;
int ledPin    = 4;
int val = 0;

void setup() {
  // put your setup code here, to run once:
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  Keyboard.begin();
}

void loop() {
  val = digitalRead(buttonPin);
  // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
  if ((val == LOW) && (digitalRead(ledPin) == LOW)) {
    // turn LED on:
    digitalWrite(ledPin, HIGH);
    Keyboard.press(KEY_LEFT_CTRL);
    Keyboard.press('r');
    delay(100);
    Keyboard.release(KEY_LEFT_CTRL);
    Keyboard.release('r');
    delay(500);
  }
  if ((val == LOW) && (digitalRead(ledPin) == HIGH)) {
    digitalWrite(ledPin, LOW);
    Keyboard.write(32);
  }
  if ((val == HIGH) && (digitalRead(ledPin) == LOW)) {
    digitalWrite(ledPin, LOW);
  }
  if ((val == HIGH) && (digitalRead(ledPin) == HIGH)) {
    digitalWrite(ledPin, HIGH);  
  }
}

Answer 1

Try something like this.

The code does not use button states in its decision making.

Instead, it looks for the transition between button states, so holding the button does not generate events.

void loop() {

  val = digitalRead(buttonPin);

  if (val != oldButtonState) {      // button state transition
    oldButtonState = val;
    if (val == LOW) {               // transition from "released" to "pressed"
      buttonClicked = true;         // set click flag
    }
  }



  if ( buttonClicked ) {

    buttonClicked = false;                // clear click flag

    if (digitalRead(ledPin) == LOW) {
      digitalWrite(ledPin, HIGH);         // turn LED on
      Keyboard.press(KEY_LEFT_CTRL);      // send CTRL-R
      Keyboard.press('r');
      delay(100);
      Keyboard.release(KEY_LEFT_CTRL);
      Keyboard.release('r');
      delay(500);
    }

    else {
      digitalWrite(ledPin, LOW);          // turn LED off
      Keyboard.write(32);                 // send space
    }
  }
}

Answer 2

Your code isn't checking for a button. It's checking the value in val. That only gets updated once per loop. Let's say the code enters the first if statement because the val is LOW and the LED is also LOW. This sets the LED to HIGH. Now the next if statement, val is still LOW and now LED is HIGH so that one is true as well.

I think what you want is to use "else if" so it will skip the rest once it finds a true one.

void loop() {
  val = digitalRead(buttonPin);
  // check if the pushbutton is pressed. If it is, the buttonState is HIGH:
  if ((val == LOW) && (digitalRead(ledPin) == LOW)) {
    // turn LED on:
    digitalWrite(ledPin, HIGH);
    Keyboard.press(KEY_LEFT_CTRL);
    Keyboard.press('r');
    delay(100);
    Keyboard.release(KEY_LEFT_CTRL);
    Keyboard.release('r');
    delay(500);
  }
  else if ((val == LOW) && (digitalRead(ledPin) == HIGH)) {
    digitalWrite(ledPin, LOW);
    Keyboard.write(32);
  }
  else if ((val == HIGH) && (digitalRead(ledPin) == LOW)) {
    digitalWrite(ledPin, LOW);
  }
  else if ((val == HIGH) && (digitalRead(ledPin) == HIGH)) {
    digitalWrite(ledPin, HIGH);  
  }
}