next topic »

[on:]

I'm trying to blink a set of LED's. A time array is used to set the timing of each blink and another array selects the LED to blink.

I'm trying to use this library: http://arduino.cc/playground/Code/Timer

Code:

timer1.pulse(n[i]*2+20, 100, LOW);

This is supposed to make a LED blink once, i.e. turn the digital pin on for 100 ms and turn it off.

When I upload my code, the first LED's stay on for a lot longer than it's supposed to.


What am I doing wrong?

Code:

#include "Timer.h"

//select output pins
int led1 = 22;
int led2 = 24;
int led3 = 26;
int led4 = 28;
int led5 = 30;
int led6 = 32;
int led7 = 34;

int index = 7;
int start = 1;

Timer timer1;


long t1[] = {0,1000,2000,2000,3000,3000,4000,5000};  //type long to store big numbers (time in ms)
int n[]= {0,1,2,3,4,5,6,1};

void setup() {

  // initialize digital pins 1-7 as an output.
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
  pinMode(led6, OUTPUT);
  pinMode(led7, OUTPUT);

  // initialize serial:
  Serial.begin(9600);

  Serial.println("Ready");

  //print what's currently in the arrays
  Serial.println("Time Sequence");
  int i;
  for (i = 0; i < index; i = i + 1) {
  Serial.println(t1[i]);
  //Serial.println(n[i]);
  }
 


}

void loop() {
  timer1.update();
 
    if(start == 1){
    Serial.println("Starting Sequence...");
    //output to pins
    int i;
    for (i = 1; i < index; i=i+1){
      delay(t1[i]-t1[i-1]);
      timer1.pulse(n[i]*2+20, 100, LOW);
      timer1.update();
    }
    Serial.println(i);
    if (i == index){
      start = 0;
        Serial.println("Sequence Stopped.");
    }
  }
 
}

[Reply #1 on:]

You need to call timer.update() repeatedly - this cannot happen if you call delay() - use the techniques as shown in blinkWithoutDelay example
to avoid calling delay and ensure the update() function is called every time round every loop...

[Reply #2 on:]

Thank you. I got it to work with the method used in the example you mentioned. 

Unfortunately, now I have a small problem at the end of the for loop. The LED at the end of the sequence does not light up. I think this is because the last instance of timer1.pulse(n*2+20, 100, LOW); gets executed, and then the for loop ends without ever doing another timer1.update(). How can I keep the loop running until the last pulse goes through?

Code:

void loop() {

  if(start == 1){
    Serial.println("Starting Sequence...");
    //output to pins
    int i;
    for (i = 1; i < index; i=i+1){
      unsigned long currentMillis = millis();
      timer1.update();

      interval = t1[i]-t1[i-1];
      if(currentMillis - previousMillis > interval){
Serial.println(currentMillis);
        // save the last time you blinked the LED
        previousMillis = currentMillis;
        timer1.pulse(n[i]*2+20, 100, LOW);
      }
      //else if (i == index){
      //}
      else{
        --i;
      }
    }
    //Serial.println(i);
   
    if (i == index){
      start = 0;
      Serial.println("Sequence Stopped.");
    }
  }

}

[Reply #3 on:]

Your code is very hard to follow. Put each { on a new line, and use Tools + Auto Format to fix the indenting.

I'm pretty sure that the problem doesn't lie in the commented out code. GET RID OF IT!

Code:

    for (i = 1; i < index; i=i+1){
      unsigned long currentMillis = millis();
      timer1.update();

      interval = t1[i]-t1[i-1];
      if(currentMillis - previousMillis > interval){
Serial.println(currentMillis);
        // save the last time you blinked the LED
        previousMillis = currentMillis;
        timer1.pulse(n[i]*2+20, 100, LOW);
      }
      //else if (i == index){
      //}
      else{
        --i;
      }
    }

If you had really understood the blink without delay philosophy, there would not be a for loop here at all. At you most certainly wouldn't be diddling with the loop index in the body of the loop.

On a side note, why do you reject the use of the ++ operator in the for statement, but embrace it inside the body? Why are you using the prefix notation?

[Reply #4 on:]

I'm trying to output a sequence of LED blinks to a set of LEDs.

I'm decrementing i so the loop runs until if(currentMillis - previousMillis > interval) has been met for the current i. What should I use instead of a for loop for this application?

the i=i+1 and --i usage is just sloppy coding on my part. I'll polish it up once the code works.

Code:

for (i = 1; i < index; i=i+1)
{
unsigned long currentMillis = millis();
timer1.update();

interval = t1[i]-t1[i-1];
if(currentMillis - previousMillis > interval)
{
Serial.println(currentMillis);
timer1.pulse(n[i]*2+20, 100, LOW);
// save the last time you blinked the LED
previousMillis = currentMillis;
}
else
--i;
}

[Reply #5 on:]

I'm decrementing i so the loop runs until if(currentMillis - previousMillis > interval) has been met for the current i. What should I use instead of a for loop for this application?

A while loop would be a better choice.

But, the best choice is no loop. The loop() function gets called over and over again. Each time, you need to decide if it is time to take some action. If it is, take the action and update the time that the action was last started.

Part of "take the action" might involve updating a PWM value, turning an LED on or off, sending some serial data, etc.

I'll polish it up once the code works.

I don't understand this attitude. You're aware of an issue, regardless of how trivial. Fix it as soon as you are aware of it.