Led On for Timed Period - using millis - SOLVED

Hi Guys

If I fade up an led from off to, say, a level of 100 and then digitalWrite it high (or analogWrite it to 255), how do I then keep it on for a set period and then switch it off? As its planned at a later date to perform other tasks as well, I want to use millis, not delay.

I have been experimenting with this seemingly simple concept for ages, but have not been able to get it working properly. The nearest I have been able to get my sketch is below. I've used elements from the bwd sketch:

const byte switchPin = 2;
const int ledPin = 3;
byte oldSwitchState = HIGH;
const unsigned long debounceTime = 10;
unsigned long switchPressTime = 0;
boolean swPressed = false;
int brightness = 0;
int fadeAmount = 5;
boolean fadeFlag = false;
unsigned long currentTime;
unsigned long loopTime;
unsigned long previousMillis = 0;
const long interval = 100;

void setup() {
  pinMode (ledPin, OUTPUT);
  pinMode (switchPin, INPUT_PULLUP);
  currentTime = millis();
  loopTime = currentTime;
}

void loop() {
  switchCall();
  if (swPressed == true) {
    fadeFlag = true;
    rampUp();
    flashLed();
  }
}

void switchCall () {	//if switch operated, make swPressed true
  // see if switch is open or closed
  byte switchState = digitalRead (switchPin);
  // has it changed since last time?
  if (switchState != oldSwitchState)
  {
    // debounce
    if (millis () - switchPressTime >= debounceTime)
    {
      switchPressTime = millis ();  // when we closed the switch
      oldSwitchState =  switchState;  // remember for next time
      if (switchState == LOW)
      {
        swPressed = true;
      }  // end if switchState is LOW
      else
      {
        swPressed = false;
      }  // end if switchState is HIGH
    }  // end if debounce time up
  }  // end of state change
}

void rampUp () {
  while (fadeFlag == true) {
    currentTime = millis();
    if (currentTime >= (loopTime + 20)) {
      analogWrite (ledPin, brightness);
      brightness = (brightness + fadeAmount); // fadeup the led
      if (brightness < 100)
        fadeFlag = true;                      // still below 100?
      else {
        fadeFlag = false;                     // above 100 so reset flag and switch led on full
        brightness = 0;
        digitalWrite (ledPin, HIGH);
      }
      loopTime = currentTime;
    }
  }
}

// Here follows the problem function.....
void flashLed () {             // Keep led on for 100ms
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis >= interval) { // which it will be initially
    previousMillis = currentMillis;
    digitalWrite(ledPin, LOW);               // this is obviously in the wrong place??
  }
}

However, this code causes the led to stay on following the fade up.

It does seem to me that the line if (currentMillis - previousMillis >= interval) in the flashLed function will always initially return true because previousMillis is 0 to start with, but then when setting previousMillis = currentMillis, it will switch the led off as well. I can't seem to get my head around this to work out how to keep the led on for the set period (100ms in this example).

I've kept each function of the led process separate for now so as to keep track on what I'm doing!

Now, I'm perfectly aware that its not the most concise code, and I do realise that it almost certainly has some areas that can be improved, but its the timing of the led on period that I'm hoping you can help with. Any other (constructive) criticism is welcome (in fact expected!), but please not at the expense of helping with the problem in hand, as its only some first draft code currently. I can address the elegance of it later once it works!

TIA
Bernie

If you call flashLed() when previousMillis = 0 it will immediately turn the LED off.

If you want the LED to stay ON for interval you need to set previousMillis = millis() immediately before you call the function or at the start of the function. BUT you must not keep updating previousMillis within loop().

This might work

void flashLed () {             // Keep led on for 100ms
  static boolean firstRun = true;
  static unsigned long previousMillis;
  unsigned long currentMillis = millis();
  if (firstRun == true) {
      previousMillis = currentMillis;
  }
  if (currentMillis - previousMillis >= interval) { // which it will be initially
    previousMillis = currentMillis;
    digitalWrite(ledPin, LOW);               // this is obviously in the wrong place??
    firstRun = false;
  }
}

...R

Many Thanks Robin2 :slight_smile:

I can't try this out until I can get to the hardware at home, but if I run the modified sketch in UnoArduSim, it still shows the led remaining permanently on after fadeup.

However, as UnoArduSim also insists on explicitly initialising ALL the variables, including the millis related ones, I don't trust its result!

I'll let you know how it goes in the real world later!

Thanks Again
Bernie

Hmm, I've substituted the revised flashLed function but just like with the ardusim, the led stays on rather than going off after 100ms.

I tried moving the unsigned long currentMillis = millis(); from the function to just before the function call, but its the same result.
Here's the amended sketch, in case I missed something. Its exactly the same as before, but with the new flashLed function.

const byte switchPin = 2;
const int ledPin = 3;
byte oldSwitchState = HIGH;
const unsigned long debounceTime = 10;
unsigned long switchPressTime = 0;
boolean swPressed = false;
int brightness = 0;
int fadeAmount = 5;
boolean fadeFlag = false;
unsigned long currentTime;
unsigned long loopTime;
unsigned long previousMillis = 0;
const long interval = 100;

void setup() {
  pinMode (ledPin, OUTPUT);
  pinMode (switchPin, INPUT_PULLUP);
  currentTime = millis();
  loopTime = currentTime;
}

void loop() {
  switchCall();
  if (swPressed == true) {
    fadeFlag = true;
    rampUp();
    flashLed();
  }
}

void switchCall () {	//if switch operated, make swPressed true
  // see if switch is open or closed
  byte switchState = digitalRead (switchPin);
  // has it changed since last time?
  if (switchState != oldSwitchState)
  {
    // debounce
    if (millis () - switchPressTime >= debounceTime)
    {
      switchPressTime = millis ();  // when we closed the switch
      oldSwitchState =  switchState;  // remember for next time
      if (switchState == LOW)
      {
        swPressed = true;
      }  // end if switchState is LOW
      else
      {
        swPressed = false;
      }  // end if switchState is HIGH
    }  // end if debounce time up
  }  // end of state change
}

void rampUp () {
  while (fadeFlag == true) {
    currentTime = millis();
    if (currentTime >= (loopTime + 20)) {
      analogWrite (ledPin, brightness);
      brightness = (brightness + fadeAmount); // fadeup the led
      if (brightness < 100)
        fadeFlag = true;                      // still below 100?
      else {
        fadeFlag = false;                     // above 100 so reset flag and switch led on full
        brightness = 0;
        digitalWrite (ledPin, HIGH);
      }
      loopTime = currentTime;
    }
  }
}

// Here follows the problem function.....
void flashLed () {             // Keep led on for 100ms
  static boolean firstRun = true;
  static unsigned long previousMillis;
  unsigned long currentMillis = millis();
  if (firstRun == true) {
    previousMillis = currentMillis;
  }
  if (currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;
    digitalWrite(ledPin, LOW);
    firstRun = false;
  }
}

I can understand Robin2's suggestion, I just don't see why it doesn't work.

Bernie

void flashLed () {             // Keep led on for 100ms
  static boolean firstRun = true;
  static unsigned long previousMillis;
  unsigned long currentMillis = millis();
  if (firstRun == true) {
    previousMillis = currentMillis;
  }
  if (currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;
    digitalWrite(ledPin, LOW);
    firstRun = false;
  }
}

How will firstRun ever be set to false?

currentMillis and previousMillis will always be the same until firstRun is false - but firstRun can only be set false if currentMillis is some amount greater than previousMillis, which can't happen as long as firstRun is true....

Thanks @DrAzzy, I had it upside-down. I think this is how it should be. Just as well I said "this might work"

void flashLed () {             // Keep led on for 100ms
  static boolean firstRun = true;
  static unsigned long previousMillis;
  unsigned long currentMillis = millis();
  if (firstRun == true) {
    previousMillis = currentMillis;
    firstRun = false;
  }
  if (currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;
    digitalWrite(ledPin, LOW);
    firstRun = true;
  }
}

...R

This gets even stranger now!

Many thanks guys, but after changing the function to DrAzzy's suggestion, the led still stays on until a second push of the switch when it will extinguish then immediately ramp up to what looks to be the brightness = 100 level and then extinguish completely without ever coming on full. Subsequent switch presses repeat the same sequence.

I'm also suspecting my switch operating function is faulty now as it was supposed to trigger on state change, but it seemingly is now repeating itself if held down.

I must admit that I was following the reasoning, but this doesn't seem to make sense now!

Bernie

your code in the main loop is not designed to handle a button being held down so once the pattern is run then it will be run again.

your first problem I see when testing is

brightness = 0;

the code is not the problem but the location is as the code was not designed to deal with someone holding the button down

once its moved further down then it works

notice the large delay I added for troubleshooting. This quickly pointed out that the main loop is constantly repeating. (sometimes when working with very small timers its hard to see whats really happening so adding some delays you will delete later can help)

void flashLed () {             // Keep led on for 100ms
  static boolean firstRun = true;
  static unsigned long previousMillis;
  unsigned long currentMillis = millis();
  if (firstRun == true) {
    previousMillis = currentMillis;
    firstRun = false;
  }
  if (currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;
    digitalWrite(ledPin, LOW);
    firstRun = true;
    brightness = 0;
    delay(2000);
  }
}

I think you would do better starting with a plan that includes all the options for user input.

if you want the led to do its thing when the button is pushed you will need to block the button from being read until the led has finished unless the plan calls for the led to do something else in that situation. What should happen when the button is released

Well, I stuck at it and despite some setbacks just as I thought it was cracked, and I have something resembling success at last :smiley:

I took the great suggestions by Robin2, DrAzzy and gpop1 and stirred them together with a big dollop of Nick Gammon's debounce method and after letting it all cook in my head for a while came up with some working code - I learnt some useful lessons doing this, so many thanks and kudos to you guys!

I've noticed that the effect is similar to a photon torpedo effect from Star Trek, so it might even be useful for some scifi modellers, so here's what it all looks like now:

// ************************************
//LED fade Up and Flash with out Delay
// ************************************

// switchCall allocations
const byte switchPin = 2;
boolean switchFlag = false;            // switch not triggered
byte oldSwitchState = HIGH;            // assume switch open because of pull-up resistor
const unsigned long debounceTime = 10; // milliseconds debounce setting
unsigned long switchPressTime;         // when the switch last changed state
boolean reader = true;                 // switch blocker (switch routine not to be called during led flash)

// rampUp allocations
const int ledPin = 3;
boolean activate = false;               // ramp up not active
unsigned long currentTime;              // for tracking time
unsigned long loopTime;                 // during ramp up
int brightness = 0;                     // brightness during ramp up
int fadeAmount = 1;                     // increments to ramp up by

// flashLed allocations
const int interval = 150;               // 150ms flash time
static unsigned long previousMillis;    // flash time counter
 static boolean firstRun = true;        // start of flash process

void setup () {
  pinMode (switchPin, INPUT_PULLUP);
  pinMode (ledPin, OUTPUT);
}  // end of setup

void loop ( ) {
  switchCall ();
  rampUp ();
  flashLed ();
}  // end of loop

void flashLed () {
  unsigned long currentMillis = millis(); 
  if (firstRun == true) {               // If start of flash
    previousMillis = currentMillis;     // advance timer
    firstRun = false;                   // flash now in progress
    reader = false;                     // set switch blocking flag
  }
  if (currentMillis - previousMillis >= interval) { // If led has been HIGH for enough time
    previousMillis = currentMillis;     // advance timer
    digitalWrite(ledPin, LOW);          // switch off led
    firstRun = true;                    // reset flash process indicator
    brightness = 0;                     // reset ready for next rampUp
    reader = true;                      // unblock switch
  }
}

void rampUp () {
  if (switchFlag == true)               // Has switch been pressed?
    activate = true; {                  // ramp up underway
    while (activate == true) { 
      currentTime = millis();
      if (currentTime >= (loopTime + 20)) {     // while rampup underway, increment time
        analogWrite (ledPin, brightness);       // write current brightness
        brightness = (brightness + fadeAmount); // fadeup the led
        if (brightness < 50)
          activate = true;              // still below 50?
        else {
          activate = false;             // above 50 so reset flag and switch led on full
          digitalWrite (ledPin, HIGH);
        }
        loopTime = currentTime;         // update time
      }
    }
  }
  activate = false;                     // process complete so reset flag
}

void switchCall () {
  if (reader == true) {                 // Has previous FlashLed activation completed?
    byte switchState = digitalRead (switchPin);  // see if switch is open or closed
    if (switchState != oldSwitchState) { // has it changed since last time?
      if (millis () - switchPressTime >= debounceTime) { // debounce
        switchPressTime = millis ();    // when we closed the switch
        oldSwitchState =  switchState;  // remember for next time
        if (switchState == LOW)
          switchFlag = true;            // end if switchState is LOW
        else
          switchFlag = false;           // end if switchState is HIGH
      }                                 // end if debounce time up
    }                                   // end of state change
  }                                     // end of function
}

Live long and prosper :smiley: :smiley:

Bernie