1

I have been working on an Arduino project that has me kind of stumped. What it needs to achieve is that two momentary safety switches need to be pushed and held within a specified time frame as defined by "PushTimeWindow" then continue to be held as a relay is engaged for the period of time as defined by "RelayOnTime" and at any point if either or both of the momentary switches are released the relay needs to turn off immediately. The number of successful pressed is then displayed on an LCD screen. I have most of that portion of the code working OK so far. It seems to do what I want it to do even if the code could be much more efficient.

I need to figure out a way to ensure the buttons are released first before another successful two hand press can be registered and another cycle initiated and they can't simply be held as the cycles continue. I have been trying to figure out how to place "state change" logic into the code but can't figure out where or how to include it properly. I created a variable called "LastButtonSafeChecked" and one called "ButtonSafeChecked" to act as a comparison but I can't get it to work properly. Where can I include that logic and is a state change the correct way to go about it in this case? Any suggestions on improving the code and implementing the missing logic are greatly appreciated as I think it is outside the scope of my coding knowledge. I included a lot of serial print lines in order to try and debug what events were happening. Here is the code:

/*
Designed to detect two button presses within specific time gap
Then turn on a relay for a specified time
Then turn it off and look for another press   
  */
// include the library code:
#include <LiquidCrystal.h>

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);

// this constant won't change:
 const int  ButtonPin1 = 2;   // the pin that pushbutton 1 is attached to
 const int  ButtonPin2 = 3;   // the pin that pushbutton 2 is attached to
 const int  RelayPin = 5;     // the pin that the Relay is attached to

// Variables will change:
int ButtonPushCounter = 0;    // counter for the number of button presses
int ButtonState1 = 0;         // current state of the button1
int ButtonState2 = 0;         // current state of button2
int LastRelayState = 0;     // previous state of relay
int ButtonSafeOn1 = 0;        // button1 time check passed
int ButtonSafeOn2 = 0;        // button2 time check passed
int ButtonSafeChecked = 0;     // Button Check passed flag
int LastButtonSafeChecked = 0;  // Last State of button check
int RelayOn = 0;            // toggles relay on off state
unsigned long RelayMillis = 0;  //on time check for relay
unsigned long Button1Millis = 0;  //on time check for Button1
unsigned long Button2Millis = 0;  //on time check for Button2
const long RelayOnTime = 1000; //Length of time for relay
const long PushTimeWindow = 1000; //Length of time to allow two pushes

void setup() {
  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);
  // Print a message to the LCD.
  lcd.print("Press Count");
   // initialize the button pins as a inputs:
   pinMode(ButtonPin1, INPUT);
   pinMode(ButtonPin2, INPUT);
   // initialize the Relay Pin as an output:
   pinMode(RelayPin, OUTPUT);
   // initialize serial communication:
   digitalWrite(RelayPin, LOW);
   Serial.begin(9600);
}


void loop() {
unsigned long currentMillis = millis();
   // read the pushbutton input pins:
   ButtonState1 = digitalRead(ButtonPin1);
   ButtonState2 = digitalRead(ButtonPin2);
//check if Button1 is HIGH
    if (ButtonState1 == HIGH) {
        Button1Millis = currentMillis;
        Serial.println("Button1"); 
     }
//check if Button2 is HIGH
    if (ButtonState2 == HIGH) {
        Button2Millis = currentMillis;
        Serial.println("Button2");
      }     
//let program know Button1 is within window  
   if (ButtonState1 == HIGH) {
      if (currentMillis - Button1Millis < PushTimeWindow){
        ButtonSafeOn1 = 1;
    } else {
        ButtonSafeOn1 = 0;
    }
   }
//let program know Button2 is within window
   if (ButtonState2 == HIGH) {
      if (currentMillis - Button2Millis < PushTimeWindow){
        ButtonSafeOn2 = 1;
    } else {
        ButtonSafeOn1 = 0;
    }
   }
//Double Check both buttons are still pushed
   if (ButtonState1 == HIGH && ButtonState2 == HIGH) {
//Check to see if both button safetys add up and Change RelayOn to 1
    if (ButtonSafeOn1 + ButtonSafeOn2 == 2){
      ButtonSafeChecked = 1;
      RelayOn = 1;
      Serial.println("Relay On");
      } else {RelayOn = 0;
      ButtonSafeChecked = 0;
     }
    }
//Turn off relay if too much time elapsed
    if (currentMillis - RelayMillis < RelayOnTime) {
      RelayOn = 0;
      ButtonSafeChecked = 0;     
    }
//Turn on relay pin if RelayOn is 1
  if (RelayOn == 1) {
       digitalWrite(RelayPin, HIGH);
       Serial.println("Relay Pin should go High");
    } else {digitalWrite(RelayPin, LOW);
    }
//update the RelayMillis if it is still on
  if (RelayOn == 1){
      RelayMillis = currentMillis;
    }
//increment counter to diplay press count 
    if (LastRelayState != RelayOn){
      ButtonPushCounter++;
      Serial.println("State of Relay");
      Serial.print(RelayOn);
      Serial.println("  ");
      Serial.println("State of Last Relay State");
      Serial.print(LastRelayState);
    }
     // Delay a little bit to avoid bouncing
     delay(75);
     // set the cursor to column 0, line 1
  // (note: line 1 is the second row, since counting begins with 0):
  lcd.setCursor(0, 1);
  // print the number of seconds since reset:
  lcd.print((ButtonPushCounter/2));   
  //Save Current Relay State for Next Loop   
   LastRelayState = RelayOn;
  //Save Current Button Safety check for comparison for state change
   LastButtonSafeChecked =  ButtonSafeChecked;
}

update 10/13/17 Here is the current code that includes the Bounce2 library as suggested by user Jot as well as the state machine he created to handle the task. Could I have implemented Bounce2 better?

// A state machine is used, only to make it more clear what is going on.
//
// When a button is pressed, the input turns HIGH.
//
// Normally only one 'enum' is used for the state, that controls the code.
// This sketch has also a 'enum' for the buttons, which is called the input state.


// define the different states for this sketch.
// An 'enum' is like a number of #define with increasing numbers.
// include the library code:
#include <Bounce2.h>
#include <LiquidCrystal.h>

// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);

enum
{
  WAIT_FOR_BOTH_OFF,
  WAIT_FOR_BOTH_ON,
  SAFETY_WINDOW,
  RELAY_ON,
} state;       // an 'enum' is automatically an integer.

// define the different input states.
enum
{
  BOTH_OFF,        // no buttons are pressed.
  BOTH_ON,         // both buttons are pressed.
  SOMETHING_ELSE,  // probably one of the buttons is pressed.
};


const int ButtonPin1 = 2;
const int ButtonPin2 = 3;
const int RelayPin = 5;

// The previousMillis is used twice for two different things.
// That is okay in this state machine.
unsigned long previousMillis;

const unsigned long RelayOnTime = 1750;    // Length of time for relay
const unsigned long PushTimeWindow = 300; // Length of time to allow two pushes

// Counters for the number of failed relay triggers and successful cycles.
// They can probably be normal integers as well.
unsigned long failed = 0;
unsigned long success = 0;

Bounce debouncer1 = Bounce();
Bounce debouncer2 = Bounce();

void setup()
{
  lcd.begin(16, 2);
  lcd.print("Press Count");
  Serial.begin(9600);
  Serial.println("Unit Online");

// Set up first button
  pinMode(ButtonPin1, INPUT);
  debouncer1.attach(ButtonPin1);
  debouncer1.interval(5); //Debounce time

// Set up second button  
  pinMode(ButtonPin2, INPUT);
  debouncer2.attach(ButtonPin2);
  debouncer2.interval(5); //Debounce time

  pinMode(RelayPin, OUTPUT);  // OUTPUT to Relay

  Serial.println("Going to WAIT_FOR_BOTH_OFF state");
  state = WAIT_FOR_BOTH_OFF;
}


void loop()
{

  unsigned long currentMillis = millis();

// Update the bounce instances
  debouncer1.update();
  debouncer2.update();

  int inputState = getInputState();

  // Process the data in a state machine.
  // The output part is also in the state machine.
  switch(state)
  {
    case WAIT_FOR_BOTH_OFF:
      // Both buttons must be released to be able to continue.
      if(inputState == BOTH_OFF)
      {
        // Prepare to go to the next state.
        Serial.println("Going to WAIT_FOR_BOTH_ON state");
        state = WAIT_FOR_BOTH_ON;
      }
      break;
    case WAIT_FOR_BOTH_ON:
      // This is the state when everything is idle and no buttons are pressed.
      // Normally both buttons are released,
      // but when both are pressed, then go to the next state.
      if(inputState == BOTH_ON)
      {
        // Prepare to go to the next state.
        Serial.println("Going to SAFETY_WINDOW state");
        previousMillis = currentMillis;
        state = SAFETY_WINDOW;
      }
      break;
    case SAFETY_WINDOW:
      if(inputState != BOTH_ON)
      {
        // During the safety time window, a button was released.
        // Return to idle.
        Serial.println("Going to WAIT_FOR_BOTH_OFF state");
        state = WAIT_FOR_BOTH_OFF;
      }
      else if(currentMillis - previousMillis <= PushTimeWindow)
      {
        // The buttons are still pressed.
        // The time window has reached the end.
        // Prepare to go to the next state.
        Serial.println("Going to RELAY_ON state");
        previousMillis = currentMillis;
        digitalWrite(RelayPin, HIGH);  // Turn ON RelayPin
        state = RELAY_ON;
      }
      break;
    case RELAY_ON:
      if(inputState != BOTH_ON)
      {
        // One of the buttons was released.
        // Stop the relay and return to idle.
        // Although the relay was triggered, it was not succesful.
        failed++;
        Serial.print("Failed=");
        Serial.println(failed);
        Serial.println("Going to WAIT_FOR_BOTH_OFF state");
        digitalWrite( RelayPin, LOW);  // Turn Off RelayPin
        state = WAIT_FOR_BOTH_OFF;
      }
      else if(currentMillis - previousMillis >= RelayOnTime)
      {
        // Everyting was okay.
        // The buttons are still pressed.
        // It was complete and succesful.
        // It is time to turn off the relay and return to idle.
        success++;
        Serial.print("Success=");
        Serial.println(success);
        Serial.println("Going to WAIT_FOR_BOTH_OFF state");
        lcd.setCursor(0,1);
        lcd.print(success);
        digitalWrite(RelayPin, LOW);  // Turn Off RelayPin
        state = WAIT_FOR_BOTH_OFF;
      }
      break;
    default:
      Serial.println("Error, unknown state");
      break;
  }
}


// This function returns the input state.
// There is no StateChangeDetection, because a state machine is used.
int getInputState()
{
  int returnInputState;

  // Read the debounce state and create a input state according to the 'enum' values.
  int value1 = debouncer1.read();
  int value2 = debouncer2.read();

  if(value1 == LOW && value2 == LOW)
  {
    returnInputState = BOTH_OFF;
  }
  else if(value1 == HIGH && value2 == HIGH)
  {
    returnInputState = BOTH_ON;
  }
  else
  {
    // Not both on, not both off, so it must be something else.
    returnInputState = SOMETHING_ELSE;
  }

  return(returnInputState);

}
  • Sorry about the formatting, I went back to edit it but it was already changed to the correct formatting. Thanks., – HiWay Sep 29 '17 at 23:37
  • Another issue the code has upon further testing is that an operator could hold one button down and then hit the other and it allows the relay to turn on. That behavior won't work either as the device is to increase the effectiveness of a safety system on some equipment and it is supposed to prevent any operator from being able to "trick" a safety system into letting it allow a user to use only one hand or to be able to force the system to cycle without hands. Thanks again for looking. – HiWay Sep 29 '17 at 23:51
  • Are both buttons equally important ? And you only have to know if both are pressed or both are released or something in between ? Then there is no need for two variables to store the millis value. For the state, either use a single variable that can be multiple states or two variables. Have you programmed a state machine before ? A state machine with StateChangeDetection for the buttons would make the sketch a lot larger but also easier to read and easier to add things to it and to change things. Without the state machine, I would create a single StateChangeDetection for both buttons together. – Jot Sep 30 '17 at 22:34
  • Hello Jot, I have used a state change in a previous device but I can't figure out how to add it in. Both buttons are equally important as they are safety switches designed to keep both of an operators hands occupied and out of the way as the relay is on. Whenever one or both switches is released the relay needs to turn off as quick as possible. The air solenoid attached to the relay automatically switches in reverse and initiates a brake system to stop the device. – HiWay Oct 2 '17 at 15:14
  • A state change does seem Ideal as the whole system needs to wait after a relay cycle is complete until both buttons are released and then depressed again in order to start a new relay cycle. – HiWay Oct 2 '17 at 15:16
3

The Arduino has a loop function, that runs over and over again.
The Arduino has a millis function for the time.
Those two combine beautifully with a state machine.

Once I made a function that returns the input value as BOTH_OFF, BOTH_ON, or SOMETHINE_ELSE, then the state machine automatically unfolds itself.

I hope you see how clean the code is. Every state has its own code. You don't have to think for hours how to make a spaghetti of if-statements. Although the sketch is now larger, there is no unnecessary code.
If you want to change something, you only have to find the state that you want to change. Adding a new state is also easy.

I have added some explanation in the sketch.

// A state machine is used, only to make it more clear what is going on.
//
// When a button is pressed, the input turns HIGH.
// Are there an external pulldown resistors ?
//
// Normally only one 'enum' is used for the state, that controls the code.
// This sketch has also a 'enum' for the buttons, which is called the input state.


// define the different states for this sketch.
// An 'enum' is like a number of #define with increasing numbers.
enum
{
  WAIT_FOR_BOTH_OFF,
  WAIT_FOR_BOTH_ON,
  SAFETY_WINDOW,
  RELAY_ON,
} state;       // an 'enum' is automatically an integer <- it is not an integer

// define the different input states.
enum
{
  BOTH_OFF,        // no buttons are pressed.
  BOTH_ON,         // both buttons are pressed.
  SOMETHING_ELSE,  // probably one of the buttons is pressed.
};


const int ButtonPin1 = 2;
const int ButtonPin2 = 3;


// The previousMillis is used twice for two different things.
// That is okay in this state machine.
unsigned long previousMillis;

const unsigned long RelayOnTime = 1000;    // Length of time for relay
const unsigned long PushTimeWindow = 1000; // Length of time to allow two pushes

// Counters for the number of failed relay triggers and successful cycles.
// They can probably be normal integers as well.
unsigned long failed = 0;
unsigned long success = 0;


void setup()
{
  Serial.begin(9600);
  Serial.println("Hello");

  pinMode(ButtonPin1, INPUT);
  pinMode(ButtonPin2, INPUT);

  pinMode(LED_BUILTIN, OUTPUT);  // LED_BUILTIN for testing

  Serial.println("Going to WAIT_FOR_BOTH_OFF state");
  state = WAIT_FOR_BOTH_OFF;
}


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

  // Collect all input data.
  int inputState = getInputState();

  // Process the data in a state machine.
  // The output part is also in the state machine.
  switch(state)
  {
    case WAIT_FOR_BOTH_OFF:
      // Both buttons must be released to be able to continue.
      if(inputState == BOTH_OFF)
      {
        // Prepare to go to the next state.
        Serial.println("Going to WAIT_FOR_BOTH_ON state");
        state = WAIT_FOR_BOTH_ON;
      }
      break;
    case WAIT_FOR_BOTH_ON:
      // This is the state when everything is idle and no buttons are pressed.
      // Normally both buttons are released,
      // but when both are pressed, then go to the next state.
      if(inputState == BOTH_ON)
      {
        // Prepare to go to the next state.
        Serial.println("Going to SAFETY_WINDOW state");
        previousMillis = currentMillis;
        state = SAFETY_WINDOW;
      }
      break;
    case SAFETY_WINDOW:
      if(inputState != BOTH_ON)
      {
        // During the safety time window, a button was released.
        // Return to idle.
        Serial.println("Going to WAIT_FOR_BOTH_OFF state");
        state = WAIT_FOR_BOTH_OFF;
      }
      else if(currentMillis - previousMillis >= PushTimeWindow)
      {
        // The buttons are still pressed.
        // The time window has reached the end.
        // Prepare to go to the next state.
        Serial.println("Going to RELAY_ON state");
        previousMillis = currentMillis;
        digitalWrite(LED_BUILTIN, HIGH);  // LED_BUILTIN for testing
        state = RELAY_ON;
      }
      break;
    case RELAY_ON:
      if(inputState != BOTH_ON)
      {
        // One of the buttons was released.
        // Stop the relay and return to idle.
        // Although the relay was triggered, it was not succesful.
        failed++;
        Serial.print("Failed=");
        Serial.println(failed);
        Serial.println("Going to WAIT_FOR_BOTH_OFF state");
        digitalWrite( LED_BUILTIN, LOW);  // LED_BUILTIN for testing
        state = WAIT_FOR_BOTH_OFF;
      }
      else if(currentMillis - previousMillis >= RelayOnTime)
      {
        // Everyting was okay.
        // The buttons are still pressed.
        // It was complete and succesful.
        // It is time to turn off the relay and return to idle.
        success++;
        Serial.print("Success=");
        Serial.println(success);
        Serial.println("Going to WAIT_FOR_BOTH_OFF state");
        digitalWrite(LED_BUILTIN, LOW);  // LED_BUILTIN for testing
        state = WAIT_FOR_BOTH_OFF;
      }
      break;
    default:
      Serial.println("Error, unknown state");
      break;
  }
}


// This function returns the input state.
// There is no StateChangeDetection, because a state machine is used.
int getInputState()
{
  int returnInputState;

  // Read the buttons and create a input state according to the 'enum' values.
  int ButtonState1 = digitalRead(ButtonPin1);
  int ButtonState2 = digitalRead(ButtonPin2);

  if(ButtonState1 == LOW && ButtonState2 == LOW)
  {
    returnInputState = BOTH_OFF;
  }
  else if(ButtonState1 == HIGH && ButtonState2 == HIGH)
  {
    returnInputState = BOTH_ON;
  }
  else
  {
    // Not both on, not both off, so it must be something else.
    returnInputState = SOMETHING_ELSE;
  }

  return(returnInputState);
}

A few extra notes about a state machine or "finite state machine" or "FSM":

In the SAFETY_WINDOW state, I use "if .. else if .." instead of two seperate "if" statements. When the state is changed due to the buttons, then I don't want to check the timeout anymore.

In the SAFETY_WINDOW state, the relay is turned on, because there I prepare for the next state. If you think that is confusing, you can add a state in between, for example called INITIATE_RELAY_ON, and turn the relay on in that state and then continue to the next state.
That is also nice about a state machine. You can add or remove states and move code around for you convenience.

In the SAFETY_WINDOW state I check the buttons first and then the timeout. I could also check the timeout first. It does not matter.
This is typical for a state machine. When delay is replaced by millis and the Arduino loop and the state machine are executed many times per second, then it does not matter which condition is checked first.

A sketch can have many state machines. Sometimes a library contains a state machine. Such libraries often have a 'update' function, that is supposed to be called every time the Arduino loop runs.

Correction nov 4, 2017
Oops, an 'enum' is not automatically an integer. An 'enum' is its own type. Its type is more like the type of a union or struct.

| improve this answer | |
  • That works brilliantly! Thank you for leaving the breadcrumbs in there to work out how it functions. I was running in circles trying to set up 4 different state changes timed with the millis function. I have been researching setting up finite state machines but I think I have been way over thinking it. I ended up thinking I needed 12 "What" states to keep track of instead of 7 like you used. I am now down the rabbit hole learning how these things are set up and know I think I will use similar logic here on out. Thanks again! – HiWay Oct 2 '17 at 20:44
  • I can see how everything is switched around and executed in the "switch...case" function. I think I was definitely missing that type of envelope that groups it together properly. I am definitely more versed in long strings of "IF" statements that are trying to switch up and toggle too many variables. – HiWay Oct 2 '17 at 21:00
  • Thanks. I have added "A few extra notes" paragraph at the end of my answer. I hope that makes the explanation complete. – Jot Oct 3 '17 at 18:30
  • Thank you for the additional information, it's really helpful. – HiWay Oct 6 '17 at 17:24
  • Hello, I built the system and made the circuit board but ran into an unforeseen issue. The OSHA approved industrial switches are big and seem to bounce like crazy and in turn toggle off the logic of the program to turn off the relay and wait for the state to go low then high again. To debounce them would it would be a matter of adding an enum and a state to define each switch as debounced or not within a const unsigned long period and count millis again to toggle between the two states of within or outside of the bounce window? Does that sound on the correct track? – HiWay Oct 11 '17 at 21:51
1

Here is the code:

your code and the code pieces posted so far suffer from two problems: 1) too specific to the application; and 2) too complicated.

here is a different approach to consider:

1) write a piece of code that reads the state of a button, with or without debouncing;

2) write a piece of code that detects no press, single press, double press, and long press (or even more if you want), using 1) above.

3) write a piece of code that detects the same but for two buttons, using 2) above.

...

you can consider the use of enum to make the code more robust.

with this approach, your code can be reused for future projects, and will be easier to maintain / understand as it is very intuitive and modular.

| improve this answer | |
0

From what I understood, this is what you're trying to do:

  • Turn a relay on as long as both buttons are pushed for 'pushWindowTime' milliseconds;
  • Keep the relay on for 'relayOnTime' milliseconds, or until at least one of the buttons are pushed.

I believe the difficulty you're facing is mostly because time is a parameter in your program, and in order to achieve that you're using 'millis()', which returns the number of milliseconds elapsed since the program starts. But rather than working with that, it may be easier in this case to use a delay associated to a loop, which is what I did.

int ButtonPin1 = 0;
int ButtonPin2 = 0;
int RelayPin = 0;

int ButtonState1 = 0;
int ButtonState2 = 0;

bool highState = false;

int pushTimeWindow = 10000;
int relayOnTime = 10000;

void setup() {
  //IOs definition
  pinMode(ButtonPin1, INPUT);
  pinMode(ButtonPin2, INPUT);
  pinMode(RelayPin, OUTPUT);

  digitalWrite(RelayPin, LOW);
}

void loop() {  

  //Loop for reading each button's state in a given period of time 'pushTimeWindow'.
  for(int i = 0; i < pushTimeWindow; i++) {

    //Reads each button's state at every iteraction
    ButtonState1 = digitalRead(ButtonPin1);
    ButtonState2 = digitalRead(ButtonPin2);

    //Flags a boolean if both buttons are pushed.
    if((ButtonState1 == HIGH)&&(ButtonState2 == HIGH))
      highState = true;

    //If a button is released, breaks the loop.
    else {
      highState = false;
      break;
    }
    //Delays the loop one second.
    delay(1000);
  }

  if(highState) {

    //Loop to activate the relay as long as both buttons are pushed, during the time limit 'relayOnTime'.
    for(int i = 0; i < relayOnTime; i++) {

      ButtonState1 = digitalRead(ButtonPin1);
      ButtonState2 = digitalRead(ButtonPin2);

      //Activates the relay if both buttons are pushed.
      if((ButtonState1 == HIGH)&&(ButtonState2 == HIGH))
        digitalWrite(RelayPin, HIGH);

      //Shuts down the relay if a button is released and breaks the loop.
      else {
        digitalWrite(RelayPin, LOW);
        break;
      }

  //Delays the loop one second.
  delay(1000);
}

  }

  //Makes sure that every time loop() starts again, the relay is shut down.
  highState = false;

}

Hope this answers your question. Let us know what you find.

| improve this answer | |
  • Hello, Thank you for taking a look at the code. The logic you stated is mostly what I am trying to do. I am trying to turn the relay on when both buttons are pushed within a time window. Both buttons need to stay pushed through the relay cycle. If either or both are released they relay must go off. After a relay cycle the system needs to wait until both buttons are released until it can initiate another relay cycle. My main concern with delays is if the system doesn't look for a button release the relay will not turn off. – HiWay Oct 2 '17 at 15:21
  • Oh, I see. Now that you mentioned, there is an error in my code. The second loop also needs a digitalRead in order to constantly update each button's value. I'll edit the answer. Also, I'm assuming you would work with times like 5 or 10 seconds for both loops, so that's why the delay is 1 second long. If you want to work with shorter time periods, you could always make the delays smaller. Let me know if it works for you. – amorimph Oct 3 '17 at 19:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.