3

I am trying to create a program that runs 4 LEDs sequentially while also being able to do other things with different inputs and outputs. Because of this, I am using the millis function and not the delay. I have the code working but not exactly how I want it. It runs through the LEDs and turns them off then runs through again like it is supposed too but there are two problems:

  1. It will pass through the first if statement, then the first again then to the second, then it will go through the first, the second, then the third, etc. I want it to cascade through the if statements, keeping the LED from the previous steps on without repeating. It is causing the LEDs to flash super quick.
  2. The code still runs even if the input is read as LOW as it is dependent on millis. I somehow need it to start from the beginning every time the button is pushed and reset when the button is not pressed.

below the code is a picture of the serial monitor showing the program running through every if statement and not staying in one and moving to the next. If needed, I can add a video showing what the LEDs do.

below is the code

//Code to build 4 sequentially run LEDs that reset everytime there is not input
int RTS_IN  = A0; //input button

//Output LEDs, assigning them to pins
int LED_5 = 5; 
int LED_6 = 6;
int LED_7 = 7; 
int LED_8 = 8; 

//1 second for each time interval
unsigned long Time1 = 1000;
unsigned long Time2 = 2000;
unsigned long Time3 = 3000;
unsigned long Time4 = 4000; 
unsigned long Time5 = 5000;

//variables for holding millis values
unsigned long currentMillis;
unsigned long newMillis = 0;

void setup() 
{
  Serial.begin(9600);
  
  pinMode(RTS_IN, INPUT);
  pinMode(LED_5, OUTPUT);
  pinMode(LED_6, OUTPUT);
  pinMode(LED_7, OUTPUT);
  pinMode(LED_8, OUTPUT);
  
}

void loop()
{
  currentMillis = millis();

  //using sequential method if the input button pressed and held
  if(digitalRead(RTS_IN) == HIGH)
  {
    sequential();
  }
  //keeping LEDs OFF if button has been released 
  else
  digitalWrite(LED_5, LOW);
  digitalWrite(LED_6, LOW);
  digitalWrite(LED_7, LOW);
  digitalWrite(LED_8, LOW);
}

//method to run LEDs sequentially
void sequential()
{
  
  //if statement checking to see if a second has passed to allow the first led to turn on
  if(currentMillis - newMillis >= Time1)
  {
    Serial.println("5 is on");
    
    digitalWrite(LED_5, HIGH);
    delay(1);
    
    //if statement checking to see if 2 seconds has passed to allow the second led to turn on
    if(currentMillis - newMillis >= Time2)
    {
      Serial.println("6 is on");
      
      digitalWrite(LED_6, HIGH);
      delay(1);

      //if statement checking to see if 3 seconds has passed to allow the third led to turn on
      if(currentMillis - newMillis >= Time3)
      {
        Serial.println("7 is on");
        
        digitalWrite(LED_7, HIGH);
        delay(1);
        
        //if statement checking to see if 4 seconds has passed to allow the fourth led to turn on
        if(currentMillis - newMillis >= Time4)
        {
          Serial.println("8 is on");
          
          digitalWrite(LED_8, HIGH);
          delay(1);
          
          //if statement to turn off LEDs and wait a second before turning the first back on
          if(currentMillis - newMillis >= Time5)
          {
            Serial.println("All off");
            
            digitalWrite(LED_5, LOW);
            digitalWrite(LED_6, LOW);
            digitalWrite(LED_7, LOW);
            digitalWrite(LED_8, LOW);

            //this is in order to add a period of this process to each of the 
            //interval times to keep up with millis without this, they would all stay on
            Time1 = Time1 + 5000;
            Time2 = Time2 + 5000;
            Time3 = Time3 + 5000;
            Time4 = Time4 + 5000;
            Time5 = Time5 + 5000;
            delay(1);

          } 
        }
      }
    }
  }
}
  

updated code below with if else statements and the other programming combined


const int RTS_IN  = A0;
const int LTS_IN = A1;

//break signal
const int BS_IN  = A2;
const int BS_OUT = 11;

//reverse signal
const int RS_IN  = A3;
const int RS_OUT = 10;

const int NR_OF_LEDS = 4;
const int RIGHTLEDS[NR_OF_LEDS] = { 6, 7, 8, 9 };
const int LEFTLEDS[NR_OF_LEDS] = { 5, 4, 3, 2 };
const int interval = 100;

int nrOfRightLedsOn = 0;
int nrOfLeftLedsOn = 0;

unsigned long t = 0;

void StateRight();
void StateRightProcess();

void StateLeft();
void StateLeftProcess();

boolean oldSwitchStateRS = LOW;
boolean newSwitchStateRS = LOW;
boolean oldSwitchStateBS = LOW;
boolean newSwitchStateBS = LOW;


void setup() 
{
  Serial.begin(9600);
  pinMode(RTS_IN, INPUT);
  pinMode(LTS_IN, INPUT);

  for (int led = 0; led < NR_OF_LEDS; led++)
  {
    pinMode(RIGHTLEDS[led], OUTPUT);
  }
  
  for (int led = 0; led < NR_OF_LEDS; led++)
  {
    pinMode(LEFTLEDS[led], OUTPUT);
  }

}

void loop() 
{
  StateRight();
  StateLeft();

  newSwitchStateBS = digitalRead(BS_IN);
   if(newSwitchStateBS != oldSwitchStateBS)
  {
    if(newSwitchStateBS == HIGH)
    {
      digitalWrite(BS_OUT, HIGH);
    }
    else
    digitalWrite(BS_OUT, LOW);
    oldSwitchStateBS = newSwitchStateBS;
  }


 newSwitchStateRS = digitalRead(RS_IN);
  if(newSwitchStateRS != oldSwitchStateRS)
  {
    if(newSwitchStateRS == HIGH)
    {
      digitalWrite(RS_OUT, HIGH);
    }
    else
    digitalWrite(RS_OUT, LOW);
    oldSwitchStateRS = newSwitchStateRS;
  }
  

}

void StateRight()

{
  if(digitalRead(RTS_IN) == LOW)
  {
    SwitchRightLedsOff();
    nrOfRightLedsOn = 0;
    t = millis() + interval;
  }
  else if (nrOfRightLedsOn < NR_OF_LEDS)
  {
    StateRightProcess();
  }
  else
  {
    if(millis() >= t + interval)
    {
      t = millis();
      SwitchRightLedsOff();
      nrOfRightLedsOn = 0;
    }
  }
}

void StateRightProcess()
{
  if(millis() >= t + interval)
  {
    t = millis();
    digitalWrite(RIGHTLEDS[nrOfRightLedsOn], HIGH);
    nrOfRightLedsOn++;
  }
}

void SwitchRightLedsOff()
{
  for (int led = 0; led < NR_OF_LEDS; led++)
  {
    digitalWrite(RIGHTLEDS[led], LOW);
  }
}




void StateLeft()
{
  if(digitalRead(LTS_IN) == LOW)
  {
    SwitchLeftLedsOff();
    nrOfLeftLedsOn = 0;
    t = millis() + interval;
  }
  else if (nrOfLeftLedsOn < NR_OF_LEDS)
  {
    StateLeftProcess();
  }
  else 
  {
    if (millis() >= t + interval)
    {
      t = millis();
      SwitchLeftLedsOff;
      nrOfLeftLedsOn = 0;
    }
  }
}

void StateLeftProcess()
{
  if (millis() >= t + interval)
  {
    t = millis();
    digitalWrite(LEFTLEDS[nrOfLeftLedsOn], HIGH);
    nrOfLeftLedsOn++;
  }
}

void SwitchLeftLedsOff()
{
  for (int led = 0; led < NR_OF_LEDS; led++)
  {
    digitalWrite(LEFTLEDS[led], LOW);
  }
}

1

1 Answer 1

5

To be honest, I don't understand exactly the problems, but I try to give some guideline and hope you can use the idea to fix your problems.

What you need is a so called 'state machine'.

I think (according to your problem) that you have 6 states:

0 Idle (no sequence)
1 One LED on
2 Two LEDs on
3 Three LEDs on
4 Four LEDs on
5 No LEDs on 

For this you can best use an enum type and global variable with similar values:

enum EState
{
    Idle,
    OneLedOn,
    TwoLedsOn,
    ThreeLedsOn,
    FourLedsOn,
    NoLedsOn
}

EState _state;

You need a function to check the new state, depending on the current state. In words something like:

current     When                          Action           New 
state                                                      state
--------    ----------------------------  ---------------- -----
Idle        digitalRead(RTS_IN) == HIGH   Start counter    OneLedOn
                                          Switch on LED 0
OneLedOn    digitalRead(RTS_IN) == LOW    Switch LEDs off  Idle
            5 seconds have passed         Switch on LED 1
                                          Start counter    TwoLedsOn
TwoLedsOn   digitalRead(RTS_IN) == LOW    Switch LEDs off  Idle
            5 seconds have passed         Switch on LED 2  
                                          Start counter    ThreeLedsOn
ThreeLedsOn digitalRead(RTS_IN) == LOW    Switch LEDs off  Idle
            5 seconds have passed         Switch on LED 3 
                                          Start counter    ThreeLedsOn
FourLedsOn  digitalRead(RTS_IN) == LOW    Switch LEDs off  Idle
            5 seconds have passed         Switch LEDs off
                                          Start counter    NoLedsOn
NoLedsOn    digitalRead(RTS_IN) == LOW    Switch LEDs off  Idle
            5 seconds have passed         Switch on LED 0  
                                          Start counter    OneLedn

(if you would draw one circle for each state and arrows between them, a so called UML State Diagram, you will see a much clearer way to depict the states and flows).

I don't have a compiler so I do it out of my head.

void Process()
{
    switch (_state)
    {
    case EState::Idle:
        if ((digitalRead(RTS_IN) == HIGH)
        {
            _time = millis();
            digitalWrite(LED_5, HIGH);
            _state = EState::OneLedOn;
        }
        break;
    case EState::OneLedOn:
        if ((digitalRead(RTS_IN) == LOW)
        {
            SwitchLedsOff();
            _state = Idle;
        }
        else if (millis() > time + 5000)
        {
            _time = millis();
            digitalWrite(LED_6, HIGH);
            _state = EState::TwoLedsOn;
        }
        break;
    case EState::TwoLedsOn:
       ...
}

The function SwitchLEDsoff sets all LEDs to low, and you only need one unsigned long _time global variable which you update when a new state starts and check in most states.

Note that high likely you can optimize this (like making each state its own function and removing almost duplicated code), but the main goal is to let you understand how to use a state machine and to use it in this sketch and future sketches.

Update

The following sketch is the same as yours but:

  • Optimized for reducing code duplication (ProcessState)
  • Introduced constants
  • Introduced array for LEDs to iterate over
  • Removed the state, instead used _nrOfLedsOn
  • Removed idle state, handled separately now
  • Prefixed global variables with an underscore (personal preference)
  • Better names for variables/functions
  • Used prototypes to put functions in logical order (high level to low level)

Code:

const int RTS_IN  = A0;
const int NR_OF_LEDS = 4;
const int LEDS[NR_OF_LEDS] = { 5, 6, 7, 8 };
const int DURATION = 300;

int _nrOfLedsOn = 0;
unsigned long _time = 0;

void ProcessState();
void ProcessLedsState();
void SwitchLedsOff();

void setup() 
{
    Serial.begin(9600);
    pinMode(RTS_IN, INPUT);
    for (int led = 0; led < NR_OF_LEDS; led++)
    {
        pinMode(LEDS[led], OUTPUT);
    }
}

void loop()
{
    ProcessState();
}

void ProcessState()
{
    if (digitalRead(RTS_IN) == LOW)
    {
        SwitchLedsOff();
        _nrOfLedsOn = 0;
        _time = millis() + DURATION + 1;
    }
    else if (_nrOfLedsOn < NR_OF_LEDS)
    {
        ProcessLedsState();
    }
    else
    {      
        if (millis() > _time + DURATION)
        {
            _time = millis();
            SwitchLedsOff();
            _nrOfLedsOn = 0;
        }
    }
}

void ProcessLedsState()
{
    if (millis() > _time + DURATION)
    {
        _time = millis();
        digitalWrite(LEDS[_nrOfLedsOn], HIGH);
        _nrOfLedsOn++;
    }
}

void SwitchLedsOff()
{ 
    for (int led = 0; led < NR_OF_LEDS; led++)
    {
        digitalWrite(LEDS[led], LOW);
    }
}
10
  • 1
    I think you nailed it! This is a lot clearer way of coding it I believe. I will try to implement this into my code and get back to you. I appreciate the quick response.
    – Myles
    Jan 25 at 20:06
  • 1
    I have implemented the new code with your help and It works great except for one thing, it doesnt enter state 5 and turn the LEDs LOW with RTS_IN HIGH. I have updated my original post
    – Myles
    Jan 25 at 21:44
  • 1
    Awesome, I will check that code out and try to implement the parts that I can understand. You have been a great help thank you.
    – Myles
    Jan 25 at 22:56
  • 1
    what is the point of have those case statements if the code just falls through. I also do not understand why you add 1 to t in the first if statement in void ProcessState(). Can you explain please?
    – Myles
    Jan 26 at 22:53
  • 1
    so do i only need 2 case statements then?
    – Myles
    Jan 26 at 23:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.