Still Frustrated With Using Millis Instead of Delay

I’ve searched and searched, posted in a few forums and I still can’t get this program to work. All I want to do is make a LED flash five times at slow rate, then flash five times at a faster rate. I can make the LED flash the way I want using delay, but as soon as it is converted to millis, the LED flashes continuously at the fast speed only. What is wrong with this method? Any help would be greatly appreciated.

// constants won’t change. Used here to set a pin number:
const int ledPin = LED_BUILTIN;// the number of the LED pin

// Variables will change:
int ledState = LOW; // ledState used to set the LED

// Generally, you should use “unsigned long” for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0; // will store last time LED was updated

// constants won’t change:
const long interval = 1000; // interval at which to blink (milliseconds)
const long intervalfast = 200;
void setup() {
// set the digital pin as output:
pinMode(ledPin, OUTPUT);
}

void loop() {

slow();

fast();
}

void slow(){
unsigned long currentMillis = millis();
for(int x=0; x<=4; x++){
if (currentMillis - previousMillis >= interval) {
//save the last time you blinked the LED

previousMillis = currentMillis;

// if the LED is off turn it on and vice-versa:
if (ledState == LOW) {
ledState = HIGH;
} else {
ledState = LOW;
}
digitalWrite(ledPin, ledState);
}
}
}
void fast(){
unsigned long currentMillis = millis();
for(int x=0; x<=4; x++){
if (currentMillis - previousMillis >= intervalfast) {
//save the last time you blinked the LED

previousMillis = currentMillis;

// if the LED is off turn it on and vice-versa:
if (ledState == LOW) {
ledState = HIGH;
} else {
ledState = LOW;
}
}
// set the LED with the ledState of the variable:
digitalWrite(ledPin, ledState);
}
}

// constants won't change. Used here to set a pin number:
const int ledPin =  LED_BUILTIN;// the number of the LED pin
bool BlinkFast = false;
int iBlinkTimes = 0;
// Variables will change:
int ledState = LOW;             // ledState used to set the LED

// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0;        // will store last time LED was updated

// constants won't change:
const long interval = 1000;           // interval at which to blink (milliseconds)
const long intervalfast = 200;
void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
}

void loop() {
  
If ( BlinkFast )
{
    slow();
   ++iBlinkTimes;
}

else
{
  
    fast();
++iBlinkTimes 
}

If ( iBlinkTimes == 5 )
{
iBlinkTimes = 0;
BlinkFast = !BlinkFast ;
}
}
  
void slow(){
  unsigned long currentMillis = millis();
  for(int x=0; x<=4; x++){
  if (currentMillis - previousMillis >= interval) {
   //save the last time you blinked the LED
    
    previousMillis = currentMillis;

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }
    digitalWrite(ledPin, ledState);
  }
}
}
void fast(){
  unsigned long currentMillis = millis();
  for(int x=0; x<=4; x++){
  if (currentMillis - previousMillis >= intervalfast) {
   //save the last time you blinked the LED
    
    previousMillis = currentMillis;

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }
  }
    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

I’m sure there is something wrong with what I wrote but your smart and you’ll figure it out. I hope the idea gets across.

Here’s a light re-write of your code OP. See if it makes sense. I wrote it this way to show that you can leave slow() or fast() while using millis() timing and do other things, then come back to them and do work if the timer has gone far enough.

// constants won't change. Used here to set a pin number:
const int ledPin =  LED_BUILTIN;// the number of the LED pin

// Variables will change:
int ledState = LOW;             // ledState used to set the LED

// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0;        // will store last time LED was updated

// constants won't change:
const long interval = 1000;           // interval at which to blink (milliseconds)
const long intervalfast = 200;
void setup() 
{
    // set the digital pin as output:
    pinMode(ledPin, OUTPUT);
}

void loop() 
{
    //this variable tells us which routine, fast() or slow() we'll run
    //it's initialized to 'false' so slow will run first 
    static bool
        bFastOrSlow = false;
        
    if( bFastOrSlow == false )
    {
        //our flag is false so we run slow()
        //i've modifed slow() so it returns a boolean (true or false)
        //if it's false, it means that slow is still counting down its flashes
        //when it's true, it means slow() is finished
        if( slow() == true )
        {
            //when slow is done, we set the flag so next time through we run fast()
            bFastOrSlow = true;
        }
    }
    else
    {
        //the flag is true so we run fast() now
        //fast also returns a bool so we know when it's done its flashes       
        if( fast() == true )
        {
            //when it's done we set the flag false so slow() runs again
            //and we keep cycling 
            bFastOrSlow = false;
            
        }//if
                    
    }//else
}
  
bool slow()
{
    //this variable counts the number of toggles of the LED state
    //it's "static" so it remains intact when we leave and has the same value
    //we left it at when we return, sort of like a global but this variable
    //is only visible within this function
    //
    static byte
        flashcount = 6;
    //this you already know; get the current millis counter    
    unsigned long currentMillis = millis();

    //if not enough time has passed for the "slow" blink, just return.
    //we return false here because we're not done flashing yet
    if( (currentMillis - previousMillis) < interval )
        return false;
    //again, you know: save this time for the next pass
    previousMillis = currentMillis;
         
    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) 
    {
        ledState = HIGH;
    }//if 
    else 
    {
        ledState = LOW;
    }//else
    
    digitalWrite(ledPin, ledState);

    //decrement the flash counter
    flashcount--;
    //when it hits zero, we're done blinking
    if( flashcount == 0 )
    {
        //reset the flash counter so when fast() is done
        //we start here at 6 again
        flashcount = 6;
        //this true tells loop() that slow() is done
        return true;
    }//if
    else
        //flashcount is not zero yet so return false
        return false;    

}//slow

//comments for slow apply here
//fast and slow differ only in the interval (interval for slow, intervalfast for fast)
bool fast()
{
    static byte
        flashcount = 6;
            
    unsigned long currentMillis = millis();

    if( (currentMillis - previousMillis) < intervalfast )
        return false;
    previousMillis = currentMillis;
         
    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) 
    {
        ledState = HIGH;
    }//if
    else 
    {
        ledState = LOW;
    }//else
    
    digitalWrite(ledPin, ledState);
    flashcount--;
    if( flashcount == 0 )
    {
        flashcount = 6;
        return true;
    }//if
    else
        return false;    
}//fast

Idahowalker,
Thanks for the code, it didn't work out of the box, but I will look it over tomorrow.

Blackfin,
Thanks for the code, it does work and I will try to understand it tomorrow.

I still don't why my code doesn't work as soon as I use millis instead of delay.

Chuck

I think if you "played computer" and carefully went through your for loop in either fast or slow functions you would see. Even if what you wrote worked the way you think it should, you are missing the point of using millis instead of delay.

Srsly, get out your pencil and step through your own code! Don't skip quickly over the part you think you understand, because you clearly do not yet - play it totally dumb, as we did when we read it for the first time and it will pop out and smack you like a damp trout.

Sometimes it's what you have to do.

a7

Loop constructs (e.g. for-loop) and millis() are not the best of friends :wink: There is also hardly a need for them as loop() already take care of the looping.

The principles of millis() based timings are that you look at the clock and do something when it's time. Your code does that; but it does that 5 times regardless if it had to do something or not.

If you look at Blackfin's code, you will see that it only increments (OK, he counts down instead of up, but that does not matter) the counter when a single flash (on and off) is completed.

/* 
    1 pushbutton, 1 LED
    1 pushbutton switch affecting 1 LED's blink rate

    pushbutton between "Pin 2" and GND
    uses "Pin 13" on-board LED
*/ 
//  pbmillis3Speeds_01
//  pb/input is accurate
//  works 2016.09.03
//
// if the pb is kept active then
// the speeds get cycled
// 

unsigned long markTime;
unsigned long elapsedTime;
unsigned long lockoutmark;  // pb-related
const unsigned long debounceTime = 250; // debounceTime

const unsigned long blinkSpeed [3] = {50,200,1000};
byte speedchoice;  // blinkSpeed[speedchoice]

boolean lockout;  // if true then PB is not checked
byte pbstate;
const byte pbpin = 2;

boolean ledstate;
const byte ledpin = 13;

void setup ()
{
  pinMode(pbpin,INPUT_PULLUP);
  pinMode(ledpin,OUTPUT);
  digitalWrite(ledpin,LOW);
  speedchoice = 0;
  lockout = false;
  markTime = millis();
}

void loop ()
{
  elapsedTime = millis();
  if(lockout == false)  // ok to read
  {
    pbstate = digitalRead(pbpin);
    if (pbstate == 0)  // pb made active
    {
      lockoutmark = millis();  // record millis when pb went LOW
      speedchoice++;
      lockout = true;
    }
    if(speedchoice > 2)
    {
      speedchoice = 0;
    }
    elapsedTime = millis();
  }
  
  elapsedTime = millis();
  if ((lockout == true) && ((elapsedTime - lockoutmark) >= debounceTime))
  {
    lockout = false; // lockout done, threshold met
  }
  
  elapsedTime = millis();
  if((elapsedTime - markTime) >= blinkSpeed[speedchoice]) //
  {
    markTime = millis();  // new mark
    ledstate = !ledstate;
    if (ledstate == true)
    {
      digitalWrite(ledpin,HIGH);
    }
    else
    {
      digitalWrite(ledpin,LOW);
    }
  }

}

Blackfin,

I've been able to adapt your program to run two stepper motors and now I'd like to put a one second delay between the fast and slow rates. I definitely don't have a clue how to do it. Could you give me a little help with that?

Chuck

Solved it myself.