Timer is Running Too Fast!

Using Arduino Programming Questions
1

I've got a weird issue while using the FastLED library. I have an Uno board, a 10 neopixel strip, a 10k potentiometer, and a tactile button. As the code below shows, I have a timer that changes the mode every 10 seconds. That works, however the timing is off. The modes switch about every 5-6 seconds when serial communication is off and every 8 when serial prints are on. What could be causing this?

Here is my sketch:

#include "FastLED.h"

//#define DEBUG
#define NUM_LEDS 10
#define DATA_PIN 12
#define BUTTON_PIN  3
#define POT_PIN A0

CRGB leds[NUM_LEDS];

int stripState = 0;
int caseMax = 7;
int buttonState = LOW;
int lastButtonState = LOW;
long lastDebounceTime = 0;
int debounceDelay = 50;
long modeTime;
int modeDelay = 10000;

void setup() {
  FastLED.addLeds<NEOPIXEL, DATA_PIN>(leds, NUM_LEDS);
  pinMode(BUTTON_PIN, INPUT_PULLUP);
  Serial.begin(9600);
}

void loop() {
  Serial.println(millis());
  button();
  modeTimer();
  mode();
  FastLED.show();
}

void potBrightness() {
  FastLED.setBrightness(map(analogRead(POT_PIN), 0, 1023, 0, 255));
}

void button() {
  int buttonReading = digitalRead(BUTTON_PIN);
  if (buttonReading != lastButtonState) {
    lastDebounceTime = millis();
  }
  if ((millis() - lastDebounceTime) > debounceDelay) {
    if (buttonReading != buttonState) {
      buttonState = buttonReading;
      if (buttonState == LOW) {
        stripState = (stripState + 1) % caseMax;
        modeTime = millis();
      }
    }
  }
  lastButtonState = buttonReading;
}

void modeTimer() {
  if (millis() > modeTime + modeDelay) {
    modeTime = millis();
    stripState = (stripState + 1) % caseMax;

  }
}
void debug() {
#ifdef DEBUG
  Serial.print("Case: ");
  Serial.print(stripState);
  Serial.print("\t");
  Serial.print("Millis: ");
  Serial.print(millis());
  Serial.println("\t");
  Serial.print("Last Switch:");
  Serial.println(modeTime);
  Serial.print("\t");
  Serial.print("Brightness");
  Serial.println(map(analogRead(POT_PIN), 0, 1000, 0, 255));
#endif
}

void mode() {
  switch (stripState) {
    case 0:
      debug();
      changeCRGBPixels(CRGB::Black, NUM_LEDS);
      break;

    case 1:
      debug();
      FastLED.setBrightness(50);
      changeCRGBPixels(CRGB::White, NUM_LEDS);
      break;

    case 2:
      debug();
      changeCRGBPixels(CRGB::White, NUM_LEDS);
      potBrightness();
      break;

    case 3:
      debug();
      changeCRGBPixels(CRGB::Blue, NUM_LEDS);
      potBrightness();
      break;

    case 4:
      debug();
      changeCRGBPixels(CRGB::Green, NUM_LEDS);
      potBrightness();
      break;

    case 5:
      debug();
      changeCHSVPixels(map(analogRead(POT_PIN), 0, 1023, 0, 255), 255, 255);
      break;

    case 6:
      debug();
      int ledsOn = map(analogRead(POT_PIN), 0, 1000, 0, NUM_LEDS);
      if (ledsOn < NUM_LEDS) {
        changeCRGBPixels(CRGB::Black, NUM_LEDS);
      }
      changeCRGBPixels(CRGB::Cyan, ledsOn);
      break;
  }
}

void changeCRGBPixels(uint32_t color, int numLEDs) {
  for (int i = 0; i < numLEDs; i++) {
    leds[i] = color;
  }
}

void changeCHSVPixels(int h, int s, int v) {
  for (int i = 0; i < NUM_LEDS; i++) {
    leds[i] = CHSV(h, s, v);
  }
}

and a more simple sketch:

#include "FastLED.h"

#define NUM_LEDS 10
#define DATA_PIN 12
#define BUTTON_PIN  3
#define POT_PIN A0

CRGB leds[NUM_LEDS];

int stripState = 0;
int caseMax = 2;
int buttonState = LOW;
int lastButtonState = LOW;
long lastDebounceTime = 0;
int debounceDelay = 50;
long modeTime;
int modeDelay = 10000;
int buttonReading;

void setup() {
  FastLED.addLeds<NEOPIXEL, DATA_PIN>(leds, NUM_LEDS);
  pinMode(BUTTON_PIN, INPUT_PULLUP);
  Serial.begin(9600);
}

void loop() {

  buttonReading = digitalRead(BUTTON_PIN);
  Serial.println(digitalRead(BUTTON_PIN));
  /*
    if (buttonReading != lastButtonState) {
      lastDebounceTime = millis();
    }
    if ((millis() - lastDebounceTime) > debounceDelay) {
      if (buttonReading != buttonState) {
        buttonState = buttonReading;
        if (buttonState == LOW) {
          stripState = (stripState + 1) % caseMax;
          modeTime = millis();
        }
      }
    }
    lastButtonState = buttonReading;
  */
  if (millis() > modeTime + modeDelay) {
    modeTime = millis();
    stripState = (stripState + 1) % caseMax;
  }

  if (stripState == 0) {
    for (int i = 0; i < NUM_LEDS; i++) {
      leds[i] = CRGB::White;
    }
  }

  if (stripState == 1) {
    for (int i = 0; i < NUM_LEDS; i++) {
      leds[i] = CHSV(analogRead(POT_PIN), 255, 255);
    }
  }
  FastLED.show();
}

test timer:

long modeTime;
int modeDelay = 10000;

void setup() {
pinMode(13, OUTPUT);
}

void loop() {
  if (millis() > modeTime + modeDelay) {
    modeTime = millis();
    digitalWrite(13, !digitalRead(13));
  }
}

I tested it with another Uno board, different power source (9v battery/USB), different sketches. I also made sure that the board works with a simple timer and no added libraries. Test turns the on board led on and off every 10 seconds.

**What could be causing this difference in time? **

2

Most here will not go and get files from pastebin

Attach your code using the </> icon on the left side of the posting menu.
Put your sketch between the code tags [code][/code]

If they are too large to post as above, attach the files.
.

3

Thanks I put my sketches in ""

4

if (millis() > modeTime + modeDelay)
Use this:
if (millis() - modeTime >= modeDelay)

modeTime should be type 'unsigned long'

5

Precisely why I left it as an int.

6

modeDelay can be an int. modeTime needs to be unsigned long because you're giving it a value from millis.

7

unsigned long modeTime;
The above is how it should be written.

BTW
get into the habit of using unsigned long for all variables used with millis() stuff
unsigned long modeDelay = 10000UL;

Doesn't take that much more RAM, but may prevent bugs in later sketches.

.

8

There's no reason modeDelay can't be an int. No good reason to waste two bytes of RAM if it is going to be 10000 and never get any bigger.

9

Delta_G:
There's no reason modeDelay can't be an int. No good reason to waste two bytes of RAM if it is going to be 10000 and never get any bigger.

Point taken but new people may not be conversed in type problem bugs.
Old guys do know this of course.
ex:
int modeDelay = 40000;

10

Delta_G:
modeDelay can be an int. modeTime needs to be unsigned long because you're giving it a value from millis.

I have changed i to an unsigned long. However that doesn't solve anything - I didn't think it would since it won't roll over for a month or so.

LarryD:
unsigned long modeTime;

That is how I wrote it.

LarryD:
unsigned long modeDelay = 10000UL;

What is the UL for?

11

Tells (forces) the compiler the number is unsigned long as opposed to int

Will you explain a bit more what you see as a problem in the minimal failing sketch.
.

12

The UL isn't needed to assign an int size quantity to a long variable. It is completely superfluous in that context.

Where it IS useful is to force a calculation to be done with unsigned long numbers instead of ints where none of the individual numbers is too big for an int but the result of the calculation is.

unsigned long Val = 60ul * 1000 *1000;
13

Just hooked up your cctry.
The timing is right on 10 seconds between colour changes

Edit:
UL also reminds you what you are dealing with, but don't us it if it is bothersome.
As Delta_G said the math example keeps things correct.

14

Yes i know the sketch is correct, but something is causing my set-up to run fast

15

The sketch is OK
My UNO timing is 10 seconds between colour changes.

Where do you think the problem is ?

16

BTW
Show us a good image of your wiring.

.

17

I did a test where I print millis and the timer variable (modeTime) in serial monitor at 9600 baud. The serial monitor shows that modeTime is 10001 after it switches modes, which is what it should be. However, the actual time was about 9 seconds. Like I said, it slows down when printing to serial monitor. When I don't have it doing serial prints the time is even faster.

I can press the button to start the timer over. So I press my phone stopwatch and the button at the same time. I can see the mode change before it hits 10 seconds on my phone even though the serial monitor shows what I want to see.

18

I changed the timer interval to 20 seconds and the difference is now about two seconds. It changes modes at just 18 seconds.

19

Make this change, remove the serial print in loop()

void modeTimer() {

  if (millis()-modeTime >= modeDelay) {
    modeTime += modeDelay;
    stripState = (stripState + 1) % caseMax;
  Serial.println(millis());  //<<<<<<<<<<<<<<<<<<<<<<<<<
  }
}

This is what is displayed in the serial monitor, every 10000ms
10000
20000
30000
40000
50000
60000
70000

If the real time is not every 10 seconds your hardware is bad.

20

You do know the oscillator for the controller chip is not that accurate.
If you what accuracy, a RTC like a DS3231 is in order.

However, I am not seeing Serial affecting the time here and the colour change is every 10 seconds real time.