Another Millis Timer off question...

I'm so sorry to bug you guys with yet another noob question, but I cannot understand why this isn't working as expected, or how to fix it.

What I'm trying to do is control a welder and a gas flow valve at the same time using a standard Arduino & 4-channel relay output module. The welder I have didn't cost as much as some higher end models, and the gas flow is manually controlled via a turn knob on the TIG torch head, which sucks. It sucks because if you forget to turn it on, there is no gas flow, and when the arc begins, it oxidizes the tungsten and trashes it.

To ease my welding tasks and avoid trashed tungsten, I need for the gas to flow automatically when the button is pressed. This is the first output and is working as expected.

The problem I'm having is with the second output for the gas valve. It is turning on when it is supposed to, but I'm trying to integrate what is called "post flow" time. Post flow allows the gas to continue flowing for 3 seconds (or whatever you set it to) after the button is released to shield the welded metal from oxidizing as it cools for a few seconds. The timing of this post flow is.... well, is sporadic at best for timing accuracy. Sometimes it stays on for 3 seconds, other times it turns off 0.5 seconds after releasing the button.

Here's my code:

int buttonPin = 2; //input pin 2
int valveOut = 3;  //output pin 3
int welderOut = 4; //output pin 4
int buttonState = 0;  //variable
int lastButtonState = 0;  //variable

unsigned long postFlow = 3000;  //gas after flow time
unsigned long currentMillis = 0;
unsigned long previousMillis = 0;

void setup() {
  pinMode (buttonPin, INPUT_PULLUP);
  pinMode (valveOut, OUTPUT);
  pinMode (welderOut, OUTPUT);
  digitalWrite(valveOut, HIGH);
  digitalWrite(welderOut, HIGH);
}

void loop() {
  currentMillis = millis();
  // read the button state, set the variables
  buttonState = digitalRead(buttonPin);
  if (buttonState == LOW) {
    digitalWrite(welderOut, LOW); // turn on welder
    digitalWrite(valveOut, LOW); // turn on valve for gas flow
    lastButtonState = 1;
    buttonState = 1;
  }

  if (buttonState == HIGH) { // button is released
    digitalWrite(welderOut, HIGH); // turn off welder
    buttonState = 0;
  }

  if ((lastButtonState == 1) && (buttonState != 1) && (currentMillis - previousMillis >= postFlow)) {
    lastButtonState = 0;
    previousMillis = currentMillis;
    digitalWrite(valveOut, HIGH); 
  }
}

Try throwing another right paren in your last if statement, encapsulating currentMillis - previousMillis.

Beyond that, I would dispense with setting currentMillis at the start of each loop. Try your test simply as millis() - previousMillis and if true, set previousMillis to millis().

Just a quick aside, the millis() function returns the value of timer0_millis. If you don't really care how long your unit has been receiving power, you can set/reset this any time you want. Perhaps at the end of each test you simply set to zero and then test for when it exceeds postFlow.

@not_in_use, ignore the two posts above. They amount to irrelevant advice and bad advice.

Do you want the long answer? The one in which you will learn something?

Or, the short answer so you can get back to welding?

You might want to look at change in state detection when handling buttons.

When a button is detected 'just-pushed' or 'just-released' you set a timer to current millis(), you then check the timer to see if times up.

And ........bring the gas on maybe 1second BEFORE the weld on.

Obviously you should heed the advice in post #3.

I would like both! I assure you I truly want to know how and what makes this thing tick. There are several other applications I could use this on around the house.

@ Coding Badly
@ DKWatson

Easy now, fellas. No need to make a stink over this.

I did try isolating the millis with additional parenthesis, to no avail. I've tried various iterations but nothing seems to work for me. I've read several online examples. I'm missing something in the order or sequence, I think. And I'm a noob, so.... hence why I'm here.

Start with everything turned off.
Go round loop() until the button becomes pressed then turn on the gas and the welder
Keep going round loop() until the button becomes released
When that happens turn off the welder, save the value of millis() as the start time and set a boolean to true.
Keep going round loop() and if the boolean is true test whether the current time minus the start time is greater than the post flow period. If not then keep going, otherwise turn off the gas and set the boolean to false.

Look at the StateChangeDetection example in the IDE to see how to detect when a pin becomes a certain state.

DKWatson:
Just a quick aside, the millis() function returns the value of timer0_millis. If you don't really care how long your unit has been receiving power, you can set/reset this any time you want. Perhaps at the end of each test you simply set to zero and then test for when it exceeds postFlow.

No, that's a very bad idea, any library that uses millis() is then completely broken.

UKHeliBob:
Start with everything turned off.
Go round loop() until the button becomes pressed then turn on the gas and the welder
Keep going round loop() until the button becomes released
When that happens turn off the welder, save the value of millis() as the start time and set a boolean to true.
Keep going round loop() and if the boolean is true test whether the current time minus the start time is greater than the post flow period. If not then keep going, otherwise turn off the gas and set the boolean to false.

Look at the StateChangeDetection example in the IDE to see how to detect when a pin becomes a certain state.

THIS is where I get confused. Won't the millis get updated each time through the program? How do you capture it only once?

cherk:
And ........bring the gas on maybe 1second BEFORE the weld on.

Yes, I agree. This TIG welder is a lift-to-start type, not the more expensive high-frequency start type. So, although it is a lower level welder, pre-flow is practically done already.

How do you capture it only once?

You capture millis() when an event happens.

not_in_use:
THIS is where I get confused. Won't the millis get updated each time through the program? How do you capture it only once?

millis() is constantly updated but you only capture the start time when an event happens

One important thing to note is that the event in question is an input becoming released, not when it is released, which will be most of the time.

Your application has two state machines. Start by trying to identify what they are (give them simple names).

Your application essentially has three inputs that drive the two state machines. Try to identify what they are.

Your application has two outputs which you have identified...

  int valveOut = 3;  //output pin 3
  int welderOut = 4; //output pin 4

There are two strategies for writing to outputs: on-transition and in-state. I believe the second will simplify your code a bit. A reasonable structure for the second is...

  • Collect inputs into local variables
  • Process transitions
  • Update outputs

Update outputs might look something like this...

  if ( WelderState == WelderOn )
  {
    digitalWrite(welderOut, LOW); // turn on welder
  }
  else
  {
    digitalWrite(welderOut, HIGH); // turn off welder
  }

  if ( GasState == GasOn )
  {
    digitalWrite(valveOut, LOW); // turn on valve for gas flow
  }
  else
  {
    digitalWrite(valveOut, HIGH); // turn off valve for gas flow
  }

I have no doubt that code looks overly verbose. But, if all goes well, you will be able to use your eyeballs and a little bit of brain to determine if the code will or will not work. And, you will be able to easily extend the code. For example, let's say you decide to add an LED to indicate the gas is on. The change is trivial...

  if ( GasState == GasOn )
  {
    digitalWrite(valveOut, LOW); // turn on valve for gas flow
    digitalWrite(valveLED, HIGH); // let the human know the gas is supposed to be flowing
  }
  else
  {
    digitalWrite(valveOut, HIGH); // turn off valve for gas flow
    digitalWrite(valveLED, LOW); // indicate the gas is stopped
  }

You need to update your previousMillis variable when you detect the button has been released, your current code does not do this.

Obviously you only want to update that variable once, when the state changes from on to off. I would look into a state machine as advised above.

Although I am a fan of state machines using switch/case the requirement here is easily met using very simple logic.

const byte welderPin = 13;
const byte gasPin = 12;
const byte buttonPin = A3;
byte currentButtonState = HIGH;
byte previousButtonState = HIGH;
unsigned long currentTime;
unsigned long postFlowStartTime;
unsigned long postFlowPeriod = 5000;
boolean postFlowInProgress = false;

void setup()
{
  Serial.begin(115200);
  digitalWrite(welderPin, HIGH);
  digitalWrite(gasPin, HIGH);
  pinMode(gasPin, OUTPUT);
  pinMode(welderPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  digitalWrite(welderPin, HIGH);
  digitalWrite(gasPin, HIGH);
}

void loop()
{
  currentTime = millis();
  previousButtonState = currentButtonState;
  currentButtonState = digitalRead(buttonPin);
  if (currentButtonState != previousButtonState)
  {
    if (currentButtonState == LOW) //button became pressed
    {
      digitalWrite(welderPin, LOW); //turn on the welder
      digitalWrite(gasPin, LOW); //turn on the gas
      postFlowInProgress = false;  //allow for welder restart during post flow period
    }
    else    //button became released
    {
      digitalWrite(welderPin, HIGH);   //turn off the welder
      postFlowStartTime = currentTime;
      postFlowInProgress = true;
    }
  }
  if (postFlowInProgress)
  {
    if (currentTime - postFlowStartTime >= postFlowPeriod)
    {
      digitalWrite(gasPin, HIGH); //turn off the gas
      postFlowInProgress = false;
    }
  }
}

UKHeliBob:
millis() is constantly updated but you only capture the start time when an event happens

One important thing to note is that the event in question is an input becoming released, not when it is released, which will be most of the time.

I get what you're saying. Perhaps something along these lines will work?

int buttonPin = 2; //input pin 2
int valveOut = 3;  //output pin 3
int welderOut = 4; //output pin 4
int buttonState = 0;  //variable
int lastButtonState = 0;  //variable

unsigned long postFlow = 3000;  //gas after flow time
unsigned long currentMillis = 0;
unsigned long previousMillis = 0;

void setup() {
  pinMode (buttonPin, INPUT_PULLUP);
  pinMode (valveOut, OUTPUT);
  pinMode (welderOut, OUTPUT);
  digitalWrite(valveOut, HIGH);
  digitalWrite(welderOut, HIGH);
}

void loop() {
  // read the push button
  buttonState = digitalRead(buttonPin);

  // if the state has changed
  if (buttonState != lastButtonState) {

    // if the state is LOW then the button went from OFF to ON
    if (buttonState == LOW) {
      lastButtonState = buttonState;
      // turn on welder & gas valve
      digitalWrite(welderOut, LOW);
      valveOut, LOW;
      // if the state is HIGH, then button went from ON to OFF
    } else if (buttonState == HIGH) {
      lastButtonState = buttonState;
      currentMillis = millis(); // grab current time
      digitalWrite(welderOut, HIGH); // turn off welder
      // check time & compare to postFlow
      if (currentMillis - previousMillis >= postFlow)
        digitalWrite(valveOut, LOW); // turn on valve
      previousMillis = currentMillis;
    } else { // if time has elapsed
      digitalWrite(valveOut, HIGH); // turn off valve
    }
  }
}

Looks like you are close but you got a bit excited here !

      valveOut, LOW;

If I were you I would rename previousMillis to startMillis or even more explicitly postFlowStartTime. The variable should only be set to millis() when the button becomes released. Is that what your code does ?

      if (currentMillis - previousMillis >= postFlow)
        digitalWrite(valveOut, LOW); // turn on valve
      previousMillis = currentMillis;

Which line or lines of code do you intend to execute if the test returns true ? My advice would be to always use {} round conditional code blocks even if the block is only 1 line. It makes the intention much clearer.

Did you see my reply #17 ?