Arduino basic led sequencer

[oldguyjn]

Good Evening All
I am attempting to turn on 4 leds in sequence eventually if I manage to sort my code I will be using a 8 ch Relay module relay board that is why I have the leds turning on with a LOW signal
I have used led 3 as a marker as soon as the code is loaded into the Arduino Nano led 3 is turned and then I use a stopwatch to chec the timing leds 4,5,6 turn on after 1,2,3 minutes respectively however led 7 instead of coming on after 4 minutes it takes 6 minutes to come on I changed the delay time on pin 7 to 7 minutes the led came on after 9 minutes..
I would appreciate if a forum member could tell me where I am going wrong with my code
Thanks oldguyjn

 unsigned long timer4 =0;
unsigned long timer5= 0;
unsigned long timer6= 0; 
unsigned long timer7= 0;



void setup() {
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  digitalWrite(4,HIGH);
  digitalWrite(5,HIGH);
  digitalWrite(6,HIGH);
  digitalWrite(7,HIGH);
 
  
}

void loop() {
   digitalWrite(3, LOW); //led 3 used as indicator for start of time sequence 

  if (millis() - timer4 > 1000UL * 60 * 1){
    digitalWrite(4, LOW); //led 4 on after 1 minute delay
     timer4 = millis();
      
  if (millis() - timer5 > 1000UL * 60 * 2){
  digitalWrite(5, LOW);  //led 5 on after 2 minute delay
  timer5 = millis(); 
  }
  if (millis() - timer6 > 1000UL * 60 * 3 ){
    digitalWrite(6, LOW);  // led 6 on after 3 minute delay 
    timer6 = millis(); 

  if (millis() - timer7 > 1000UL * 60 * 3to 7minutes) {
    digitalWrite(7,LOW);
    timer7 = millis(); // led on 7 after 4 minute delay (actual delay is 6 minutes )

   
 }
 }
 }
 }

[anon57585045]

@oldguyjn

  if (millis() - timer7 > 1000UL * 60 * 3to 7minutes) {

Does this compile without errors? I don't think the compiler understands Oxford English.

Are you tinkering with someone else's code?

[oldguyjn]

Hi aarg
Typing code it should be
if (millis() - timer7 > 1000UL * 60 * 4 ) { and yes it does compile without errors
Thanks oldguyjn

[anon57585045]

@oldguyjn

Typing code it should be

I have no idea what that means. Please post the exact actual sketch that you have a problem with, verbatim. It's best to copy paste using the "copy for forum" option in the IDE menu. Post in a new message, don't edit the original post.

It can be helpful to auto-format using ctrl-T in the IDE, to highlight the code blocks with indentation in a standard way.

[oldguyjn]

Thanks aarg
please find sketch without typing error

unsigned long timer4 = 0;
unsigned long timer5 = 0;
unsigned long timer6 = 0;
unsigned long timer7 = 0;



void setup() {
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  digitalWrite(4, HIGH);
  digitalWrite(5, HIGH);
  digitalWrite(6, HIGH);
  digitalWrite(7, HIGH);
}

void loop() {
  digitalWrite(3, LOW);  //led used as indicator for start of time sequence

  if (millis() - timer4 > 1000UL * 60 * 1) {
    digitalWrite(4, LOW);  //led on after 1 minute delay
    timer4 = millis();

    if (millis() - timer5 > 1000UL * 60 * 2) {
      digitalWrite(5, LOW);  //led on after 2 minute delay
      timer5 = millis();
    }
    if (millis() - timer6 > 1000UL * 60 * 3) {
      digitalWrite(6, LOW);  // led on after 3 minute delay
      timer6 = millis();

      if (millis() - timer7 > 1000UL * 60 * 4) {
        digitalWrite(7, LOW);
        timer7 = millis();  // led on after 4 minute delay (actual delay is 6 minutes )
      }
    }
  }
}

[DaveX]

The indentation suggests that several of the if(...){...} are nested and conditional on each other. Which makes their tests only be checked if the parents happen to be true.

I'm not sure exactly what you want from your description, but you might try un-nesting the if() statements.

Here's a Wokwi simulation with lots of pre-wired-up LEDS (active HIGH) that you could drop some code into:

PWMallPins - Wokwi ESP32, STM32, Arduino Simulator

[anon57585045]

@DaveX

The indentation suggests that several of the if(...){...} are nested and conditional on each other.

Yep. As I suspected. Witness the amazing power of ctrl-T.

[qubits-us]

a bit confusing..
made some subtle changes..
seems to work good now..
also added a reset button to start process over..

UnoSwitchMins

#define RST_BTN 2
int mins = 0;
int lastMins = 0;
unsigned long offset = 0;

unsigned long lastDebounce = 0;
int intervalDebounce = 50;
byte lastBtn = 1;

void setup() {
  Serial.begin(115200);
  pinMode(RST_BTN, INPUT_PULLUP);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  digitalWrite(4, HIGH);
  digitalWrite(5, HIGH);
  digitalWrite(6, HIGH);
  digitalWrite(7, HIGH);
  digitalWrite(3, LOW); //led 3 used as indicator for start of time sequence

}


void loop() {

  mins = (millis() - offset) / 60000UL;
  if (mins != lastMins) {
    lastMins = mins;
    Serial.println(mins);

    switch (mins) {
      case 1: digitalWrite(4, LOW); break;
      case 2: digitalWrite(5, LOW); break;
      case 3: digitalWrite(6, LOW); break;
      case 4: digitalWrite(7, LOW); digitalWrite(3, HIGH); break;
    }
  }

  //reset button code to restart sequence..
  if (millis() - lastDebounce >= intervalDebounce) {
    byte btn = digitalRead(RST_BTN);
    if (btn != lastBtn) {
      lastDebounce = millis();
      lastBtn = btn;
      if (btn == LOW) {
        digitalWrite(4, HIGH);
        digitalWrite(5, HIGH);
        digitalWrite(6, HIGH);
        digitalWrite(7, HIGH);
        digitalWrite(3, LOW);
        offset = millis();
        mins = 0;
        lastMins = 0;
      }
    }
  }
}

good luck.. ~q

[PaulRB]

@oldguyjn

turn on 4 leds... using a 8 ch Relay module relay board

Really? What kind of LEDs? (Please post a link to the specs)

[paulpaulson]

Hello oldguyjn

Consider this small Arduino Basis LED Sequencer program tailored from a traffic ligth project.

Try it, check it and play around with the settings inside of the ledSequences[] structured array.

// https://forum.arduino.cc/t/arduino-basic-led-sequencer/1151727/6
#define ProjectName "Arduino basic led sequencer"
// make names
enum TimerControl {Halt, Run};
// make variables
constexpr uint8_t LedPins[] {3, 4, 5, 6, 7};
constexpr uint8_t ButtonPin {A0};
// make structures
struct TIMER
{
  uint32_t previousTime;
  uint32_t intervalTime;
  uint8_t control;
} sequenceTimer {0, 1000, Run};

struct LEDSEQUENCE
{
  uint32_t timeOfCycle;
  uint8_t pattern[sizeof(LedPins)];
} ledSequences[]
{
  {
    1000, {1, 0, 0, 0, 0}
  },
  {
    1000, {1, 1, 1, 1, 1}
  },
  {
    1000, {1, 0, 1, 1, 1}
  },
  {
    1000, {1, 0, 0, 1, 1}
  },
  {
    1000, {1, 0, 0, 0, 1}
  },
  {
    1000, {1, 0, 0, 0, 0}
  },
  {
    1000, {0, 0, 0, 0, 0}
  },
};
// make support
void heartBeat(int LedPin, uint32_t currentMillis)
{
  static bool setUp = false;
  if (!setUp) pinMode (LedPin, OUTPUT), setUp = !setUp;
  digitalWrite(LedPin, (currentMillis / 500) % 2);
}
void setup()
{
  Serial.begin(115200);
  Serial.println(ProjectName);
  for (auto Pin : LedPins) pinMode(Pin, OUTPUT);
  pinMode(ButtonPin, INPUT_PULLUP);
  uint8_t number = 0;
  uint8_t element = 0;
  for (auto &Pin : LedPins) digitalWrite(Pin, ledSequences[element].pattern[number++]);
}
void loop()
{
  uint32_t currentMillis = millis();
  heartBeat(LED_BUILTIN, currentMillis);
  if ((sequenceTimer.control == Halt) and (digitalRead(ButtonPin) ? LOW : HIGH))
  {
    sequenceTimer.control = Run;
    sequenceTimer.previousTime = currentMillis;
    uint8_t number = 0;
    uint8_t element = 0;
    for (auto &Pin : LedPins) digitalWrite(Pin, ledSequences[element].pattern[number++]);
    Serial.println("restart");
  }
  if (currentMillis - sequenceTimer.previousTime >= sequenceTimer.intervalTime and sequenceTimer.control)
  {
    sequenceTimer.previousTime = currentMillis;

    static uint8_t element = 0;
    element = (element + 1) % (sizeof(ledSequences) / sizeof(ledSequences[1]));
    Serial.print("sequence is ");
    Serial.println(element == 0 ? "halted" : "running");
    if (element == 0)
    {
      sequenceTimer.control = Halt;
      Serial.println("press button to restart");
    }
    else
    {
      uint8_t number = 0;
      for (auto &Pin : LedPins) digitalWrite(Pin, ledSequences[element].pattern[number++]);
    }
  }
}

hth

Have a nice day and enjoy coding in C++.

[oldguyjn]

Hi Guys
Thanks for all the replies qubits-us a big thankyou for posting the code I will try it out tonight.
PaulRB perhaps I did not explain what I was trying to achieve , I was using 4 leds to test the sketch if the sketch worked I was going to add addonal lines for 8 outputs and instead of leds would be using a 8 channel relay board however once I tested the sketch pins 4,5,6 timed out as I wanted however pin 7 instead of the led coming on after 4 minutes was taking 6 minutes and no matter what changes I madeto the sketch I could not get the led to come on after 4 minutes . I decided to reach out to the arduino forum for help and once again thanks for all the replies.
oldguyjn

[PaulRB]

Ok, me try to explain in another way.

  if (millis() - timer4 > 1000UL * 60 * 1) {
    digitalWrite(4, LOW);  //led on after 1 minute delay
    timer4 = millis();

timer4 is set to zero initially. So this if statement won't be true until 1 minute after startup. At that time, the first relay switches on, just as you intended. Plus, timer4 is updated to the current time. This means that this if-statement will be true again after another minute has passed, and every minute after that. Each minute, the first relay is switched on again, but because it never got switched off inbetween, this has no effect.

"So what?" you may think. "It's not doing any harm and it's not causing my problem". True. But there is no } after the line that updates timer4, so the other if-statements are "cascaded" to the first if-statement. Meaning they only execute when the first if-statement becomes true, which is once every 1 minute.

This is no problem for second if-statement, because it is true after 2 minutes and every 2 minutes after that, so it always gets executed exactly when it needs to anyway.

At this point you have correctly (but inconsistently) added a }, so the third if-statement is not cascaded to the second if-statement (but it is still cascaded to the first if-statement). So the third if-statement is also works as desired because it gets executed every minute.

Between the third and fourth if-statements there is another missing }. As a result, the fourth if-statement only gets executed every 3 minutes because the third if-statement is true once every 3 minutes. So the fourth if-statement "misses" the desired time and isn't able to spot that until 6 or 9 minutes (2x or 3x 3 minutes).

Why not write your code this way?

void loop() {
  digitalWrite(3, LOW);  //led used as indicator for start of time sequence
  delay(1000UL * 60 * 1);
  digitalWrite(4, LOW);  //led on after 1 minute delay
  delay(1000UL * 60 * 1);
  digitalWrite(5, LOW);  //led on after 2 minute delay
  delay(1000UL * 60 * 1);
  digitalWrite(6, LOW);  // led on after 3 minute delay
  delay(1000UL * 60 * 1);
  digitalWrite(7, LOW);
}

But I'm using the dreaded delay() which must not be used? Well, based on what you have told us about your project so far, there's no reason not to use delay().

[oldguyjn]

Thanks PaulRB and to all that took time to read and posted sketches to my post its very much appreciated.
oldguyjn