Sequencing LEDs using millis() funktion

Other Hardware LEDs and Multiplexing
1

Hi.

I'm from Serbia, English is not my native language and I'm sorry for my mistakes, if I make any.

I'm really new in Arduino, bought my UNO five days ago. I made my first project, wich works just fine. It was something like "Thunder and lightning" that is well known except I implemented RF remote controller for sound volume levels. MP3 DFPlayer is used for playing sound and its BUSY pin for triggering LED's. Timing of DFPlayer is controlled using millis(). Sequence of LED's was controlled using delay() funktion. Only issue that occurred was lag in controlling sound volume due to delay(). It is possible only when lights are LOW.

Now I'm trying to replicate that light sequence using millis(). And I did it. Sort of... I wrote two codes separately.

First code for sequence that turns on first LED for 500ms than turns it of, turns on second LED for 500ms than turns it of and turns on third LED for 500ms than turns it of. And that works.

Second code for sequence that turns on all LED's at fast rate. And that works too...

But when I joined two codes together all I got is all LED's blinking at fast rate.

It skips first sequence and execute second one continuously.

I'm missing something but don't know what. I've tried to find solution on forum but it's specific problem.

Any help would be appreciated. Thank's.

Here is my code:

int ledPin8=8;
int ledPin9=9;
int ledPin10=10;

int ledState8 = LOW;             
int ledState9 = LOW;
int ledState10 = LOW;
int ledStateXX = LOW;

unsigned long previousMillis8 = 0;        // will store last time LED was updated
unsigned long previousMillis9 = 0;
unsigned long previousMillis10 = 0;

unsigned long interval_100 = 100;        // interval at which to blink (milliseconds)
unsigned long interval_500 = 500;


void setup()
{
  Serial.begin(19200);  
  pinMode(ledPin8, OUTPUT);
  pinMode(ledPin9, OUTPUT);
  pinMode(ledPin10, OUTPUT);
  }

void loop(){

        
       
       unsigned long currentMillis = millis();
       
// First LED=HIGH for 500ms , others=LOW    
          
          if(
          ((ledState8==HIGH) && ( currentMillis - previousMillis8 >= interval_500 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          )
        {
          ledState8=LOW;
          
          digitalWrite(ledPin8,ledState8);
          digitalWrite(ledPin9,ledState8);
          digitalWrite(ledPin10,ledState8);
          previousMillis8 = currentMillis;
                    
        }
        
        else
        
        if(
          (ledState8==LOW) && ( currentMillis - previousMillis8 >= interval_500 )
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          )
        {
          ledState8=HIGH;
          
          digitalWrite(ledPin8,ledState8);         
          digitalWrite(ledPin9,ledStateXX);
          digitalWrite(ledPin10,ledStateXX);
          previousMillis8 = currentMillis;
           
        }
         
        
// Second LED=HIGH for 500ms , others=LOW 

        
        if(
          ((ledState9==HIGH) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          )
        {
          ledState9=LOW;
          
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin8,ledState9);
          digitalWrite(ledPin10,ledState9);
          
          previousMillis9 = currentMillis;
               
        }
        
        else
        
        if(
          ((ledState9==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          )
        {
          ledState9=HIGH;
          
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin8,ledStateXX);
          digitalWrite(ledPin10,ledStateXX);
          
          previousMillis9 = currentMillis;

        }
          
        
// Third LED=HIGH for 500ms , others=LOW 
           
        if(
          ((ledState10==HIGH) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))          
          )
          {
          ledState10=LOW;
          
          digitalWrite(ledPin10,ledState10);
          digitalWrite(ledPin8,ledStateXX);
          digitalWrite(ledPin9,ledStateXX);
          
          previousMillis10 = currentMillis;
         }
         
         else
         
         if(
          ((ledState10==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          
          )
          {
          ledState10=HIGH;
          
          digitalWrite(ledPin10,ledState10);
          digitalWrite(ledPin8,ledStateXX);
          digitalWrite(ledPin9,ledStateXX);
          
          previousMillis10 = currentMillis;
          
         }
          
      
// All LEDs in HIGH/LOW sequence at fast rate
      
        
        if(
          ((ledState10==LOW) && ( currentMillis - previousMillis10 >= interval_100 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis9 >= interval_100 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis8 >= interval_100 ))
        )
        {
          ledState8=HIGH;
          ledState9=HIGH;
          ledState10=HIGH;
          previousMillis8  = currentMillis;
          previousMillis9  = currentMillis;
          previousMillis10 = currentMillis;
          digitalWrite(ledPin8,ledState8);
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin10,ledState10);
        }
        
        else
        
        if(
        ((ledState10==HIGH) && ( currentMillis - previousMillis10 >= interval_100 ))
          &&
          ((ledState9==HIGH) && ( currentMillis - previousMillis9 >= interval_100 ))
          &&
          ((ledState8==HIGH) && ( currentMillis - previousMillis8 >= interval_100 ))
        )
        {
          ledState8  = LOW;
          ledState9  = LOW;
          ledState10 = LOW;
          previousMillis8 = currentMillis;
          previousMillis9 = currentMillis;
          previousMillis10 = currentMillis;
          digitalWrite(ledPin8,ledState8);
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin10,ledState10);
          
        }
        
      
       
}
2

I've also tried with bool() funktion but with same results. All LED's blinking at fast rate...

Here is code with bool() funktion...

int ledPin8=8;
int ledPin9=9;
int ledPin10=10;

int ledState8 = LOW;             
int ledState9 = LOW;
int ledState10 = LOW;
int ledStateXX = LOW;

unsigned long previousMillis8 = 0;        // will store last time LED was updated
unsigned long previousMillis9 = 0;
unsigned long previousMillis10 = 0;
 
unsigned long interval_100 = 100;  // interval at which to blink (milliseconds)
unsigned long interval_500 = 500;

bool firstLine = true;
bool secondLine = true;
bool thirdLine = true;
bool fourthLine = false;

void setup()
{
  Serial.begin(19200);  
  pinMode(ledPin8, OUTPUT);
  pinMode(ledPin9, OUTPUT);
  pinMode(ledPin10, OUTPUT);
  
  }

void loop(){

          firstLine = true;       // At first attempt, these lines were at the end of loop
          secondLine = true;   // trying to reset values                                                                      
          thirdLine = true;      // but results were the same                                                                                                                       
          fourthLine = false;   // whether on top or bottom line of loop                      
       
          unsigned long currentMillis = millis();
       
        
          if( firstLine == true)
          {       
          if(
          ((ledState8==HIGH) && ( currentMillis - previousMillis8 >= interval_500 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          )
          {
          ledState8=LOW;
          
          digitalWrite(ledPin8,ledState8);
          digitalWrite(ledPin9,ledState8);
          digitalWrite(ledPin10,ledState8);
          previousMillis8 = currentMillis;
                    
          }
        
          else
        
          if(
          (ledState8==LOW) && ( currentMillis - previousMillis8 >= interval_500 )
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis8 >= interval_500 ))
          )
          {
          ledState8=HIGH;
          
          digitalWrite(ledPin8,ledState8);         
          digitalWrite(ledPin9,ledStateXX);
          digitalWrite(ledPin10,ledStateXX);
          previousMillis8 = currentMillis;
           
          }
          
          }
          firstLine = false;

          if (secondLine == true)
          {
          if(
          ((ledState9==HIGH) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          )
          {
          ledState9=LOW;
          
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin8,ledState9);
          digitalWrite(ledPin10,ledState9);
          
          previousMillis9 = currentMillis;
               
          }
        
          else
        
          if(
          ((ledState9==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          &&
          ((ledState10==LOW) && ( currentMillis - previousMillis9 >= interval_500 ))
          )
          {
          ledState9=HIGH;
          
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin8,ledStateXX);
          digitalWrite(ledPin10,ledStateXX);
          
          previousMillis9 = currentMillis;

          }
         
          }
          secondLine = false;

          if (thirdLine == true)
          {   
          if(
          ((ledState10==HIGH) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))          
          )
          {
          ledState10=LOW;
          
          digitalWrite(ledPin10,ledState10);
          digitalWrite(ledPin8,ledState10);
          digitalWrite(ledPin9,ledState10);
          
          previousMillis10 = currentMillis;
          }
         
          else
         
          if(
          ((ledState10==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis10 >= interval_500 ))
          
          )
          {
          ledState10=HIGH;
          
          digitalWrite(ledPin10,ledState10);
          digitalWrite(ledPin8,ledStateXX);
          digitalWrite(ledPin9,ledStateXX);
          
          previousMillis10 = currentMillis;
          
          }
        
          }
          thirdLine = false;

          if (thirdLine == false){
          fourthLine = true;
          }

          if (fourthLine == true)
          {
        
          if(
          ((ledState10==LOW) && ( currentMillis - previousMillis10 >= interval_100 ))
          &&
          ((ledState9==LOW) && ( currentMillis - previousMillis9 >= interval_100 ))
          &&
          ((ledState8==LOW) && ( currentMillis - previousMillis8 >= interval_100 ))
          )
          {
          ledState8=HIGH;
          ledState9=HIGH;
          ledState10=HIGH;
          previousMillis8  = currentMillis;
          previousMillis9  = currentMillis;
          previousMillis10 = currentMillis;
          digitalWrite(ledPin8,ledState8);
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin10,ledState10);
          }
        
          else
        
          if(
          ((ledState10==HIGH) && ( currentMillis - previousMillis10 >= interval_100 ))
          &&
          ((ledState9==HIGH) && ( currentMillis - previousMillis9 >= interval_100 ))
          &&
          ((ledState8==HIGH) && ( currentMillis - previousMillis8 >= interval_100 ))
          )
          {
          ledState8  = LOW;
          ledState9  = LOW;
          ledState10 = LOW;
          previousMillis8 = currentMillis;
          previousMillis9 = currentMillis;
          previousMillis10 = currentMillis;
          digitalWrite(ledPin8,ledState8);
          digitalWrite(ledPin9,ledState9);
          digitalWrite(ledPin10,ledState10);
          
          }
        
          } 
       
}
3

your description is not clear.

do you want

loop
switch on pin 8 for 500ms
switch off pin 8
switch on pin 9 for 500ms
switch off pin 9
switch on pin 10 for 500ms
switch off pin 10
switch on pin 8, 9 & 10 for 100ms
switch off pin 8, 9 & 10 for 100ms

If not, make a time diagram/drawing what you want to program.

4

Sorry to be picky, but that is truly dreadful and confused code!

A couple of things to help:

  • Do not repeat relatively complex tests blindly six times. If your test is: "if (currentMillis - previousMillis >= interval_500) then perform that test and contingent on it matchng, decide which other things may require action.

In short, "Is it time to do something? If it is, we can move on but let's figure out what it is we need to do." :sunglasses:

  • Setting previousMillis to currentMillis is always done when the above matches, so make it the first action before you decide what else to do. But it is wrong anyway! Notice that you use a ">=" test? Well, if is equal, then you just set it to match, but if it is greater then you will have lost one or more counts. So in fact you must simply add the relevant interval to previousMillis instead, and ignore currentMillis.

  • Updating three "ledPin"s from "ledState"s is a complete function, so make it a function. Even then, since you do it every time you perform an action when an interval expires, it should appear just once at the end of the list of actions, before you close the braces for the processing of that interval.

  • You need to use Control-T - "Auto Format" to properly indent your code, otherwise it is too confusing to read and follow where the braces start and end! :astonished:

acikasrb:
But when I joined two codes together all I got is all LED's blinking at fast rate.
It skips first sequence and execute second one continuously.
I'm missing something but don't know what. I've tried to find solution on forum but it's specific problem.

Is it not pretty obvious that if you are going to do one thing at a short interval, or doing something else after a longer interval and doing either will re-set the interval, then the longer interval will simply not happen! :grinning:

5

I would get rid of lot of globals
define all intervals in an array
and let a state machine do the rest

something like

/*
  by noiasca
  https://forum.arduino.cc/index.php?topic=666044
*/

const uint16_t interval[] {500, 0, 500, 0, 500, 0, 100, 100};  // time to wait in each intervall

const byte ledPinA = 8;
const byte ledPinB = 9;
const byte ledPinC = 10;

//const byte ledPinA = 5;    // just for me as I'm using different pins ...
//const byte ledPinB = 6;
//const byte ledPinC = 7;

void handleLeds()
{

  static uint32_t previousMillis = 0;
  static byte state = 7;
  if (millis() - previousMillis >= interval[state])
  {
    // it's time for next state
    state++;
    state = state % 8;
    Serial.print(F("state=")); Serial.println(state);

    // act according state
    switch (state)
    {
      case 0:
        digitalWrite(ledPinA, HIGH);
        digitalWrite(ledPinB, LOW);
        digitalWrite(ledPinC, LOW);
        break;
      case 1:                              // these cases are all the same, so lets spare some lines of codes and combine the cases
      case 3:
      case 5:
      case 7:
        digitalWrite(ledPinA, LOW);
        digitalWrite(ledPinB, LOW);
        digitalWrite(ledPinC, LOW);
        break;
      case 2:
        digitalWrite(ledPinA, LOW);
        digitalWrite(ledPinB, HIGH);
        digitalWrite(ledPinC, LOW);
        break;
      case 4:
        digitalWrite(ledPinA, LOW);
        digitalWrite(ledPinB, LOW);
        digitalWrite(ledPinC, HIGH);
        break;
      case 6:
        digitalWrite(ledPinA, HIGH);
        digitalWrite(ledPinB, HIGH);
        digitalWrite(ledPinC, HIGH);
        break;
    }
    previousMillis = millis();
  }
}

void setup() {
  Serial.begin(115200);
  Serial.println(F("\nStart"));
  pinMode(ledPinA, OUTPUT);
  pinMode(ledPinB, OUTPUT);
  pinMode(ledPinC, OUTPUT);
}

void loop() {
  handleLeds();  // checks if something is todo with the LEDs
  //do other important stuff here
}
6

noiasca:
I would get rid of lot of globals
define all intervals in an array
and let a state machine do the rest

Than's for Your reply. I will try this code later and let you know how does it works.

7

Paul__B:
Sorry to be picky, but that is truly dreadful and confused code!

Thank's for Your reply. You said a lot and I'll find a way to understand what You were saying.

As I said before I'm really new in arduino and my knowledge of programming is equal to zero.

The only reference I've found was

https://www.arduino.cc/en/Tutorial/BlinkWithoutDelay

I was blindly following this code on trial/error principle until I've got some results that works.

Since it is better way to manage time-related funktions, made leap of fate into millis().

Evidently, that leap ended in broken glass.

8

acikasrb:
The only reference I've found was https://www.arduino.cc/en/Tutorial/BlinkWithoutDelay
I was blindly following this code on trial/error principle until I've got some results that works.
Since it is better way to manage time-related functions, made leap of fate into millis().
Evidently, that leap ended in broken glass.

By no means. If you want to do more than one thing at a time - and you almost always do - then this technique, broadly called the "state machine", is how it is done; there is no practical alternative.

You just have to understand the design process. The succession of steps is not the order of steps in the code, it is built into the decision code, a series of "If this has happened, then switch on the next part" where the "switch" is a variable that is updated at each step.

You have been using the complex state of the LEDs as the switch. A basic state machine would use a variable - you could call it "step" to denote where you are, and a "switch" statement to decide what (if anything) to do as you run through the loop. You can name the steps for the switch with useful descriptions such as "LED8", "LED9", "NoLED" and so on, corresponding to switching those LEDs on (and others off) in the defined step. This way you do not need to check the LEDs, just know which is the current step.

So if each step is timed, then you first perform the timeout check, if that fails then you skip through to any other major process in the loop (which is how you actually perform quite different and separate tasks "simultaneously") which then repeats. If the timeout has occurred, then you perform the switch and act on it. You may also use another variable to specify the timeout and alter it to correspond to what is required for the current pattern.

Have I confused you further? :grinning:

9

Have I confused you further? :grinning:

Calculating new route... :o

10

noiasca:
I would get rid of lot of globals
define all intervals in an array
and let a state machine do the rest

It works exactly as I wanted. Thank you.

Now I have more questions.

How this code could be modified to change sequence? For example if I want all LEDs to blink at fast rate several times? Is it for() loop applicable in switch/case scenario? Or there is something else I've never heard of...

noiasca:
static byte state = 7;

This line I can't undersand. Why #7? Is it equal to "case 7:"? Or is it something from library?

11

noiasca:

    state = state % 8;

And what does this line of code do?

12

acikasrb:
And what does this line of code do?

Secures state stays in a 0-7 range, this would often be placed after a state++ line or combines into one line: state = (state+1) % 8;

13

HKJ-lygte:
Secures state stays in a 0-7 range, this would often be placed after a state++ line or combines into one line: state = (state+1) % 8;

I see. So, it would be %10 for nine states, %5 for four states etc?

Now I undestand why "static byte state =7". There are 7 states. Its just telling from wich interval to start.

Correct me if I'm wrong, I'm just thinking outloud:

<state = (7+1) % 8;> results in <state=0> so the code starts at <interval[0]>. From that point it adds +1 for every case until reaches 7 again.

Thank you.

14

Your count is one low.
When you use state % 8 the valid states are 0, 1, 2, 3, 4, 5, 6 and 7, i.e. 8 states.

15

HKJ-lygte:
Your count is one low.
When you use state % 8 the valid states are 0, 1, 2, 3, 4, 5, 6 and 7, i.e. 8 states.

This is line from code:

/*
  by noiasca
  https://forum.arduino.cc/index.php?topic=666044
*/

// time to wait in each intervall
const uint16_t interval[] {500, 0, 500, 0, 500, 0, 100, 100};

If each equals to dedicated interval and counting starts from zero, there are 7 states. Am I missing something again?

16

acikasrb:

const uint16_t interval[] {500, 0, 500, 0, 500, 0, 100, 100};

If each equals to dedicated interval and counting starts from zero, there are 7 states. Am I missing something again?

I can see 8 values in the table.

17

HKJ-lygte:
I can see 8 values in the table.

Now I can see it too. I was counting from zero... :-* Silly me. Sorry. Thank you.

18

How this code could be modified to change sequence? For example if I want all LEDs to blink at fast rate several times? Is it for() loop applicable in switch/case scenario?

currently the FSM changes states only by time.
Either

  • pre-calculate the total time blinkrate*blinkTimes for the new state and do the "blinking" by a separate "BlinkWithoutDelay or
  • modify the program logic so that the new case (state) will not be effected by the
    if (millis() - previousMillis >= interval[state]) and switch over to the next state after you have reached your desired blink intervals

regarding

static byte state = 7;

This line I can't undersand. Why #7? Is it equal to "case 7:"?

the variable state holds the actual state of the fsm (grml).
static is needed, as it is declared in the function and I want to keep it even if we finish the function.

7 was needed to initialize / set the start state. As one of the first things what will happen will be the increase of the state (state++), I needed the start value at the end of the range.

P.S.: I'm very sorry for this hardcoded "%8" and "=7". I thought it is obvious and easier to understand if its written in clear, than a more preferred construct of enum / sizeof...

19

noiasca:
currently the FSM changes states only by time.

You helped me alot. Thank's. I'll try to modify code and publish it for evaluation.