1 button code was perfect. 2 button code is causing problem with button 1

Dear All,
I have an issue caused by adding a second “button” in to my code that is causing the initial button to not register correctly anymore.
My initial code using a single momentary button acts as follows…
When the code starts, the pushbutton counter goes to 1. When the button is pressed it goes to 2 and turns on the electronics. When pressed again it goes to 3 and turns electronics off, when pressed again it goes to 4 and turns back on again etc etc:

I then wanted RF control so I didn’t have to be local to the unit to switch the electronics on so I added a 433MHz RF receiver/decoder unit and plugged its output in to a different digital pin to the first push button and called it rfButton. I added a second buttonState variable (buttonState2) and second lastButtonState variable (lastButtonState2) and then I modified the if statements so that they incorporated an OR logic statement. Basically, the idea is that if either the pushButton or the rfButton are pressed, the pushButtonCounter should increment by 1 only. The problem in this code is that when rfButton is pressed, the pushButtonCounter does indeed only increment by 1, but if the pushButton is pressed, it increments by 2, which shouldn’t happen.
As a result, what physically happens is that the electronics switch on and off perfectly when the rfButton is pressed. But only switch on if you hold the momentary button down so it’s effectively manually latched. As soon as you let go it turns off again. I cannot for the life of me understand why!
I’ve tried changing the delay() function for debounce but that’s just made the code slower.
I’ve not yet tried changing my “If – OR” statements to switch-case statements and wonder if that’s what I should be doing? Logically, I don’t think that should make any difference though.

I look forward to any advice you may be able to give me!

My original code working perfectly with JUST the pushButton is below. The newer code with both a pushButton and an rfButton will follow after…

Note... I've removed some of the code that is the electronics instruction that happens as a result of the button press as that is irrelevant to the question and is also sensitive data.

const int buttonPin = 2;     // the number of the pushbutton pin
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 pushbutton pin as an input:
  pinMode(buttonPin, INPUT_PULLUP);
 
  // initialize serial communication:
  //baud rate needs to be high in order for loop to run at correct speed
  Serial.begin(500000);
}

void loop()
{
/*The code in this loop will detect if a button is pressed and if so, it will turn on some electronics
  if it is pressed again it will turn the electronics off.
  */
      //read the state of the pushbutton value:
    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 on to off:
        buttonPushCounter++;
        Serial.println("off");
        Serial.print("number of button pushes: ");
        Serial.println(buttonPushCounter);
      } else {
        // if the current state is LOW then the button went from off to on:
        Serial.println("on");
      }
      // Delay a little bit to avoid bouncing
      delay(200);
    }
    // save the current state as the last state, for next time through the loop
    lastButtonState = buttonState;


    // turns on the electronics every two 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) {
        while (value not true) {
            //read the state of the pushbutton value during the while loop
            buttonState = digitalRead(buttonPin);

            // compare the buttonState to its previous state during the while loop
           //if button pressed during the while loop, turn off electronics and reset them
           buttonState = HIGH;
           return(0); 
       }  
    //turn on the electronics here
    buttonPushCounter++;
    Serial.print("number of button pushes: ");
    Serial.println(buttonPushCounter);          
    } 
    
    else {
      //turn off the electronics here
      return(0);  
    }
    
}
const int buttonPin = 2;     // the number of the pushbutton pin
const int rfPin = 4;         // the number of the rf pin
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState1 = 0;        // current state of the pushbutton
int buttonState2 = 0;        // current state of the rfbutton
int lastButtonState1 = 0;    // previous state of the pushbutton
int lastButtonState2 = 0;    // previous state of the rfbutton

void setup()
{
  // initialize the pushbutton and rf pins as inputs:
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(rfPin, INPUT);

  // initialize serial communication:
  //baud rate needs to be high in order for loop speed to be high enough
  Serial.begin(500000);
}

void loop()
{
/*The code in this loop will detect if a button is pressed and if so, it will turn on some electronics and if pressed again, will the electronics off.
  */
      
    buttonState1 = digitalRead(buttonPin); //read the state of the pushbutton value
    buttonState2 = digitalRead(rfPin); // read the state of the rfbutton value

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


    // turns on the electronics every two 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) {
        while (stepCount != stepsToTake) {
        buttonState1 = digitalRead(buttonPin); //read the state of the pushbutton value during the while loop
        buttonState2 = digitalRead(rfPin); // read the state of the rfbutton value during the while loop

        // compare the buttonState to its previous state during the while loop
        //if button pressed during the while loop, turn off electronics and reset them
        if (buttonState1 != lastButtonState1 || buttonState2 != lastButtonState2) {
          //code to turn off electronics here
          buttonState1 = HIGH;
          buttonState2 = LOW;
          return(0);
        }
        //code to turn on electronics here
     }
  buttonPushCounter++;
  Serial.print("number of button pushes: ");
  Serial.println(buttonPushCounter);
  Serial.print("PushButton State: ");
  Serial.println(buttonState1);
  Serial.print("rfButton State: ");
  Serial.println(buttonState2);          
  }
  else {
      //code to turn off electronics here
      return(0);  
   }
}

I think it's to do with how you combined your button tests (both manual and rf) into one test. If you release the rf button, that counts as a change, but then inside there, if the manual button is high (released) that increments the counter although it wasn't a change in the manual button.

I'd try duplicating your original test section for the rf button, and keep them independent.

So a bit like this?

void loop()
{
/*The code in this loop will detect if a button is pressed and if so, it will turn on the electronics
  if it is pressed again it will turn off the electronics.
  */
      
    buttonState1 = digitalRead(buttonPin); //read the state of the pushbutton value
    buttonState2 = digitalRead(rfPin); // read the state of the rfbutton value

    // compare the buttonState to its previous state
    if (buttonState1 != lastButtonState1 || buttonState2 != lastButtonState2) {
      
      // if the state has changed, increment the counter
      Switch (buttonState1) {
        case HIGH:
          buttonPushCounter++;
          Serial.println("off");
          Serial.print("number of button pushes: ");
          Serial.println(buttonPushCounter);
          Serial.print("PushButton State: ");
          Serial.println(buttonState1);
          Serial.print("rfButton State: ");
          Serial.println(buttonState2);
          break;
     
        case LOW:
          Serial.println("on");
          break;
      }
    
      Switch (buttonState2) {
        case LOW:
          buttonPushCounter++;
          Serial.println("off");
          Serial.print("number of button pushes: ");
          Serial.println(buttonPushCounter);
          Serial.print("PushButton State: ");
          Serial.println(buttonState1);
          Serial.print("rfButton State: ");
          Serial.println(buttonState2);
          break;
      
        case HIGH:
          Serial.println("on");
          break;
      }
      
      // Delay a little bit to avoid bouncing
      delay(200);
  }

}

I'd recommend an object-oriented approach:

Code:


---




```
class PushButton
{
public:
PushButton(uint8_t pin) // Constructor (executes when a PushButton object is created)
: pincolor=#000000[/color] { // remember the push button pin
pinMode(pin, INPUT_PULLUP); // enable the internal pull-up resistor
};
bool isPressedcolor=#000000[/color] // read the button state check if the button has been pressed, debounce the button as well
{
bool pressed = false;
bool state = digitalReadcolor=#000000[/color];               // read the button's state
int8_t stateChange = state - previousState;  // calculate the state change since last time

if (stateChange == falling) { // If the button is pressed (went from high to low)
       if (milliscolor=#000000[/color] - previousBounceTime > debounceTime) { // check if the time since the last bounce is higher than the threshold
         pressed = true; // the button is pressed
       }
     }
     if (stateChange == rising) { // if the button is released or bounces
       previousBounceTime = milliscolor=#000000[/color]; // remember when this happened
     }

previousState = state; // remember the current state
     return pressed; // return true if the button was pressed and didn't bounce
   };
 private:
   uint8_t pin;
   bool previousState = HIGH;
   unsigned long previousBounceTime = 0;

const static unsigned long debounceTime = 25;
   const static int8_t rising = HIGH - LOW;
   const static int8_t falling = LOW - HIGH;
};

/* -------------------------------------------------------------------------------------------------------------------------------- */

PushButton button_A = { 2 };  // Create a new PushButton object on pin 2
PushButton button_B = { 3 };

const uint8_t ledPin_A = 12;
const uint8_t ledPin_B = 13;

void setupcolor=#000000[/color] {
   pinMode(ledPin_A, OUTPUT);  // Set the LED pin to output mode
   pinMode(ledPin_B, OUTPUT);
}

void loopcolor=#000000[/color] {
   static bool ledState_A = LOW;
   static bool ledState_B = LOW;
   
   if color=#000000[/color] {  // If the button is pressed
       ledState_A = !ledState_A;  // Flip the state of the LED
       digitalWrite(ledPin_A, ledState_A);  // Write the new state to the LED
   }

if color=#000000[/color] {
       ledState_B = !ledState_B;
       digitalWrite(ledPin_B, ledState_B);
   }
}
```

|

This code takes input from two buttons, and toggles two outputs when the corresponding buttons are pressed. You can easily change the code to allow for a second input for the same output.

Pieter

No, I mean separate the two tests completely.

The nested "if" you had for the manual button, just copy/paste it and use the same idea for the rf button: don't do any of that OR-ing at all.

So, the test for the manual button is unchanged. A press will increment.

And the new, similar test for the rf button (but with the pressed /not pressed logic corrected since I see it doesn't have a pullup so it's active high) will increment as well, but independently.

Thank you gentlemen, I shall try LesserMole's approach first, for simplicity's sake as it will be easier for me to create this code.

Following on from that I will have a go at doing it in the Object Oriented way, as I understand from my uni days that it's superior to do things this way and if I eventually go separately compiled modules approach, it should be more adaptable.

Thank you both, I shall report back - I really value your input.

So using LesserMole's advice, the two separate IF statements works!

This is my code...

void loop()
{
/*The code in this loop will detect if a button is pressed and if so, it will turn on the electronics
  if it is pressed again it will turn off the electronics.
  */
      
    buttonState1 = digitalRead(buttonPin); //read the state of the pushbutton value
    buttonState2 = digitalRead(rfPin); // read the state of the rfbutton value

    
    // compare buttonState1 to its previous state
    if (buttonState1 != lastButtonState1) {
      if (buttonState1 == HIGH) {
        // if the current buttonState1 is HIGH then the button went from on to off:
        buttonPushCounter++;
        Serial.println("off");
        Serial.print("number of button pushes: ");
        Serial.println(buttonPushCounter);
        Serial.print("PushButton State: ");
        Serial.println(buttonState1);
        Serial.print("rfButton State: ");
        Serial.println(buttonState2);
      } else {
        // if the current buttonState1 is LOW or if the current buttonState2 is HIGH then the button went from of to on:
        Serial.println("on");
      }
      // Delay a little bit to avoid bouncing
      delay(200);
    }
    // save the current state as the last state, for next time through the loop
    lastButtonState1 = buttonState1;

    // compare buttonState2 to its previous state
    if (buttonState2 != lastButtonState2) {
      if (buttonState2 == LOW) {
        // if the current buttonState1 is HIGH then the button went from on to off:
        buttonPushCounter++;
        Serial.println("off");
        Serial.print("number of button pushes: ");
        Serial.println(buttonPushCounter);
        Serial.print("PushButton State: ");
        Serial.println(buttonState1);
        Serial.print("rfButton State: ");
        Serial.println(buttonState2);
      } else {
        // if the current buttonState1 is LOW or if the current buttonState2 is HIGH then the button went from of to on:
        buttonPushCounter++;
        Serial.println("on");
        Serial.print("number of button pushes: ");
        Serial.println(buttonPushCounter);
        Serial.print("PushButton State: ");
        Serial.println(buttonState1);
        Serial.print("rfButton State: ");
        Serial.println(buttonState2);
      }
      // Delay a little bit to avoid bouncing
      delay(200);
    }
    // save the current state as the last state, for next time through the loop
    lastButtonState2 = buttonState2;

So now I have something that is fully functional. Thank you LesserMole.

Pieter, I'll investigate going object oriented - what you've posted is really helpful and when I have a couple of days I may re-work the whole project to be that way inclined. Who doesn't love a function?!

Mr_Si:
So now I have something that is fully functional. Thank you LesserMole.

My pleasure.

Hi Pieter,

Moving toward your code, I was just considering it this morning but I was wondering if it would work as a class for both buttons as the RF button has reverse logic to the push-button. Is this possible to do? Or can I change something (other than using a pull-down resistor circuit) so that the logic remains the same for either pushbutton press?

It would certainly be nice to make my code less C and more C++

Thanks in advance.
Simon

PieterP:
I'd recommend an object-oriented approach...

...This code takes input from two buttons, and toggles two outputs when the corresponding buttons are pressed. You can easily change the code to allow for a second input for the same output.

Pieter