Timing trouble with millis() for LED control

Hello everyone!
I am new around here and new to Arduino. I thought it would be fun to learn by designing a project and working to completion. I didn’t realize how much trouble my little project would give me.

I am making a small marquee sign in the shape of an arrow. I have been working to program in different light patterns that I can change with the press of a button. Like all beginners, I started using delay(). It took me a while to realize that the delay() was what was causing intermittent and unpredictable input from my button. I then discovered millis() which should allow me to replace delay(). I sketched out some code and got the button working on a single LED, which I can then implement into my larger sketch. I am now trying to re-sketch my light patterns (avoiding delay() ) and I am having problems.

I don’t think I fully understand how millis() works. I have read multiple threads here, as well as several other pages that explain it and watched handfuls of YouTube videos. I have spent hours playing with the “blink without delay” and “several things at the same time” sketches, but it does not seem to be sinking in. For the life of me, I cannot figure out how to code the same patterns I had while using delay(). I don’t understand how millis() can program in a “pause” between actions.

For instance, I have a sequential lighting of the LEDs and then they sequentially turn off. This was easy using delay(). Here is a section of that code:

/*
This pattern will:
Sequentially light 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 one at a time
Sequentially light 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 
Stay on for 1.5 seconds
Sequentially turn off 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
Repeat
*/

//Declarations
int ledPins[] = {2, 3, 4, 5, 6, 7, 8, 9, 10, 11}; //Sets up output pin array
int pinCount = 10;                                //Declares number of pins in array
int timer = 500;

void setup(){
 //Sets pins 2 through 11 as output pins
  for (int thisPin = 0; thisPin < pinCount; thisPin++)
    {pinMode(ledPins[thisPin], OUTPUT);}
    delay(timer);}

void loop() {
 // loop from the lowest pin to the highest:
  for (int thisPin = 0; thisPin < pinCount; thisPin++) { 
    // turn the pin on:
    digitalWrite(ledPins[thisPin], HIGH);   
    delay(timer);                  
    // turn the pin off:
    digitalWrite(ledPins[thisPin], LOW);
  delay(timer / 10);}
 delay(timer);
 
 // Sequential light
  for (int thisPin = 0; thisPin < pinCount; thisPin++) { 
    // turn the pin on:
    digitalWrite(ledPins[thisPin], HIGH);   
    delay(timer / 5);}
 delay(timer * 3);

 // Sequential off
  for (int thisPin = 0; thisPin < pinCount; thisPin++) { 
    // turn the pin on:
    digitalWrite(ledPins[thisPin], LOW);   
    delay(timer / 5);}
 delay(timer);

}

I tried to recreate this pattern using millis() and failed miserably. Tried again, failed again. Tried again... you get the picture. Frustrated, I searched for ways to do it not using millis(). I found a library called elapsedmillis (Arduino Playground - elapsedMillis Library) that I thought would help me solve the problem, but I can’t seem to get it to work either. It gives me random lighting of the LEDs, not in the order that I have them listed in the sketch and I do not understand why. On top of that, if I change the interval, the LEDs light in a different random pattern. This piece of code has been simplified to just 5 LEDs to see if I could get it to function on a smaller scale, but I am currently having issues with it (here I am just trying to turn the LEDs on sequentially and leave them on):

#include <elapsedMillis.h>


void setup() {
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);     
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(12, OUTPUT);
  pinMode(13, OUTPUT);
  Serial.begin(9600);
}

elapsedMillis elapsedTime;

unsigned int interval = 250;

void loop() {

  if (elapsedTime >= interval) {
      digitalWrite(13, HIGH);
      Serial.println(elapsedTime);
      elapsedTime = 0;
    }
  
  if (elapsedTime >= interval) {
      digitalWrite(12, HIGH);
      Serial.println(elapsedTime);
      elapsedTime = 0;
    }

  if (elapsedTime >= interval) {
      digitalWrite(11, HIGH);
      Serial.println(elapsedTime);
      elapsedTime = 0;
    }
    
  if (elapsedTime >= interval) {
      digitalWrite(10, HIGH);
      Serial.println(elapsedTime);
      elapsedTime = 0;
    }
    
  if (elapsedTime >= interval) {
      digitalWrite(9, HIGH);
      Serial.println(elapsedTime);
      elapsedTime = 0;
    }
}

This code makes the LEDs light in a random order, but the serial.println(elapsedTime); will almost always print out the 250 milliseconds that the timer makes. Occasionally it will throw out a 251.

I have gotten frustrated and my brain seems to have stopped making any sense of this at all. I would like a fresh set of eyes to look at it and maybe help me understand why I cannot get it to work like I want it to.

I am fairly sure that my problem is with the code because my breadboard works perfectly with delay(). But, just in case, here is a render of how my breadboard and Arduino are connected.

I think I am on the verge of completing my first real Arduino project if I can just rebuild my light patterns, I can then implement them with my other state change code with the working button and assemble the final product.

In all honesty, I have worked on this for over a year on and off. After several months of not touching it, I have broken it all out again in the hopes that I can finally complete the project.

Any guidance that you can give will be greatly appreciated. And, I would love to show off the final project when done. Thank you for taking the time to read this, and for any help you can throw my way. It is all very much appreciated.

You may be having problems because you are using a library rather than trying to use the BWD technique directly.

Here is an example that compares a blocking and non blocking for the same task:

// program to sequentially turn on and turn off 3 LEDs
 
const byte LED1 = 2;
const byte LED2 = 3;
const byte LED3 = 4;
const byte heartBeat = 13;
 
boolean blockingFlag = true;
 
unsigned long Millis13;
unsigned long lastMillis;
unsigned long wait = 3000;
 
enum States {
  StateStart, State2, State3, State4, State5, State6, State7
};
States mState = StateStart;
 
void setup()
{
  pinMode(heartBeat, OUTPUT);
 
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
 
  delay(3000);
 
} //END of setup()
 
 
void loop()
{
  //**********************************
  //some code to check for blocking sketches
  if (millis() - Millis13 >= 200)
  {
    digitalWrite(heartBeat, !digitalRead(heartBeat)); //toggle D13
    Millis13 = millis(); //re-initalize
  }
 
  //********************************** delay() being used here
  if (blockingFlag == true)
  {
    blockingFlag = false;
    digitalWrite(13, LOW);
 
    blockingMethod();
   }
 
  //********************************** BWD being used here
  if (millis() - lastMillis >= wait)
  {
    nonBlockingMethod();
  }
 
  //Other non blocking code goes here
 
} //END of loop()
 
 
 
 
// ******************* Functions *******************
void blockingMethod()
{
  //LEDs ON
  digitalWrite(LED3, HIGH);    // turn on LED3
  delay(450);                  // wait for 450ms
  digitalWrite(LED2, HIGH);    // turn on LED2
  delay(450);                  // wait for 450ms
  digitalWrite(LED1, HIGH);    // turn on LED1
  delay(3000);                 // wait for 3000ms
 
  //LEDs OFF
  digitalWrite(LED3, LOW);     // turn off LED3
  delay(450);                  // wait for 450ms
  digitalWrite(LED2, LOW);     // turn off LED2
  delay(450);                  // wait for 450ms
  digitalWrite(LED1, LOW);     // turn off LED1
  delay(3000);                 // wait for 3000ms
 
} //END of blockingMethod()
 
 
void nonBlockingMethod()
{
  switch (mState)
  {
    //***************************    //LEDs ON
    case StateStart:
      {
        digitalWrite(LED3, HIGH);    // turn on LED3
        wait = 450;                  // wait for 450ms
        lastMillis = millis();
        mState = State2;
      }
      break;
 
    //***************************
    case State2:
      {
        digitalWrite(LED2, HIGH);    // turn on LED2
        wait = 450;                  // wait for 450ms
        lastMillis = millis();
        mState = State3;
      }
      break;
 
    //***************************
    case State3:
      {
        digitalWrite(LED1, HIGH);    // turn on LED1
        wait = 3000;                 // wait for 3000ms
        lastMillis = millis();
        mState = State4;
      }
      break;
 
    //***************************    //LEDs OFF
    case State4:
      {
        digitalWrite(LED3, LOW);     // turn off LED3
        wait = 450;                  // wait for 450ms
        lastMillis = millis();
        mState = State5;
      }
      break;
 
    //***************************
    case State5:
      {
        digitalWrite(LED2, LOW);     // turn off LED2
        wait = 450;                  // wait for 450ms
        lastMillis = millis();
        mState = State6;
      }
      break;
 
    //***************************
    case State6:
      {
        digitalWrite(LED1, LOW);     // turn off LED1
        wait = 3000;                 // wait for 3000ms
        lastMillis = millis();
        mState = State7;
      }
 
      break;
 
    //***************************
    case State7:
      {
        blockingFlag = true;
        wait = 3000;                 // wait for 3000ms
        mState = StateStart;
      }
 
      break;
 
    //***************************
    default:
      {
      }
      break;
 
  } //END of switch case
 
} //END of nonBlockingMethod()
 
// ************** END of sketch ****************

.

Think of millis() as the clock on the wall.

Imagine I have to switch a light on or off every 10 minutes. I look at the clock and decide if 10 minutes has expired. Last time I switched the switch it was 10:42 so I wait until I see 10:52 before I go and switch the light. If I see 10:55, then I know I'm late, but I go and switch it anyway. Then I have a choice - do I add 10 minutes to 'now' and schedule the next change for 11:05 or do I add 10 minutes to the original time and plan the next one for 11:02?

Now you give me a different pattern. The light goes on for 1 minute, off for 2, on for 1... So, I look at the clock several times per minute. When I'm in the 1-minute-on period, I will switch it as soon as I see the next minute. Now I move my finger down the page and I see that I need to wait 2 minutes. So, when 2 minutes has elapsed since I last switched it, I'll do the switch.

When I un-delayed a greenhouse automation, I included the timers in with the state machines (for serial, 1-wire and GSM non-blocking functions) like this. Note the static locals for state and timing, keeps it all a bit more contained.

inline void nonBlockingMethod()
{
  static byte nbmState = StateStart; 
  static unsigned long nbmWait = 0;
  static unsigned long nbmLastMillis = 0;

  // **********************             the timer
  if ( nbmWait ) // 1-shot wait, clears on timeout, only runs if nbmWait is non-zero.
  {
    if (millis() - nbmLastMillis < nbmWait)  return;
    nbmWait = 0;
  }

  switch (nbmState)
  {
    //***************************    //LEDs ON
    case StateStart:
      {
        digitalWrite(LED3, HIGH);    // turn on LED3
        nbmWait = 450;                  // wait for 450ms
        nbmLastMillis = millis();
        nbmState = State2;
      }
      break;
 
    //***************************
    case State2:
      {
        digitalWrite(LED2, HIGH);    // turn on LED2
        nbmWait = 450;                  // wait for 450ms
        nbmLastMillis = millis();
        nbmState = State3;
      }
      break;
 
    //***************************
    case State3:
      {
        digitalWrite(LED1, HIGH);    // turn on LED1
        nbmWait = 3000;                 // wait for 3000ms
        nbmLastMillis = millis();
        nbmState = State4;
      }
      break;
 
    //***************************    //LEDs OFF
    case State4:
      {
        digitalWrite(LED3, LOW);     // turn off LED3
        nbmWait = 450;                  // wait for 450ms
        nbmLastMillis = millis();
        nbmState = State5;
      }
      break;
 
    //***************************
    case State5:
      {
        digitalWrite(LED2, LOW);     // turn off LED2
        nbmWait = 450;                  // wait for 450ms
        nbmLastMillis = millis();
        nbmState = State6;
      }
      break;
 
    //***************************
    case State6:
      {
        digitalWrite(LED1, LOW);     // turn off LED1
        nbmWait = 3000;                 // wait for 3000ms
        nbmLastMillis = millis();
        nbmState = State7;
      }
 
      break;
 
    //***************************
    case State7:
      {
        blockingFlag = true;
        nbmWait = 3000;                 // wait for 3000ms
        nbmState = StateStart;
      }
 
      break;
 
    //***************************
    default:
      {
      }
      break;
 
  } //END of switch case
 
} //END of nonBlockingMethod()
 
// ************** END of sketch ****************

.

Thanks for the replies and the guidance everyone, it is really appreciated.

LarryD:

I tried using the BWD sketch initially and was still having problems. That’s when I found the elapsedMillis library, thinking it would help me resolve the issue. Reading the page for the library, it seemed to be just the ticket.

I downloaded your code and ran it. I noticed that you broke each LED into its own “on” case and its own “off” case. I never thought of that! (Truth be told, I'm not real familiar with 'case' yet.) I’m guessing that maybe it is not possible doing things as straight forward as delay() allows? That is what I have been thinking, but making them have their own case make a bit of sense to.

I also noticed it seems you made the non-blocking method into its own function. That is what the
'void nonBlockingMethod()' means, right? There are some other items I do not recognize in your code, such as 'byte' and 'enum.' I will need to look these up, and I am sure I will be back to this thread afterwards.

I’m also wondering if this is going to cause things to be harder because I was planning on using cases to change to different lighting patterns. I’ve got some homework looking into these things, but I’ll be back. Thanks for the information!

MorganS:

I like your analogy. It makes sense looking at it like watching the clock. There is still something I don’t understand though.

Let’s say that I want to do something like:

Wait for 1 minute.
Do thing A.
Wait for 30 seconds.
Do thing B.

With delay(), it is easy. I can say:

delay(1 minute)
Do thing A.
delay (30 seconds.)
Do thing B.

But with millis() I can’t seem to figure out the “wait” part. Maybe I am thinking about it wrong. Can you use millis() to create a “wait” or does it only work like a scheduler? For instance, for every time period specified, do a thing?

Also, if the action is late, with the Arduino processing around 16,000 instructions per second, it wouldn’t cause a noticeable “misstep” in the timing of the LEDs since there are only 10 things (the 10 different LEDs) to look at at any give time, correct?

GoForSmoke:

I see you also broke the LEDs out into individual actions inside a case. I'm starting to notice a pattern here... Is there a specific reason why, or does it just make the coding easier?

You have also used things that I am not familiar with, mainly 'static' and 'inline.' I will have to look these up.

If I'm reading your code right, when you have the 'nbmWait = 450;' inside the case, that is what is setting the trigger for when to do the next action. So, changing the 450 to 1000 would make the next LED come on after a 1 second wait?

Thanks again, all three of you. I am going to try to put some of these ideas into action and play with your recommendations. After I get some experience with this and do my homework, I will be back!

Your millis() approach possibly failed due to the use of for-loops. loop() is executed repeatedly so in general there is no need for loops to perform actions.

(Truth be told, I'm not real familiar with 'case' yet.)

'switch/case' is nothing more than 'if / else if / else iif / else if/ .... / else'. A switch/case only works with numbers though and is less flexible from that perspective.

Let's say that I want to do something like:

Wait for 1 minute.
Do thing A.
Wait for 30 seconds.
Do thing B.

You broke it already up in pieces. Each piece becomes a state (case). This is shown in below code and implements a' blink without delay'. It uses a state machine (like @LarryD used). @LarryD used an enum for the states, I use #defines for it.

I hope the comments make clear how it works.

// the states that our program can be in
#define ACTION_1 0  // execute action for step 1
#define ACTION_2 1  // execute action for step 2
#define WAIT_1 2    // delay before step 1
#define WAIT_2 3    // delay before step 2

// our delays
#define DELAY_ACTION_1 2000  // delay after action A has been done
#define DELAY_ACTION_2 3000  // delay after action B has been done

// current state; iinitialised to WAIT_1 so program starts with that
byte currentState = WAIT_1;
// current time; global so it can be access from everywhere; updated in loop()
unsigned long currentTime;

void setup()
{
  pinMode(13, OUTPUT);
  digitalWrite(13, LOW);
}

void loop()
{
  // remember the previous time
  static unsigned long previousTime = 0;

  // get current time
  currentTime = millis();

  switch (currentState)
  {
    // wait till first delay has lapsed
    case WAIT_1:
      // if delay has lapsed
      if (currentTime - previousTime >= DELAY_ACTION_1)
      {
        // change state; next iteration of loop case 'ACTION_1' will be executed
        currentState = ACTION_1;
      }
      break;
    // switch led 13 on
    case ACTION_1:
      // action for step 1
      digitalWrite(13, HIGH);
      // remember time that this step was done
      previousTime = currentTime;
      // change state; next iteration of loop case 'WAIT_2' will be executed
      currentState = WAIT_2;
      break;
    // wait till second delay has lapsed
    case WAIT_2:
      // if delay has lapsed
      if (currentTime - previousTime >= DELAY_ACTION_2)
      {
        // change state; next iteration of loop case 'ACTION_2' will be executed
        currentState = ACTION_2;
      }
      break;
    // switch led 13 on
    case ACTION_2:
      // action for step 2
      digitalWrite(13, LOW);
      // remember time that this step was done
      previousTime = currentTime;
      // change state; next iteration of loop case 'WAIT_1' will be executed (again)
      currentState = WAIT_1;
      break;
  }
}

Don't pay much attention to inline, it's only a hint to the compiler and has absolutely no significance at all. In fact, it's highly unlikely that the compiler would ever inline a function that large. Nothing adverse will happen if you ignore it and omit it.

Also, get rid of the elapsedMillis library. It does work for it's intended purpose, but it is nothing more than a convenient wrapper around the basic time interval calculation. At the moment, while you are still learning, it does nothing except obfuscate the code's working. By the time you would be comfortable with the concepts enough to use it, it would be trivial to recreate yourself.

A function is evaluated from top to bottom as quickly as possible. When you are using delay, it's trivial to create a fixed sequence because delay stops everything in it's tracks. The technical term for that is blocking, as mentioned in LarryD's post. This is perfectly fine if you only need to do one thing in a fixed sequence. The moment you need to do a second thing at the same time (like check for button presses), things get tough.

The reason your second set of code is causing random light patterns to occur is because you have nothing forcing a certain sequence. When it passes through loop, it checks the first conditional, checks the second conditional, checks the first conditional...all the way down the line. This happens very quickly, literally tens of thousands of times per second. The millis() timer is ticking up in the background, so which one of those conditions eventually evaluates true is random.

What you need to do is to define your sequence in some way that you can select one part of the sequence with a variable. LarryD and GoForSmoke have shown you a way with switch() statements, but that may be overkill if you are just going to cycle through a fixed sequence. You can also use arrays containing a value representing which LEDs you want to turn on at that point in the sequence, and a value representing the interval until the next part of the sequence.

The second thing you need is a way to keep track of what point in the sequence you are at, and every time the interval expires advance that variable to the next point in the sequence. LarryD and GoForSmoke used that variable as the input to their switches, you can also use it as an index for the array method I mentioned.

Here is an example that will flash two LEDs (on 12 and 13) in the following sequence with a 1,000 millisecond interval in between transitions, except when both LEDs are on, then it's 2,000 ms:

XX
XO
OO
OX

// Array of pin numbers to use.
const uint8_t PINS[] = {12, 13};
const uint8_t PINS_NO = sizeof(PINS) / sizeof(PINS[0]);

// The state of both LEDs are packed in the single
// pattern variable, and will be unpacked with bitwise
// operators.
struct seq_entry
{
  uint8_t pattern;
  uint32_t interval;
};

uint32_t lastIntervalChangeTime = 0;
uint32_t intervalLength;

const seq_entry SEQUENCE[] = {{0b00, 1000},
                              {0b01, 1000},
                              {0b11, 2000},
                              {0b10, 1000}};
const uint8_t SEQUENCE_NO = sizeof(SEQUENCE) / sizeof(SEQUENCE[0]);
uint8_t sequence_index = 0;

void setup()
{
  for( int i=0; i<PINS_NO; ++i )
    pinMode( PINS[i], OUTPUT );

  // Load the first sequence onto the LEDs.
  advanceSequence();
}

void loop()
{
  if( millis() - lastIntervalChangeTime >= intervalLength )
  {
    // The order of the next two statements is critical. 
    // You ~MUST~ update the interval accumulator before
    // advancing the sequence.
    // Swap the order and see if you can understand why it wigs out. :)
    lastIntervalChangeTime += intervalLength;
    advanceSequence();
  }
}

void advanceSequence()
{
  for( int i=0; i<PINS_NO; ++i )
    digitalWrite( PINS[i], SEQUENCE[sequence_index].pattern & (0x1<<i));

  intervalLength = SEQUENCE[sequence_index].interval;

  ++sequence_index;
  if( sequence_index==SEQUENCE_NO )
    sequence_index = 0;
}

There's quite a few things there, so study it and pick out what it's doing. The definition of the sequence is in the SEQUENCE[] array, and sequence_index is used to keep track of the position in the sequence each time the timer goes out and you need to advance to the next step.

I would have posted this example far sooner, except for a subtle bug that I had to solve that took longer than I would like to admit to find. I've commented it in loop(). Good thing I tested it before posted. :grinning:

I think of an arduino loop() function as being like a goldfish circling a bowl, forgetting everything that happened last time it did a circuit. So you have to write things down - that's what global (aka: static) variables are for.

Each time the loop() runs, you have to answer one question: "what am I supposed to do right now?"

Most of the time, the answer is "nothing". But every now and then the time changes, or an input alters state, and loop() needs to respond to it.

With delay(), it is easy. I can say:

delay(1 minute)
Do thing A.
delay (30 seconds.)
Do thing B.

But with millis() I can't seem to figure out the "wait" part. Maybe I am thinking about it wrong. Can you use millis() to create a "wait" or does it only work like a scheduler? For instance, for every time period specified, do a thing?

Try this...

  • Look at clock
  • Is it time to do thing A? -> Start doing thing A
  • Is it time to do thing B? -> Start doing thing B
  • Repeat forever

PaulMurrayCbr:
Most of the time, the answer is "nothing". But every now and then the time changes, or an input alters state, and loop() needs to respond to it.

I recently had to do some timing analysis on a project running on a Teensy (higher clock speed than a standard Arduino.) It needs to read a sensor 400 times per second, send some output 100 times per second and react to other events. The minimum time that it takes to execute a loop is 6 microseconds. The average time is 7 microseconds. It's doing hundreds and hundreds of things but really, the loop is executing 130,000 times per second mostly just looking at the clock and waiting.

Welcome to the forum!

My early fumbling with Arduino involved the exact same type of sketch.
and is how I learned both BWOD and state machine's(switch/case).

My solution was to have a single BWOD to control all timing.
Then one switch/case within loop() to control which pattern I was on.
And another switch/case within each pattern function to control which step of that pattern i was on.

Pressing the button would increment to the next pattern and reset the patternstep.
There was also a potentiometer to adjust the base patternstep interval but each patternstep could also modify the interval for that particular step.

I managed to lose the final version. but here is a version that works except some of the patterns are not fully debugged.

Gregs_Flasher_V2.ino (10.8 KB)

Wow! Thanks everyone for the replies and all the information! I really appreciate it.

I have been busy looking up all the mentioned terms that I am unfamiliar with at the arduino reference. There are a few things that I didn’t find there and I am wondering if they are important for me to know at this point. Are these worth me worrying about right now?

  • enum
  • uint8_t and uint32_t
  • struct

If there is something important here, could you guide me to where I can look it up? I didn't find any of these on the arduino reference page.

sterretje:

Would you mind giving a little clarification?

sterretje:
Your millis() approach possibly failed due to the use of for-loops. loop() is executed repeatedly so in general there is no need for loops to perform actions.

Are you referring to the first block of code or the last block? The first block is the one that works and it is also the one using the for loops. Did you mean 'if'? In that second block of code (the one that doesn't work) I don't have any for loops. I'm just a little confused.

Also, thanks for explaining what a switch/case is. I can understand looking at it like an 'if/else'. Your block of code with all those comments does help me wrap my brain around what is happening. Thanks for taking the time to write that out!

Jiggy-Ninja:

Jiggy-Ninja:
Don't pay much attention to inline, it's only a hint to the compiler and has absolutely no significance at all. In fact, it's highly unlikely that the compiler would ever inline a function that large. Nothing adverse will happen if you ignore it and omit it.

Ok, I can look past that. Still makes me curious what it means. I couldn't find it in the arduino reference. Is it something mostly used for C++?

At the moment, while you are still learning, it does nothing except obfuscate the code's working. By the time you would be comfortable with the concepts enough to use it, it would be trivial to recreate yourself.

Seems like good advice. I think I am going to re-write my code and not use the library.

The reason your second set of code is causing random light patterns to occur is because you have nothing forcing a certain sequence...

So, basically, the arduino is processing so rapidly that the sequence I want gets "skipped" at times? The math doesn't add up to the sequence and it will turn the light on eventually when the if statement is fulfilled? I was wondering if it had something to do with the arduino processing over the code so rapidly, but I didn't know any way to stop that. Enter switch/case!

Also, thanks for the example. When I get a bit of time I am going to run all of these through my arduino and play with them a bit. Maybe I can get some working code written soon.

PaulMurray:

PaulMurrayCbr:
I think of an arduino loop() function as being like a goldfish circling a bowl...

That's a good analogy. And since static variables are only available to one particular function, I can use that to remember where I am in the lighting sequence, correct?

Thanks for your input. Much appreciated!

MorganS:

MorganS:
Try this...

  • Look at clock
  • Is it time to do thing A? -> Start doing thing A
  • Is it time to do thing B? -> Start doing thing B
  • Repeat forever

So, instead of just sitting and waiting for time to pass, the code is watching the clock and looking to see if it is time to do a thing. Basically like when sit and bounce my leg waiting for my time to go into an interview or a doctor appointment. I'm not just sitting in delay() letting time pass, I am actively waiting, checking the time to see if it is my turn.

Hutkikz:

Hutkikz:
My solution was to have a single BWOD to control all timing.
Then one switch/case within loop() to control which pattern I was on.
And another switch/case within each pattern function to control which step of that pattern i was on.

I'm going to have to look into this. Maybe it can simplify what I am trying to do.

I managed to lose the final version. but here is a version that works except some of the patterns are not fully debugged.

Thanks for the code sample. I downloaded it, but I haven't had a chance to dig through it. Hopefully in the next couple days I can get a closer look at it.

Again, everyone, thanks for the input, I really appreciate it. I am going to try to look over all of these code samples and get something worked out. I'm sure I should get something going after all the advice you've given me!

I will be back... :sunglasses:

MorganS:
Try this...

  • Look at clock
  • Is it time to do thing A? -> Start doing thing A
  • Is it time to do thing B? -> Start doing thing B
  • Repeat forever

The problem with that example is that is shows how to do two things in parallel without blocking, but it does not show how to do a timed sequence without blocking like BoJack wants to do. It's not wrong, but it's insufficient for this purpose.

It will be useful when he starts to add the button code in, but not yet while he's still just trying to understand how to recreate the fixed time sequence.

BoJack:
Wow! Thanks everyone for the replies and all the information! I really appreciate it.

I have been busy looking up all the mentioned terms that I am unfamiliar with at the arduino reference. There are a few things that I didn’t find there and I am wondering if they are important for me to know at this point. Are these worth me worrying about right now?

  • enum
  • uint8_t and uint32_t
  • struct

If there is something important here, could you guide me to where I can look it up? I didn't find any of these on the arduino reference page.

cppreference.com (It's probably not the best to learn it from, but it'll have everything there. Other sites will have more practical tutorials, but you should search for those on your own.)

The Arduino IDE is currently using the C++11 standard, so disregard anything on that site marked C++14 or C++17.

enum and struct will be in the Language > keywords section, uint8_t and uint32_t are in Utilities Library > Type support, find the Fixed width integer types link.

struct (and the related class) is definitely important to focus on. If you open up the elapsedMillis.h file you downloaded for that library, you will find that it is a simple class with a few constructors and a lot of overloaded operators, but it's fundamentally a wrapper around (millis() - startOfInterval).

Enumerations are useful, but nowhere near as much as structures and classes. They are probably most useful in switches, to give names to the difference cases.

Jiggy-Ninja:

Ok, I can look past that. Still makes me curious what it means. I couldn't find it in the arduino reference. Is it something mostly used for C++?

http://en.cppreference.com/w/cpp/language/inline

The original intent of the inline keyword was to serve as an indicator to the optimizer that inline substitution of a function is preferred over function call, that is, instead of executing the function call CPU instruction to transfer control to the function body, a copy of the function body is executed without generating the call. This avoids overhead created by the function call (copying the arguments and retrieving the result) but it may result in a larger executable as the code for the function has to be repeated multiple times.

Since this meaning of the keyword inline is non-binding, compilers are free to use inline substitution for any function that's not marked inline, and are free to generate function calls to any function marked inline. Those optimization choices do not change the rules regarding multiple definitions and shared statics listed above.

The key there is that it is a non-binding indicator that only effected the compiler's optimizer. It is free to ignore the presence of absence of inline. Because of that, the meaning will be changing when the new C++17 standard is finalized.

Because the meaning of the keyword inline for functions came to mean "multiple definitions are permitted" rather than "inlining is preferred", that meaning was extended to variables.
(since C++17)

So, basically, the arduino is processing so rapidly that the sequence I want gets "skipped" at times? The math doesn't add up to the sequence and it will turn the light on eventually when the if statement is fulfilled? I was wondering if it had something to do with the arduino processing over the code so rapidly, but I didn't know any way to stop that. Enter switch/case!

Also, thanks for the example. When I get a bit of time I am going to run all of these through my arduino and play with them a bit. Maybe I can get some working code written soon.

It's not a matter of it being fast or slow. You could slow the clock speed down by 100,000 times and you'd still get the same erratic behavior. The problem was that you were checking all of your individual LEDs in parallel. You had no control over where it would be in the loop when the timer interrupt finally ticked millis() over the interval you needed.

The three of us that posted code examples have all shown you a way to define the sequence and advance through it. For a fixed sequence of outputs like this, I would normally use an array which is why I gave an example of that method. sterretje's and GoForSmoke's examples using switches are a bit overkill for this kind of task, but can be essential if you are coding more complicated sequences that may involve more inputs affecting the advancement and more complicated pathways that are more than just a straight line. The general term for that sort of thing is a finite state machine.

There is an enormous amount of depth you can get into with this hobby, and you've only dipped your toe in to test the water so far. The device you possess is not some watered down toy like some of the education electricity kits that are basically Legos with battery holders, it is a proper electronic device that is capable of being used in industrial controllers. The Arduino core does abstract away some details, and sometimes these details are important, but it does nothing to prevent you from diving into the guts of the machine.

Come on in, the water's great. :smiley: Just stay in the shallow end so you don't drown first.

MorganS:

So, instead of just sitting and waiting for time to pass, the code is watching the clock and looking to see if it is time to do a thing. Basically like when sit and bounce my leg waiting for my time to go into an interview or a doctor appointment. I'm not just sitting in delay() letting time pass, I am actively waiting, checking the time to see if it is my turn.

Good enough, if that helps you make sense of it. The important thing is that because you aren't stuck twiddling your thumbs in a delay() loop, you can check on the time for multiple things at once, letting you multi-task.

However, like I said above, you aren't multi-tasking yet. You're still learning how to single-task in a way that lets you multi-task when you start added the other features (like the button).

Hutkikz

#define DEBUG false                        // Set false to REMOVE Serial
#define Serial if (DEBUG)Serial            // WARNING: Use only when Serial is used for Debugging only (THIS REMOVES Serial COMPLETLY!!!)

Off-topic criticism, but there's a better way to do your debug printing than that.

//#define PRINT_DEBUGS   // uncomment to enable debug printing.

#ifdef PRINT_DEBUGS
  #define DEBUG_PRINT(x) Serial.print(x);
  #define DEBUG_LINE(x) Serial.println(x);
#else
  #define DEBUG_PRINT(x)
  #define DEBUG_LINE(x)
#endif

You can use it like DEBUG_LINE(F("Starting program")). When the PRINT_DEBUGS symbol is defined, the macro (basically a pseudo-function) expands to Serial.println(F("Starting program"));. If the PRINT_DEBUGS symbol is not defined, it expands to nothing; the preprocesor eats it and it takes up no space in the final compiled code.

BoJack:
sterretje:

Would you mind giving a little clarification?

Are you referring to the first block of code or the last block? The first block is the one that works and it is also the one using the for loops. Did you mean 'if'? In that second block of code (the one that doesn't work) I don't have any for loops. I'm just a little confused.

Also, thanks for explaining what a switch/case is. I can understand looking at it like an 'if/else'. Your block of code with all those comments does help me wrap my brain around what is happening. Thanks for taking the time to write that out!

I was referring tothe first block of code; I for some reason missed the second one. Sorry for the confusion.

@ Jiggy-Ninja

Hutkikz

#define DEBUG false                        // Set false to REMOVE Serial

#define Serial if (DEBUG)Serial            // WARNING: Use only when Serial is used for Debugging only (THIS REMOVES Serial COMPLETLY!!!)




Off-topic criticism, but there's a better way to do your debug printing than that.

I found that very early on. Since then I've heard enough scare stories about the preprocessor that I just avoid macro's altogether.(for now anyway)
Because it's so easy I do still use this one but I really shouldn't since I still don't quite understand it fully.

Yours is easy to understand and leaves Serial available for other usage. Are there any other reason's that it is better?

Jiggy-Ninja:

Jiggy-Ninja:
cppreference.com (It's probably not the best to learn it from, but it'll have everything there.

Thanks for the lead on the reference and all of the information! I will use it to look up the things I don't understand. Hopefully I will be able to make sense of it!

Jiggy-Ninja:
There is an enormous amount of depth you can get into with this hobby, and you've only dipped your toe in to test the water so far. The device you possess is not some watered down toy like some of the education electricity kits that are basically Legos with battery holders, it is a proper electronic device that is capable of being used in industrial controllers. The Arduino core does abstract away some details, and sometimes these details are important, but it does nothing to prevent you from diving into the guts of the machine.

Yeah, this is seeming to be the case. This is such an inspirational concept. There are a half-dozen projects in my head right now, and they are only limited by my ability to write the code. I am doing what I can to learn that part, and I appreciate all the help I have gotten here. Can't wait to start my next project!

Jiggy-Ninja:
Come on in, the water's great. :smiley: Just stay in the shallow end so you don't drown first.

Hah! Sounds about right!

Jiggy-Ninja:
The three of us that posted code examples have all shown you a way to define the sequence and advance through it. For a fixed sequence of outputs like this, I would normally use an array which is why I gave an example of that method.

I finally have gotten a chance to take a preliminary look at the code you guys have posted and I have written some new code! I based my code off of the switch/case idea that LarryD and GoForSmoke showed me because I could visualize it better at this point. I can see each individual block and trace through what it is doing.

I am only slightly familiar with arrays, but only in what I think is their simplest form. I used one in my code in the first post. I got a little lost while looking at the array method you posted Jiggy-Ninja. I am going to study this and see if I can make sense of it.

Ok on to the NEW CODE!!!

Ok, so first off, I realize that this code probably isn't very efficient. At least not for the amount of space it takes up. From my understanding, things like arrays make the code more space efficient because you can iterate through them by ++. Right now, at least for this project, I'm not too worried about that. I should have plenty of space for my code using this 'brute force' method. I would like to optimize it, but right now, I think that may be a little over my head. So maybe I can revisit to make it more optimized. However, if there is anything that is really obvious and within what any of you think I could understand, please, let me know.

I have some questions about what to do now. As you can see, I used cases for each step in the sequence. I was planning on using case to integrate the button press. So, when I press the button, it changes to a new case and therefore a new lighting pattern. Can you have a case inside of a case?

I was also thinking that instead of case inside of case, I could make each light pattern its own function. Then the button presses could call up each function as necessary. Would this be a better way to do it? And if I did it this way, as long as the button press was remembered each iteration through the loop, it would stay within that function, correct?

And, there is one final question... Is it ok for two different functions to have the same state names inside of them? For instance, if I have function1() and function2() can each function contain a state called state1 and state2? I am working on combining two light patterns into one. I made two different sketches and I am wondering if I am going about it in the right way.

I ran out of word space for this post, so I couldn't post the code in code tags. I have attached them below. Please, take a look and let me know what you think!

Sequential_Run_Non_Blocking.ino (9.01 KB)

Sequential_Train_Non_Blocking.ino (8.89 KB)

Head_Shaft_Tail_Sequential_Non_Blocking.ino (8.25 KB)

You might be missing the point a little bit. Every iteration of loop() you check if it's time to do something and if so, do it. What is done depends where you are in sequence1().

You can vary which sequence to is selected in loop(). A very basic example (not taking bouncing into account) based on Head_Shaft_Tail_Sequential_Non_Blocking.ino. The below will execute sequence1 if a button is in one state and execute sequence2 if the button is in the other state

// Main Loop (runs continually)
void loop()
{
  //**********************************
  //some code to check for blocking sketches
  if (millis() - Millis13 >= 200)
  {
    digitalWrite(heartBeat, !digitalRead(heartBeat)); //toggle D13
    Millis13 = millis(); //re-initalize
  }
 

  //********************************** BWD being used here
  if (millis() - lastMillis >= wait)
  {
    if(digitalRead(yourButton) == HIGH)
      sequence1();
    else
      sequence1();
  }
 
  // Other non blocking code goes here
  // This is where the checking for the button happens.
 
} //END of loop()

Note that each sequence is a statemachine on it's own and hence basically will have it's own state names (some or all can be the same depending on what the state machine needs to do) in its own enum. You should actually give those states in the enum (and the switch/case) sensible names; e.g. StateTail, StateShaft, ...)

You can expand for more sequences by keeping track of the selected state. The below is extremely primitive (using delays and hence blocking) and I leave it up to you to sort that.

void loop()
{
  static byte selectedSequence = 1;

  // if button pressed
  if(digitalRead(yourButton) == ACTIVE)
  {
    // debounce delay
    delay(100);
    // next sequence
    selectedSequence++;
    // limit number of sequences
    if(selectedSequence > maxSequences)
      selectedSequence = 1;
    // wait for button to be released
    while(digitalRead(yourButton) == ACTIVE)
      delay(100);
  }

  if (millis() - lastMillis >= wait)
  {
    switch(selectedSequence)
    {
      case 1:
        sequence1();
        break;
      case 2:
        sequence2();
        break;
    }
  }

With regards to state names

// States for the state machine (sequence 1)
enum StatesSeq1 {
  StateStart, State2, State3, State4, State5,
  State6, State7, State8, State9, State10,
  State11, State12, State13, State14, State15,
  State16, State17, State18, State19, State20
};

// States for the state machine (sequence 2)
enum StatesSeq2 {
  StateStart, State2, State3, State4, State5,
  State6, State7, State8, State9, State10,
  State11, State12, State13, State14, State15,
  State16, State17, State18, State19, State20
};

Not modified with sensible names.

Yes you can have a case() inside a case(). The language would be very limited if you could not do this.

Yes, two functions can have local variables with the same name and they don't interfere. However it does indicate that you have a poor design. If two functions make different LED light patterns then it would make more sense to have one function that drives the LEDs and send it different data. You should learn about arrays and pointers, but not immediately. Get your first project working first.

const byte LED01 = 12; // Varible Name = Pin #
const byte LED02 = 11;
const byte LED03 = 10;
const byte LED04 = 9;
const byte LED05 = 8;
const byte LED06 = 7;
const byte LED07 = 6;
const byte LED08 = 5;
const byte LED09 = 4;
const byte LED10 = 3;

Array, right here.

 //********************************** BWD being used here
  if (millis() - lastMillis >= wait)
  {
    sequence1();
  }

...

  switch (mState)
  {
    //***************************
    case StateStart:
      {
        digitalWrite(LED01, HIGH);    // Turn on LED 1
        wait = waits;                 // Wait before next state
        lastMillis = millis();        // Reset millisecond counter
        mState = State11;             // Switch to specified state
      }
      break;

This is bad form. It will work, but now your code is now horribly entangled with itself like that one string of Christmas lights you threw into the bottom of the box last year and now when you pull the ball it takes more time to unravel that string than to put up all the other decorations combined.

Make things easy on yourself, and take that lastMillis = millis(); statement out of all the switch cases. Put it just before the sequence1(); call. Put all the code that's related to the same functionality in one location. That way it's easier to see that all the steps have been done in the right order, and you don't have to worry whether you've missed performing some necessary task buried 5 pages down in some obscure branch of your code that you might think is impossible for the code to go through but it does anyway.

Avoid duplicating statements. If you find yourself copying the same line(s) into a dozen different places in the code, that's a sign that you either need to relocate it to a more convenient place or spin it off into its own function.

sterretje:

// Main Loop (runs continually)

void loop()
{
 //**********************************
 //some code to check for blocking sketches
 if (millis() - Millis13 >= 200)
 {
   digitalWrite(heartBeat, !digitalRead(heartBeat)); //toggle D13
   Millis13 = millis(); //re-initalize
 }

//********************************** BWD being used here
 if (millis() - lastMillis >= wait)
 {
   if(digitalRead(yourButton) == HIGH)
     sequence1();
   else
     sequence2();
 }

// Other non blocking code goes here
 // This is where the checking for the button happens.

} //END of loop()

Looking at this, sequence1() and sequence2() are functions, right? So the button press (or release) chooses one function or the other, then these functions then comprise the state machines with my code inside...?

You have brought up a couple ideas that I had not considered yet...

  • One, I have only seen button debounce that uses delay(). I am going to need to look into this. Think I can search around and find out how to do it without delay().

  • Another is that I can break the state machine states into two different enums based upon the future function names I choose. I have not given them descriptive names yet because I was looking at them as "steps" of what to do. StateStart is step 1, state2 is step 2 and so on... This way I could mentally step through what I want to do as I am writing the code for the first time. I plan to go back and give them better names, but this is also why I added all the //comments.

sterretje:
You can expand for more sequences by keeping track of the selected state. The below is extremely primitive (using delays and hence blocking) and I leave it up to you to sort that.

I believe this sample is how I have my button working, I just need to de-delay() it.

sterretje:

// States for the state machine (sequence 1)

enum StatesSeq1 {
 StateStart, State2, State3, State4, State5,
 State6, State7, State8, State9, State10,
 State11, State12, State13, State14, State15,
 State16, State17, State18, State19, State20
};

// States for the state machine (sequence 2)
enum StatesSeq2 {
 StateStart, State2, State3, State4, State5,
 State6, State7, State8, State9, State10,
 State11, State12, State13, State14, State15,
 State16, State17, State18, State19, State20
};

Yes, this is how I envision it. Glad to know that my thinking is on the right track.

Jiggy-Ninja:
Make things easy on yourself, and take that lastMillis = millis(); statement out of all the switch cases. Put it just before the sequence1(); call.

Whoa... I had no idea I could do that!

Ok, I have gone and tweaked my code for Head_Shaft_Tail_Sequential_Non_Blocking.ino a little bit. I started with this one as it seemed to be the easiest to modify because it has fewer parts to it. I made an array for initializing the pins for the LEDs, and I used an array with a for loop to set them to OUTPUT. I also moved the lastMillis = mills(); statement out of the switch cases as suggested and it blew my mind that it still worked!

This is what I am trying to figure out now:

Jiggy-Ninja:
Avoid duplicating statements. If you find yourself copying the same line(s) into a dozen different places in the code, that's a sign that you either need to relocate it to a more convenient place or spin it off into its own function.

So, I am looking for a way to remove all the state2, state3, state4, etc. I learned earlier that to control the LEDs in the order that I want, I need to command them individually to do things at the times I want. Switch/case let me do that. Now I know that this is bad form. I haven't come up with a way to replace it yet. Jaggy-Ninja, I am still trying to understand the method you posted earlier. I am still lost. I know nothing yet about bitwise math, and I am getting lost trying to decipher the code.

I am not sure what to do now. I think I need to make each section of the flashing pattern its own function, but I just made them cases and it seems to me like I am backtracking. I guess I need to recreate the flashing pattern but not put each piece in its own case?

Looking back at your example Jiggy, I can understand that (on my circuit)

const seq_entry SEQUENCE[] = {{0b00, 1000},
                              {0b01, 1000},
                              {0b11, 2000},
                              {0b10, 1000}};

is turning on the LED on pin 12 for 1 second, both 12 and 13 for 2 seconds, and then the LED on pin 13 for 1 second, but I have no idea what the '0b' is all about. And I am not sure how I would get all ten LEDs in there. Would it be something like this for 5 LEDs?

{0b00000, 500},
{0b00001, 500},
{0b00010, 500},
{0b00100, 500},
{0b01000, 500},
{0b10000, 500}}

I ran out of word space again, so I will re-attach my updated code. Same filename, but updated. If anyone can drop hints on how to move my patterns out of the cases into something more efficient that I can understand, that would be awesome.

As always, I am very grateful for all the help you all have given me. It is very much appreciated. Thanks again!!!

Head_Shaft_Tail_Sequential_Non_Blocking.ino (7.07 KB)