Run a task until triggered to do otherwise?

I've been struggling in vain all day with the subject objective. It's my first attempt at using millis (apart from relatively simple tutorials) but I still can't get it working.

Before extending the approach to a more complex servo project I want to do the following in a simple LED sketch:
The pin 13 LED is blinking ON for 0.5s and OFF for 4.5s. Until a LOW on D6 triggers it to immediately go ON for 2s, and then return to the 5s cycling.

Would greatly appreciate learning why neither of the following two versions work please.

// LED is on for 500 ms every 5 s.
// If D6 goes low, LED goes on for 2000 ms then returns to the 5 s cycling
// BUT NOT WORKING. LED is always on.

unsigned long ledON = 500; // LED ON for 1 s, then OFF for 4 s
unsigned long ledOFF = 4000; // Total interval = 5s (1s on, 4s off)
unsigned long interval = 5000;
unsigned long previousMillis = 0; // millis() returns an unsigned long.
unsigned long currentMillis = 0;
int triggerPin = 6;



void setup()
{
  pinMode(13, OUTPUT);
  pinMode(triggerPin, INPUT_PULLUP);
  Serial.begin(115200);
  Serial.println("BlinkLED-MillisPlusInterrupt-2");
  Serial.print("interval = ");
  Serial.println(interval);
}


void loop()
{
  // Each time the loop() starts, get the current value of millis().
  unsigned long currentMillis = millis();
  // check if specified interval has passed
  if ((currentMillis - previousMillis) >= interval)
  {
    Serial.println("LED ON for 1 s");
    digitalWrite(13, HIGH);
  }
  // Check if another 1 s has elapsed
  if ((millis() - currentMillis) >= 1000)
  {
    // Now switch LED off
    digitalWrite(13, LOW);
    // save the "current" time, ready for next time through the loop.
    previousMillis = millis(); // UNSURE WHERE THIS GOES, TRIED SEVERAL!
  }
  if (triggerPin = LOW)
  {
    Serial.println("Perform the task triggered by D6 having just gone low");
    digitalWrite(13, HIGH);
    delay(2000);
    digitalWrite(13, LOW);
  }
}

Also tried this version:

// LED is on for 500 ms every 5 s.
// If D6 goes low, LED goes on for 2000 ms then returns to the 5 s cycling
// BUT NOT WORKING. LED is always on. Tx also blinking slowly.
// FWIW the yellow Tx LED is blinking on/off slowly

unsigned long ledON = 500; // LED ON for 1 s, then OFF for 4 s
unsigned long ledOFF = 4000; // Total interval = 5s (1s on, 4s off)
unsigned long interval = 5000;
unsigned long previousMillis = 0; // millis() returns an unsigned long.
unsigned long currentMillis = 0;
int triggerPin = 6;



void setup()
{
  pinMode(13, OUTPUT);
  pinMode(triggerPin, INPUT_PULLUP);
  Serial.begin(115200);
  Serial.println("BlinkLED-MillisPlusInterrupt-2");
  Serial.print("interval = ");
  Serial.println(interval);
}


void loop()
{
  // Each time the loop() starts, get the current value of millis().
  unsigned long currentMillis = millis();
  // Now check if specified interval has passed
  if ((currentMillis - previousMillis) >= interval)
  {
    Serial.println("LED ON for 1 s");
    digitalWrite(13, HIGH);
    // Check if another 1 s has elapsed
    if ((millis() - currentMillis) >= 1000)
    {
      // Now switch LED off
      digitalWrite(13, LOW);
    }
    // save the "current" time, ready for next time through the loop.
    previousMillis = millis(); // UNSURE WHERE THIS GOES, TRIED SEVERAL!
  }
  if (triggerPin = LOW)
  {
    Serial.println("Perform the task triggered by D6 having just gone low");
    digitalWrite(13, HIGH);
    delay(2000);
    digitalWrite(13, LOW);
  }
}

Here's the problem:

  if ((millis() - currentMillis) >= 1000)

You're comparing millis to currentMillis. A few microseconds before, you set currentMillis to be millis. There is no way that a second has passed since then, hence the LED will never be turned off.

What's the difference?

One of these if statements is correct, guess which one?

Assignment of a value to a variable

if (triggerPin = LOW)

Compare a value to a value.

if (triggerPin == LOW)

In both versions where is the line to check the state of the trigger pin?

See digitialRead() digitalRead() - Arduino Reference

If your code doesn't check the state of the pin then how does the code know if button has been pressed?

  unsigned long currentMillis = millis();
  // A few microseconds later:

  // Check if another 1 s has elapsed
  if ((millis() - currentMillis) >= 1000)
  {

That expression is never going to be true because 'millis()' has barely had time to increment once since you saved the value, let alone 1000 times.

You need a State Machine. You stay in a state until something causes you to leave. In your case, the states might be "BlinkOn", "BlinkOff", and "ButtonPressed".

@IdahoWalker.

Thanks, should have spotted that!

But with it corrected and all the millis stuff removed, it's still not recognising my low on D6.

// Aim:
// LED on for 500 ms every 5 s.
// If D6 goes low, LED goes on for 2000 ms then returns to the 5 s cycling
// BUT NOT WORKING.

unsigned long ledON = 500; // LED ON for 1 s, then OFF for 4 s
unsigned long ledOFF = 4000; // Total interval = 5s (1s on, 4s off)
unsigned long interval = 5000;
unsigned long previousMillis = 0; // millis() returns an unsigned long.
unsigned long currentMillis = 0;
unsigned long LEDgoesHigh;
int triggerPin = 6;

void setup()
{
  pinMode(13, OUTPUT);
  pinMode(triggerPin, INPUT_PULLUP);
  Serial.begin(115200);
  Serial.println("LEDwithMillisPlusTriggerD6");
  Serial.print("interval = ");
  Serial.println(interval);
}


void loop()
{
  if (triggerPin == LOW)
  {
    digitalWrite(13, HIGH);
    delay(2000);
    digitalWrite(13, LOW);
  }
}

@johnwasser, @wildbill,
Thanks both. Presumably my latest fails for similar reason?
Will study STATES and try implementing.

// LED is on for 500 ms every 5 s.
// If D6 goes low, LED goes on for 2000 ms then returns to the 5 s cycling
// BUT NOT WORKING. LED is always on. Tx also blinking slowly.
// FWIW the yellow Tx LED is blinking on/off slowly

unsigned long ledON = 500; // LED ON for 1 s, then OFF for 4 s
unsigned long ledOFF = 4000; // Total interval = 5s (1s on, 4s off)
unsigned long interval = 5000;
unsigned long previousMillis = 0; // millis() returns an unsigned long.
unsigned long currentMillis = 0;
int triggerPin = 6;



void setup()
{
  pinMode(13, OUTPUT);
  pinMode(triggerPin, INPUT_PULLUP);
  Serial.begin(115200);
  Serial.println("LEDwithMillisPlusTriggerD6-2edit");
  Serial.print("interval = ");
  Serial.println(interval);
}


void loop()
{
  // Each time the loop() starts, get the current value of millis().
  unsigned long currentMillis = millis();
  // Now check if specified interval has passed
  if ((currentMillis - previousMillis) >= interval)
  {
    Serial.println("LED ON for 1 s");
    digitalWrite(13, HIGH);
     unsigned long LEDgoesHigh = millis();
    // Check if another 1 s has elapsed
    if ((millis() - LEDgoesHigh) >= 1000)
    {
      // Now switch LED off
      digitalWrite(13, LOW);
    }
    // save the "current" time, ready for next time through the loop.
    previousMillis = millis(); // UNSURE WHERE THIS GOES, TRIED SEVERAL!
  }
  if (triggerPin == LOW)
  {
    Serial.println("Perform the task triggered by D6 having just gone low");
    digitalWrite(13, HIGH);
    delay(2000);
    digitalWrite(13, LOW);
  }
}

How fast is this code running

[color=#222222]// Aim:[/color][color=#222222][/color]
[color=#222222]// LED on for 500 ms every 5 s.[/color][color=#222222][/color]
[color=#222222]// If D6 goes low, LED goes on for 2000 ms then returns to the 5 s cycling[/color][color=#222222][/color]
[color=#222222]// BUT NOT WORKING.[/color][color=#222222][/color]
[color=#222222][/color]
[color=#222222]unsigned long ledON = 500; // LED ON for 1 s, then OFF for 4 s[/color][color=#222222][/color]
[color=#222222]unsigned long ledOFF = 4000; // Total interval = 5s (1s on, 4s off)[/color][color=#222222][/color]
[color=#222222]unsigned long interval = 5000;[/color][color=#222222][/color]
[color=#222222]unsigned long previousMillis = 0; // millis() returns an unsigned long.[/color][color=#222222][/color]
[color=#222222]unsigned long currentMillis = 0;[/color][color=#222222][/color]
[color=#222222]unsigned long LEDgoesHigh;[/color][color=#222222][/color]
[color=#222222]int triggerPin = 6;[/color][color=#222222][/color]
[color=#222222][/color]
[color=#222222]void setup()[/color][color=#222222][/color]
[color=#222222]{[/color][color=#222222][/color]
[color=#222222]  pinMode(13, OUTPUT);[/color][color=#222222][/color]
[color=#222222]  pinMode(triggerPin, INPUT_PULLUP);[/color][color=#222222][/color]
[color=#222222]  Serial.begin(115200);[/color][color=#222222][/color]
[color=#222222]  Serial.println("LEDwithMillisPlusTriggerD6");[/color][color=#222222][/color]
[color=#222222]  Serial.print("interval = ");[/color][color=#222222][/color]
[color=#222222]  Serial.println(interval);[/color][color=#222222][/color]
[color=#222222]}[/color][color=#222222][/color]
[color=#222222][/color]
[color=#222222][/color]
[color=#222222]void loop()[/color][color=#222222][/color]
[color=#222222]{[/color]
[color=#222222][/color]
[color=#222222]  if (triggerPin == LOW)[/color][color=#222222][/color]
[color=#222222]  {[/color][color=#222222][/color]
[color=#222222]    digitalWrite(13, HIGH);[/color][color=#222222][/color]
[color=#222222]    delay(2000);[/color][color=#222222][/color]
[color=#222222]    digitalWrite(13, LOW);[/color][color=#222222][/color]
[color=#222222]  }[/color][color=#222222][/color]
[color=#222222]}

Obviously you did not read what digitalRead() does or you did not apply what you read about that digitalRead() does.

if digitalRead(triggerpin)==low then barf.

HOW WILL YOU KNOW WHEN D6 has went low when you do NOT read D6?

This will never be more than 1000:

     unsigned long LEDgoesHigh = millis();
    // Check if another 1 s has elapsed
    if ((millis() - LEDgoesHigh) >= 1000)

Idahowalker:
if digitalRead(triggerpin)==low then barf.

HOW WILL YOU KNOW WHEN D6 has went low when you do NOT read D6?

Thanks. Another elementary error. Duly fixed so at least now getting a result on taking it low.
BTW, I’m curious about all those colour tags in your previous reply?

The colour tags are some problem with the forums (or perhaps the IDE). It’s a recent issue.

Terrypin:
Thanks.

You're welcome.

Here's an example that does what you're looking for (I think.) It doesn't use a true FSM but does track LED states and modes and looks for a change of state on the trigger pin (rather than its level.)

See if it makes sense to you:

// LED is on for 500 ms every 5 s.
// If D6 goes low, LED goes on for 2000 ms then returns to the 5 s cycling

const uint8_t ledPin = LED_BUILTIN;
const uint8_t triggerPin = 6;
//
const uint32_t  ALT_STROBE_LEN = 2000ul;        //mS    LED on time when trigger transitions to low
const uint32_t  NORM_ON_TIME = 500ul;           //mS    LED on time in "normal" mode
const uint32_t  NORM_OFF_TIME = 4500ul;         //mS    LED off time in "normal" mode
const uint32_t  TRIG_READ_INTERVAL = 50ul;      //mS    time between trigger pin checks
//

#define NORMAL      true                        //normal operating mode: 0.5 / 4.5 on/off flashing
#define ALTERNATE   false                       //alternate mode: 2-sec strobe

uint32_t
    timeReadTrigger;
uint8_t
    trigNow,
    trigLast;
    
void setup()
{
    pinMode(ledPin, OUTPUT);
    pinMode(triggerPin, INPUT_PULLUP);
    Serial.begin(115200);
    Serial.println("LEDwithMillisPlusTriggerD6-2edit");
    
    trigLast = digitalRead( triggerPin );
    timeReadTrigger = millis();
    
}//setup


void loop()
{
    //default mode to NORMAL blip
    bool flashMode = NORMAL;

    //no need to read the trigger pin every few microseconds
    //so we read it every 50mS
    uint32_t timeNow = millis();
    if( timeNow - timeReadTrigger >= TRIG_READ_INTERVAL )
    {
        timeReadTrigger = timeNow;
        //read the state of the pin now        
        trigNow = digitalRead( triggerPin );
        //if not the same as last time it has changed state
        if( trigNow != trigLast )
        {
            //save this new state
            trigLast = trigNow;
            //if low now, we saw a high-to-low transition (button press)
            if( trigNow == LOW )
                //in that case, select alternate strobe mode
                flashMode = ALTERNATE;
            
        }//if

    }//if

    //pass mode to LED control
    LEDControl( flashMode );
    
}//loop

void LEDControl( bool mode )
{
    static bool
        bLEDState = false;
    static uint32_t
        timeDelay,
        timeLED;
    uint32_t
        timeNow = millis();

    //if mode is ALT...
    if( mode == ALTERNATE )
    {
        //...turn on the LED and set the delay time
        digitalWrite( ledPin, HIGH );                
        timeDelay = ALT_STROBE_LEN;
        timeLED = timeNow;
        //next time through, mode will be NORMAL and the blip mode will
        //resume when the ALT_STROBE_LEN delay expires
        bLEDState = true;                
        
    }//if
    else 
    {
        //has the delay time expired?
        if( (timeNow - timeLED) >= timeDelay )            
        {
            //yes; save time now for next timing check
            timeLED = timeNow;
            
            if( bLEDState == false )
            {
                //LED is off now; turn it on
                digitalWrite( ledPin, HIGH );
                //for 500mS                
                timeDelay = NORM_ON_TIME;
                //indicate LED is on
                bLEDState = true;
                
            }//if
            else
            {
                //LED is on now; turn it off
                digitalWrite( ledPin, LOW );
                //for 4.5-seconds
                timeDelay = NORM_OFF_TIME;
                //and indicate it off
                bLEDState = false;
                
            }//else
            
        }//if
        
    }//if
                                
}//LEDControl

ToddL1962:
This will never be more than 1000:

     unsigned long LEDgoesHigh = millis();

// Check if another 1 s has elapsed
   if ((millis() - LEDgoesHigh) >= 1000)

Thanks, understood. Try as I might, I keep forgetting that these commands do NOT stop at the end of my typed program (or repeat at my request), but continue indefinitely at high speed!

Blackfin:
Here's an example that does what you're looking for (I think.) It doesn't use a true FSM but does track LED states and modes and looks for a change of state on the trigger pin (rather than its level.)

See if it makes sense to you:

Thanks a bunch @Blackfin, really appreciate the time it must have taken you to write that. It works perfectly. But I'm afraid I'll use it only as a last resort as so much of it is above my know-how level.

I'm an experienced electronics hobbyist, starting decades ago in the TTL and CMOS eras. But not a programmer, and no micro-controller experience until starting Arduino enthusiastically last year. Although I make heavy use of copy/paste I do like to grasp most of the stuff I plagiarise. I confess I even had to reach for the search tools on your very first command to learn that const uint8_t means what I've been writing as const byte.
And then another hurdle a few lines later,

#define NORMAL      true                        //normal operating mode: 0.5 / 4.5 on/off flashing

with its apparent introduction of 'modes', prompted the decision to just run it, not try to understand it!

With apologies for digressing: this isn't the first time that I've been tempted to give up on coding what seems to me a fairly trivial electronics project and just do it with a few components. The actual task (for which the LED sketch is a simplified analogy) is to repeatedly take a photo by pressing a camera button once with a servo every 170 seconds to keep it powered up beyond its 180 second automatic power down. So that when triggered overnight by one of several sensors the same servo will deliver a sequence of movements that will take a 40s video.

I have each of these tasks working separately with appropriate sketches, but I have not yet been able to combine them. Your sketch, with major editing, would do that. A few hours in my shed workshop would do so too. Meanwhile, video of the occasional fox and badger has been achieved with a sketch that powers up the camera when triggered, followed by the servo commands and delays, and then power down. Very intuitive - but with drawbacks that prompted the 'keep-alive' method under discussion.

There are a number of programming concepts that baffle beginners. Once you reach enlightenment, it's quite hard to understand why you were ever baffled at all.

Pointers are the classic example, FSMs are another. It looks like use of millis is a third. Stick with it - it'll click eventually :slight_smile:

Here is how I would implement your sketch as a simple Finite State Machine:

// Example Simple Finite State Machine
//
// The pin 13 LED is blinking ON for 0.5s and OFF for 4.5s.
// Until a LOW on D6 triggers it to immediately go ON for 2s, and then return to the 5s cycling.


// The states we can be in:
enum States {BLINK_ON, BLINK_OFF, BUTTON_PRESSED} State = BLINK_ON;


const unsigned long BLINK_ON_INTERVAL = 500;
const unsigned long BLINK_OFF_INTERVAL = 4500;
const unsigned long BUTTON_PRESSED_INTERVAL = 2000;


const byte Button_IPPin = 6;  // INPUT_PULLUP pin for the button
// Note:  LED_BUILTIN is already defined by the IDE


unsigned long StateStartTime;  // The time in millis() when the current state was entered.


void setup()
{
  Serial.begin(115200);


  pinMode(Button_IPPin, INPUT_PULLUP);
  pinMode(LED_BUILTIN, OUTPUT);


  // Starting in BLINK_ON state
  State = BLINK_ON;
  digitalWrite(LED_BUILTIN, HIGH); // ON
  StateStartTime = millis();
}


void loop()
{
  unsigned long currentTime = millis();


  switch (State)
  {
    case BLINK_ON:
      // Is the interval over?
      if (currentTime - StateStartTime >= BLINK_ON_INTERVAL)
      {
        State = BLINK_OFF;
        Serial.println("Off");
        digitalWrite(LED_BUILTIN, LOW); // OFF
        StateStartTime = currentTime;
      }


      // Has the button been pressed?
      if (digitalRead(Button_IPPin) == LOW)
      {
        State = BUTTON_PRESSED;
        Serial.println("Pressed");
        digitalWrite(LED_BUILTIN, HIGH); // ON
        StateStartTime = currentTime;
      }
      break;


    case BLINK_OFF:
      // Is the interval over?
      if (currentTime - StateStartTime >= BLINK_OFF_INTERVAL)
      {
        State = BLINK_ON;
        Serial.println("On");
        digitalWrite(LED_BUILTIN, HIGH); // ON
        StateStartTime = currentTime;
      }


      // Has the button been pressed?
      if (digitalRead(Button_IPPin) == LOW)
      {
        State = BUTTON_PRESSED;
        Serial.println("Pressed");
        digitalWrite(LED_BUILTIN, HIGH); // ON
        StateStartTime = currentTime;
      }
      break;


    case BUTTON_PRESSED:
      // Is the interval over?
      if (currentTime - StateStartTime >= BUTTON_PRESSED_INTERVAL)
      {
        State = BLINK_OFF;
        Serial.println("Off");
        digitalWrite(LED_BUILTIN, LOW); // OFF
        StateStartTime = currentTime;
      }
      break;
  } // End switch(State)
}

When your FSM gets more complex it becomes too cumbersome to cram everything into one ‘switch’ statement. It is also a problem that the set-up for each state has to be in the previous state (s). For example, setting the LED on for the BUTTON_PRESSED state was in both BLINK_ON and BLINK_OFF states. To make the states more self-contained we can put each in a function and have an argument to tell the state if it is entering, running, or exiting.

This sketch does the same thing as the previous sketch, but with each state in a separate function:

// Example Complex Finite State Machine
//
// The pin 13 LED is blinking ON for 0.5s and OFF for 4.5s.
// Until a LOW on D6 triggers it to immediately go ON for 2s, and then return to the 5s cycling.


// The states we can be in:
enum FSMStates {BLINK_ON, BLINK_OFF, BUTTON_PRESSED} State;


enum FSMStages {ENTERING, RUNNING, EXITING};
void BlinkOn(enum FSMStages stage);
void BlinkOff(enum FSMStages stage);
void ButtonPressed(enum FSMStages stage);
void (*FSMStateFunctions[])(FSMStages stage) = {BlinkOn, BlinkOff, ButtonPressed};


void (*CurrentState)(FSMStages stage);


const unsigned long BLINK_ON_INTERVAL = 500;
const unsigned long BLINK_OFF_INTERVAL = 4500;
const unsigned long BUTTON_PRESSED_INTERVAL = 2000;


const byte Button_IPPin = 6;  // INPUT_PULLUP pin for the button


void setup()
{
  Serial.begin(115200);


  pinMode(Button_IPPin, INPUT_PULLUP);
  pinMode(LED_BUILTIN, OUTPUT);


  // Starting in BLINK_ON state
  FSMEnterState(BLINK_ON);
}


void loop()
{
  (*CurrentState)(FSMStages::RUNNING);
}




void BlinkOn(enum FSMStages stage)
{
  static unsigned long startTime = 0;
  unsigned long currentTime = millis();
  switch (stage)
  {
    case FSMStages::ENTERING:
      Serial.println("On");
      digitalWrite(LED_BUILTIN, HIGH); // ON
      startTime = currentTime;
      break;


    case FSMStages::RUNNING:
      // Is the interval over?
      if (currentTime - startTime >= BLINK_ON_INTERVAL)
      {
        FSMChangeState(BLINK_OFF);
      }


      // Has the button been pressed?
      if (digitalRead(Button_IPPin) == LOW)
      {
        FSMChangeState(BUTTON_PRESSED);
      }
      break;


    case FSMStages::EXITING:
      break;
  }
}




void BlinkOff(enum FSMStages stage)
{
  static unsigned long startTime = 0;
  unsigned long currentTime = millis();
  switch (stage)
  {
    case FSMStages::ENTERING:
      Serial.println("Off");
      digitalWrite(LED_BUILTIN, LOW); // OFF
      startTime = currentTime;
      break;


    case FSMStages::RUNNING:
      // Is the interval over?
      if (currentTime - startTime >= BLINK_OFF_INTERVAL)
      {
        FSMChangeState(BLINK_ON);
      }


      // Has the button been pressed?
      if (digitalRead(Button_IPPin) == LOW)
      {
        FSMChangeState(BUTTON_PRESSED);
      }
      break;


    case FSMStages::EXITING:
      break;
  }
}


void ButtonPressed(enum FSMStages stage)
{
  static unsigned long startTime = 0;
  unsigned long currentTime = millis();


  switch (stage)
  {
    case FSMStages::ENTERING:
      Serial.println("Pressed");
      digitalWrite(LED_BUILTIN, HIGH); // ON
      startTime = currentTime;
      break;


    case FSMStages::RUNNING:
      // Is the interval over?
      if (currentTime - startTime >= BUTTON_PRESSED_INTERVAL)
      {
        FSMChangeState(BLINK_OFF);
      }
      break;


    case FSMStages::EXITING:
      break;
  }
}


// Enter a new state
void FSMEnterState(enum FSMStates state)
{
  CurrentState = FSMStateFunctions[state];
  (*CurrentState)(FSMStages::ENTERING);
}


// Exit current state
void FSMExitState()
{
  (*CurrentState)(FSMStages::EXITING);
}


void FSMChangeState(enum FSMStates state)
{
  FSMExitState();  // Exit current state
  FSMEnterState(state);  // 
}

Note: With this setup, there can be only one FSM because it uses the global “CurrentState”. Writing the FSM as an object would allow multiple instances.

Thank you. I'll sleep on it and get stuck in again tomorrow.