I am working on a sketch using an addressable Led strip, I would like to have one led on at a time while moving down the strip, to give the appearance of the light chasing down the strip.
I have used the blink without delay as a starting point, however something appears to be going wrong with the increment in the for loop.
When I run the sketch, the first led continuously blinks on and off instead of moving down the strip.
What am I missing?
Thanks
#include "FastLED.h"
#define NUM_LEDS 60
#define DATA_PIN 8
CRGB leds[NUM_LEDS];
unsigned long previousMillis = 0;
const long interval = 1000;
void setup() {
FastLED.addLeds<NEOPIXEL, DATA_PIN>(leds, NUM_LEDS);
leds[0] = CRGB(0, 0, 0);
}
void loop() {
unsigned long currentMillis = millis();
for (int i = 0; i < NUM_LEDS; i++) { // i = Led number
if (currentMillis - previousMillis >= interval) {
previousMillis = currentMillis;
if (leds[i] == CRGB( 0, 0, 0)) {
leds[i] = CRGB( 100, 50, 50); // Led on
FastLED.show();
}
else {
leds[i] = CRGB( 0, 0, 0); // Led off
FastLED.show();
}
}
}
}
Why do you think the for loop isn't incrementing?
Does that seem a likely thing?
Hi TolpuddleSartre, I'm not quite sure I understand the comment, to my understand i should be incrementing by 1 every time the for loop runs?
Have you considered the number of times that for loop will complete in one second?
My thinking is that the below bit of code would control the timing and result in a 1 second delay before running the for loop again. When I check the status of i in the serial monitor I can see that the loop is running once per second, however i stays at 0.
for (int i = 0; i < NUM_LEDS; i++) { // i = Led number
if (currentMillis - previousMillis >= interval) {
previousMillis = currentMillis;
I feel as though I am missing something very basic....
Hi gfvalvo
Good attempt, but has some problems. Try something like this (untested, but it compiles):
I have tried your code and it does exactly what mine was intended to do, I have also read through it and understand the approach which you took. Thank you.
But please comment on where my attempt is failing as I would like to understand what I am doing wrong?
i = 0 -> the if statement is correct -> previous = current
i = 1 -> current - previous is always zero -> if statement false
i = 2 -> if statement false
...
When you finish the for loop, then your currentMillis gets updated.
JamieDemos:
Hi TolpuddleSartre, I'm not quite sure I understand the comment, to my understand i should be incrementing by 1 every time the for loop runs?
By my understanding, it is.
I can see no proof from you to show that it is not.
By my understanding, it is.
I can see no proof from you to show that it is not.
See the attached screenshot of the serial monitor, each time 1000ms passes i remains at 0.
Hi Speklap
You don't update currentMillis.
What happens is
i = 0 -> the if statement is correct -> previous = current
i = 1 -> current - previous is always zero -> if statement false
i = 2 -> if statement false
...
When you finish the for loop, then your currentMillis gets updated.
Try to rewrite this.
From the screenshot you can see currentMillis is indeed updating.
Regardless I did try moving the unsigned long currentMillis = millis();
around in the program however this did not help.
The original code runs through the numbers 0-59 very very fast. Thousands of times per second. The Arduino can easily count to 60 in less than 1 millisecond.
Then once a second the timer expires and whatever LED it's pointing to gets checked. That will be basically random.
What is wrong with gfvalvo's code? Does it do something like you expect?
The original code runs through the numbers 0-59 very very fast. Thousands of times per second. The Arduino can easily count to 60 in less than 1 millisecond.
Then once a second the timer expires and whatever LED it's pointing to gets checked. That will be basically random.
What is wrong with gfvalvo's code? Does it do something like you expect?
Thanks for the input MorganS, I think I understand now, gfvalvo's code works perfectly, I just wanted to understand where I had gone wrong for future reference.
If I understand correctly, you would typically use a delay at the end of the for loop to control the timing of the for loop.
I have instead tried to use millis to control the timing (incorrectly). Is it possible to use millis to control the timing of a for loop, how would you go about this? or is there a different method to control the timing of a for loop (other than delay)?
See the attached screenshot of the serial monitor, each time 1000ms passes i remains at 0.
Hi Speklap
From the screenshot you can see currentMillis is indeed updating.
Regardless I did try moving the unsigned long currentMillis = millis();
around in the program however this did not help.
Thanks for the input guys.
Still trying to figure this out.
No it isn't updating, it is updating after/before the loop (depending how you look at it).
--> start program
currentmillis = millis();
do the for loop
2.1 if current - previous > interfal -> current = previous
2.2 if statement always false
2.3 if statement always false
2.4 ...
currentmillis = millis();
do the for loop
4.1 ..
4.2 ..
currentmillis = millis();
The arduino goes through the for loop in a few microseconds. And never updates in those microseconds. So only when i == 0, currentMillis can be bigger than previousMillis + interval
If I understand correctly, you would typically use a delay at the end of the for loop to control the timing of the for loop.
I have instead tried to use millis to control the timing (incorrectly). Is it possible to use millis to control the timing of a for loop, how would you go about this? or is there a different method to control the timing of a for loop (other than delay)?
Why use a for loop when the loop() function is doing what its name implies ?
start of loop()
current time = millis()
if it is time to display the LEDS
set boolean updatingLeds to true to start
save current millis() as start time
set currentLed to zero
end if
if updateLeds is true
updateLeds();
end if
end of loop()
start of updateLeds
if currentTime - start time >= period
increment currentLed
turn on currentLed
save current time as start time
end if
end of updateLeds
JamieDemos:
Thanks for the input MorganS, I think I understand now, gfvalvo's code works perfectly, I just wanted to understand where I had gone wrong for future reference.
If I understand correctly, you would typically use a delay at the end of the for loop to control the timing of the for loop.
If you truly understand my code in Reply #2, then you should know that your last statement is totally incorrect.
JamieDemos:
I have instead tried to use millis to control the timing (incorrectly). Is it possible to use millis to control the timing of a for loop, how would you go about this? or is there a different method to control the timing of a for loop (other than delay)?
Thanks
The point is to NOT control the speed of any for() loop. Let it run fast, updating anything it needs to update as fast as possible. What you need to slow down is the decisions of what to update.
If you want a single LED 'chasing' along the strip then change which LED is lit once per second or whatever. The for() loop can use this information to quickly update the whole strip once that decision is made.
Comments on ORIGINAL code , may be irrelevant by now.
you are not checking current time in for() loop
if (currentMillis - previousMillis >= interval) {
should be
if (millis() - previousMillis >= interval) {
2.AS written with "previousMillis" , the if() code is dependent on HOW long has program been running - millis() is a counter initialized to zero at program start.
the if() will always initially evaluate to true ( millis() - 0 > interval) initially
next line has same issue as #1, and initially "preiviousMillis" will be set pretty close to "currentMilis"
previousMillis = currentMillis;
and it is not clear what is "previousMillis" function anyway.
Basically - you do not need "currentMillis" variable - use millis() function it its place.
Safer and "self documenting" that way.
It is really unnecessary to name "previousMillis" if you really want to know "lastTime" the function / event was used.
.
Basically - you do not need "currentMillis" variable - use millis() function it its place.
Safer and "self documenting" that way.
Whilst it is not necessary to save the value of millis() in a variable it is good practice to do so.
(1) The name of the variable makes it obvious what the value represents
(2) Calling millis() several times in loop() is an overhead
(3) The value of miilis() may change during the execution of the loop() function but the value of the variable won't
(4) If a decision is based on the current value of millis() then if millis() is read again as part of the action then it may well read a different value because time has moved on.
SUMMARY :
It is good programming practice to save the value of millis() at the start of loop() and use the saved value throughout loop() for consistent timing and lowest overhead.
UKHeliBob:
Whilst it is not necessary to save the value of millis() in a variable it is good practice to do so.
(1) The name of the variable makes it obvious what the value represents
(2) Calling millis() several times in loop() is an overhead
(3) The value of miilis() may change during the execution of the loop() function but the value of the variable won't
(4) If a decision is based on the current value of millis() then if millis() is read again as part of the action then it may well read a different value because time has moved on.
SUMMARY :
It is good programming practice to save the value of millis() at the start of loop() and use the saved value throughout loop() for consistent timing and lowest overhead.
OK, so how do you handle / synchronize the initial value of millis() even when it is close to zero?
Should the initial value be set as "baseTime = miliis()" in Setup and then the rest ft the code compared to it?
I just see these "fence posts errors" causing all kinds of grief.
Basically - you do not need "currentMillis" variable - use millis() function it its place.
Safer and "self documenting" that way.
...and wrong.
So, @232,using your "solution", you'd be happy to allow to allow timing to slip in the following (or similar cases) case using the blink-without-delay/cooperative multitasking example:
if (millis() - lastEvent >= processInterval)
{
doSomethingThatCouldTakeFiveMilliseconds (forExample, couldTakeTwoMilliseconds);
lastEvent = millis (); // that is not going to end well.
}
Would that be a fair assessment of your argument?
Sometimes, when you apply the KISS principle, you do too much of the KIS, but you end-up with just that last S.
I plead ignorance - you had "previous Milllis " which I did not bother to figure out where it was declared and defined / used. I figured when the "currentMillis" is taken care off then take a look at "previousMillis" .
One "baby step" at a time. Hope to see the "final version " soon.
so how do you handle / synchronize the initial value of millis() even when it is close to zero?
By setting currentMillis and previousMillis to millis() in setup() or even at the start of loop() with a boolean preventing it happening at every iteration of loop().
If you are dealing with really short periods then use micros() instead.
I just see these "fence posts errors" causing all kinds of grief.
Sorry, you have lost me. Would you care to explain ?
UKHeliBob:
By setting currentMillis and previousMillis to millis() in setup() or even at the start of loop() with a boolean preventing it happening at every iteration of loop().
If you are dealing with really short periods then use micros() instead.
Sorry, you have lost me. Would you care to explain ?
Sorry. old term for problems at the start or end of the program.
