Blink Four LEDs

Using Arduino Programming Questions
1

Hello everybody.

This is my very first message here and I'm a almost total noob using arduino so sorry in advance if I'm saying/doing any weird thing, but after all we can't learn without failing.

I have a little problem I hope you can help me with (I don't expect you to write any code for me as the tittle of this section is warning) but anyway.

I'm trying to make blink 4 leds in a specific order inside an old Simon game I've made empty. I made a pretty simple code that worked just as I wanted but after 30 minutes the leds stop blinking in the order I wanted and only one of them blinks quickly while the others do nothing.

I've read that it can be related to using Delay. Anyway I'm leaving here the code just in case you can give me any advice about it. I know it's very simple, I'm a noob after all. Thank you in advance!

// the number of the pushbutton pin
const int ledPin = 13;
const int ledPin2 = 12;
const int ledPin3 = 11;
const int ledPin4 = 10;

void setup() {

pinMode(ledPin, OUTPUT);
pinMode(ledPin2, OUTPUT);
pinMode(ledPin3, OUTPUT);
pinMode(ledPin4, OUTPUT);
}

void loop(){

digitalWrite(ledPin, HIGH);
digitalWrite(ledPin2, HIGH);
digitalWrite(ledPin3, HIGH);
digitalWrite(ledPin4, HIGH);
delay (3000);
digitalWrite(ledPin, LOW);
digitalWrite(ledPin2, LOW);
digitalWrite(ledPin3, LOW);
digitalWrite(ledPin4, LOW);
delay (3000);
// encender leds por orden.
digitalWrite(ledPin, HIGH);
delay (3000);
digitalWrite(ledPin, LOW);
delay (1000);
digitalWrite (ledPin2, HIGH);
delay (3000);
digitalWrite(ledPin2, LOW);
delay (1000);
digitalWrite (ledPin3, HIGH);
delay (3000);
digitalWrite(ledPin3, LOW);
delay (1000);
digitalWrite(ledPin4, HIGH);
delay (3000);
digitalWrite(ledPin4, LOW);
delay (1000);
digitalWrite(ledPin2, HIGH);
delay (3000);
digitalWrite(ledPin2, LOW);
delay (3000);
}

2

This is my very first message here a

No, it's your second, but it's almost identical to your other one.

3

..and it’s entirely the wrong place to post it.
Start your own thread?

4

@Alvferhid, do not cross-post, do not post in the tutorials section, do not hijack.

5

Review the this tutorial.

https://forum.arduino.cc/index.php?topic=526696.0 https://forum.arduino.cc/index.php?topic=526696.0

6

There is nothing wrong with your code, and there is nothing wrong with using delay() for your purpose although larryd's link will be useful for you later on. Consider reading it anyway :slight_smile:
I do not see any reason why it should stop blinking after 30 minutes. Is this the actual code you are using?

7

Which pin is it that is blinking.

This is a red flag:

const int ledPin =  13;

Pin 13 is traditionally used as the on-board LED pin. Strange things may happen when you use this in your project as well. But I agree with olf2012. The code is good.

I would look at other things. Start with the power supply. Is there a possibility you are getting a reset due to a brownout or or power spike in line or somthing? What kind of power supply are you using?

8

Code seems very “manual”...it’s just a list of instructions in order...I am very new here so others may disagree but I would consider putting your pin data into an arrays as follows;

Put PIN numbers in array;

Int pins[4]{10,11,12,13};

Put high times (your delay values) into array in same order as pins;

Int hightimes[4]{3000,3000,3000,3000};

Same for low;

Int lowtimes[4]{1000,1000,1000,1000};

And lastly, a 2d array to hold on and off times in millis ()

Int ledState[2][4]{
{0,0,0,0}
{0,0,0,0}
};

Why?

This allows you to set up all you pin modes as follows;

void setup(){
  // declare pinModes
  for (p=0;p<=3,p++) {
    pinMode(pins[p], OUTPUT);
  }
}

Similarly you could then use millis() and another loop to control all the pins

// pin control loop
start = millis();       //sets start time
// loop to set all pins high
for (p=0;p<=3,p++) {
    digitalWrite(pins[p],HIGH);
    ledState[1][p]=millis();   //ledstate[1] = on times, ledState [2] = off times.
}

Do{
for (p=0;p<=3,p++) {
 if (ledState[1][p]>ledState[0][p]){                 // led is ON (time turned on later than time turned off)
  if (millis()-ledstate[1][p] >= hightimes[p]) { // if the led has been on long enough...
    digitalWrite(pins[p],LOW);
    ledState[0][p]=millis();
}
}
else     // led is OFF
{
if (millis()-ledstate[0][p] >= lowtimes[p]) { // if the led has been off long enough...
    digitalWrite(pins[p],HIGH);
    ledState[1][p]=millis();
}while (1!=0);

That do while loop is really bad practice as it’s an infinite loop. You could use a runtime value instead, as millis does “roll over” at some point so you should really rest the values if this is going to run for a long time.

Disclaimer: all code was typed by hand into the browser and may contain typos and syntax errors - and because I’m a noob myself, may not work at all! There’s a good link to a relevant tutorial for the array stuff here;

But it still uses delay, not millis()

9 
ledstate[1] = on times, ledState [2] = off times.

The ledState array does not have a level 2

as millis does "roll over" at some point so you should really rest the values if this is going to run for a long time.

No need to worry about millis() rollover as long as unsigned long variables are used, the test for expiry of the timing period is done using subtraction and the period does not exceed 49 and a bit days.

More to the point, if all the OP really wants to do is to blink the 4 LEDs in sequence then there is nothing wrong with using delay(), but I suspect that he has been put off by the complicated solutions offered here.

10

UKHeliBob:

ledstate[1] = on times, ledState [2] = off times.

The ledState array does not have a level 2

See disclaimer, LOL!

Thanks Bob, just a typo, should read;

ledstate[1] = on times, ledState [0] = off times.

l

UKHeliBob:
No need to worry about millis() rollover as long as unsigned long variables are used, the test for expiry of the timing period is done using subtraction and the period does not exceed 49 and a bit days.

More to the point, if all the OP really wants to do is to blink the 4 LEDs in sequence then there is nothing wrong with using delay(), but I suspect that he has been put off by the complicated solutions offered here.

All true. Comment about rollover was just part of offering an alternative to my lazy “while (1!=0)” infinite loop.

11

All true. Comment about rollover was just part of offering an alternative to my lazy "while (1!=0)" infinite loop.

No need to create an infinite loop when you have the loop() function. Its name rather gives away its purpose.

12

All part of my miseducation in Arduino.

For ages I’ve been defining variables at the start of (loop), because that’s what I saw in the first sketch example I read.

But that resets them every time it runs, which isn’t always what you want, so I started using my infinite do...while.

I tried declaring in (setup), but just got “variable x was not declared at this stage”.

Have now realised I can declare variables BEFORE setup in which case they apply globally, so all is well.

But since OP is also new, thought he might have seen same sketch as me...hence 1!=0...

13

GreyArea:
Have now realised I can declare variables BEFORE setup in which case they apply globally, so all is well.

be careful about getting too comfortable with this situation as it unnecessarily takes up memory space.

as you progress to more complicated code using various functions, you will learn when to use variables limited to said functions - this takes up less memory space, very important when using Arduino and 32KB of flash memory.

14

BabyGeezer:
as you progress to more complicated code using various functions, you will learn when to use variables limited to said functions - this takes up less memory space, very important when using Arduino and 32KB of flash memory.

Be careful about getting too comfortable with this approach, as nested functions can take up memory space, possibly unnecessarily.

On a microcontroller with limited RAM, you must always be mindful of all techniques of memory usage.

15

But that resets them every time it runs, which isn't always what you want

Not if you declare them as static

Try this

void setup()
{
  Serial.begin(115200);
  Serial.println();
}

void loop()
{
  static byte count = 0;
  Serial.println(count);
  count++;
  delay(1000);
}
16

Thanks BG...good to know. Most of my routines are low memory...just animating a few LEDs...but it’s good to be made aware of these things...I’ll make sure I just declare the variables before setup that I do NOT want overwritten every time the loop starts...

..sound of 12x12 array being hastily cut and pasted back to the start of (loop)...

There. :slight_smile:

17

UKHeliBob:
Not if you declare them as static

Try this

void setup()

{
  Serial.begin(115200);
  Serial.println();
}

void loop()
{
  static byte count = 0;
  Serial.println(count);
  count++;
  delay(1000);
}

I’ll have a look at that...I might have misinterpreted what static did...

18

Although there's nothing in OP's code to suggest it would go wrong at 30 minutes, I set up to test it.

So far, 40 minutes in and still running properly.

Will report later.

Edit: Ran ok for just over an hour then I accidentally nudged the USB and killed the power.

19

Over 6 hours on a new test, no signs of distress.

So OP, pretty sure the problem isn't your code (not that anyone thought it was).