How can I make my code not go in order?

My setup is essentially 4 x momentary buttons that move a single servo from 0 to 90 degrees. The variable for each button is that they each make the servo stay at 90deg for four differing times (1, 2, 3 and 4 seconds) before returning to 0deg.

The code runs perfectly, but only if I cycle through button 1, 2, 3 and 4 in order. For example, it won’t let me go from button 2 back to button 1, I have to go through 3 and 4 before I can go back to 1.

I am a beginner and I’m learning code slowly so I don’t quite understand the mistake I’m making here. Help would be very much appreciated. Thank you.

#include <Servo.h>           //servo library included
Servo myservo;              //my servo is myservo

void setup()                //do these things once
{
  Serial.begin(9600);           //data transmission rate to pc
  myservo.attach(9);        //servo to PWM 9
  myservo.write(0);        //servo start at 0 position

  pinMode(8,INPUT_PULLUP);      //pin X forced to HIGH when there is no external input
  pinMode(7,INPUT_PULLUP);      
  pinMode(4,INPUT_PULLUP);      
  pinMode(2,INPUT_PULLUP);      
  
}


void loop()                     //loop forever
{


  //1 second @ 90 degrees
  while(digitalRead(8) == HIGH) //as long as pin 8 is HIGH,...
    {
    myservo.write(0);      //servo position is 0 degrees
    Serial.print("open");       //print the word "open"
    Serial.println("");         //print nothing, go to next line
    }
  myservo.write(90);       //otherwise, servo position is THETA degrees
  Serial.println("");           //print nothing, go to next line
  Serial.println("closed");     //print the word "closed"
  Serial.print("waiting 2 seconds"); //print "waiting T seconds"
  Serial.println("");           //print nothing, go to next line
  delay(1000);              //waits for T seconds before re-doing the loop


  
  //2 seconds @ 90 degrees
  while(digitalRead(7) == HIGH) 
    {
    myservo.write(0);      
    Serial.print("open");   
    Serial.println("");       
    }
  myservo.write(90);       
  Serial.println("");           
  Serial.println("closed");     
  Serial.print("waiting T seconds"); 
  Serial.println("");        
  delay(2000);              
  

  //3 seconds @ 90 degrees
  while(digitalRead(4) == HIGH) 
    {
    myservo.write(0);      
    Serial.print("open");    
    Serial.println("");        
    }
  myservo.write(90);       
  Serial.println("");           
  Serial.println("closed");     
  Serial.print("waiting T seconds"); 
  Serial.println("");       
  delay(3000);             

  //4 seconds @ 90 degrees
  while(digitalRead(2) == HIGH) 
    {
    myservo.write(0);      
    Serial.print("open");    
    Serial.println("");        
    }
  myservo.write(90);       
  Serial.println("");           
  Serial.println("closed");     
  Serial.print("waiting T seconds"); 
  Serial.println("");        
  delay(4000);              

}

You need to read all the buttons at the same time at the start of loop() and save their values.

And you should not use WHILE as it blocks the Arduino until it finishes. Just use IF.

And you must remove all the delay()s as they also block the Arduino. The demo Several Things at a Time illustrates the use of millis() to manage timing without blocking. It may help with understanding the technique.

Have a look at Using millis() for timing. A beginners guide if you need more explanation.

...R Planning and Implementing a Program

Thanks for the reply!

How do I read the buttons at the start and reflect that into my code later?

I will switch the while with if statements and try.

I will give that link a read.

How do I read the buttons at the start and reflect that into my code later?

One possibility is:

byte save_button_1_state = digitalRead(button_1_pin);

...

if (save_button_1_state == 1) do_something();

If you declare save_button_1_state to be a global variable, it will be available in functions as well.

jremington: One possibility is:

byte save_button_1_state = digitalRead(button_1_pin);

...

if (save_button_1_state == 1) do_something();




If you declare save_button_1_state to be a global variable, it will be available in functions as well.

Using this method though I can't run my pulse through the non-PWM digital though anymore can I? I'm currently wiring my buttons into non-PWM. I'll have to run through the PWM enabled slots on the Arduino right?

I can't reconcile it.

ardoyouknowwhoiam: Using this method though I can't run my pulse through the non-PWM digital though anymore can I?

Why not?

Post the program that represents your best attempt?

...R

Using this method though I can't run my pulse through the non-PWM digital though anymore can I?

Sorry, I don't understand. My "method" simply reads the pin state and saves it for "later".

Here’s my attempt. I’ve changed a few things:

  1. Named constants for everything. The only bare number that appears in loop() or setup() is zero.
  2. Using arrays for the buttons and the times associated with the buttons.
  3. Using millis() for timing instead of delays.
    {added in edit}4. Setup prints out what program this is and when it was compiled. Useful for tracking different versions.

This re-starts the interval every time a button is pushed. If you pressed ‘1’ and then half a second later pressed ‘2’, it would run for 2 seconds from the moment that ‘2’ was pressed, making a total of 2.5 seconds.

#include <Servo.h>           //servo library included
Servo myservo;              //my servo is myservo
const byte servoPin = 9;                 //The servo is plugged into this Arduino pin
const int servoPositionClose = 0;        //The 'closed' position in degrees on the servo
const int servoPositionOpen = 90;        //The open position in degrees

const byte numButtons = 4;               //The number of buttons connected (arrays below must have the same number of elements)
const byte buttonPins[] = {8, 7, 4, 2};  //The Arduino pin numbers the buttons are plugged into
const unsigned long buttonDelays[] = {1000, 2000, 3000, 4000}; //Milliseconds - the delay for each button
bool buttonState[numButtons];            //we're going to use this to remember which button(s) was previously pushed
unsigned long openTimeStart=0;           //record the time that we most recently moved to the open position
unsigned long currentDelay=0;              

void setup()                //do these things once
{
  Serial.begin(9600);           //data transmission rate to pc
  myservo.attach(servoPin);        
  myservo.write(servoPositionClose);        //servo start at 0 position

  for(int i=0; i<numButtons; i++) {
    pinMode(buttonPins[i],INPUT_PULLUP);      //pin X forced to HIGH when there is no external input
  }  

  
  Serial.println("Servo tester started");
  Serial.print("Compiled on ");
  Serial.print(__DATE__);
  Serial.print(" ");
  Serial.print(__TIME__);
  Serial.println("\n");
}


void loop()                     //loop forever
{
  bool newButtonState[numButtons];
  //read the current state of all the buttons
  for(int i=0; i<numButtons; i++) {
    newButtonState[i] = digitalRead(buttonPins[i]);     
  }  

  //work through the buttons to see what changed - which button has most-recently been pressed?
  for(int i=0; i<numButtons; i++) {
    
    if(newButtonState[i] == LOW && buttonState[i] != LOW) {
      //this button has been pushed and it wasn't before
      Serial.print("open ");
      Serial.println(i);
      currentDelay = buttonDelays[i];
      openTimeStart = millis(); 
    }
    
    //record the status of every button, for next time through loop
    buttonState[i] = newButtonState[i];
  }

  if(openTimeStart > 0 && millis() - openTimeStart < currentDelay) {
    myservo.write(servoPositionOpen);
  } else {
    myservo.write(servoPositionClose);
  }   

}

MorganS:
Here’s my attempt. I’ve changed a few things:

  1. Named constants for everything. The only bare number that appears in loop() or setup() is zero.
  2. Using arrays for the buttons and the times associated with the buttons.
  3. Using millis() for timing instead of delays.

This re-starts the interval every time a button is pushed. If you pressed ‘1’ and then half a second later pressed ‘2’, it would run for 2 seconds from the moment that ‘2’ was pressed, making a total of 2.5 seconds.

#include <Servo.h>           //servo library included

Servo myservo;              //my servo is myservo
const byte servoPin = 9;                //The servo is plugged into this Arduino pin
const int servoPositionClose = 0;        //The ‘closed’ position in degrees on the servo
const int servoPositionOpen = 90;        //The open position in degrees

const byte numButtons = 4;              //The number of buttons connected (arrays below must have the same number of elements)
const byte buttonPins = {8, 7, 4, 2};  //The Arduino pin numbers the buttons are plugged into
const unsigned long buttonDelays = {1000, 2000, 3000, 4000}; //Milliseconds - the delay for each button
bool buttonState[numButtons];            //we’re going to use this to remember which button(s) was previously pushed
unsigned long openTimeStart=0;          //record the time that we most recently moved to the open position
unsigned long currentDelay=0;

void setup()                //do these things once
{
  Serial.begin(9600);          //data transmission rate to pc
  myservo.attach(servoPin);       
  myservo.write(servoPositionClose);        //servo start at 0 position

for(int i=0; i<numButtons; i++) {
    pinMode(buttonPins[i],INPUT_PULLUP);      //pin X forced to HIGH when there is no external input
  }

Serial.println(“Servo tester started”);
  Serial.print(“Compiled on “);
  Serial.print(DATE);
  Serial.print(” “);
  Serial.print(TIME);
  Serial.println(”\n”);
}

void loop()                    //loop forever
{
  bool newButtonState[numButtons];
  //read the current state of all the buttons
  for(int i=0; i<numButtons; i++) {
    newButtonState[i] = digitalRead(buttonPins[i]);   
  }

//work through the buttons to see what changed - which button has most-recently been pressed?
  for(int i=0; i<numButtons; i++) {
   
    if(newButtonState[i] == LOW && buttonState[i] != LOW) {
      //this button has been pushed and it wasn’t before
      Serial.print("open ");
      Serial.println(i);
      currentDelay = buttonDelays[i];
      openTimeStart = millis();
    }
   
    //record the status of every button, for next time through loop
    buttonState[i] = newButtonState[i];
  }

if(openTimeStart > 0 && millis() - openTimeStart < currentDelay) {
    myservo.write(servoPositionOpen);
  } else {
    myservo.write(servoPositionClose);
  }

}

Very nice , perhaps adding some comments how you have separated the tasks to keep track of things. .
I would suggest to put it into "Arduino examples " as “pushing multiple buttons template”.

Kudos.

MorganS:
Here’s my attempt. I’ve changed a few things:

  1. Named constants for everything. The only bare number that appears in loop() or setup() is zero.
  2. Using arrays for the buttons and the times associated with the buttons.
  3. Using millis() for timing instead of delays.
    {added in edit}4. Setup prints out what program this is and when it was compiled. Useful for tracking different versions.

This re-starts the interval every time a button is pushed. If you pressed ‘1’ and then half a second later pressed ‘2’, it would run for 2 seconds from the moment that ‘2’ was pressed, making a total of 2.5 seconds.

#include <Servo.h>           //servo library included

Servo myservo;              //my servo is myservo
const byte servoPin = 9;                //The servo is plugged into this Arduino pin
const int servoPositionClose = 0;        //The ‘closed’ position in degrees on the servo
const int servoPositionOpen = 90;        //The open position in degrees

const byte numButtons = 4;              //The number of buttons connected (arrays below must have the same number of elements)
const byte buttonPins = {8, 7, 4, 2};  //The Arduino pin numbers the buttons are plugged into
const unsigned long buttonDelays = {1000, 2000, 3000, 4000}; //Milliseconds - the delay for each button
bool buttonState[numButtons];            //we’re going to use this to remember which button(s) was previously pushed
unsigned long openTimeStart=0;          //record the time that we most recently moved to the open position
unsigned long currentDelay=0;

void setup()                //do these things once
{
  Serial.begin(9600);          //data transmission rate to pc
  myservo.attach(servoPin);       
  myservo.write(servoPositionClose);        //servo start at 0 position

for(int i=0; i<numButtons; i++) {
    pinMode(buttonPins[i],INPUT_PULLUP);      //pin X forced to HIGH when there is no external input
  }

Serial.println(“Servo tester started”);
  Serial.print(“Compiled on “);
  Serial.print(DATE);
  Serial.print(” “);
  Serial.print(TIME);
  Serial.println(”\n”);
}

void loop()                    //loop forever
{
  bool newButtonState[numButtons];
  //read the current state of all the buttons
  for(int i=0; i<numButtons; i++) {
    newButtonState[i] = digitalRead(buttonPins[i]);   
  }

//work through the buttons to see what changed - which button has most-recently been pressed?
  for(int i=0; i<numButtons; i++) {
   
    if(newButtonState[i] == LOW && buttonState[i] != LOW) {
      //this button has been pushed and it wasn’t before
      Serial.print("open ");
      Serial.println(i);
      currentDelay = buttonDelays[i];
      openTimeStart = millis();
    }
   
    //record the status of every button, for next time through loop
    buttonState[i] = newButtonState[i];
  }

if(openTimeStart > 0 && millis() - openTimeStart < currentDelay) {
    myservo.write(servoPositionOpen);
  } else {
    myservo.write(servoPositionClose);
  }

}

Took me a while to get back to you. This works perfectly. Very clean code. Thank you!