Should this not be in the setup function?
And anyway it just fires off all the LEDs every second.
You have nothing to impose a sequence on what led is on when.
You need just one timer that fires off each second. That function will then increment a variable and look at that value to determine which LED is on and which is off. When the variable reaches the limit of your flashing sequence you should set it back to zero.
I would suggest you start with some of the arduino tutorials that are on line, and possibly the arduino cookbook. I am curious, have you considered looking for "arduino traffic light code" using google? I found that there are a lot of examples complete with hard and software.
You could also solve this by using an array with delayed looping. I've modified an example created by David Mellis and Tom Igoe for you below. Good luck with your project!
/*
The circuit:
- LEDs from pins 10 through 12 (on Adafruit Feather) to ground
originally created 2006
by David A. Mellis
modified 30 Aug 2011
by Tom Igoe
modified 04/02/21
by me!
This example code is in the public domain.
http://www.arduino.cc/en/Tutorial/Array
*/
int timer = 1000; // The higher the number, the slower the timing.
int ledPins[] = {
10, 11, 12,
}; // an array of pin numbers to which LEDs are attached
int pinCount = 3; // the number of pins (i.e. the length of the array)
void setup() {
// the array elements are numbered from 0 to (pinCount - 1).
// use a for loop to initialize each pin as an output:
for (int thisPin = 0; thisPin < pinCount; thisPin++) {
pinMode(ledPins[thisPin], OUTPUT);
}
}
void loop() {
// loop from the lowest pin to the highest:
for (int thisPin = 0; thisPin < pinCount; thisPin++) {
// turn the pin on:
digitalWrite(ledPins[thisPin], HIGH);
delay(timer);
// turn the pin off:
digitalWrite(ledPins[thisPin], LOW);
}
}
You could also solve this by using an array with delayed looping.
Agreed on the arrays.
But it's really not a good idea to encourage the use of delay(). Sure, it may not matter in this case, but it's a bad habit to get into.
In any case, you actually took the OP a step backwards here, since the SimpleTimer library they edit are using were trying to use already uses delay()-less millis()-based timing. From its Playground page:
Theory
The base goal is to be able to execute a particular piece of code every n milliseconds, without using interrupts.
The algorithm looks like this:
lastMillis = 0
forever do:
if (millis() - lastMillis > n)
call the particular piece of code
lastMillis = millis()
end
end
But it's really not a good idea to encourage the use of delay(). Sure, it may not matter in this case, but it's a bad habit to get into.
In any case, you actually took the OP a step backwards here, since the SimpleTimer library they edit are using were trying to use already uses delay()-less millis()-based timing. From its Playground page:
Theory
The base goal is to be able to execute a particular piece of code every n milliseconds, without using interrupts.
The algorithm looks like this:
lastMillis = 0
forever do:
if (millis() - lastMillis > n)
call the particular piece of code
lastMillis = millis()
end
end
You already know what I meant by using millis.. This is exactly what I did by using while loop. That's the difference. The only thing I want to emphasize here is using delay is a very very bad habit.
ramesh4nani:
You already know what I meant by using millis..
Yes, I do, and others do too; but not everyone will understand what you meant without seeing your code. Just strikes me as good forum etiquette to share your code to help others in the long run.
I did by using while loop
That said, it's likely unnecessary to have used a while loop... I wonder if you have actually made code just as blocky as using delay(), but with millis() instead. We'll never know....