Help needed with millis

Hi, I've been working on this issue for far too long. What I have is a start signal connected to pin 2 on my Arduino Uno, pins 3-13 are connected to air valves that move a carriage carrying 10 suction cups. Using the delay feature, I had this working great, except that if it gets a second signal before it's finished doing it's thing, it just ignores everything until it's done (I know it's because of the delay function). I need it to happen in steps.

  1. The signal is received
  2. Short delay (determined by a 10k pot).
  3. The carriage will activate.
  4. Another short delay to ensure the carriage is in place.
  5. All suction cups will activate.
  6. Another short delay (to allow time to latch on)
  7. The carriage will return to its starting position,
  8. yet another short delay.
  9. First suction cup releases.
  10. short delay.
  11. second suction cup releases.
    etc.
    the following code is basically what I've got, I've removed most of what isn't needed for the purpose of posting it to make it easier for someone to look over, such as remove pots and program selection features.
    I've connected LED's to see what is actually happening. Currently I have all outputs firing at once, then one turns off, then on again, then two turn off, then on, then three and so on... it seems to be doing what it's supposed to but each instance is starting over immediately after. I need it to wait for a start signal. also this is only happening as long as the signal is active, my signal only lasts roughly half a second. Any help with this would be greatly appreciated. here is my code. Thank you in advance.
//Digital Pin Assignment
//Suction cups
int Cup0 = 3;
int Cup1 = 4;
int Cup2 = 5;
int Cup3 = 6;
int Cup4 = 7;
int Cup5 = 8;
int Cup6 = 9;
int Cup7 = 10;
int Cup8 = 11;
int Cup9 = 12;

int Car = 13; //carriage
//Start signal
int MSig = 2;

int MVsig = 0;

//timing variables
unsigned long previousMillis0 = 0;
unsigned long previousMillis1 = 0;
unsigned long previousMillis2 = 0;
unsigned long previousMillis3 = 0;
unsigned long previousMillis4 = 0;
unsigned long previousMillis5 = 0;
unsigned long previousMillis6 = 0;
unsigned long previousMillis7 = 0;
unsigned long previousMillis8 = 0;
unsigned long previousMillis9 = 0;
unsigned long previousMillis10 = 0;
unsigned long previousMillis11 = 0;
unsigned long previousMillis12 = 0;
unsigned long previousMillis13 = 0;
unsigned long previousMillis14 = 0;
unsigned long previousMillis15 = 0;
unsigned long previousMillis16 = 0;

int spread = 300;
int delay1 = 913;
int delay2 = 318;
int delay3 = 438;
int delay4 = 1054;
int delay5 = delay1 + delay2 + delay3 + delay4;
int delay6 = delay5 + spread;
int delay7 = delay6 + delay4;
int delay8 = delay7 + spread;
int delay9 = delay8 + delay4;
int delay10 = delay9 + spread;
int delay11 = delay10 + delay4;
int delay12 = delay11 + spread;
int delay13 = delay12 + delay4;
int delay14 = delay13 + spread;
int delay15 = delay15 + delay4;
int totaldelay = delay15 + 200;

void setup(){

pinMode(Car,OUTPUT);
pinMode(Cup0,OUTPUT);
pinMode(Cup1,OUTPUT);
pinMode(Cup2,OUTPUT);
pinMode(Cup3,OUTPUT);
pinMode(Cup4,OUTPUT);
pinMode(Cup5,OUTPUT);
pinMode(Cup6,OUTPUT);
pinMode(Cup7,OUTPUT);
pinMode(Cup8,OUTPUT);
pinMode(Cup9,OUTPUT);
pinMode(MSig,INPUT);

digitalWrite(Cup0,LOW);
digitalWrite(Cup1,LOW);
digitalWrite(Cup2,LOW);
digitalWrite(Cup3,LOW);
digitalWrite(Cup4,LOW);
digitalWrite(Cup5,LOW);
digitalWrite(Cup6,LOW);
digitalWrite(Cup7,LOW);
digitalWrite(Cup8,LOW);
digitalWrite(Cup9,LOW);
digitalWrite(Car,LOW);

}
 
void loop(){

MVsig = digitalRead(MSig);

	bool sigRec2 = false;
	bool sigRec3 = false;
	bool sigRec4 = false;
	bool sigRec5 = false;
	bool sigRec6 = false;
	bool sigRec7 = false;
	bool sigRec8 = false;
	bool sigRec9 = false;
	bool sigRec10 = false;
	bool sigRec11 = false;
	bool sigRec12 = false;
	bool sigRec13 = false;
	bool sigRec14 = false;
	bool sigRec15 = false;

if(MVsig == 1){
		
  // check to see if it's time to change the state of the LED
  unsigned long currentMillis = millis();

	//Set signal varibles
	bool sigRec1 = true;
	bool sigRec2 = true;
	bool sigRec3 = true;
	bool sigRec4 = true;
	bool sigRec5 = true;
	bool sigRec6 = true;
	bool sigRec7 = true;
	bool sigRec8 = true;
	bool sigRec9 = true;
	bool sigRec10 = true;
	bool sigRec11 = true;
	bool sigRec12 = true;
	bool sigRec13 = true;
	bool sigRec14 = true;
	bool sigRec15 = true;
  
 //step1
  if((currentMillis - previousMillis1 >= delay1 ) && (sigRec1))  {
    previousMillis1 = currentMillis;
	
    digitalWrite(Car, HIGH);
	sigRec1 = false;
	
 //step2
  if((currentMillis - (previousMillis2) >= delay2) && (sigRec2))  {
     previousMillis2 = currentMillis;
digitalWrite(Cup0,HIGH);
digitalWrite(Cup1,HIGH);
digitalWrite(Cup2,HIGH);
digitalWrite(Cup3,HIGH);
digitalWrite(Cup4,HIGH);
digitalWrite(Cup5,HIGH);
digitalWrite(Cup6,HIGH);
digitalWrite(Cup7,HIGH);
digitalWrite(Cup8,HIGH);
digitalWrite(Cup9,HIGH);
sigRec2 = false;
	
//step3
  if((currentMillis - (previousMillis3) >= delay3) && (sigRec3))  {
    previousMillis3 = currentMillis;
    digitalWrite(Car, LOW);
	 sigRec3 = false;
	
//step4
  if((currentMillis - (previousMillis4) >= delay4) && (sigRec4))  {
    previousMillis4 = currentMillis;
    digitalWrite(Cup0, LOW);
	 sigRec4 = false;
	
//step5
  if((currentMillis - (previousMillis5)) >= delay5 && (sigRec5))  {
	previousMillis5 = currentMillis;
	digitalWrite(Cup1, LOW);
	 sigRec5 = false;
	
//step6
  if((currentMillis - (previousMillis6) >= delay6) && (sigRec6))  {
	previousMillis6 = currentMillis;
	digitalWrite(Cup2, LOW);
	 sigRec6 = false;
	
//step7
  if((currentMillis - (previousMillis7) >= delay7) && (sigRec7))  {
	previousMillis7 = currentMillis;
	digitalWrite(Cup3, LOW);
	 sigRec7 = false;
	
//step8
  if((currentMillis - (previousMillis8) >= delay8) && (sigRec8))  {
  previousMillis8 = currentMillis;
	digitalWrite(Cup4, LOW);
	 sigRec8 = false;
	
//step9
  if((currentMillis - (previousMillis9) >= delay9) && (sigRec9))  {
	previousMillis9 = currentMillis;
	digitalWrite(Cup4, LOW);
	 sigRec9 = false;
	
//step10
  if((currentMillis - (previousMillis10) >= delay10) && (sigRec10))  {
	previousMillis10 = currentMillis;
	digitalWrite(Cup5, LOW);
	 sigRec10 = false;
	
//step11
  if((currentMillis - (previousMillis11) >= delay11) && (sigRec11))  {
	previousMillis11 = currentMillis;
	digitalWrite(Cup5, LOW);
	 sigRec11 = false;
	
//step12
  if((currentMillis - (previousMillis12) >= delay12) && (sigRec12))  {
	previousMillis12 = currentMillis;
	digitalWrite(Cup6, LOW);
	 sigRec12 = false;
	
//step13
  if((currentMillis - (previousMillis13) >= delay13) && (sigRec13))  {
	previousMillis13 = currentMillis;
	digitalWrite(Cup7, LOW);
	 sigRec13 = false;
	
//step14
  if((currentMillis - (previousMillis14) >= delay14) && (sigRec14))  {
	previousMillis14 = currentMillis;
	digitalWrite(Cup8, LOW);
	 sigRec14 = false;
	
//step15
  if((currentMillis - (previousMillis15) >= delay15) && (sigRec15))  {
	previousMillis15 = currentMillis;
	digitalWrite(Cup9, LOW);
	 sigRec15 = false;
	 }
}
}}}}}}}}}}}}}
}
else{}
}

edit - is this better? :slight_smile:

What you have going on right now is if commands all inside each other. If I interpreted you correctly, you want it to work this way: if a sensor is triggered, it should (wait, latch on all suction cups at once, wait, then release them one at a time). If this is the case, your problem was not in the delay, but in the hard logic of it. The coding follows strict logic, so if the sensor is activated, it will start doing the rest, but if the sensor is deactivated it will stop in the middle of the program because it no longer follows the if. The simple solution is to have a variable.

void setup()
int x;

void loop()

//set x to false
x = 0;

//if you trigger the action, x will be true
if (pin2 is triggered)
{x = 1;}

// if x is true, do the action. if x is false, ignore action
if (x == 1)
{do action;
//set x back to false for next time
x = 0}

If I understand your problem right, this should be your solution and delay should not be the problem. If you want it to wait during delay, or you want it to set pins to on/off in time intervals, then delay is properly used.

Write your description down in steps

  1. wait for start signal
  2. short delay
  3. activate carriage
  4. short delay
    ...
    ...

Next you can use a state machine (using e.g. switch/case)

// sensible names for your states
#define ST_WAIT_START 1
#define ST_START_DELAY 2
#define ST_START_CARRIAGE 3
#define ST_CARRIAGE_DELAY 4
...
...
// current state of the state machine; initial value waiting for a start signal
byte currentState = ST_WAIT_START;

// current time
unsigned long currentTime;

void loop()
{
  // start time for delay
  static unsigned long startTime;
  
  // get current time
  currentTime = millis();

  switch(currentState)
  {
    // wait for start signal
    case ST_WAIT_START:
      if (digitalRead(yourButton) == HIGH)
      {
        // set start time of delay
        startTime = currentTime;

        // next step
        currentState = ST_START_DELAY;
      }
      break;
    // delay after start
    case ST_START_DELAY:
      // check if it's time; if so, next step
      if(currentTime - startTime >=1000)
        currentState = ST_START_CARRIAGE;
      break;
    // move carriage
    case ST_START_CARRIAGE:
      // motor on
      digitalWrite(motorPin, HIGH);

      // set start time of delay
      startTime = currentTime;

      // next step
      currrentState= ST_CARIAGE_DELAY;
      break;
    case ST_CARIAGE_DELAY:
      if(currentTime - startTime >=1000)
        currentState = ...
      break;
  }
}

That should get you going :wink: Not tested

@Donuteh, please edit your post and use paragraphs to make it understandable.

…R

nunofyourbusiness, you're saying I should have all of them in their own if statements (not inside of each other) and have an if statement to just change the variable that will trigger them? I'm just about to alter my code to try this, I'll let you know if it works. Thank you.

sterretje, your approach is similar to my program select, I'm using a single analog pin and different resistors to select one a 5 different configurations. This would determine which suction cups are in use. I never thought to try this approach. Thank you.

tl:;dr
Google "C arrays"

}
}}}}}}}}}}}}}
}
else{}

Don't you hate it when that happens?

Thank you for editing your post - now it is much easier to read.

To create a responsive program you need to use millis() to manage timing and not use the delay() function at all. See the demo Several Things at a Time

You may also find something useful in Planning and Implementing a Program

...R

Much easier to understand now with the formatting change. It seems like you only need one signal to trigger for the rest to happen, all in order, correct? If so, you only need 2 if statements, one to set the variable to true and one to start the process if the variable is true. You may have overthought your code.

void loop()

//set x to false
x = 0;

//if you trigger the action, x will be true
if (pin2 is triggered)
{x = 1;}

// if x is true, do the action. if x is false, ignore action
if (x == 1)
{Short delay (determined by a 10k pot);
The carriage will activate;
Another short delay to ensure the carriage is in place;
All suction cups will activate;
Another short delay (to allow time to latch on);
The carriage will return to its starting position;
yet another short delay;
First suction cup releases;
short delay;
second suction cup releases;
etc. ;
//set x back to false for next time
x = 0}

Donuteh:
this is only happening as long as the signal is active, my signal only lasts roughly half a second.

This is because your if statement was based on the signal, so when the signal stopped, the if statement would no longer be true, so it would stop the process. This is what the variable is for. When the signal is received, the variable is set to true, then the if statement is true until the entire process is finished. Just make sure you set the variable back to false after the entire process, but still in the brackets following the if statement.

Donuteh:
(I know it's because of the delay function).

Unless you need the Arduino to do something else while it is waiting, the delay function is doing no harm. It may have seemed like the perpetrator because the Arduino could not determine the if statement false while waiting, so it would determine it false after it finished waiting, but before continuing with the code. This caused it to stop directly after it finishes the delay, making the delay seem like the problem.

Thank you Nunofyourbusiness, a lot of what you said makes a sense. However I'm looking at my code, I'm not seeing how I can get away with only 2 if statements. every example of how to use millis, I've seen requires an if statement each time there is a bit of a delay, which makes sense because we have to look at the time and say, if(the difference between current time and previous time is equal to my desired time) do my task. Is there another way to do this that I have overlooked?

I definitely see what your saying about the signal stopping and stopping my program. This wasn't a problem with the delay function because while the Arduino is counting, it's ignoring everything else. So it will only check for the signal when I tell it to. with millis, if I understand correctly, I'm basically saying run if signal is high, don't run if it's low (don't know how I missed that, obvious now that you point it out).

The reason the delay function didn't work for me was because I also have to watch the signal, if it goes high before the task is complete, it needs to shut off all the outputs and wait for the next signal.

One thing I'm looking at is I have 16 different points in time being recorded (previousmillis1 to 16), am I right in thinking that having only one and basing my math and times on that would be better?

Currently I'm looking at several things at t time from Robin2, it's interesting and contrary to popular belief, I'm actually learning. My problem is I don't seem to understand how intervals work for an application like mine where everything turns on at the same time, then off at different times which are determined mainly by my potentiometers (absent in the code I provided. if I use the approach I see in every example I've found so far, the first signals would flash, while it waits for the longer ones. I could have the sum of the remaining delays as the equivalent to what you have as variable onBoardLedInterval (except on my code, the first signal would activate at delay1 but be off for delay15). There has got to be an easier way than that.

As far as I can see, you don't need 16 previousMillis. Your steps are sequential; you're not doing several things at the same time but things in sequence. So you can get away with only one (I called it startTime in my previous reply).

It would be different if you e.g. want to release each suction cup at e.g. 1 second intervals while the carriage is moving for e.g. 15 seconds.

Donuteh:
The reason the delay function didn't work for me was because I also have to watch the signal, if it goes high before the task is complete, it needs to shut off all the outputs and wait for the next signal.

Ok, now I see why you are having trouble with delay. Since you are using millis, yes, it does have a bunch of if statements. It would only need a couple if you were using delay. What I think your best bet would be to set another condition that if the signal turns on again, it sets your variable to false and stops the action. I would not have your original steps 3-15 inside each other, though.

Right now I can't think of how exactly to make a condition where it stops whenever the signal is triggered again. It probably involves a while command and maybe another variable. I just can't put it together right now. I'm thinking something along the lines of this:

void loop()
if (sensor is triggered)
{x = 1}
if (x == 1)
{ while (sensor is not triggered)
{do all of the actions}
}

As for the timing part, it looks much more complicated than it needs to be. I would have just one variable that keeps millis under control and base my if statements off of millis. Even if your timing is based on the reading of the pot, (is it or is it static amounts?) you could take a small equation out of them.

void setup()
int y;

void loop()
//time will not start counting until sensor is triggered
if (sensor is triggered)
{y = 1}
if (y == 0)
{millis() = 0;}

//then instead of all the other millis stuff you have. This is just an example
if (millis() == 2000)
{digitalWrite(Cup1, LOW);}

//or if you have it based on the pot (still just the idea, not necessarily /pot value)
if ((millis()/ pot value) == 2000)
{digitalWrite(Cup1, LOW);}

//add at the end of the process and in the condition if the process is cut short
y=0;