2 buttons performing separate loops problem

Projects Programming
1

Hi all. I've tried searching the forum and Google on this question, but I haven't quite found the answer I'm looking for.

For starters what I want to do is have two push buttons hooked up to two inputs and when I press one button, I want it to loop a function indefinitely until the other button is pressed at which point I want another separate function to loop indefinitely.

I know it has something to do with reading whether the button state has changed and if it has, perform "x", else if the other button has changed states then perform "y" but I can't quite figure out how to get to work right. It only performs each function while the buttons are held down. Here is the code.

EDIT: I have the inputs hooked up to pull-up resistors, hence I'm checking to see if the input goes LOW.

const int button_1 = 52;
const int button_2 = 53;
bool button_1_counter = false;
bool button_2_counter = false;

void setup() 
{
  pinMode (button_1, INPUT);
  pinMode (button_2, INPUT);
  pinMode (51, OUTPUT);
  pinMode (50, OUTPUT);
  Serial.begin (9600);
}

void loop() 
{
  int button_1_state = digitalRead(button_1);
  int button_2_state = digitalRead(button_2);
      
if (button_1_state == LOW)
    {
      delay (10);
      button_1_counter = true;
      button_2_counter = false;
    
      if (button_1_counter == true)
      {
        Serial.println ("Button 1");
        delay (500);
      }
    }
    
else if (button_2_state == LOW)
    {
      delay (10);
      button_1_counter = false;
      button_2_counter = true;

      if (button_2_counter == true)
      {
        Serial.println ("Button 2");
        delay (500);
      }
    }
}
2

So I went to the State Change example and attempted to edit it for what I was trying to do, and I'm getting closer, but still can't quite get it right. Now I have two if statements. The second one (as you will see in the code below) works to indefinitely run a loop. However, if I press button 1 it does not interrupt the loop that pushing button 2 started.

So basically-

When Button 1 is pressed first: It prints out "Button 1" for every press of button 1.

When Button 2 is pressed first: It continuously runs the loop in the "for" statement and prints out "Button 2", but does not stop if button 1 is pushed.

Here's the code:

const int button_1_pin = 52;// Button 1 input pin
const int button_2_pin = 53;// Button 2 input pin

int button_1_state = 0;      // Current state of button 1
int button_2_state = 0;      // Current state of button 2
int last_button_1_state = 0; // Previous state of button 1
int last_button_2_state = 0; // Previous state of button 2

void setup() 
{
  pinMode (button_1_pin, INPUT); // Set pin 52 as input
  pinMode (button_2_pin, INPUT); // Set pin 53 as input
  Serial.begin (9600);
}

void loop() 
{
  button_1_state = digitalRead (button_1_pin); // Read state of button 1
  button_2_state = digitalRead (button_2_pin); // Read state of button 2
    
    if (button_1_state != last_button_1_state) // Checks to see if button 1 was just pressed
    {
      if (button_1_state == LOW)
      {
      delay (10); // Delay a little bit to avoid bouncing
      Serial.println ("Button 1");
      }
    } 
    
    else if (button_2_state != last_button_2_state) // Checks to see if button 2 was just pressed
    {
      if (button_2_state == LOW)
      {
      int y = 1;
      for (int x = 0; x < 10; x = x + y)
        {
        delay (1000); // Delay a little bit to avoid bouncing
        Serial.println("Button 2");
        (y = -1);
        }
      }
    }
  last_button_1_state = button_1_state; // Save current state as last state for button 1
  last_button_2_state = button_2_state; // Save current state as last state for button 2
}
3

AT or near button2 press code:

      int y = 1;
      // this essentially says "for x=0 to 9" and by the way change x = (x+y)
      // BUT SEVERAL LINES DOWN YOU SET Y= (-1) THAT'S NEGATIVE 1
      // YOU WILL NEVER EXIT THIS LOOP UNTIL X ROLLS AROUND TO 10
      for (int x = 0; x < 10; x = x + y)
        {
        delay (1000); // Delay a little bit to avoid bouncing
        Serial.println("Button 2");
        (y = -1);  // <<<< FIX THIS LINE
        }
4 
[quote author=Dan95 link=msg=3095348 date=1485014689]
AT or near button2 press code:

        (y = -1);  // <<<< FIX THIS LINE
        }

[/quote]

Yeah, I realize that it just reverses it self and counts up to 9 and back down and then back up again, but that's the only way I could figure out how to create an infinite loop once button 2 was pressed.

As for your suggestion, I did remove the "(y = -1)" command from the code, but of course it only runs that statement while x < 10, so as soon as x = 9 it stops the statement.

I want "statement A" to run indefinitely after button 1 is pressed. I also want "statement B" to run indefinitely after button 2 is pressed. But I also want button 1 to be able to interrupt "statement B" if it's running, and begin running "statement A" and vice versa.

5

It seems to me that what is wanted is a state machine with 2 states. The easiest way to implement this is with switch/case and a variable which I will name state

When state is 0 run one set of code
When state is 1 run the other set of code

Put the button detection code in loop() and set the value of the state variable to 0 or 1 depending on which button became pressed last.

Now the crucial part. The code that runs in either state must not block the free running of the code. This means no delay()s, no whiles and no big for loops that might cause the code to stay in the switch/case otherwise the button detection will be sluggish. Using millis() for timing and the looping nature of the loop() function is used instead to keep the code moving and to enable the buttons to be read frequently.

6

@UKHeliBob

Thank you!!

Using a combination of if/else and switch/case statements I got the program to do exactly what I wanted. It turned out to be extremely simple as well. Below is the code I'm using now to accomplish what I want.

const int button_1_pin = 3;
const int button_2_pin = 2;

int button_1_state = 0;
int button_2_state = 0;

int lastbutton_1_state = 0;
int lastbutton_2_state = 0;

int switch_var = 0;

void setup() 
{

pinMode (button_1_pin, INPUT);
pinMode (button_2_pin, INPUT);
Serial.begin (9600);

}

void button_1_loop ()
{
  Serial.println ("Button 1");
  delay (1000);
}

void button_2_loop ()
{
  Serial.println ("Button 2");
  delay (1000);
}


void loop() 
{
  button_1_state = digitalRead (button_1_pin);
  button_2_state = digitalRead (button_2_pin);

  if (button_1_state == LOW)
  {
    switch_var = 1;
    button_1_state = lastbutton_1_state;
  }

  else if (button_2_state == LOW)
  {
    switch_var = 2;
    button_2_state = lastbutton_2_state;
  }

  switch (switch_var)
  {
    case 1:
    button_1_loop ();
    break;

    case 2:
    button_2_loop ();
    break;
  }
}
7

UKHeliBob:
Now the crucial part. The code that runs in either state must not block the free running of the code. This means no delay()s, no whiles and no big for loops that might cause the code to stay in the switch/case otherwise the button detection will be sluggish. Using millis() for timing and the looping nature of the loop() function is used instead to keep the code moving and to enable the buttons to be read frequently.

Hmm, I did just go back and re-read your comment though and this part got me thinking. So the end-goal of my program is to have an LED display that performs different displays depending on which button you push.

For example:

If button 1 is pressed then make the LED display do PWM fades on each LED one at a time.

If button 2 is pressed do a Night Rider type effect where the LEDs light up back and forth in a row.

However, after reading your statement it would seem that if I wanted to change the state, let's say midway trough case 1, then it may not respond and vice versa.

Do you have any suggestions on how to get it to where the code is constantly reading the input and can change state as soon as button is pushed? Because as you can imagine, the code for each of the LED display examples would include several delays.

8

Do you have any suggestions on how to get it to where the code is constantly reading the input and can change state as soon as button is pushed?

Yes.
Each time through loop() test whether the required time has elapsed since the LEDs last changed and if so, change them to the next pattern. To determine whether the time has elapsed save the millis() value when you change the LED pattern and compare it with millis() next time through loop(). This is generally known as the BlinkWithoutDelay principle, named for the example of the same name.

If you do it this way you can read the input(s) in the main body of loop() which will occur very frequently.

9

I apologize, but that is completely going over my head. I opened up the BlinkWithoutDelay example and I'm not understanding what the code is saying and the comments aren't very clear to me either.

Could you please dumb down what you are saying for me?

10

BWOD works by comparing the current millis() value with its value the last time the state of the LED was changed.

  if(currentMillis - previousMillis >= interval)

If the required interval has elapsed then the test will return true and action can be taken. Otherwise the program continues on its way and repeats the loop() function with no interruption. BWOD does not actually do anything else in loop() but it could, for instance read a digital input and act on it.

11

Ok, I'm tracking how that program works now. The value of "millis()" continuously increases and once the difference between "current" and "previous" equals 1000 it stores the current value of "millis" into "previous" and millis keeps counting up until the difference is 1000 again, and then it stores the new value into "previous".

However, I'm having a read hard time determining how to incorporate that into my switch/case program.

Would I set the value of "const int interval = 5;" and every time the difference between "current" and "previous" is 5 do a "digitalRead" for buttons 1 and 2 and then place the rest of my code to run after that?

EDIT:

So I tried the above and it's still not working. The below is the code I currently have set up. I still have to hold down the button for it to recognize. It seems like the amount of time I have to hold down the button is related to the "button_1_loop" and "button_2_loop" functions and the amount of time it takes for each function to loop. Any thoughts?

const int button_1_pin = 3;
const int button_2_pin = 2;

int button_1_state = 0;
int button_2_state = 0;

int lastbutton_1_state = 0;
int lastbutton_2_state = 0;

int switch_var = 0;

unsigned long previous_millis = 0;
const long interval = 5;

void setup() 
{
pinMode (button_1_pin, INPUT);
pinMode (button_2_pin, INPUT);
pinMode (5, OUTPUT);
Serial.begin (9600);
}

void button_1_loop ()
{
  Serial.println ("Button 1");
  analogWrite (5, 255);
  delay (100);
  analogWrite (5, 0);
  delay(100);
}

void button_2_loop ()
{
  Serial.println ("Button 2");
  int x = 1;
  for (int i = 1; i >= -1; i = i + x)
   {
    analogWrite(5, i);
    if (i == 255) x = -1;
    delayMicroseconds (700);
   }
}

void loop() 
{
  unsigned long current_millis = millis();

  if (current_millis - previous_millis >= 5)
  {
      button_1_state = digitalRead (button_1_pin);
      button_2_state = digitalRead (button_2_pin);

      previous_millis = current_millis;

      if (button_1_state == LOW)
      {
       switch_var = 1;
       button_1_state = lastbutton_1_state;
      }

     else if (button_2_state == LOW)
      {
       switch_var = 2;
       button_2_state = lastbutton_2_state;
      }

      switch (switch_var)
      {
       case 1:
       button_1_loop ();
       break;

       case 2:
       button_2_loop ();
       break;
      }
  }
}
12

Would I set the value of "const int interval = 5;" and every time the difference between "current" and "previous" is 5 do a "digitalRead" for buttons 1 and 2 and then place the rest of my code to run after that?

No, that is not what you do.

Let's make up a specification and see if we can invent some pseudo code that meets it.

OK, here is the specification.
There are 2 button inputs, button1 and button2 and 2 LEDs, led1 and led2. When button1 becomes pressed led1 starts to blink and led2 is turned off. When button2 becomes pressed led2 start to blink and led1 is turned off.

Now the "code" to implement that.

state = 0
period = 1000

start of loop()
  if button1 becomes pressed
    state = 0
    turn off led2
    start time = millis()
  end if
  
  if button2 becomes pressed
    state = 1
    turn off led1
    start time = millis()
  end if
  
  current time = millis()
  
  switch based on state
    case 0
      if current time - start time >= period
        change the state of led 1
        start time = millis()
      end if
    end case
    
    case 1
      if current time - start time >= period
        change the state of led 2
        start time = millis()
      end if
    end case
  end of switch
end of loop()

Can you see that loop() is not blocked at any point and that the buttons are read and acted upon promptly ? At the same time the appropriate LED blinks but does not stop loop() running freely.

13

It seems like the amount of time I have to hold down the button is related to the "button_1_loop" and "button_2_loop" functions and the amount of time it takes for each function to loop. Any thoughts?

Yes, the two loops are blocking and you can not read the new state of a button while they are running. The millis timer and blink without delay type code needs to be within the two loops. You do not need use the millis() timers to read the buttons. Read them every pass through loop.

First off, you are not reading the buttons to detect the state change when they are pressed, rather than reading while they are pressed.

I have the inputs hooked up to pull-up resistors, hence I'm checking to see if the input goes LOW.

The button detection code needs to be like this.

if (button1State == LOW && lastButton1State == HIGH)
        {
          switchVar = 1;
        }
 lastButton1State = button1State;

You will need to rewrite the two button loops to be non blocking by using the millis() timers to switch the led on and off, and to increment the fades. Rely on the loop() to enter the active case thousands of times per second. Do not use delay or a for loop.

const int button_1_pin = 3;
const int button_2_pin = 2;

int button_1_state = 0;
int button_2_state = 0;

int lastbutton_1_state = 0;
int lastbutton_2_state = 0;

int switch_var = 0;

unsigned long previous_millis = 0;
const long interval = 5;

void setup()
{
  pinMode (button_1_pin, INPUT);
  pinMode (button_2_pin, INPUT);
  pinMode (5, OUTPUT);
  Serial.begin (9600);
}

void button_1_loop ()
{
  //recode to use millis() timer for on/off action
  //Serial.println ("Button 1");
  //analogWrite (5, 255);
  // delay (100);
  // analogWrite (5, 0);
  // delay(100);
}

void button_2_loop ()
{
  //recode to use millis() timer for each increment of i
  //Serial.println ("Button 2");
  // int x = 1;
  // for (int i = 1; i >= -1; i = i + x)
  // {
  //  analogWrite(5, i);
  // if (i == 255) x = -1;
  // delayMicroseconds (700);
}


void loop()
{
  // unsigned long current_millis = millis();

  // if (current_millis - previous_millis >= 5)
  //{
  button_1_state = digitalRead (button_1_pin);
  button_2_state = digitalRead (button_2_pin);

  // previous_millis = current_millis;

  if (button_1_state == LOW && lastbutton_1_state == HIGH)
  {
    switch_var = 1;
    // button_1_state = lastbutton_1_state;
  }

  else if (button_2_state == LOW && lastbutton_2_state == HIGH)
  {
    switch_var = 2;
    //button_2_state = lastbutton_2_state;
  }

  lastbutton_1_state = button_1_state;
  lastbutton_2_state = button_2_state;



  switch (switch_var)
  {
    case 1:
      button_1_loop();
      break;

    case 2:
      button_2_loop();
      break;
  }
  // }
}
14

Still not quite tracking a couple of things. First off, I don't have the slightest clue how to re-write the PWM fade loop by recoding using millis timer. I replaced the blink loop with the "blink without delay" method, but I'm not sure how to do that for the PWM fade.

Also, if you do this:

   if (button_1_state == LOW && lastbutton_1_state == HIGH)
  {
    switch_var = 1;
  }

  else if (button_2_state == LOW && lastbutton_2_state == HIGH)
  {
    switch_var = 2;
  }
  lastbutton_1_state = button_1_state;
  lastbutton_2_state = button_2_state;

Is it necessary to set lastbutton_state = button_state? The way I'm reading it is every time the loop starts it reads the buttons and assigns their value to the button_state. In this case, assuming no button is currently being pressed, should assign a "HIGH" to both button_state values.

However, if the button is being pressed it will assign a "LOW" to the button_state. The "if" statements then take over and say "if the button_state is LOW and the last button_state was HIGH then assign either a 1 or a 2 to the switch_variable. "

However, at this point the button_state is still LOW and the lastbutton_state is also LOW because I initialized the "last_button_state" variable to a "0" in the setup.

int last_button_1_state = 0;

Then the code says to assign the value of "button_state" to "last_button_state".

lastbutton_1_state = button_1_state;
lastbutton_2_state = button_2_state;

Would this not mean you are assigning a LOW to the "last_button_state" and that's what the value of "last_button_state" will be the next time the loop runs? This would mean that the "if" statement would never evaluate as true because you now assigned a "LOW" to the "last_button_state".

i.e.

This will assign a "LOW" to "button_state" when the button is pushed.

button_state = digitalRead (button_pin)

Then the "if" statement evaluates the expression. At this point in the loop I'm assuming that "button_state = LOW" and "last_button_state = LOW" because of the "int last_button_state = 0;" that I had in the setup.

if (button_state == LOW && last_button_state == HIGH)

But then, the following statement executes...

last_button_state = button_state;

Would this not mean that the new values would be "last_button_state = LOW" and "button_state = LOW"?

I'm absolutely sure I'm wrong, but I'm still learning and trying to understand what the code is actually doing and saying.

Also it seems like the only difference in the code you posted vs. mine is that you moved the "lastbutton = current_button" statements outside the "if" braces, changed the button_loop functions to use millis() timers instead of delays, and added to the condition of the "if" evaluations.

I'm still not quite tracking why this was done. When "delay" is used, does it mean that it is completely stopping the program as opposed to using the millis() timer which has the program keep running but just compares values and makes changes when certain conditions are met?

15

I'm still learning and trying to understand what the code is actually doing and saying.

You are almost there.

You want the code to record a button press only when it becomes pressed, when the state changes from not pressed to pressed. That is, when the current button state ==LOW and the last button state == HIGH.

All the other current and last conditions LOW/HIGH HIGH/HIGH/ LOW/LOW will not change the switch variable. Only the other button will do that. Once the switch variable is set, the case will be entered each pass of the loop.

In fact, if you wanted you could have one button toggle between 1 and 2 each time it is pressed. You don't need two buttons.

The key to understanding is that you want to detect when the button state changed from not pressed to pressed. You are not interested in when it remains pressed.

When "delay" is used, does it mean that it is completely stopping the program as opposed to using the millis() timer which has the program keep running but just compares values and makes changes when certain conditions are met?

Yes you understand this correctly. You always want the loop() to be running, looking for button presses, and constantly entering the selected case where it will see the millis() timer and see if its time to do something.

First off, I don't have the slightest clue how to re-write the PWM fade loop by recoding using millis timer

In the case of the fade, it looks like your timer will be set to do something every 700 microseconds.
Try this

 if (micros() - lastMicros >= 700)
  {
    lastMicros = micros();
    static byte i = 0;
    static byte x = 0;
    if (i == 0)
      x = 1;
    if (i == 255)
      x = -1;
    i = i + x;
    analogWrite(5, i);
  }
16

cattledog:
You want the code to record a button press only when it becomes pressed, when the state changes from not pressed to pressed. That is, when the current button state ==LOW and the last button state == HIGH.

This is what I don't quite understand. If I initialized "last_button_state" as "LOW" in the setup, when would it ever be "HIGH" to make the "if" statement evaluate as true?

Especially since this statement places the value of "button_state" (being "LOW") into the "last_button_state" variable:

last_button_1_state = button_1_state;

In fact, if you wanted you could have one button toggle between 1 and 2 each time it is pressed. You don't need two buttons.

Yeah I know. I figure it'd probably be easier to do it that way too but the overall end-goal is to have say 5 or so different loop functions and I want to be able to choose which one to execute as opposed to having to scroll through the different loops by pressing one button over and over again. Which now that I think about it, it might be more efficient to toggle instead of having 5 or more buttons on a circuit board, but that's for another discussion. I want to be able to have two separate buttons execute two different lines of code and then expand from there.

The key to understanding is that you want to detect when the button state changed from not pressed to pressed. You are not interested in when it remains pressed.

Would this not mean that I would have to tell the code to assign values to variables for the next time it loops so that the if statements evaluate as true? For example, in my previous post I stated that when the loop first starts from the beginning that it would be storing the values of the variables from the last go around. If this is the case, would that mean that after the if statements execute that it would assign a "LOW" to "last_button_state" and therefore the next time the loop runs then the value of "last_button_state" would be "LOW" therefore never allowing the if statement to evaluate as true?

Also, I copied and pasted that PWM fade code you suggested into my program and it didn't work. I'm not familiar with the static and byte functions so I have no idea how to go about debugging at the moment.

17

If I initialized "last_button_state" as "LOW" in the setup, when would it ever be "HIGH" to make the "if" statement evaluate as true?

The very first pass through the loop when button state == HIGH and last button state = button state.

Especially since this statement places the value of "button_state" (being "LOW") into the "last_button_state" variable:

This only happens when the button is pressed. During that period when your finger is on the button they are both LOW. As soon as you release, button state ==HIGH and last button state ==LOW until the next pass of the loop brings it into line with the current state. Indeed, testing on the condition of current == HIGH and last ==LOW is the way to test for the moment of button release.

If this is the case, would that mean that after the if statements execute that it would assign a "LOW" to "last_button_state" and therefore the next time the loop runs then the value of "last_button_state" would be "LOW" therefore never allowing the if statement to evaluate as true?

Only for one loop cycle. The value will very quickly be set back to HIGH on the next one. Since the value of the switch case has already been set, it is of no consequence, as the selected case will be entered.

18

My fade function was not correct.

  static byte x = 0;

byte is unsigned 0-255 and needs to be
static int x = 0;

  static unsigned long lastMicros;
  static byte i = 0;
  static int x = 0;

  if (micros() - lastMicros >= 700)
  {
    lastMicros = micros();
    if (i == 0)
      x = 1;
    else if (i == 255)
      x = -1;

    i = i + x;
    
    analogWrite(5, i);
  }