TLC5940 with adjustable speed, help

I am using Arduino Duemilanove with two TLC5940's hooked up to 24 LEDs. I modified the Basic Use (Knight Rider) example slightly to the following:

#include "Tlc5940.h"

void setup()
{
  Tlc.init();
}

void loop()
{
  int direction = 1;
  for (int channel = 0; channel < NUM_TLCS * 12; channel += direction) {
    Tlc.clear();
    if (channel == 0) {
      direction = 1;
    } else {
      Tlc.set(channel - 1, 1000);
    }
    Tlc.set(channel, 4095);
    if (channel != NUM_TLCS * 12 - 1) {
      Tlc.set(channel + 1, 1000);
    } else {
      direction = -1;
    }
    Tlc.update();
    delay(75);
  }
}

It works fine, but I want to add a pot to change the speed without using delay. Here's what I came up with:

#include "Tlc5940.h"

int potPin = 0;  // pot to change speed
int ledSpeed;  // speed of led chase
unsigned long changeTime;

void setup()
{
  Tlc.init();
}

void loop()
{
  ledSpeed = analogRead(potPin);
  int direction = 1;
  for (int channel = 0; channel < NUM_TLCS * 12; channel += direction) {
    Tlc.clear();
    if (channel == 0) {
      direction = 1;
    } else {
      Tlc.set(channel - 1, 1000);
    }
    Tlc.set(channel, 4095);
    if (channel != NUM_TLCS * 12 - 1) {
      Tlc.set(channel + 1, 1000);
    } else {
      direction = -1;
    }
    Tlc.update();
    if ((millis() - changeTime) > ledSpeed) {
    changeTime = millis();
  }
 }
}

When I upload and run this, the LEDs just flash very quickly and the pot doesn't do anything when turned.

Every time loop() is being called it is doing all the work so it is a very busy sketch. The delay time being set by the pot doesn't matter in this situation because the effective delay is almost zero anyway (on a human scale).

You have to wrap the entire loop code in your delay code like this:

if ((millis() - changeTime) > ledSpeed) {
changeTime = millis();

DO EVERYTHING ELSE HERE!
}

Also, in setup I would initialize changetime:

void setup()
{
changeTime = millis();
Tlc.init();
}

Otherwise you have an uninitalized variable which is bad practice.

Revised code is below, but it didn't change anything, the LEDs still flash rapidly and turning the pot does nothing.

#include "Tlc5940.h"

int potPin = 0;  // pot to change speed
int ledSpeed;  // speed of led chase
unsigned long changeTime;

void setup()
{
  changeTime = millis();
  Tlc.init();
}

void loop()
{
  if ((millis() - changeTime) > ledSpeed) {
    changeTime = millis();
  ledSpeed = analogRead(potPin);
  int direction = 1;
  for (int channel = 0; channel < NUM_TLCS * 12; channel += direction) {
    Tlc.clear();
    if (channel == 0) {
      direction = 1;
    } else {
      Tlc.set(channel - 1, 1000);
    }
    Tlc.set(channel, 4095);
    if (channel != NUM_TLCS * 12 - 1) {
      Tlc.set(channel + 1, 1000);
    } else {
      direction = -1;
    }
    Tlc.update();
  }
 }
}

It looks OK to me now as long as ledSpeed is some reasonable value. Any chance you can take a voltage reading on pin A0 and verify that it is not zero or near zero? A zero or near zero reading would cause this loop to run very fast also. Have you ever used Serial Monitor to debug your code? You may want to have the Arduino send the value of ledSpeed back to your computer to verify that it is reasonable and Serial Monitor is all sorts of help in other ways when you want to know the state of your running code. Read this:

http://www.ladyada.net/learn/arduino/lesson4.html

The bit that does the LED changing is still inside the loop, you need to seprate out a change in lights from the loop.
Try playing being the processor, read through the code and remember that everything in the loop happens very fast.
You will see you are doing your sequence very fast, then all the pot is doing is delaying slightly the time before that whole sequence happens again.

Tried this, but it still reacts the same way:

#include "Tlc5940.h"

int potPin = 0;  // pot to change speed
int ledSpeed;  // speed of led chase
unsigned long changeTime;

void setup()
{
  changeTime = millis();
  Tlc.init();
}

void loop()
{
  ledSpeed = analogRead(potPin);
  if ((millis() - changeTime) > ledSpeed) {
    changeLights();
    changeTime = millis();
 }
}

void changeLights(){
  int direction = 1;
  for (int channel = 0; channel < NUM_TLCS * 12; channel += direction) {
    Tlc.clear();
    if (channel == 0) {
      direction = 1;
    } else {
      Tlc.set(channel - 1, 1000);
    }
    Tlc.set(channel, 4095);
    if (channel != NUM_TLCS * 12 - 1) {
      Tlc.set(channel + 1, 1000);
    } else {
      direction = -1;
    }
    Tlc.update();
 }
}

Your change lights function goes through ALL the sequence in one go.
As I keep saying you need to separate out just one step and do that each time your timer times out.

It is your 'for' statement that is doing this. Remove it and replace it with a structure that only changes one of the light states any time it is entered. To do this you need a 'state variable' one that shows where you are up to in your sequence. You used the loop variable i for this but now you no longer have a for loop you will have to replace it with a variable you declare as static in the loop function.
A static variable is one that is initialised only once the first time it is encountered and after that it's value remains the same.

Grumpy_Mike:
It is your 'for' statement that is doing this. Remove it and replace it with a structure that only changes one of the light states any time it is entered. To do this you need a 'state variable' one that shows where you are up to in your sequence. You used the loop variable i for this but now you no longer have a for loop you will have to replace it with a variable you declare as static in the loop function.
A static variable is one that is initialised only once the first time it is encountered and after that it's value remains the same.

Is there an example of this? I searched but didn't see anything that might help. I may have missed something though.

Ok then make that function this:-

void changeLights(){
  static int dir = 1;
  static int channel = 1;

     Tlc.clear();
     if (channel == 1) dir = 1;
     if (channel == 14) dir = -1;
     Tlc.set(channel - 1, 1000);
     Tlc.set(channel, 4095);
     Tlc.set(channel + 1, 1000);
     Tlc.update();
    channel += dir;
}

Thanks Mike, it's working now. There is one minor issue though. When I turn the pot, most of the speed changes happen at the end of the pot that makes it go fastest rather than gradually and evenly change speed. How can I fix this? I am using a B100K pot.

It sounds like you are using a log pot and not a linear one.
Try swapping over the 5V and ground connections on the pot to move the log section to the other end.
If you do have a log pot and do not have a linear one, you can put resistors across it to linearise it.
See:- The Secret Life of Pots

Also that function will take a certain amount of time to run, if your delay is shorter than this then you won't see any change in speed. To gt round this add a fixed number to your analogue input reading.

Grumpy_Mike:
Also that function will take a certain amount of time to run, if your delay is shorter than this then you won't see any change in speed. To gt round this add a fixed number to your analogue input reading.

I tried this but I'm guessing it's not what you meant, because it didn't seem to be any different:

int ledSpeed = 100;  // speed of led chase

No I meant

  ledSpeed = analogRead(potPin) + 200;

Or some other such constant to make a minimum delay value.

I'm working on adding additional patterns to this and changing them by pushing a button, and still keep the pot, but one thing that I'll need to change is this:

void loop()
{
  ledSpeed = analogRead(potPin);
  if ((millis() - changeTime) > ledSpeed) {
    changeLights();
    changeTime = millis();
 }
}

The changeLights() part in the above, how would I change this and where would I put it so that turning the pot changes speed on all patterns? In other words, if my first pattern looks like this:

void changeLights(){
  static int dir = 1;
  static int channel = 1;

     Tlc.clear();
     if (channel == 1) dir = 1;
     if (channel == 14) dir = -1;
     Tlc.set(channel - 1, 1000);
     Tlc.set(channel, 4095);
     Tlc.set(channel + 1, 1000);
     Tlc.update();
    channel += dir;
}

Would that part to change speed still be in the loop section, or would it be in the void changeLights() section?

Instead of just calling changeLights() you use an if statement or a switch statement to pick what function to change.