Trouble with multiple LED flash patterns using Millis

Totally new to the forum and Arduino, but have been really enjoying my journey into coding so far. I am trying to create a program that will cycle through two flash patterns after a defined period of time. I have 4 LEDs. Flash pattern one would alternate LED 1 and 4 (LED 1 on for 200ms, LED 4 off for 200ms, LED 1 off for 200ms, LED 4 on for 200ms) while LED 2 and 3 flash together (LED 2 & 3 on for 100ms, LED 2 & 3 off for 100ms). After the first pattern I want the program to move on to the second pattern after 5 seconds. Flash pattern two flashes all LEDs at the same rate (LED 1-4 on for 200ms, LED 1-4 off for 200ms). Then back to flash pattern one. When my program starts, pattern one works correctly, but pattern two will changes after each pass through the loop and only occasionally works as intended. Any help is greatly appreciated. Thanks. I even tried a port manipulation to set all my pins low before the second flash pattern, but I can’t seem to get it to work right. Here is my sketch:

const int ledPin =  2;
const int ledPin2 =  3;
const int ledPin3 =  4;
const int ledPin4 =  5;
int ledState = LOW;
int ledState2 = LOW;
unsigned long previousMillis = 0;
unsigned long previousMillis2 = 0;
unsigned long interval = 200;
unsigned long interval2 = 100;
unsigned long interval3 = 200;
unsigned long interval4 = 200;

unsigned long starttime;
unsigned long endtime;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(ledPin3, OUTPUT);
  pinMode(ledPin4, OUTPUT);
  DDRD = B1111111;
  Serial.begin(9600);
}

void loop() {
  PORTD = B0000000;
  Serial.println ("Pins Low");
  Serial.println (millis());
  Serial.println ("Pattern One Begin");
  Serial.println (millis());
  starttime = millis();
  endtime = starttime;
  while ((endtime - starttime) <= 5000) // do this loop for up to 5000mS
  {
    LED_pattern1();

    endtime = millis();
    //delay(2000);



  }
  PORTD = B0000000;
  Serial.println ("Pins Low");
  Serial.println (millis());
  Serial.println ("Pattern Two Begin");
  Serial.println (millis());
  starttime = millis();



  while ((endtime - starttime) <= 5000) 
  {
    LED_pattern2();


    endtime = millis();
    //delay(2000);


  }
}
void LED_pattern1() {
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis > interval) {
    previousMillis = currentMillis;
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    digitalWrite(ledPin, ledState);
    digitalWrite(ledPin4, !ledState);

  }
  if (currentMillis - previousMillis2 > interval2) {
    previousMillis2 = currentMillis;
    if (ledState2 == LOW)
      ledState2 = HIGH;
    else
      ledState2 = LOW;
    digitalWrite(ledPin3, ledState2);
    digitalWrite(ledPin2, ledState2);

  }
}
void LED_pattern2() {
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis > interval3) {
    previousMillis = currentMillis;
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    digitalWrite(ledPin, ledState);
    digitalWrite(ledPin4, ledState);

  }
  if (currentMillis - previousMillis2 > interval4) {
    previousMillis2 = currentMillis;
    if (ledState2 == LOW)
      ledState2 = HIGH;
    else
      ledState2 = LOW;
    digitalWrite(ledPin3, ledState2);
    digitalWrite(ledPin2, ledState2);


  }
}

I don't have an Uno handy to test your code, but please use CTRL-T in the IDE to format the sketch.

Nothing bad jumps out to me, but it is bad practice to use while() statements with millis(). The whole idea of using millis() is to not have blocking code in your loop(). while() is blocking code.

Also, again not a hard rule, you should only copy millis()once in each loop. Usually the first thing you do in loop(). then inside the loop, each thing you want to test should have its own long int to test. (You re-use endtime and starttime for two different timings. Very confusing).

HI sobredosis,

still not completely clear to me what you want to do:
flash-pattern 1

lash pattern one would alternate LED 1 and 4 while LED 2 and 4 flash together.

LED1 On
LED2 OFF
LED3 OFF
LED4 OFF

LED1 OF
LED2 OFF
LED3 OFF
LED4 ON

LED1 OFF
LED2 ON
LED3 ON
LED4 OFF

repeat

flash-pattern 2:

Flash pattern two flashes all LEDs at the same rate.

LED1, LED2, LED3, LED4 ON
LED1, LED2, LED3, LED4 OFF

repeat

I'm not sure if you mean it that way. Best thing would be to draw a picture by hand a line for each LED horizontal axle is time vertical is ON/OFF

best regards Stefan

I had a mistype and forgot to " control T" in my first post. Sorry for the confusion. I edited it to hopefully be a little more clear as to what I'm trying to accomplish. Thanks

If you want me to post suggestions I need feedback if my understanding of what you are trying to do is correct.
best regards Stefan

You really just need one 5 second timer and then some way to indicate which pattern to use. Here is what I came up with. It could be even shorter if you used arrays.

const int ledPin =  2;
const int ledPin2 =  3;
const int ledPin3 =  4;
const int ledPin4 =  5;
const unsigned long interval = 200;
const unsigned long interval2 = 100;
const unsigned long interval3 = 200;
const unsigned long interval4 = 200;
const unsigned long patternInterval = 5000;

int ledState = LOW;
int ledState2 = LOW;
int patternCnt = 0;
unsigned long previousMillis = 0;
unsigned long previousMillis2 = 0;
unsigned long prevMillisPattern = 0;

unsigned long starttime;
unsigned long endtime;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(ledPin3, OUTPUT);
  pinMode(ledPin4, OUTPUT);
  DDRD = B1111111;
  Serial.begin(9600);
}

void loop() 
{
  if (millis() - prevMillisPattern > patternInterval)
  {
    prevMillisPattern = millis();
    PORTD = B0000000;
    patternCnt++;
    patternCnt %= 2;
    Serial.println("Switch patterns");
  }
  switch (patternCnt)
  {
    case 0:
  	  LED_pattern1();
      break;
    case 1:
  	  LED_pattern2();
      break;
  }
}

void LED_pattern1() {
  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis > interval) {
    previousMillis = currentMillis;
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;
    digitalWrite(ledPin, ledState);
    digitalWrite(ledPin4, !ledState);
  }

  if (currentMillis - previousMillis2 > interval2) {
    previousMillis2 = currentMillis;
    if (ledState2 == LOW)
      ledState2 = HIGH;
    else
      ledState2 = LOW;
    digitalWrite(ledPin3, ledState2);
    digitalWrite(ledPin2, ledState2);
  }
}

void LED_pattern2() {
  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis > interval3) {
    previousMillis = currentMillis;
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;
    digitalWrite(ledPin, ledState);
    digitalWrite(ledPin4, ledState);
  }

  if (currentMillis - previousMillis2 > interval4) {
    previousMillis2 = currentMillis;
    if (ledState2 == LOW)
      ledState2 = HIGH;
    else
      ledState2 = LOW;
    digitalWrite(ledPin3, ledState2);
    digitalWrite(ledPin2, ledState2);
  }
}

Thanks Stefan. I had a mistype in my first post that caused some confusion. I looking for the following pattern:

Pattern 1

LED1 - ON (200ms)
LED2 - ON (100ms)
LED3 - ON (100ms)
LED4 - OFF (200ms)

then

LED1 - OFF (200ms)
LED 2 - OFF (100ms)
LED 3 - OFF (100ms)
LED 4 - ON (200ms)

repeat pattern for 5 seconds ... then

Pattern 2

LED1 - ON (200ms)
LED2 - ON (200ms)
LED3 - ON (200ms)
LED4 - ON (200ms)

then

LED1 - OFF (200ms)
LED 2 - OFF (200ms)
LED 3 - OFF (200ms)
LED 4 - OFF (200ms)

repeat pattern for 5 seconds...then back to pattern one.

I'm trying to get a handle of using Millis, but my pattern two doesn't want to cooperate. It's frustrating but in a fun kind of way. I've only been at this for a week or so, but have been learning a lot. Just can't seem to find an answer online. In the meantime I'll keep cracking at it. Thanks.

Thanks for your post Todd. I just beginning to learn how to use cases and counters so I'll definitely be staring at your sketch for a bit. I did load it into my UNO and I am experiencing the same problem with pattern 2. I'm going to try some debugging through the serial port, but it seems to me the LED pins are remaining in their previous states when pattern one ends instead of starting in the LOW position at the start of pattern 2. It causes pattern 2 to have different pattern each pass through the loop. Thanks

sobredosis:
Thanks for your post Todd. I just beginning to learn how to use cases and counters so I'll definitely be staring at your sketch for a bit. I did load it into my UNO and I am experiencing the same problem with pattern 2. I'm going to try some debugging through the serial port, but it seems to me the LED pins are remaining in their previous states when pattern one ends instead of starting in the LOW position at the start of pattern 2. It causes pattern 2 to have different pattern each pass through the loop. Thanks

I tested it and it worked fine for me. I slowed the patterns down to make sure.

You could use a library like TDuino which would make it quite easy - code is untested:

#include <TDuino.h>

void timerCallback(byte timerIndex);

#define LED_COUNT 4
const byte LED_PINS[LED_COUNT] = {2, 3, 4, 5};
const byte PATTERN1[LED_COUNT] = {200, 200, 200, 200};
const byte PATTERN2[LED_COUNT] = {200, 100, 100, 200};

byte currentPattern = 255;
TTimer timer(timerCallback);
TPinOutput leds[4];

void timerCallback(byte timerIndex)
{
    if (currentPattern == 0)
    {
        for (byte i = 0; i < LED_COUNT; i++) leds[i].pulse(PATTERN1[i]);
        currentPattern = 1;
    }
    else
    {
        for (byte i = 0; i < LED_COUNT; i++) leds[i].pulse(PATTERN2[i]);
        currentPattern = 0;
    }
}

void setup()
{
    for (byte i = 0; i < LED_COUNT; i++) leds[i].attach(LED_PINS[i]);
    timer.set(5000);
    timerCallback(0);
}

void loop()
{
    for (byte i = 0; i < LED_COUNT; i++) leds[i].loop();
    timer.loop();
}

OK if two LEDs shall blink with the same frequency you can both switch with one "timer".

There is even more simplification possible in your case because LED2 and LED3 blink at double the frequency of LED1 and LED 4 in pattern so change state of LED1 and LED4 is done every second change of LED2 / LED3

In pattern 2 all four LEDs blink with the same frequency. This means one timer is enough because they all blink the same way ON/OFF

every 5 seconds there shall be a change from pattern 1 to 2 and back.
This can be understood as a "blinking" ON/OFF too

So a boolean flag that switches between "pattern1" and "pattern2" could be used
Just like switching ON/OFF an LED

I don't have LEDs handy right now. Here is a code that compiles so give it a try if it works as expected

const int ledPin1 =  2;
const int ledPin2 =  3;
const int ledPin3 =  4;
const int ledPin4 =  5;

int ledState = LOW;
int ledState2 = LOW;

unsigned long CurrentMillisPattern1_2  = 0;
unsigned long previousMillisPattern1_2 = 0;

unsigned long CurrentMillisBlink  = 0;
unsigned long previousMillisBlink = 0;

unsigned long intervalPattern1_2 = 5000;
unsigned long intervalBlink = 100;
unsigned int BlinkCount = 0;

boolean DoPattern1 = true;

void setup() {
  pinMode(ledPin1, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(ledPin3, OUTPUT);
  pinMode(ledPin4, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  CurrentMillisPattern1_2 = millis();
  
  if (CurrentMillisPattern1_2 - previousMillisPattern1_2 >= intervalPattern1_2) {
    previousMillisPattern1_2 = CurrentMillisPattern1_2;
    DoPattern1 = !DoPattern1;  // the "!" is the NOT operator which just inverts a boolean    
  }

  if (DoPattern1) {
    LED_pattern1();
  }
  else {
    LED_pattern2();
  }    
}


void LED_pattern1() {
  static unsigned long CurrentMillisBlink;
  static unsigned long previousMillisBlink;

  CurrentMillisBlink = millis();
  if (CurrentMillisBlink - previousMillisBlink >= intervalBlink) {
    previousMillisBlink = CurrentMillisBlink;

    BlinkCount++; // increment blinkcount by 1 every 100 ms

    // blink LED1, LED4 only every 2 times 100ms passed by = blink with 200ms 
    if (BlinkCount == 2) {
      BlinkCount = 0;
      ledState2 = ! ledState2; // invert state with the NOT-operator
      digitalWrite(ledPin1, ledState2);
      digitalWrite(ledPin4, !ledState2);  // LED4 is always in opposite state as LED1 so invert status
    }
    
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    digitalWrite(ledPin2, ledState);
    digitalWrite(ledPin3, ledState);
  }
}


void LED_pattern2() {
  static unsigned long CurrentMillisBlink;
  static unsigned long previousMillisBlink;

  CurrentMillisBlink = millis();
                                               // double interval = 2 * 100 ms = 200 ms
  if (CurrentMillisBlink - previousMillisBlink >= 2 * intervalBlink) {
    previousMillisBlink = CurrentMillisBlink;

    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    digitalWrite(ledPin1, ledState);
    digitalWrite(ledPin2, ledState);
    digitalWrite(ledPin3, ledState);
    digitalWrite(ledPin4, ledState);
  }
}

best regards Stefan

Thank you all so much for your responses!!!!

Todd: I'm still playing with your sketch, but I'm still running into problems with pattern two not turning on all 4 LEDs on and off at the same time. It's more than likely on my end, but thank you so much for your help.

Stefan: Your sketch is doing exactly what I was looking for. It's a new way for me to think about how to accomplish the end goal...and I'll be really staring at this code until I really get it. Thank you. Now to figure out how to add more patterns....

My journey is just starting with this arduino stuff, and I hope to return the advice soon.

Another option is to use my MultiBlink sketch that allows you to define the pattern you want in data and then it runs them for you. See GitHub - MajicDesigns/MultiBlink: Blink LEDs in different configurable patterns, MultiBlink4 is probably the best for you.