3

I am using millis() to flash some lights (neopixels) on an arduino pro mini. The problem is that I cannot flash the lights for the desired period. More specifically, the lights flash for ~250 ms, whereas I want to flash for 9 seconds! Here is a simplified version of what I am doing.

In loop(), I have a while loop (let's call it loop 1) that should run for 250 ms. Inside loop 1, I call a function flash(), that has to run for 9 seconds! I put another loop (loop 2) after loop 1, that would run for 3 seconds.

Here is the bigger picture...I actually want a radio to wake up for 250 ms. If in that time interval it receives anything, flash() will be called that will flash the lights for 9 secs. If it doesn't receive anything it will simply go to sleep for 3 secs, then come back alive for 250 ms, so on and so on

void loop() { 
    previousMillis = millis();
    while (millis() - previousMillis < awake_interval) {  //awake_interval = 250 ms    
        flash();  //flash for 9 seconds
    }

    previousMillis = millis();
    while (millis() - previousMillis < sleep_interval);  //sleep_interval = 3 secs
}

The flash() function looks like this,

void flash() {
    unsigned long x = millis();
    while(millis() - x < flash_time){  //flash_time = 9 secs 
        lights_on();
        delay(t1);
        lights_off(); 
        delay(t2); 
    }
}

I put loop 2 in there because without it the lights flash forever. This makes sense because all the code is in an infinite loop - void loop().

For debugging, I put serial prints after entering the loops. It seems that nesting two loops (in my case flash() inside loop 1), both of which uses millis() is the source of the problem. If that is true, how else should I do this? If that is not the problem, any ideas will be appreciated.

Edit: The suggestions made by @VE7JRO and @ratchet freak are neat but I was still having problems making my thing work. Turns out it was a problem with one my initializations. I am posting my whole code below (that uses LEDs). I guess I should post the correct code as an answer.

const long sleep_interval = 3000; 
const long awake_interval = 250; 
const int ledPin = 3;

const uint8_t flash_time = 9000;  //bug 
const uint16_t t1 = 5; 
const uint16_t t2 = 495;

void flash() {  
    unsigned long x = millis();
    while(millis() - x < flash_time){ 
        digitalWrite(ledPin, HIGH);
        delay(t1);
        digitalWrite(ledPin, LOW);
        delay(t2);
    }   
}   

void setup() {
    Serial.begin(57600);  
    pinMode(ledPin, OUTPUT);
}

void loop() {
    unsigned long x = millis();
    while(millis() - x < awake_interval){
        Serial.println("flash time!");
        flash();  //overflow will occur    
    }

    unsigned long y = millis();; 
    while (millis() - y < sleep_interval) {  //sleep_interval = 3 secs
        Serial.println("Sleep time!");
    }
}
3
  • 3
    Post your full code.
    – gre_gor
    Oct 5, 2017 at 16:25
  • your 'while (millis() - previousMillis < sleep_interval);' is nothing else like delay(sleep_interval). rewrite your code to one main loop, executing lights_on() and lights_off() on 'timeline schedule'
    – Juraj
    Oct 5, 2017 at 16:52
  • @Juraj I don't understand what you mean by "timeline schedule". And I cannot use delay, because it's blocking, and this won't go with the rest of the (bigger) program
    – Tahseen
    Oct 5, 2017 at 17:24

5 Answers 5

2

I was wrong being too confident about my whole code. When people suggested posting my whole code, I should have just done it!!!

Anyway, here is the mistake I made. I initialized the variable flash_time as an 8-bit integer. Of course, 8 bit can only hold up to 255. I am trying to store 9000!!! The following code has this bug fixed.

const long sleep_interval = 3000; 
const long awake_interval = 250; 
const int ledPin = 3;

const uint16_t flash_time = 9000;  //bug fixed by using uint16_t instead of uint8_t
const uint16_t t1 = 5; 
const uint16_t t2 = 495;

void flash() {  
    unsigned long x = millis();
    while(millis() - x < flash_time){ 
        digitalWrite(ledPin, HIGH);
        delay(t1);
        digitalWrite(ledPin, LOW);
        delay(t2);
    }   
}   

void setup() {
    Serial.begin(57600);  
    pinMode(ledPin, OUTPUT);
}

void loop() {
    unsigned long x = millis();
    while(millis() - x < awake_interval){
        Serial.println("flash time!");
        flash();    
    }

    unsigned long y = millis();; 
    while (millis() - y < sleep_interval) {  //sleep_interval = 3 secs
        Serial.println("Sleep time!");
    }
}

So, nested loops that use millis() to determine the terminating conditions work just fine, even if loop 1 runs for 250 ms and loop 2 (that's inside loop 1) runs for much longer, like 9 seconds.

Apologies if I have wasted too much of people's time.

1

Your logic is wrong. If you update previousMillis with millis() value in the first line, you are always subtracting the time difference between the the first and second lines of your code (which is the same you are doing in the loop inside flash()). If what you need is to get the time passed since the program started running, you need to have a fixed variable for that in setup() instead of updating it every time in loop().

Even so, I don't see why millis() is necessary in your case. Using delay() would be much more simple and still get the job done. Example:

void setup() {
delay(250);
}

void loop(){

    for(int i = 0; i < 9; i++){
        digitalWrite(LED, HIGH);
        delay(time);
        digitalWrite(LED, LOW);
        delay(time);

        delay(1000 - 2*time); //Delays the loop exactly one second per iteraction
    }

}
4
  • I cannot use delay, because it's blocking, and this won't go with the rest of the (bigger) program. Could you please expand a bit more on your idea of putting a fixed variable in setup()? My while loops are trying to determine the period of time to stay inside.
    – Tahseen
    Oct 5, 2017 at 17:27
  • 1
    @Tahseen: You wrote “I cannot use delay” and yet you are using it in flash(). So what’s the deal? Oct 5, 2017 at 18:20
  • Exactly... I didn't understand what you mean by "it's blocking", what's the reason you can't use it?
    – amorimph
    Oct 5, 2017 at 18:25
  • I used it in flash() to have the effect of lights turning on and off. But if I use it in the actual while loop, other things that I hope to incorporate in the future cannot work. For example, I want to use a radio that wakes up for 250 ms, sees if it has any command, then goes to sleep for 3 secs. If it received a command in that 250 ms interval, the arduino will flash the lights for 9 secs
    – Tahseen
    Oct 5, 2017 at 19:04
1

Use a state machine instead:

bool  flashing;
unsigned long lastflashingChange;

void loop() { 
    unsigned long currentMillis = millis;
    if(flashing){
        flash();  //update flash state and change IO pin
        if (lastflashingChange - currentMillis >= awake_interval) {  //awake_interval = 250 ms    
            flahsing = false;
            //turn off the led
            lastflashingChange = currentMillis;
        }
    } else {
        if (lastflashingChange - currentMillis >= sleep_interval) {  //awake_interval = 250 ms    
            flashing = true; 
            //possible initialize flash state
            lastflashingChange = currentMillis;
        }
    }
}

And a similar bit of code inside flash()

bool flashOn;
unsigned long lastFlashLightChange;
void flash() {
    unsigned long x = millis();

    if(flashOn){
        if (lastFlashLightChange - x >= t1) {
            flashOn = false;
            lights_off();
            lastFlashLightChange = currentMillis;
        }
    } else {
        if (lastFlashLightChange - x >= t2) {
            flashOn = true;
            lights_on();
            lastFlashLightChange = currentMillis;
        }
    }

}
4
  • This one looks really good...going to try it. But the line previousMillis = millis(); in your first block of code isn't really necessary, right?
    – Tahseen
    Oct 5, 2017 at 19:23
  • @Tahseen no it isn't necessary. Oct 5, 2017 at 19:27
  • This is smart but it will not flash the lights for 9 seconds, but only 250 ms (that is awake_interval). Here is the bigger picture...I actually want a radio to wake up for 250 ms. If in that time interval it receives anything, flash() will be called that will flash the lights for 9 secs. If it doesn't receive anything it will simply go to sleep for 3 secs, then come back alive for 250 ms, so on and so on
    – Tahseen
    Oct 5, 2017 at 20:02
  • @Tahseen you can choose other ways to change state than just the time having elapsed. Just change the condition in the if. You can also have more states, change the bool to an enum and change the if/else to a switch. Oct 5, 2017 at 20:08
1

Here is a sketch to get you started. It doesn't use delay or any libraries. I don't have any NeoPixel devices so I used the Arduino built in LED for testing purposes. Just remove the "myTimer4" function and related code/variables to stop the blinking LED. I don't know what type of "radio" you are using so I used a character inputed in the serial monitor to simulate a radio signal being received. There are several serial print statements throughout the code for testing purposes as well.

const unsigned long delayTime = 3000;
const unsigned long delayTime2 = 250;
const unsigned long delayTime3 = 9000;
const unsigned long delayTime4 = 500;
unsigned long previousMillis = 0;
unsigned long previousMillis2 = 0;
unsigned long previousMillis3 = 0;
unsigned long previousMillis4 = 0;
byte checkForData = 0;
byte dataReceived = 0;
byte flashLED = 0;
byte doOnce = 0;

void setup(){
  Serial.begin(9600);
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, LOW);
}

void loop(){

  // Get current time.
  unsigned long currentMillis = millis();

  // First event.
  if(checkForData == 0){
    if(myTimer1(delayTime, currentMillis) == 1){
      Serial.println("3 second timer done");
      checkForData = 1;
      previousMillis2 = currentMillis;
    }
  }

  // Second event.
  if(checkForData == 1){

    // Your radio code here to check for data.

    // I'm simulating data input from your device using the serial monitor.
    if(Serial.read() > 0){dataReceived = 1;}

    if(myTimer2(delayTime2, currentMillis) == 1){
      Serial.println("250 ms timer done");
      checkForData = 0;
      previousMillis = currentMillis;
      if(dataReceived == 1){
        flashLED = 1;
        previousMillis3 = currentMillis;
        dataReceived = 0;
      }
    }
  }

  // Third event.
  if(flashLED == 1){
    if(doOnce == 0){
      flash(1);
      doOnce = 1;
    }
    if(myTimer3(delayTime3, currentMillis) == 1){
      Serial.println("9 second timer done");
      flashLED = 0;
      flash(0);
      doOnce = 0;
      digitalWrite(LED_BUILTIN, LOW);
    }
  }

  // Fourth event. 
  if(myTimer4(delayTime4, currentMillis) == 1 && flashLED == 1){
    //Serial.println("500 ms timer done");
    digitalWrite(LED_BUILTIN, !digitalRead(LED_BUILTIN));
    previousMillis4 = currentMillis;
  }

}

// Function to send data to the NeoPixel device.
void flash(byte startStop){
  if(startStop == 1){Serial.println("flash function started");}
  if(startStop == 0){Serial.println("flash function ended");}  
}

// First event timer.
byte myTimer1(unsigned long delayTime, unsigned long currentMillis){
  if(currentMillis - previousMillis >= delayTime){previousMillis = currentMillis;return 1;}
  else{return 0;}
}

// Second event timer.
byte myTimer2(unsigned long delayTime2, unsigned long currentMillis){
  if(currentMillis - previousMillis2 >= delayTime2){previousMillis2 = currentMillis;return 1;}
  else{return 0;}
}

// Third event timer.
byte myTimer3(unsigned long delayTime3, unsigned long currentMillis){
  if(currentMillis - previousMillis3 >= delayTime3){previousMillis3 = currentMillis;return 1;}
  else{return 0;}
}

// Fourth event timer.
byte myTimer4(unsigned long delayTime4, unsigned long currentMillis){
  if(currentMillis - previousMillis4 >= delayTime4){previousMillis4 = currentMillis;return 1;}
  else{return 0;}
}
0

the lights flash for ~250 ms, whereas I want to flash for 9 seconds!

that logic doesn't make sense. Essentially you want to eat a 2 hour meal within 5 seconds.

your code reflects that kind of logic: when the execution exits flash(), it must also exit the while loop in loop().

so think exactly what you want to do, write it down and then code to it.

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.