Blinking one led after the other using millis() and function

Hi all,

I've a problem that give me headaches.this sketch here below is OK. In using 2 function: ligths2() and lights13() 2 leds are blinking at the same time
and it is ok.

[code]
const int ledpin2 = 2;
const int ledpin13 = 13;
unsigned long count = 0;
bool invert = LOW;

void setup() {

  pinMode(ledpin2, OUTPUT);
  pinMode(ledpin13, OUTPUT);

}





void loop()  {

  lights13();
  lights2();
}






void lights13() {

  static unsigned long lastTime = 0;
  const long interval = 400ul;
  static bool state = 0;

  unsigned long now = millis();

  if ( now - lastTime > interval && state == 0 ) {
    state = 1;
    lastTime = now;
    digitalWrite(ledpin13, HIGH);

  }

  if ( now - lastTime > interval && state == 1 ) {
    state = 0;
    lastTime = now;
    digitalWrite(ledpin13, LOW);

  }

}

void lights2() {

  static unsigned long lastTime = 0;
  const long interval = 400ul;
  static bool state = 0;

  unsigned long now = millis();

  if ( now - lastTime > interval && state == 0 ) {
    state = 1;
    lastTime = now;
    digitalWrite(ledpin2, HIGH);

  }

  if ( now - lastTime > interval && state == 1 ) {
    state = 0;
    lastTime = now;
    digitalWrite(ledpin2, LOW);



  }


}


[/code]

but I would want  the  2 leds running sequencely by example: 4 blinks for one and eventually 4 blinks for the other. In the "void loop()" I added  these codes and I don't have any clue where I've been mistaken.
thank for your help.


void loop()  {

  invert = !invert;

  if (invert = 0) {
    for (count = 0; count < 4; count ++)   {
      lights13();
    }
  }

  else  {
    for  (count = 0; count < 4; count ++)   {
      lights2();
    }
  }
}

Oops

= vs ==

bool invert = LOW;
static bool state = 0;

Variables of type bool should only be set to true or false, not numbers or LOW/HIGH.

It will work if you use 0 or 1 or LOW or HIGH, because of the way C language works, but that does not make it a good idea. Other languages will not let you do such things.

When comparing a bool variable, you should not compare it to 0, 1, LOW, HIGH etc either. Nor should you compare it to true or false. You should simply write

if (invert) ...

or

if (!invert) ...

Also, "state" is not a good name for a bool variable, because the name does not indicate what true and false mean. "invert" is a better name because it is clear to the reader that true means that something is upside-down or reversed, and false means that it is not those things.

Sorry,il is the first time I post on this site, this is the text missing.

But I would want  the  2 leds running sequencely by example: 4 blinks for one and eventually 4 blinks for the other. In the "void loop()" I added  these codes and I don't have any clue where I've been mistaken.
thank for your help.

That text is not code, so you should not put it in code tags. You can see that the text is not wrapped and the reader must scroll a long way to the right to read the whole sentence, which is inconvenient.

@bardaffe
You have LED1 connected at DPin-2 and LED2 (L) connected at DPin-13.
You want only LED1 should blink (ON-timeDelay-OFF-timeDelay) 4 times; where timeDelay would be genrated by millis() function. After that only LED2 would blink 4 times; and then LED1 again. Is it this what you want to achieve? How much is this timeDelay?

Normally when you want things to happen in a particular sequence, or for a number of repetitions, you simply write lines of code in that same sequence, with loops where you want repetition. As you have probably learned, that style of coding is known as "blocking code", because it relies on using delay() for timing but also because it relies on code lines being written in the sequence you want them to run in, and loops when you want repetition. But if you want to use the "blink without delay" technique, you cannot write code lines in the sequence you want or use loops like that, because that is still "blocking code", even if you are not using delay().

Instead, you need to use the "state machine" technique. In this technique, you use a variable to record where your sketch is in the sequence you need. For example you could have a variable which counts from 0 to 7. When that variable is 0 to 3, one led flashes. When the variable is 4 to 7, the other led flashes. Each time a flash is completed, the variable is incremented.

yes,indeed
that's the scenario, but LED1is call ledpin2 connected on port2 and ledpin13 to port 13 in order i can follow easily the result on my board.

Are you still waiting to see it working?

the timedelay 400ms.

I thought my code was correct because no error when compiling.
but I change it following what you said.

[code]
const int ledpin2 = 2;
const int ledpin13 = 13;
unsigned long count = 0;
bool invert = LOW;

void setup() {

  pinMode(ledpin2, OUTPUT);
  pinMode(ledpin13, OUTPUT);

}





void loop()  {

  invert = !invert;

  if (invert) {
    for (count = 0; count < 4; count ++)   {
      lights13();
    }
  }

  else  {
    for  (count = 0; count < 4; count ++)   {
      lights2();
    }
  }
}






void lights13() {

  static unsigned long lastTime = 0;
  const long interval = 400ul;
  static bool invert = LOW;

  unsigned long now = millis();

  if ( now - lastTime > interval && invert == LOW ) {
    invert = HIGH;
    lastTime = now;
    digitalWrite(ledpin13, HIGH);

  }

  if ( now - lastTime > interval && invert == HIGH ) {
    invert = LOW;
    lastTime = now;
    digitalWrite(ledpin13, LOW);

  }

}

void lights2() {

  static unsigned long lastTime = 0;
  const long interval = 400ul;
  static bool invert = LOW;

  unsigned long now = millis();

  if ( now - lastTime > interval && invert == LOW ) {
    invert = HIGH;
    lastTime = now;
    digitalWrite(ledpin2, HIGH);

  }

  if ( now - lastTime > interval && invert == HIGH ) {
    invert == LOW;
    lastTime = now;
    digitalWrite(ledpin2, LOW);



  }


}


[/code]

Since you care about the number of blinks, I'd add that as a state variable in your functions:
(untested)

int lights13(int blinks) {
  static unsigned long lastTime = 0;
  const long interval = 400ul;
  static bool state = 0;
  static int count = 0; 
  unsigned long now = millis();
  if (blinks >= 0){
    count = blinks;
  }
  if(count <= 0){ // all ffinished 
    return 0;
  }
  if ( now - lastTime > interval && state == 0 ) {
    state = 1;
    lastTime = now;
    digitalWrite(ledpin13, HIGH);
  }
  if ( now - lastTime > interval && state == 1 ) {
    state = 0;
    count--;
    lastTime = now;
    digitalWrite(ledpin13, LOW);
  }
  return count;
}

Then I'd make loop toggle between one of the other:

void loop()  {
  static enum {LED13,LED2} internal_state = LED13;
  switch (internal_state) {
    LED13: 
       if (lights13(-1) == 0) { // all finished 
          lights2(4);  // start 4 blinks
          internal_state = LED2;
       } 
       break;
    LED2:
      if (lights2(-1) == 0) { // all finished 
          lights13(4);   // start 4 blinks
          internal_state = LED13;
       }    
    }  
}

(*** Edited to add `static` keyword for loop's internal_state. ***)
(*** Edited to count down ***)

The compiler doesn't have a mind. It can't understand the meaning of your code. It can only screen it for correct grammar and usage.

1 Like

I don't see what's different. I still see those for-loops.

1 Like

Learn to factor your code:

if ( now - lastTime >= interval ) {
  lastTime = now;
  
  if ( state == false ) {
    state = true;
    digitalWrite(ledpin13, HIGH);
  }
  else {
    state = false;
    digitalWrite(ledpin13, LOW);
  }
}

Also don't assign numerical values to a boolean variable. That's what true and false are for.

2 Likes

it is about boolean variable: high and low instead of 0 and 1.
As far as for-loop I've not yet known this technique.
I don't understand for instance when lights13() is over I cant launch it an other time and so on.
I am sorry that is not possible using for-loop in what i wanted to do.
Thanks.

thank Davix,
I will give an eye at your codes.

sorry, I mend the state machine of PaulRB

The difference is the non-blocking code versus blocking code. How long to you think lights13() takes to run? It does not take 800ms, it takes less than a 1ms to check whether it needs to do anything, then passes control back to your for loop, when then passes it back to loop, which ends, and gets called again. Which toggles invert etc... loop() probably runs 10000+ times/sec, calling lights13() and lights2() 40000 times each.

...And that's perfectly OK because lights13() and lights2() are non-blocking and don't do anything but return if it isn't the right time.

Thanks for replying to me. I was beginning to think you were ignoring me.

None of those values should be used with Boolean variables, only true and false.

I saw for-loops in your code, you do know that technique! But that technique will not work for you this time, as you have discovered.

I think you have not understood what the non-blocking or "blink without delay" way to write code is for or why you might need it. I think this might be because you have not discovered the problem of blocking code yet, so you cannot appreciate the difference. So please explain what is your reason for using millis() instead of delay().

1 Like