Dealing with multiple inputs during timer operation.

I am using a digital sensor to control a valve, and an interlock bypass loop. I want to turn on the valve based on a signal from the input, and open/close an interlock bypass based on the same input. I need to open the valve during the process, and leave it open for some interval after the process finishes. I am using millis for timing, since I need to deal with the possibility of the process coming back on during the delay interval.

I am having problems due to a lack of experience with programming. I have had some C in school, but it's been a while. I've been looking at a lot of example sketches, and trying to adapt my code, but I think my logic may be flawed in one way or another.

I'm using led's to represent the valve and interlock circuit, but I've wired in the relay board I'll be using, and it works fine as well.

My sketch works as follows:

  1. idle condition starts and runs fine.
  2. when the signal (I'm using a tact switch) is activated one time, it works fine... it turns on the purge, turns off the bypass and waits for the interval to complete before going back to idle.
  3. if the switch is released, and pressed again during the delay interval, I only get the remainder of the delay interval.

Here is the code.

Any help would be much appreciated.

Thanks,
Steve

P.S.
This is my first post to this forum. I have really tried to do all my homework on this, but the solution has eluded me thus far.

/*
N2 Purge Control
To control an exhaust line purge for dirty vacuum chamber process.  We want to turn on purge when vacuum chamber is in use, and 
leave it on for some appropriate time (using 10 seconds for testing the sketch).  We also have to control an interlock bypass.  
When the purge is active, we want to interlock the process based on a flow switch on the purge, but need to be able to bypass 
the interlock when we purposefully turn off the purge.  Processes are typically shorter than the overpurge time, and start/stop 
automatically so we need to remain aware of process starts/stops during purge.

Broken down into idle() and processRunning() functions.  Trying to set it up as a state machine; as in 
Blink Without Delay sketch.  
*/
 
const int gasBoxSignal = 5;               // take a signal from Gas Box pneumatic reed valve or something.
const int purgePin = 9;                   // used to turn on the purge.
const int bypassPin = 8;                  // used to control the GB interlock bypass.   

int signalState = HIGH;                    // keep track of the state of the signal pin.   
int bypass_delay = 1000;                   // delay turning off interlock bypass to avoid GB alarms. 
unsigned long interval   = 10000;          // continue to purge "interval" milliseconds after GB signal shuts off.
unsigned long previousMillis = 0;
unsigned long currentMillis;

void setup()
{
  pinMode(gasBoxSignal, INPUT_PULLUP);     // sets the input signal pin to normally high, closing the switch will sink it.
  pinMode(purgePin, OUTPUT);               // output signal pin to turn on/off the purge.
  pinMode(bypassPin, OUTPUT);              // output signal pin to open/close the bypass.
  Serial.begin (115200);                   // initialize serial comms at 115200 baud for debuging.  
  idle();                                  // initialize to idle state.
}
 
void loop()
{   
  currentMillis = millis();
  Serial.print (F("currentMillis is: ")); Serial.println(currentMillis);
  Serial.print (F("previousMillis is: ")); Serial.println(previousMillis);
  if ((signalState == HIGH) && (currentMillis - previousMillis >= interval)) { 
    idle();
    currentMillis = millis();
    previousMillis = currentMillis;
  }
  else if (signalState == LOW) {
    processRunning();
    currentMillis = millis();
    previousMillis = currentMillis;
  }  
}


void idle()
{  
  while (signalState == HIGH) {                         // no process running.
    digitalWrite(bypassPin, HIGH);                      // bypass GB interlock.
    delay(bypass_delay);                                // delay 1 second. 
    digitalWrite(purgePin, LOW);                        // turn off the purge.
    signalState = (digitalRead(gasBoxSignal));
    Serial.print(F("Process Idle: time = ")); Serial.println(millis());
    Serial.print (F("The State of the gas box signal is ")); Serial.println(signalState);    
  }
  Serial.println (F("Exiting idle"));
}

void processRunning()
{  
  while (signalState == LOW) {                          // process is running.
    digitalWrite(purgePin, HIGH);                       // turn on the purge.
    delay(bypass_delay);                                // wait 1 sec. before taking GB out of bypass. 
    digitalWrite(bypassPin, LOW);                       // Take GB out of bypass. 
    signalState = (digitalRead(gasBoxSignal));
    Serial.print(F("Process Running: time = ")); Serial.println(millis());
    Serial.print (F("The State of the gas box signal is ")); Serial.println(signalState);
  }   
  Serial.println (F("Exiting running"));  
}

You mention BlinkWithoutDelay in your comments then use delay() in the program. Hmmm....

Anyway, look at this

   currentMillis = millis();
    previousMillis = currentMillis;

What will the value of previousMillis be after this ? You are not setting previousMillis to the previous value at all.

It's ironic, that I mentioned BlinkWithoutDelay, and then used delays in my sketch, yes. Those delays are 1 secs. delays that are being used to give the valve time to open/close in order to keep from causing interlocks to occur.

In the two lines you quoted, I'm setting currentMillis to the current time, then setting previousMillis equal to currentMillis, which resets the timer to zero. Is that not right?

That is not right. Unless YOU are keeping some sort of timer, there is no timer to reset to zero.
Just like a clock on your wall, the time provided by millis() is always increasing (except as described in the next sentence).
Just like a clock on your wall, the time provided by millis() reaches a maximum value and then goes ("rolls over") to its minimum value.
The maximum value of a clock on your wall is 12:59:59.999999... The maximum value of millis() is something else, but YOU can never force its value to zero.

That's a modern example of "spaghetti code." You are calling functions based on the value of global variables and then those functions also look at those globals, "just to be sure" that they're still in that state?

Read Robin's Planning and Implementing An Arduino Program. This recommends an approach to read all of your inputs, make some calculations or decisions and then do your outputs. So the output functions should not be reading the inputs. If you think your output function needs to read the "emergency stop" input while it's outputting then you need to re-think how you have structured your program. (Yes, there are different structures you can use which allow that, but this one is usually the best for microcontroller projects.)

Also have a look at switch debouncing. You want an action to occur once when a switch is pressed and you don't really care if the switch is held down for 0.01 second or 100 seconds. But you don't want to do the action several times when you get a few 'bounces' in 0.1 second.

In the two lines you quoted, I'm setting currentMillis to the current time, then setting previousMillis equal to currentMillis, which resets the timer to zero. Is that not right?

No. You might as well set previousMillis to millis() for all the difference it will make.

The BWD principle is that you check frequently (and that means no blocking code) whether the required period has elapsed since an event occurred. If so, act on it. If not go round loop() again until the period has elapsed. In order to start a BWD "timer" you need a start time (saved from millis() when the start event occurs) and a current time derived from millis()

In this code section

  if ((signalState == HIGH) && (currentMillis - previousMillis >= interval)) { 
    idle();
    currentMillis = millis();
    previousMillis = currentMillis;
  }

when the time period has elapsed you need to save the time that it occurred. The name previousMillis is not really appropriate. It will work but the code would read better if the variable were named startTime or perhaps stateStartTime then the code would read more like a sentence as in

  if ((signalState == HIGH) && (currentTime - stateStartTime >= interval)) { 
    idle();
    currentMillis = millis();
    previousMillis = currentMillis;
  }

Inside that if test you are effectively setting previousMillis to millis() which does reset the start time but you can do it in one statement and don't need to involve currentMillis. What I don't understand is why you keep setting previousMillis to millis() whether the test succeeds or not. If you think about it previousMillis and currentMillis will always be the same so the period will never elapse.

What does your serial output show you ?

I appreciate all the feedback. It's back to the drawing board for me. I'll start with the Tutorial MorganS pointed out.

Thanks again,

Steve