Blink without delay not working as expected?

Hi,
I am sending signals to my arduino from my phone (via bluetooth). I can get it to turn an LED on and off - sweet. I try to make it blink without using the delay function and it just turns on and stays on. I can still turn it on or off but I can't get it to blink. So my question is, why is this not blinking and how can I make it blink? I think it's because once it goes through the blinking function it never goes back again, right? I tried changing the code so that it held onto the last command instead of resetting it but that didn't work. Thanks heaps!
Troublesome section:

void redSides() {
    int redSidesLedState = LOW;             // ledState used to set the LED
    long redSidesPreviousMillis = 0;        // will store last time LED was updated
    unsigned long redSidesCurrentMillis = millis();  //Put the result fromt the millis() function into a variable
    
    if(redSidesCurrentMillis - redSidesPreviousMillis > redSidesBlinkRate) {  //Check if it is time to blink
    redSidesPreviousMillis = redSidesCurrentMillis;  //Store the last blink time of the light

    if (redSidesLedState == LOW) //Turn the light on if it's off, or vice versa
      redSidesLedState = HIGH;
    else
      redSidesLedState = LOW;

    digitalWrite(redSidesPin, redSidesLedState);  //Write the state to the light
}
}
  
//---------------------------- Our main "loop" ------------------------------------//
void loop() {
  while (Serial.available()){  //Is there a serial byte that can be read
  delay(10); //To make things stable
  char c = Serial.read(); //Attempt to read whatever came through via serial
  if (c == '#') {break;} //Break out of the loop after #
  command += c; //The same as: command = command + c
  }
  if (command.length() > 0) { //If the command is longer than 0 characters print it
    Serial.println(command);
  }
    
   //Commands for controlling the redSides lights
  if(command == "o") digitalWrite(redSidesPin, HIGH);
  else if(command == "f") digitalWrite(redSidesPin, LOW);
  else if(command == "b") redSides();
  
  //Commands for controlling the redTop lights
  else if(command == "*brake light on") redTop("on");
  else if(command == "*brake light off") redTop("off");
  else if(command == "*brake light blink") redTop("blink");
  
  //Commands for controlling the left indicator lights
  else if(command == "*left indicator on") yellowLeft("on");
  else if(command == "*left indicator off") yellowLeft("off");
  else if(command == "*left indicator blink") yellowLeft("blink");
  
  //Commands for controlling the right indicator lights
  else if(command == "*right indicator on") yellowRight("on");
  else if(command == "*right indicator off") yellowRight("off");
  else if(command == "*right indicator blink") yellowRight("blink");
  
  //Commands for controlling the buzzer
  else if(command == "*buzzer on" || "buzzer full on") buzzerFunc("on", 100);
  else if(command == "*buzzer 75 percent on" || "*buzzer 3 quarters on") buzzerFunc("on", 75);
  else if(command == "*buzzer half on" || "*buzzer 50 percent on") buzzerFunc("on", 100);
  else if(command == "*buzzer 25 percent on" || "*buzzer a quarter on" || "*buzzer one quarter on") buzzerFunc("on", 75);
  else if(command == "*buzzer off") buzzerFunc("off", 0);
  else if(command == "*buzzer blink") buzzerFunc("blink", 100);
  else if(command == "*buzzer blink 75 percent" || "*buzzer blink 3 quarters") buzzerFunc("blink", 75);
  else if(command == "*buzzer blink half" || "*buzzer blink 50 percent") buzzerFunc("on", 100);
  else if(command == "*buzzer blink 25 percent" || "*buzzer blink a quarter" || "*buzzer blink one quarter") buzzerFunc("blink", 25);
  
  //-----------------------------------------------------------------------// 
  command = ""; //Reset the variable to nothing after we are finished using it

}

It wouldn't fit in the other post so here is my full code (when I get this to work I will replicate it across the rest of my code):
Also, I removed some of the irrelevant functions because it still wouldn't fit.

//------------------------------ Define Stuff --------------------------------------// 
String command; //defines our string that will hold the command
int redSidesPin = 13; //Connects to the two constantly on, red tail lights transistor
int redTopPin = 6; //Connects to the top  only on when braking, transistor
int yellowLeftPin = 8; //Connects to the left indicator transistor
int yellowRightPin = 9; //Connects to the right indicator transistor
int buzzerPin = 10; //Connects to the buzzer
long redSidesBlinkRate = 500;  //Blink rate
long redTopBlinkRate = 1000;  //Blink rate
long yellowLeftBlinkRate = 1000;  //Blink rate
long yellowRightBlinkRate = 1000;  //Blink rate
long buzzerBlinkRate = 1000;  //Blink rate



//------------------------ Our many functions ------------------------------------------------------------------------------------//
//--------------------------- Our setup function ----------------------------------// 
void setup() {
  Serial.begin(9600); //Initialise the serial connection at a baud rate 9600
  pinMode(redSidesPin, OUTPUT);  //Set this pin as an output
  pinMode(redTopPin, OUTPUT);  //Set this pin as an output
  pinMode(yellowLeftPin, OUTPUT);  //Set this pin as an output
  pinMode(yellowRightPin, OUTPUT);  //Set this pin as an output
  pinMode(buzzerPin, OUTPUT);  //Set this pin as an output
  
}

void redSides() {
    int redSidesLedState = LOW;             // ledState used to set the LED
    long redSidesPreviousMillis = 0;        // will store last time LED was updated
    unsigned long redSidesCurrentMillis = millis();  //Put the result fromt the millis() function into a variable
    
    if(redSidesCurrentMillis - redSidesPreviousMillis > redSidesBlinkRate) {  //Check if it is time to blink
    redSidesPreviousMillis = redSidesCurrentMillis;  //Store the last blink time of the light

    if (redSidesLedState == LOW) //Turn the light on if it's off, or vice versa
      redSidesLedState = HIGH;
    else
      redSidesLedState = LOW;

    digitalWrite(redSidesPin, redSidesLedState);  //Write the state to the light
}
}
  
//---------------------------- Our main "loop" ------------------------------------//
void loop() {
  while (Serial.available()){  //Is there a serial byte that can be read
  delay(10); //To make things stable
  char c = Serial.read(); //Attempt to read whatever came through via serial
  if (c == '#') {break;} //Break out of the loop after #
  command += c; //The same as: command = command + c
  }
  if (command.length() > 0) { //If the command is longer than 0 characters print it
    Serial.println(command);
  }
    
   //Commands for controlling the redSides lights
  if(command == "o") digitalWrite(redSidesPin, HIGH);
  else if(command == "f") digitalWrite(redSidesPin, LOW);
  else if(command == "b") redSides();
  
  //Commands for controlling the redTop lights
  else if(command == "*brake light on") redTop("on");
  else if(command == "*brake light off") redTop("off");
  else if(command == "*brake light blink") redTop("blink");
  
  //Commands for controlling the left indicator lights
  else if(command == "*left indicator on") yellowLeft("on");
  else if(command == "*left indicator off") yellowLeft("off");
  else if(command == "*left indicator blink") yellowLeft("blink");
  
  //Commands for controlling the right indicator lights
  else if(command == "*right indicator on") yellowRight("on");
  else if(command == "*right indicator off") yellowRight("off");
  else if(command == "*right indicator blink") yellowRight("blink");
  
  //Commands for controlling the buzzer
  else if(command == "*buzzer on" || "buzzer full on") buzzerFunc("on", 100);
  else if(command == "*buzzer 75 percent on" || "*buzzer 3 quarters on") buzzerFunc("on", 75);
  else if(command == "*buzzer half on" || "*buzzer 50 percent on") buzzerFunc("on", 100);
  else if(command == "*buzzer 25 percent on" || "*buzzer a quarter on" || "*buzzer one quarter on") buzzerFunc("on", 75);
  else if(command == "*buzzer off") buzzerFunc("off", 0);
  else if(command == "*buzzer blink") buzzerFunc("blink", 100);
  else if(command == "*buzzer blink 75 percent" || "*buzzer blink 3 quarters") buzzerFunc("blink", 75);
  else if(command == "*buzzer blink half" || "*buzzer blink 50 percent") buzzerFunc("on", 100);
  else if(command == "*buzzer blink 25 percent" || "*buzzer blink a quarter" || "*buzzer blink one quarter") buzzerFunc("blink", 25);
  
  //-----------------------------------------------------------------------// 
  command = ""; //Reset the variable to nothing after we are finished using it

}

Try...

static unsigned long redSidesPreviousMillis = 0;

Hi, thanks for the suggestion but it still just stays on whenever I send it a 'b' over Bluetooth. I don't understand why it isn't flashing... I have virtually copied this code from blink without delay example and have just added the function name to front of everything (except millis() ). On another note the arduino keeps randomly resetting - on two different ones, cold it be the code?

Here is my latest code (still not working) but I have changed everything around so now they all at leat turn on and off correctly. The only thing wrong with this codes is the blink functions do not work.

EDIT:
For some reason the code did not show up?? Anyway here it is again, I uploaded the .ino because it was to big to put in here: https://drive.google.com/file/d/0BwqZI88ckbEYZDF4R0dWRmo1R3c/edit?usp=sharing

Here is my latest code

???

Thanks for pointing that out- the link didn't show for some reason... Anyway, I edited it and inserted it back in there as it was to big to put all of the code in a post for some reason. Here is the link to the .ino ; https://drive.google.com/file/d/0BwqZI88ckbEYZDF4R0dWRmo1R3c/edit?usp=sharing

I don't know very well (and I don't went see the code of the link), but I'll guess that the line:

    int redSidesLedState = LOW;             // ledState used to set the LED

it's the problem. If all the times that the program enters this function it puts the state as LOW, when he changes it (a few lines down) it will changes for HIGH (all the times).

jabelone:
Troublesome section:

void redSides() {
       long redSidesPreviousMillis = 0;        // will store last time LED was updated

Every time you call redSides() you're resetting this to 0, which makes nonsense of:

if(redSidesCurrentMillis - redSidesPreviousMillis > redSidesBlinkRate) {  //Check if it is time to blink
    redSidesPreviousMillis = redSidesCurrentMillis;  //Store the last blink time of the light

Put that line before setup() (make it a global). Better yet would be to make it an unsigned long (as you should for all time variables) and make it equal to millis(), not zero, so that the first delay is consistent.

OK, thanks. I have just tried it and it still didn't work... Also, depending on user input, up to five of these functions could potentially be running (flashing) at once. In this case it is preferred to have them blinking at the same time. In other situations though it would not be favourable. I have played around with the code and still can't get it to work. I moved all of the (now)
unsigned long redSidesPreviousMillis = millis(); to the top outside of any function. Thanks for that suggestion.

unsigned long redSidesPreviousMillis;

is outside the function, but

redSidesPreviousMillis = millis();

is inside, right?

BTW,

int redSidesLedState = LOW;

is outside too, right?

void redSides() {
int redSidesLedState = LOW;
long redSidesPreviousMillis = 0;

The declaration of these two variables is one part of your problem. These are automatic variables by default. This means that they are allocated on the stack and do not persist between function calls. It also means that they are initialized each the the function is called. Thus redSidesLedState is always LOW and redSidesPreviousMillis is always 0 when the function is called. Any changes to these variables are lost when the function returns. You clearly need these variables values to persist between function calls. The way to do that is use the static keyword in the declaration. This will allocate the variables on the stack and the initialization happens at startup rather than each time the function is called.

Also since millis() returns unsigned long, redSidesPreviousMillis also needs to be unsigned long.

The other part of your problem is that the led will only blink (change state) when you call redsides(). And redsides() is only called when you recieve a b command. If you want the led to blink you need to call redsides() each time loop() is called. Of course you now need to account for the commanded state of the led, you don't want it to blink if it has been commanded on or off.

Hey guys, I ended up finding a library that can toggle a pin high/low at a set interval!! I got everything working exactly how I originally imagined it!! Thank you so much for your help along the way, I really appreciate it. :slight_smile: I think this is probably the most code I have ever written. Here is the library: Google Code Archive - Long-term storage for Google Code Project Hosting.

Here is my fully working, complete code:

/*
Wireless bike trailer lights by Jaimyn Mayer (known as jabelone online) (C) 2014
You may modify and re-use this code for anything other than commercial use as
long as this copyright notice is not removed or modifed.
*/

//-------------------------------- Define Stuff ----------------------------------------// 
String command; //defines our string that will hold the command
int redSidesPin = 13; //Connects to the two constantly on, red tail lights transistor
int redTopPin = 12; //Connects to the top  only on when braking, transistor
int yellowLeftPin = 10; //Connects to the left indicator transistor
int yellowRightPin = 8; //Connects to the right indicator transistor
int buzzerPin = 6; //Connects to the buzzer

long redSidesBlinkRate = 500; //Blink rates (speeds) in milli seconds
long redTopBlinkRate = 500;
long yellowLeftBlinkRate = 500;
long yellowRightBlinkRate = 500;
long buzzerBlinkRate = 500;

//----------Booleans variables for our blinking functions------------------------------//
//False (off) by default.  If you want something blinking by default, untill you turn it
//off, then change that one to true.
boolean redSidesBoolean = false;
boolean redTopBoolean = false;
boolean yellowLeftBoolean = false;
boolean yellowRightBoolean = false;
boolean buzzerBoolean = false;

//------------------ All of our blinking library setup code ----------------------------//
#include <Timing.h>
SimpleBlink redSidesBlink = SimpleBlink(redSidesBlinkRate);
SimpleBlink redTopBlink = SimpleBlink(redTopBlinkRate);
SimpleBlink yellowLeftBlink = SimpleBlink(yellowLeftBlinkRate);
SimpleBlink yellowRightBlink = SimpleBlink(yellowRightBlinkRate);
SimpleBlink buzzerBlink = SimpleBlink(buzzerBlinkRate);

//------------------------ Our many functions ------------------------------------------------------------------------------------//
//------------------------------ Our setup function -------------------------------------// 
void setup() {
  Serial.begin(9600); //Initialise the serial connection at a baud rate 9600
  pinMode(redSidesPin, OUTPUT);  //Set these pins as outputs
  pinMode(redTopPin, OUTPUT);
  pinMode(yellowLeftPin, OUTPUT);
  pinMode(yellowRightPin, OUTPUT);
  pinMode(buzzerPin, OUTPUT);
  
  //These pins are being used as ground to make wiring easier - not reccomended though!
  pinMode(11, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(7,OUTPUT);
  pinMode(5,OUTPUT);
  digitalWrite(11, LOW);
  digitalWrite(9, LOW);
  digitalWrite(7, LOW);
  digitalWrite(5, LOW);
    
  //Let us know by flashing all the lights 4 times that it has run the setup() function
  //This only happens at startup or when a reset happens
  allon();
  delay(100);
  alloff();
  delay(100);
  allon();
  delay(100);
  alloff();
  delay(100);
  allon();
  delay(100);
  alloff();
  delay(100);
  allon();
  delay(100);
  alloff();
}

//-------------------------------------------------------- Our main "loop" ------------------------------------------------------------------------//
void loop() {
  while (Serial.available()){  //Is there a serial byte that can be read
  delay(10); //To make things stable
  char c = Serial.read(); //Attempt to read whatever came through via serial
  if (c == '#') {break;} //Break out of the loop after #
  command += c; //The same as: command = command + c
  }
  if (command.length() > 0) { //If the command is longer than 0 characters print it
    Serial.println(command);
  }
  if (redSidesBoolean == true) digitalWrite(redSidesPin, redSidesBlink.check());
  if (redTopBoolean == true) digitalWrite(redTopPin, redTopBlink.check());
  if (yellowLeftBoolean == true) digitalWrite(yellowLeftPin, yellowLeftBlink.check());
  if (yellowRightBoolean == true) digitalWrite(yellowRightPin, yellowRightBlink.check());
  if (buzzerBoolean == true) digitalWrite(buzzerPin, buzzerBlink.check());
  
  //All off or on
  if(command == "*all on" || command == "*everything on") allon();
  else if(command == "*all off" || command == "*everything off") alloff();
  
  //redSidesLights
  else if(command == "*backlight on" || command == "*back light on") redSidesON();
  else if(command == "*back light off" || command == "*backlight off" || command == "*backlight of" || command == "*back light of") redSidesOFF();
  else if(command == "*back light blink" || command == "*backlight blink") redSidesBoolean = true;
  
  //redTop lights
  else if(command == "*brake light on") redTopON();
  else if(command == "*brake light off") redTopOFF();
  else if(command == "*brake light blink") redTopBoolean = true;
  
  //yellowLeft lights
  else if(command == "*left indicator on" || command == "*left blinker on") yellowLeftON();
  else if(command == "*left indicator off" || command == "*left indicator of" || command == "*left blinker off" || command == "*left blinker of") yellowLeftOFF();
  else if(command == "*left indicator blink") yellowLeftBoolean = true;
  
  //yellowRight lights
  else if(command == "*right indicator on" || command == "*right blinker on") yellowRightON();
  else if(command == "*right indicator off" || command == "*right indicator of" || command == "*right blinker off" || command == "*right blinker of") yellowRightOFF();
  else if(command == "*right indicator blink") yellowRightBoolean = true;
  
  //buzzer Lights
  else if(command == "*buzzer on" || command == "*buzzer full on") yellowLeftON();
  else if(command == "*buzzer off") buzzerOFF();
  else if(command == "*buzzer blink") buzzerBoolean = true;
  
  //Blink all lights
  else if(command == "*blink all" || command == "*blink all lights" || command == "*all blink" || command == "*all lights blink") allblink();
  
  command = ""; //Reset the variable to nothing after we have finished using it
}

//---------- Functions to control the state of everything ---------//
//---------Turn on all lights---------//
void allon() {
redSidesON();
redTopON();
yellowLeftON();
yellowRightON();
buzzerON();
}

//--------Turn off all lights---------//
void alloff() {
redSidesOFF();
redTopOFF();
yellowLeftOFF();
yellowRightOFF();
buzzerOFF();
}

//-----Blink all lights (hazard)-----//
void allblink() {
redSidesBoolean = true;
redTopBoolean = true;
yellowLeftBoolean = true;
yellowRightBoolean = true;
buzzerBoolean = true;
}

//-------Turns on redSidesPin --------//
void redSidesON() {
  redSidesBoolean = false; 
   digitalWrite(redSidesPin, HIGH);
}

//-------Turns off redSidesPin --------//
void redSidesOFF() {
  redSidesBoolean = false; 
   digitalWrite(redSidesPin, LOW);
}

//-------Turns on redTopPin --------//
void redTopON() {
  redTopBoolean = false; 
   digitalWrite(redTopPin, HIGH);
}

//-------Turns off redTopPin --------//
void redTopOFF() {
  redTopBoolean = false; 
   digitalWrite(redTopPin, LOW);
}

//-------Turns on yellowLeftPin --------//
void yellowLeftON() {
  yellowLeftBoolean = false; 
   digitalWrite(yellowLeftPin, HIGH);
}

//-------Turns off yellowLeftPin --------//
void yellowLeftOFF() {
  yellowLeftBoolean = false; 
   digitalWrite(yellowLeftPin, LOW);
}


//-------Turns on yellowRightPin --------//
void yellowRightON() {
  yellowRightBoolean = false; 
   digitalWrite(yellowRightPin, HIGH);
}

//-------Turns off yellowRightPin --------//
void yellowRightOFF() {
  yellowRightBoolean = false; 
   digitalWrite(yellowRightPin, LOW);
}

//-------Turns on buzzerPin --------//
void buzzerON() {
  buzzerBoolean = false; 
   digitalWrite(buzzerPin, HIGH);
}

//-------Turns off buzzerPin --------//
void buzzerOFF() {
  buzzerBoolean = false; 
   digitalWrite(buzzerPin, LOW);
}

Aside from the issues pointed out already with your redSides() function, there is also the fact that you only call the function once, when new serial data arrives, and the serial data is a 'b'.

You need(ed) to separate the reading and parsing of serial data from the handling of the last command received.

As it is, you've used a library that you don't understand to accomplish a relatively simple goal. That is NOT the way to learn.

Hi Paul,
I know that is not the way to learn but it is part of a school assignment that is due very soon so I just needed a quick fix. We have to design and manufacture a two piece article. Most people are doing a simple wooden band saw box or a pair of wooden salt and pepper shakers. Me, being me, decided to make something overly complicated. We only had 5 weeks to completely design, manufacture and evaluate and that ends this week. :slight_smile:

My design is some wireless indicator lights and a way to parse a potentiometer reading to a trailer attached to my bicycle for controlling a motor. :wink: I have a controller with buttons etc on it (piece 1) and a receiver (piece 2) with transistors etc for driving the LED strips and motor controller. I managed to get the milling machine at school working and have just made a perfectly good PCB on it as part of this project also. If you want, I can send you the design folio if you are interested. :slight_smile:

During the school holidays, which is in about 3 weeks, I will have time to go through and rewrite the code and learn how to do it properly. I kind of realised that I was only calling the function once and tried a few different ways to keep calling it but none worked and I needed to move on to the assembly phase. Also, thanks for the advice about separating the reading and parsing of serial data I will remember that in a few weeks. I will also be doing an instructable as well.

Again, thanks heaps to everyone who helped! :slight_smile:

jabelone:
During the school holidays, which is in about 3 weeks, I will have time to go through and rewrite the code and learn how to do it properly.

Since it does what you set out to do, it's not incorrect. But wow you're going to be amazed how simple this can be done once you grasp blink without delay.

Very much looking forward to seeing the bike trailer in action.
Geoff