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:
- 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.
- 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);
}
}
-
Thanks for the wonderful question. here is the Arduino project link which you can use for further discussion. I am sure, you will find it very easy to share your project in this way wokwi.com/arduino/projects/321913849735283283ArduinoFan– ArduinoFan2022年01月27日 05:36:10 +00:00Commented Jan 27, 2022 at 5:36
1 Answer 1
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);
}
}
-
1I 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– Myles2022年01月25日 20:06:44 +00:00Commented Jan 25, 2022 at 20:06
-
1I 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 postMyles– Myles2022年01月25日 21:44:32 +00:00Commented Jan 25, 2022 at 21:44
-
1Awesome, 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– Myles2022年01月25日 22:56:58 +00:00Commented Jan 25, 2022 at 22:56
-
1what 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– Myles2022年01月26日 22:53:12 +00:00Commented Jan 26, 2022 at 22:53
-
1so do i only need 2 case statements then?Myles– Myles2022年01月26日 23:03:32 +00:00Commented Jan 26, 2022 at 23:03
Explore related questions
See similar questions with these tags.