Help with millis function

I'm making a project with 4 inputs and 4 outputs, the basic idea is that should any input turn HIGH once the corresponding output will turn HIGH for a period (5mins). As this the program require continuous checking of all 4 inputs, I can't use delay function and started using millis function. I tried using the code below and I'm able to turn on 4 different output at different timing but they will turn off at same timing. How can I make it so I can turn the output on at different timing but for the same period of 5mins?

unsigned long time_nowA= 0;   //millis() for box A
unsigned long time_nowB= 0;   //millis() for box B
unsigned long time_nowC= 0;   //millis() for box C
unsigned long time_nowD= 0;   //millis() for box D

unsigned long interval; 

int boxA =0;
int boxB =0;
int boxC =0;
int boxD =0;


void setup() {
  // put your setup code here, to run once:

  //5mins = (1000*60*5)

// Configure input from 24v relay   
    pinMode(A0, INPUT);
    pinMode(A1, INPUT);
    pinMode(A2, INPUT);
    pinMode(A3, INPUT);

//Congfigure output for 5v relay
      pinMode(2, OUTPUT);
      pinMode(3, OUTPUT);
      pinMode(4, OUTPUT);
      pinMode(5, OUTPUT);

    Serial.begin(9600);
}


void loop() {
  // put your main code here, to run repeatedly:
  
  boxA = digitalRead(A0);
  boxB = digitalRead(A1);
  boxC = digitalRead(A2);
  boxD = digitalRead(A3);

/////////////////////////////////////////////////////////////
    interval =10000;                        //period to wait (for testing purposes)
    unsigned long currentMillis = millis(); //start millis
    if(boxA==HIGH)                      //if A0 == HIGH
    {
    digitalWrite(5, HIGH);                  //Set output D5==HIGH to turn on the relay
    } 
    if((boxA==LOW)&&(currentMillis -time_nowA>interval))    //if A0==LOW and current millis>interval
    {
      digitalWrite(5, LOW);                 //Set output D5==LOW to turn off the relay
      time_nowA =currentMillis;            
    }
    
/////////////////////////////////////////////////////////////
  
    if(boxB==HIGH)
    {
    digitalWrite(4, HIGH);
    } 
    if((boxB==LOW)&&(currentMillis-time_nowB>interval))
    {
      digitalWrite(4, LOW);
      time_nowB = currentMillis;
    }
    
//////////////////////////////////////////////////////////////

    if (boxC==HIGH)
    {
    digitalWrite(3, HIGH);
    } 
    if((boxC==LOW)&&(currentMillis-time_nowC>interval))
    {
      digitalWrite(3, LOW);
      time_nowC = currentMillis;
    }
    
/////////////////////////////////////////////////////////////////

    if (boxD==HIGH)
    {
    digitalWrite(2, HIGH);
    } 
    if((boxD==LOW)&&(currentMillis - time_nowD>interval))
    {
      digitalWrite(2, LOW);
      time_nowD = currentMillis;
    }
//////////////////////////////////////////////////////////////////
}

Thank you for your time looking through the code.

  if (boxA == HIGH)                   //if A0 == HIGH
  {
    digitalWrite(5, HIGH);                  //Set output D5==HIGH to turn on the relay
  }
  if ((boxA == LOW) && (currentMillis - time_nowA > interval)) //if A0==LOW and current millis>interval
  {
    digitalWrite(5, LOW);                 //Set output D5==LOW to turn off the relay
    time_nowA = currentMillis;
  }

Shouldn't you be setting time_nowA to the current time when you turn the relay on rather than when you turn it off ?

UKHeliBob:  if (boxA == HIGH)                   //if A0 == HIGH  {    digitalWrite(5, HIGH);                  //Set output D5==HIGH to turn on the relay  }  if ((boxA == LOW) && (currentMillis - time_nowA > interval)) //if A0==LOW and current millis>interval  {    digitalWrite(5, LOW);                 //Set output D5==LOW to turn off the relay    time_nowA = currentMillis;  }

Shouldn't you be setting time_nowA to the current time when you turn the relay on rather than when you turn it off ?

Thank you for the reply, so i guess i should run the code this way for all 4?

 if(boxA==HIGH)                      //if A0 == HIGH
    {
    time_nowA =currentMillis;
    digitalWrite(5, HIGH);                  //Set output D5==HIGH to turn on the relay
    } 
    if((boxA==LOW)&&(currentMillis -time_nowA>interval))    //if A0==LOW and current millis>interval
    {
      digitalWrite(5, LOW);                 //Set output D5==LOW to turn off the relay            
    }
    
/////////////////////////////////////////////////////////////
  
    if(boxB==HIGH)
    {
    time_nowB = currentMillis;
    digitalWrite(4, HIGH);
    } 
    if((boxB==LOW)&&(currentMillis-time_nowB>interval))
    {
      digitalWrite(4, LOW);
    }
    
//////////////////////////////////////////////////////////////

    if (boxC==HIGH)
    {
    time_nowC = currentMillis;
    digitalWrite(3, HIGH);
    } 
    if((boxC==LOW)&&(currentMillis-time_nowC>interval))
    {
      digitalWrite(3, LOW);
    }
    
/////////////////////////////////////////////////////////////////

    if (boxD==HIGH)
    {
    time_nowD = currentMillis;
    digitalWrite(2, HIGH);
    } 
    if((boxD==LOW)&&(currentMillis - time_nowD>interval))
    {
      digitalWrite(2, LOW);
    }

That looks better. Yes you must do the same for each of the input/output pairs.

You can do it more effectively by using arrays or a struct to hold the timing information and iterate through them using a for loop.

UKHeliBob: That looks better. Yes you must do the same for each of the input/output pairs.

You can do it more effectively by using arrays or a struct to hold the timing information and iterate through them using a for loop.

I will try the code with my project first, if this work then I will look more into arrays and for loop for it. I'm still new to programming and learning. Thank you!

Why do you have 4 different nows? That's an absolutely terrible way to name variables. They aren't storing now, they're storing the start of the timing interval. Name them appropriately.

Jiggy-Ninja: Why do you have 4 different nows? That's an absolutely terrible way to name variables. They aren't storing now, they're storing the start of the timing interval. Name them appropriately.

You are right, I will rename them as startTimeA/B/C/D. Thanks for the suggestion!

I will rename them as startTimeA/B/C/D. Thanks for the suggestion!

Even better would be to create an array, called startTimes.

Except, that there is no clue what started, so startTimes is not that good a name. The times are when you turned an output pin HIGH aren't they? So, onTimes or highTimes would make even more sense.

PaulS: Even better would be to create an array, called startTimes.

Except, that there is no clue what started, so startTimes is not that good a name. The times are when you turned an output pin HIGH aren't they? So, onTimes or highTimes would make even more sense.

I'm not sure about creating array yet, but yes, onTimes or highTimes makes more sense. Thank you!

Hi, the above code works! I’m now looking into arrays and for loop for it but i encounter this issue of unable to complie.
Below is the original code

void loop() {
  // put your main code here, to run repeatedly:
  
  boxA = digitalRead(A0);
  boxB = digitalRead(A1);
  boxC = digitalRead(A2);
  boxD = digitalRead(A3);

Changed to and having problem

void loop () {

for (int i = 0; i < 4; i++){
box[i] = digitalRead ([inputPins]);
}

How do i actually implement it properly?

This is the top portion if it helps:

int box[4];
int pinCount = 4;
int outputPins [ ] = {5, 4, 3, 2};
int inputPins [ ] = {A0, A1, A2, A3};

void setup() {
for (int thisPin =0 ; thisPin<pinCount; thisPin++){
pinMode(inputPins[thisPin], INPUT);
}

for (int thisPin =0 ; thisPin<pinCount; thisPin++){
pinMode(outputPins[thisPin], OUPUT);
}
Serial.begin(9600);
}

Thanks for your time!

for (int i = 0; i < 4; i++){
box[i] = digitalRead ([inputPins]);
}

You declared box to be an array, and figured out how to index into that array. Then, you declared inputPins to be an array, but completely butchered the use of/indexing into that array.

for (int i = 0; i < 4; i++)
{
   box[i] = digitalRead (inputPins[i]);
}

Putting each { and each } on lines by themselves makes for better looking code.

Thank you very much. I understand the concept and change all the codes to

unsigned long onTime[4];

unsigned long interval = 10000;  

int box[4];

int pinCount = 4;
int outputPins[] = {5, 4, 3, 2};
int inputPins[] = {A0, A1, A2, A3};


void setup() {

  for (int thisPin = 0; thisPin < pinCount; thisPin++)
      {
      pinMode(inputPins[thisPin], INPUT);
      }

  for (int thisPin = 0; thisPin < pinCount; thisPin++)
      {
      pinMode(outputPins[thisPin], OUTPUT);      
      }
      
    Serial.begin(9600);
}


void loop() {

  for (int i = 0; i < 4; i++)
  {
  box[i] = digitalRead (inputPins[i]);
  }
  
    unsigned long currentMillis = millis();
    for (int i = 0; i <4; i++)
    {
      if(box[i] == HIGH)                   
      {
        digitalWrite(outputPins[i], HIGH);      
        onTime[i] = currentMillis;             
      }
      if( (box[i] == LOW) && (currentMillis - onTime[i] > interval) ) 
      {
        digitalWrite(outputPins[i], LOW);       
      }
    }
}

Is my understanding correct?

  for (int i = 0; i < 4; i++)

You gave the value 4 a nice name. You should use that name here.

      if(box[i] == HIGH)                  
      {
        digitalWrite(outputPins[i], HIGH);      
        onTime[i] = currentMillis;            
      }
      if( (box[i] == LOW) && (currentMillis - onTime[i] > interval) )
      {
        digitalWrite(outputPins[i], LOW);      
      }

You are writing to the pin the same value, in both cases, that is in box[ i ].

Other than this, does the code do what you want?