Can't get timing right for LEDs with Limit Switches

Hello Everyone,

I am in need of a bit of assistance regarding my project with Millis(). I surely hope what I’m asking makes sense and thank you in advance for the help.

A quick run through of what my project does.

I have two limit switches for forward and reverse limits. I have a motor (controlled by two relays for forward and reverse motion) driving a linear rod that has a screw block moving back and forth on it. When I press a single “enter” button, the screw block moves in the opposite direction from whichever limit limit switch it is engaged in. For example, if the block is hitting the reverse limit switch, when I press the “enter” button, it will move forward towards the forward limit switch and vice versa. This will go back and forth each time I press the “enter” button. I also have the arduino check during initial power-up, if either any of the limit switches are engaged and defaults to the “reverse position” if none are engaged.

Now I would like to add two LED light indicator that will light for about 30 seconds after each limit switch is hit, RED LED for reverse and Green LED for forward.

First off, I am at a beginner level with the Arduino and I apologize if my coding does not look orderly.
I place lots of Serial.println statements to check my work and get values to I can understand more of what is going on. Hopefully my code isn’t too confusing. And also, I am not sure if my approach is even correct. The problem I’m having is that I can’t seem to get the values from Millis() for this statement correct because it will be at random times when the “enter” button is pressed. After each condition is ran, for example, if the reverse limit switch is pressed and the “enter” button is pressed, the arduino will send the screw block forward. I then added my if (currentMillis -previousMillis >= interval) and I can’t seem to get the builtin LED (used for testing for now) to turn on and stay on even when I change the values “1000” (from 100 to 5000). And also I think the issue is previousMillis = currentMillis; after each condition doesn’t work in my case because the “enter” button is pressed at random times all the while currentMillis = millis(); keeps on counting.

void setup() {

pinMode (Relay1, OUTPUT);
pinMode (Relay2, OUTPUT);
pinMode (LimitFWD, INPUT_PULLUP);
pinMode (LimitREV, INPUT_PULLUP);
pinMode(TriggerSW, INPUT_PULLUP);
digitalWrite(Relay1, LOW);
pinMode(LED_BUILTIN, OUTPUT);

// Startup Limit SW check and Reverse 
while ((digitalRead(LimitFWD)) && (digitalRead(LimitREV)))    
 { 
   digitalWrite(Relay2, HIGH);
   delay(5);                       
 }
digitalWrite(Relay2, LOW);

for (int i = 0; i < NUMRELAYS; i++)
 {
   digitalWrite(relayPins[i], !RELAYACTIVE);
   pinMode(relayPins[i], OUTPUT);
 }
 pinMode(TriggerSW, INPUT_PULLUP);
}


void loop() {

if (buttonPressed())
 {

  
 if ( (digitalRead(LimitREV))&& (!digitalRead(LimitFWD)))
    { 
     unsigned long currentMillis = millis();

     while (digitalRead(LimitREV)) 
       {
         digitalWrite(Relay2, RELAYACTIVE);
       }
     digitalWrite(Relay2, !RELAYACTIVE);
     unsigned long currentMillis = millis();
     
    }
    

   
   
  if ((!digitalRead(LimitREV)) && (digitalRead(LimitFWD)))  
   {
    unsigned long currentMillis = millis();

    while (digitalRead(LimitFWD)) 
     {
       digitalWrite(Relay1, RELAYACTIVE);
     }
     digitalWrite(Relay1, !RELAYACTIVE);
     Serial.println("Relay 1 is off");
     previousMillis = currentMillis;
    return;
   }

     
 if ( (digitalRead(LimitFWD))&& (digitalRead(LimitREV)))
  {
     Serial.println("Reset");
     while (digitalRead(LimitREV)) 
     {
       digitalWrite(Relay2, RELAYACTIVE);
     }
        digitalWrite(Relay2, !RELAYACTIVE);
     Serial.println("Reverse Reset Stop");
   }
   currentRelay++;
   
   if (currentRelay >= NUMRELAYS) currentRelay = 0;
    
 } 
   
 }

The first thing to do is to post your code properly in code tags to avoid it being mangled like yours has been

See read this before posting a programming question

while ((digitalRead(LimitFWD)) && (digitalRead(LimitREV)))    
 {
   digitalWrite(Relay2, HIGH);
   delay(5);                      
 }

The relay pin should be written to before the while statement. The Arduino doesn't charge you each time it reads a pin, so there is no reason to have a delay() in the body of the statement.

//BOOLEAN
boolean buttonPressed() // Reads Enter  button presses

What value does that first comment add?

You're going to be screwed if you add another switch that needs to be read. If the function took an argument, a pin number for instance, you'd be OK. If the function clearly defined which switch it cares about, you'd be OK. As it is, you need to make sure you have some Vaseline handy.

     while (digitalRead(LimitREV))
       {
         digitalWrite(Relay2, RELAYACTIVE);
       }

Again, you don't need to keep doing that.

     unsigned long currentMillis = millis();
     Serial.println("REV");
     Serial.println("Relay 2 is on");
     while (digitalRead(LimitREV))
       {
         digitalWrite(Relay2, RELAYACTIVE);
       }
     digitalWrite(Relay2, !RELAYACTIVE);
     unsigned long currentMillis = millis();

Maybe you need more variables at the same scope named currentMillis...

With the code you have now, you'll never get where you want to be. You need to get rid of all the while statements. On any given pass through loop(), it may be necessary to change the direction that the motor is moving. Or it may not. But, that has nothing to do with whether a limit switch has become pressed. And, it has nothing to do with whether an LED should be turned on, or off.

You need to learn to write, and call, functions. The loop() function should look like:

void loop()
{
   checkForEnter();
   checkLimitSwitchOne();
   checkLimitSwitchTwo();
   turnOffRedLED();
   turnOffGrnLED();
}

It should be fairly obvious that you need some global variables, so that functions can share data. It should be obvious what each of these functions should do. It should also be obvious that each of these functions will be called many, many times per second, and that most of the time, they will do nothing.