Ughh… My Arduino Code ain't workin'… And i just couldn't see any problems… Any thoughts?

Question

There's Latest Update!

So i'm workin' on a counter just for fun. How it works is when you press BUTTON1 the LED Lights up red, the buzzer beeps and the variable "counter" adds one to itself; After counting, u can press BUTTON2 then it will first show a transition animation then blink&beep while taking away the value of "counter". After the value of "counter" is less than 1, everything will start from the beginning.

But what actually happens is after the transition animation played, the LED and the buzzer just won't do any thing. I'm sure my phisical connection is all correct, and the buttons are also pulled to ground with a resistor& a 100nf cap to prevent accidental pressing and vibration.

---

Note that I'm usin' a common anode RGB LED, so the anode pin is always HIGH but to light up individual colors i'll have to pull the cathode pins LOW (remember how electrons flow?)

Update @Tom Carpenter's latest answer still didn't solve the problem. I found the "else" function inside the "if" didn't work at all. So if i put the counter code inside the "else" function, the result would be none. But if i put the counter code in the "if" inside the primary "if", it would, again, count at clock speed, which results blinking forever! So is there anyway for the code to only count once per press&release cycle without losing?

The latest code:

---

#define RLED 10
#define GLED 11
#define BLED 12

#define B1 4
#define B2 5

#define BZZ 3

boolean OB1V;

int counter;

void setup() {

  pinMode(RLED, OUTPUT);
  pinMode(GLED, OUTPUT);
  pinMode(BLED, OUTPUT);

  pinMode(B1, INPUT);
  pinMode(B2, INPUT);

  pinMode(BZZ, OUTPUT);

  OB1V = digitalRead(B1);

}

void loop() {

  digitalWrite(RLED, HIGH);
  digitalWrite(GLED, HIGH);
  digitalWrite(BLED, HIGH);

  boolean NB1V = digitalRead(B1);

  if (NB1V != OB1V)
    {
      digitalWrite(RLED, LOW);
      digitalWrite(BZZ, HIGH);
      counter = 0;
    }
  else
    {
      digitalWrite(RLED, HIGH);
      digitalWrite(BZZ, LOW);
      counter = counter +1;
    }

  if ( digitalRead(B2) == HIGH )
    {
      digitalWrite(GLED, LOW);
      delay(300);
      digitalWrite(GLED, HIGH);
      digitalWrite(RLED, LOW);
      delay(300);
      digitalWrite(RLED, HIGH);
      digitalWrite(GLED, LOW);
      delay(300);
      digitalWrite(GLED, HIGH);
      digitalWrite(BLED, LOW);
      delay(100);
      digitalWrite(BLED, HIGH);
      delay(300);

      for ( counter; counter >= 1; counter -- )
        {
          digitalWrite(GLED, LOW);
          digitalWrite(BZZ, HIGH);
          delay(200);
          digitalWrite(GLED, HIGH);
          digitalWrite(BZZ, LOW);
          delay(200);
        }
    }
}

---

Answer 1

Your problems come down to the fact that in the first code you are trying to do things on level sensitivity rather than edge sensitivity.

To try and understand the difference, some examples. Level sensitivity is effectively:

Do 'x' while 'y'

In other words, if 'y' is true (for example an input is high), then keep doing 'x' (e.g. adding to a counter). This is what your code is doing at the moment, but not what you want it to be doing.

Edge sensitivity on the other hand is:

Do 'x' when 'y changed'

In other words, when the state of 'y' changes (for example from true to false), then doing 'x' once.

The key difference between the two is the difference between 'while' and 'when'.

---

So how do you do edge sensitivity in code?

The way you have done it requires that the button be high and then low at exactly the right moment for the code to catch it. What happens if the button goes low at the end of the loop? Your code will miss the edge because at the start of the next loop it is not high, so doesn't get in to the first if statement.

Lets think about what 'changes' means. It means when the current value is different from the previous value. That implies that we need to keep track of what the previous value is in order to compare it to the current value. In fact that is exactly what we do:

boolean oldButtonValue; //It would be better to use a static variable in loop, but for simplicity of the example lets use a global.

void setup() {
    oldButtonValue = digitalRead(buttonPin); //This line is executed once to initialise the variable.
}

void loop() {

    //Each loop we read our current value
    boolean newButtonValue = digitalRead(buttonPin);

    //Has it changed?
    if (newButtonValue != oldButtonValue) {

        //Do something on edge.

    }

    //Update the old button value to match the new one (so we can check it again next loop
    oldButtonValue = newButtonValue;
}

What this does is each loop it compares the current state of the button with the state of it from the previous loop. If the value changes, it does something.

---

Now the trouble with that code is it executes on any edge. That means that each time you press the button, it will run twice, once on press and again on release. So lets modify it slightly:

    ...

    //Has it changed?
    if (newButtonValue != oldButtonValue) {

        //Was it a rising edge?
        if (newButtonValue) {
            //If the button value is now high, it must have been a rising edge
        } else {
            //Otherwise it was a falling edge

        }
    }

    ...

The extra comparison allows us to determine whether it was a rising edge or a falling edge. So you can now do something either when the button is pressed, or when it is released (or both if you want).

---

You can have a second boolean variable to keep track of the old state for the second button. Then you use the same approach to do edge sensitivity detection approach for the second button.

Answer 2

There are a number of problems with your code. Your code does not deal with releasing the button. Once you press B1 (assuming pressing B1 causes the input to go high) the counter (if correctly implemented (that is, it should be counter++)) would count up to 32K then start over at zero and count up again. Only stopping when you release B1. The value of counter could be any number between 0 and 32K.

Working with buttons is difficult. And the 100nF capacitor will do little to help. It would be better if you learned how to debounce buttons and treat both the press and the release with equal importance.

There is an Arduino library called Bounce that will do some of this for you. It will not help with your logical errors. You still need to treat pressing and releasing the button with equal importance. But it will help debounce button presses so your results will more closely match your expectation.

added later...

Your altered code will still not work. You need to keep track of the state of the button in order to increment the counter by one for each button press. You can call it edge detection code or state machine code. But the code needs history to decide what to do next. Here is an example of code which does this.