Trouble 'multitasking' with arduino

Hello everyone!

I should first off say I am very much a beginner to Arduino, and was hoping to get advice on some code that I have written.

I've recently been working on a small project as a favor to my sister - she wants a light-up cloud that she can hang from her ceiling. Sure, I could just throw some Christmas tree lights in it, but that's no fun. So I came up with an Arduino set up, and plan to incorporate it into the cloud so it can do things like pulse, and have lightning effects, and control individual leds through PMW- all controlled by a single button that will switch between different modes.

Anyhow, I have code that works, and it does all that I want it to. My problem however lies with the push button that switches between modes. I need it to detect the button while also doing whatever it is supposed to do during whatever mode it is in.

My first solution was to remove as many "delay()" functions as possible and replace them with 'millis()' timers and whatnot. That helped for the 3/4 of the modes, however my fourth mode - the 'lightning()' loop, seems to not have enough time to detect the button change while also processing the code.

I included the sketch as it currently is, and the original 'lightning()' loop that used a bunch of delays commented at the bottom. Thank you in advance for any help! :slight_smile:

int redPin1 = 5;                // dont let 'red' 'green' and 'blue' be confusing
int greenPin1 = 6;            // basically, its just 3 different leds that i never renamed
int bluePin1 = 7;

int buttonPin = 1;

int modeCount = 8;          //begins at number that is definitely higher than 4

void setup(){
  pinMode(redPin1, OUTPUT);
  pinMode(greenPin1, OUTPUT);
  pinMode(bluePin1, OUTPUT);

  pinMode(buttonPin, INPUT_PULLUP);
  modeChange();
}

void loop(){
  
  button();
  
  if(modeCount == 0){
    off();
  }
  if(modeCount == 1){
    solid();
  }
  if(modeCount == 2){
    pulse();
  }
  if(modeCount == 3){
    lightning();                        // this is the mode that seems to 'hogs' cpu cycles
  }
  if(modeCount >= 4){
    modeCount = 0;
  }
}



void button(){                      // a debounced button cycles through various 'modes' of the light
  int buttonState;
  int lastButtonState = 0;
  int reading = digitalRead(buttonPin);
  int debouncePeriod = 10;

  unsigned long oldButtonMillis;
  unsigned long buttonMillis;
  
  if(reading != lastButtonState){
    oldButtonMillis = millis();
  }

  if((millis() - oldButtonMillis) > debouncePeriod){
    if (reading != buttonState){
      buttonState = reading;
      if(buttonState == LOW){
        modeCount += 1;
        modeChange();                  //small effect to show that the mode has been changed
      }
    }
  }
  lastButtonState = reading;
}



void off(){                                // first mode
  digitalWrite(redPin1, LOW);
  digitalWrite(greenPin1, LOW);
  digitalWrite(bluePin1, LOW);
}



void solid(){                             // second mode
  digitalWrite(greenPin1, HIGH);
}



void pulse(){                            // third mode
  int value;
  int period = 8000;
  unsigned long time = 0;
  
  time = millis();
  value = 128+127*cos(2*PI/period*time);
  analogWrite(greenPin1, value);
}



void lightning(){                        // fourth, troublesome mode
  int firstFlash = random(150,256);
  unsigned long currentMillis = millis();
  unsigned long oldMillis = 0;
  int interval = random(2,6);
  int section = 0;
  int i = firstFlash;
  
  while((i > 0)&&(section == 0)){           //this is the initial 'big flash'
    currentMillis = millis();
    if(currentMillis - oldMillis > interval){
      oldMillis = currentMillis;
      analogWrite(redPin1, i);
      analogWrite(greenPin1, i);
      analogWrite(bluePin1, i);
      i--;
      if(i == 0){
        section = 1;
        analogWrite(redPin1, 0);
        analogWrite(greenPin1, 0);
        analogWrite(bluePin1, 0);
      }
    }
  }

  int k = random(200,401);                  // this is a pause before the next flash
  interval = 1;
  while(k > 0){
    currentMillis = millis();
    if(currentMillis - oldMillis > interval){
      oldMillis = currentMillis;
      k--;
    }
  }
  
  int afterFlash = random(50,200);
  int repeatValue = random(0,10);
  int j = repeatValue;
  while((j > 0)&&(section == 1)){          //this is the following flashes that happen after the first big flash
    afterFlash = afterFlash/2;
    i = afterFlash;
    while(i > 0){
      currentMillis = millis();
      delay(random(4,10));
      interval = random(6);
      if(currentMillis - oldMillis > interval){
        oldMillis = currentMillis;
        analogWrite(redPin1, i);
        analogWrite(greenPin1, i);
        analogWrite(bluePin1, i);
        i--;
        if(i == 0){
          analogWrite(redPin1, 0);
          analogWrite(greenPin1, 0);
          analogWrite(bluePin1, 0);
        }
      }
    }
    j--;
    if(j == 0){
      section = 2;
    }
  }

  k = random(601);                // another brief pause
  interval = 1;
  while(k > 0){
    currentMillis = millis();
    if(currentMillis - oldMillis > interval){
      oldMillis = currentMillis;
      k--;
    }
  }
  
  repeatValue = random(20);                  //this is code creates small 'distant' flashes on one led at a time.
  j = repeatValue;
  while((j > 0)&&(section == 2)){
    delay(random(10,800));
    int glowFlash = random(40);
    int flashSelect = random(5,8);
    i = glowFlash;
    while(i > 0){
      interval = random(6);
      currentMillis = millis();
      if(currentMillis - oldMillis > interval){
        oldMillis = currentMillis;
        analogWrite(flashSelect, i);
        i--;
        if(i == 0){
          analogWrite(flashSelect, 0);
        }
      }
    }
    j--;
    if(j == 0){
      section = 0;
    }
  }
}



void modeChange(){
  analogWrite(redPin1, 150);
  delay(100);
  analogWrite(redPin1, 0);
  delay(100);
  analogWrite(greenPin1, 150);
  delay(100);
  analogWrite(greenPin1, 0);
  delay(100);
  analogWrite(bluePin1, 150);
  delay(100);
  analogWrite(bluePin1, 0);
  delay(100);
}





// this is the original sketch for my lightning code
// it actually worked better, however because I used 
// delay();, the arduino couldn't detect a button change
// to change mode. Unfortunately my solution doesn't
// do that either, hence my problem.


//void lightning(){
//  int firstFlash = random(150,256);
//
//  for(int i = firstFlash; i > 0; i--){      // initial big flash
//      analogWrite(redPin1, i);
//      analogWrite(greenPin1, i);
//      analogWrite(bluePin1, i);
//      delay(random(2,6));
//  }
//  analogWrite(redPin1, 0);
//  analogWrite(greenPin1, 0);
//  analogWrite(bluePin1, 0);
//  delay(random(200,400));                   // brief pause after flash
//  
//  int afterFlash = random(50,200);          // successive repeated flashes/
//  int repeatValue = random(0,10);
//  for(int i = repeatValue; i>0; i--){
//    afterFlash = afterFlash/2;
//    delay(random(4,10));
//    for(int i = afterFlash; i>0; i--){
//      analogWrite(redPin1, i);
//      analogWrite(greenPin1, i);
//      analogWrite(bluePin1, i);
//      delay(random(6));
//    }
//  }
//  analogWrite(redPin1, 0);
//  analogWrite(greenPin1, 0);
//  analogWrite(bluePin1, 0);
//
//  delay(random(601));                         // brief pause after repeated flashes
//  repeatValue = random(0,20);                 
//  for(int i = repeatValue; i>0; i--){         // smaller distant flashes
//    int glowFlash = random(40);
//    int flashSelect = random(5,8);
//    delay(random(10,800));
//    for(int j = glowFlash; j >0; j--){
//      analogWrite(flashSelect, j);
//      delay(random(3,20));
//    }
//    analogWrite(flashSelect, 0);
//    delay(random(200));
//    int repeatGlowFlash;
//    repeatGlowFlash = random(4);
//    for(int j = repeatGlowFlash; j>0; j--){
//      analogWrite(flashSelect,j);
//      delay(random(6));
//    }
//    delay(random(100));
//    analogWrite(flashSelect, 0);
//  }
//}

If you want to maintain lastButtonState:
int lastButtonState = 0;
Should be:
static int lastButtonState = 0;

However, you do not need any debounce other than reading your switch every 50ms.

Why are you using delay() in the modeChange() function? :wink:

You caught me! I suppose it's such a fast effect, I didn't expect the button to be pushed during that time, so I cheated haha. but thank you for that suggestion!

EDIT: Removing the debounce helped. I guess my problem is just sloppy code! So that was a step in the right direction - thank you.

  interval = 1;

while(k > 0){
    currentMillis = millis();
    if(currentMillis - oldMillis > interval){
      oldMillis = currentMillis;
      k--;
    }
  }

This is EXACTLY the same as writing delay(k); Actually worse.

There's two ways to go here: 1. write it properly so no function takes longer than a millisecond or two and always returns control back to loop() so that loop() can read the button again. Or 2. write your own myDelay() function a little bit like the code above which reads the button during the delay and sets a global variable to let loop() know that the button was pressed at some time during the delay.

I actually had started to try the second option at one point - however I didn't really know how to write the actual delay code any simpler, so I didn't bother. I will try that though. Thank you.

For your switch sampling, here is one method:

. . .
  //is it time to check the switche
  if (currentMillis - SwitchMillis >= debounceMillis)
  {
    //code here runs every debounceMillis ms
    //get ready for the next iteration
    SwitchMillis += debounceMillis; 
    //go and check the switches
    checkSwitches();    
  } 

. . .

//****************** c h e c k S w i t c h e s ( ) *********************
//switches are checked every debounceValue milli seconds 
//no minimum switch press time is validated with this code (i.e. No glitch filter)
void checkSwitches()  
{
  boolean thisState;    

  //******************************
  //check if this switch has changed state
  thisState = digitalRead(startSwitch);
  if (thisState != laststartSwitchState)
  {  
    //update the switch state
    laststartSwitchState = thisState;  

    //this switch position has changed, do some stuff

    //"HIGH condition code"
    //switch went from LOW to HIGH
    if(thisState == HIGH)        
    {
      //Do some HIGH switch stuff here
    }

    //"LOW condition code"
    //switch went from HIGH to LOW
    else                          
    {
      //Do some LOW switch stuff here  
    }

  } //END of startSwitch code

  //*****************************************  
  //similar code for other switches goes here 
  //*****************************************  

} //END of checkSwitches()

//**********************************************************************

MorganS:

  1. write it properly so no function takes longer than a millisecond or two

This is exactly what I don't know what to do. Could you, or someone else point me in the right direction? Obviously that's a pretty broad topic to be able to easily answer, but I guess I don't understand why something like the following code is taking so long (longer than what is intended anyways):

  interval = 1;
  while(k > 0){
    currentMillis = millis();
    if(currentMillis - oldMillis > interval){
      oldMillis = currentMillis;
      k--;
    }
  }

Thanks again, you guys are a big help!

This is exactly what I don't know what to do

Do not use for loops that contain delays. If you have them then break the for loop down so that only one iteration gets done in one call.

Read this

http://www.thebox.myzen.co.uk/Tutorial/State_Machine.html

@grumpy_mike - thank you. That write up was very helpful and informative. I've been working to change my code. That is a much better way of doing it for sure. I'm having difficulty getting my 'checkSwitch' working right (it keeps looping nonstop after first press), but I think I should be able to figure that out.

Edit: It does work, however it seemed to not work when I had the Serial monitor running, and sending a message every button check. So it does work. Thank you!

Christian427:
@grumpy_mike - thank you. That write up was very helpful and informative. I’ve been working to change my code. That is a much better way of doing it for sure. I’m having difficulty getting my ‘checkSwitch’ working right (it keeps looping nonstop after first press), but I think I should be able to figure that out.

Edit: It does work, however it seemed to not work when I had the Serial monitor running, and sending a message every button check. So it does work. Thank you!

Of course you need to show what you have if you want comments.

Edit: I’m dumb and already see a mistake in another spot. If you want to see the code anyways though, there it is.

ReEdit: Ok, this time it works, but my pulse() loop is changing very very slowly. Like, one increment every few seconds, even though I told it to do it every millisecond. Why is this? I updated the code below to what I currently have. Thanks.

Ok, I’m still having trouble. Everything is working fine accept for my ‘pulse()’ loop. I broke down the original for() loop like grumpy_mike suggested, but the LED just sits there sorta half lit and kinda flickering. Here’s the code so far- thanks in advance.

int redPin1 = 5; 
int greenPin1 = 6;
int bluePin1 = 7;
int buttonPin = 1;

long int offMillis;
long int solidMillis;
long int pulseMillis;
long int buttonMillis;

int offTiming = 10000;
int solidTiming = 10000;
int pulseTiming = 1;
int buttonTiming = 50;

int value = 0;
boolean directionState = 0;

boolean pushState = HIGH;
unsigned long buttonHoldTime;

int modeCount = 0;

unsigned long k;

void setup() {
  pinMode(redPin1, OUTPUT);
  pinMode(greenPin1, OUTPUT);
  pinMode(bluePin1, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);

  modeChange();
  pushState = digitalRead(buttonPin);
  buttonHoldTime = millis();
  buttonMillis = millis();
  offMillis = millis();
  solidMillis = millis();
  pulseMillis = millis();
}

void loop() {
  if (millis() >= buttonMillis) checkButton();
  if ((millis() >= offMillis) && (modeCount == 0)) off();
  if ((millis() >= solidMillis) && (modeCount == 1)) solid();
  if ((millis() >= pulseMillis) && (modeCount == 2)) pulse();
}

void checkButton() {
  boolean reading = digitalRead(buttonPin);
  if (reading != pushState) {
    if ((millis() - buttonHoldTime) > 20) {
      pushState = reading;
      if (reading == LOW) {
        offTiming = millis();
        solidTiming = millis();
        pulseTiming = millis();
        if (modeCount > 2) {
          modeCount = 0;
          modeChange();
        }
        else {
          modeCount++;
          modeChange();
          offMillis = millis();
          solidMillis = millis();
          pulseMillis = millis();
        }
      }
    }
    else {
      buttonHoldTime = millis();
    }
  }
  buttonMillis = millis() + buttonTiming;
}



void solid() {
  digitalWrite(greenPin1, HIGH);
  solidMillis = millis() + solidTiming;
}



void pulse() {
  if (directionState == 0) {
    analogWrite(greenPin1, value);
    value++;
    if (value == 255) {
      directionState = 1;
    }
  }
  if (directionState == 1) {
    analogWrite(greenPin1, value);
    value--;
    if (value == 0) {
      directionState = 0;
    }
  }
  pulseMillis = millis() + pulseTiming;
}



void off() {
  digitalWrite(redPin1, LOW);
  digitalWrite(greenPin1, LOW);
  digitalWrite(bluePin1, LOW);

  offMillis = millis() + offTiming;
}

void modeChange() {
  analogWrite(redPin1, 100);
  delay(80);
  analogWrite(redPin1, 0);
  delay(80);
  analogWrite(greenPin1, 100);
  delay(80);
  analogWrite(greenPin1, 0);
  delay(80);
  analogWrite(bluePin1, 100);
  delay(80);
  analogWrite(bluePin1, 0);
  delay(80);
}

You need to read about ‘Data Types’.

boolean (8 bit) - simple logical true/false
byte (8 bit) - unsigned number from 0-255
char (8 bit) - signed number from -128 to 127. The compiler will attempt to interpret this data type as a character in some circumstances, which may yield unexpected results
unsigned char (8 bit) - same as 'byte'; if this is what you're after, you should use 'byte' instead, for reasons of clarity
word (16 bit) - unsigned number from 0-65535
unsigned int (16 bit)- the same as 'word'. Use 'word' instead for clarity and brevity
int (16 bit) - signed number from -32768 to 32767. This is most commonly what you see used for general purpose variables in Arduino example code provided with the IDE
unsigned long (32 bit) - unsigned number from 0-4,294,967,295. The most common usage of this is to store the result of the millis() function, which returns the number of milliseconds the current code has been running
long (32 bit) - signed number from -2,147,483,648 to 2,147,483,647
float (32 bit) - signed number from -3.4028235E38 to 3.4028235E38. Floating point on the Arduino is not native; the compiler has to jump through hoops to make it work. If you can avoid it, you should. We'll touch on this later.

You also need to ‘fully understand’ how you use millis() in timer creation.
Several things at the same time.

Please do not make major edits in your posts as it can make our comments moot.

Just make a new post for updates.

Ok, thank you.

Most of this post seems to have been invalidated by an edit. I'll leave it here for now.

millis() >= buttonTiming

The reason why you can't do this is called "millis rollover". See Nick Gammon's excellent page on how to avoid it.

I would also switch those if() statements around. The reason is because C (and C++) can "shortcut" an if() condition. If it sees a construction like A&&B then if it discovers A is false, then combined statement A&&B can never be true, so there's no point in evaluating B. In this case millis() is a somewhat-complex function and takes more time to evaluate than a simple modeCount==0. I put the simpler expression or the one most-likely to be false at the beginning, so that the shortcut can avoid calculating irrelevant things.

Note that shortcutting may not be good in every situation. That's why it's legal to write if(A && Serial.print(A)>1) but it's not a good idea - the serial print will not occur if A evaluates to false. (Serial.print returns the number of characters printed, if you were wondering.)

Then after doing that, it looks like you could have used a switch() statement here instead.

Which Arduino? Pin 7 is not capable of analogWrite() on an UNO.

There's nothing obviously wrong with the below but I do notice a few things that will cause you problems later...

void pulse() {
  if (directionState == 0) {
    analogWrite(greenPin1, value);
    value++;
    if (value == 255) {
      directionState = 1;
    }
  }
  if (directionState == 1) {
    analogWrite(greenPin1, value);
    value--;
    if (value == 0) {
      directionState = 0;
    }
  }
  pulseMillis = millis() + pulseTiming;
}

First, what happens if directionState starts with some value other than 0 or 1? Nothing happens. It's stuck. With a simple name like "directionState" it's possible that this might have been re-used elsewhere. What if you had some other function that used directions like left and right and up and down? Either give this a distinctive name like pulseDirectionState or make it a local variable inside this function. Since the value must be remembered after the function exits, make it static. Then you could have many different functions use the name directionState but they don't interfere with each other.

Second, value has a similar problem. What if it got set to something other than 0-255 by another function? Make it local and static.

Third, what if I changed directionState to start at 1 but I left value starting at zero? It will first change to -1 and keeps going after that. Instead of looking for exactly equal, use less-than-or-equal (or greater-than) so that even if it got damaged, it can be brought back in line. Then if you decide in the future that you prefer to take steps of 3, then you don't need to work out what factor of 3 is closest to 255.

Fourth, if(directionState==1) could just as easily be replaced by if(directionState). You don't care if it's 1 or 100, just if it's true or false. Then you could re-write the whole function to use if(){}else{} and you don't need to examine the same variable twice.

You still have a useless debounce - you set buttonTiming to 50 but then look to see if the bounces are less than 20ms apart. The buttonTiming is unnecessary. Remove that part and check the button on every single loop(). Then, looking at it closer, it appears you never update buttonTiming, so it is checking the button on every single loop.

modeCount can reach 3. You check if it's not greater than 2 and then increment it. If it is 2 then it will be incremented to 3 and that seems to have no light pattern associated with it.

Made a few changes.

Re-worked the checkButton code.

Do you see how the checkButton timer works?

You will still have to make other timer changes. To do, you still need to get rid of those delay()s :wink:

//Version 1

byte redPin1   = 5;
byte greenPin1 = 6;
byte bluePin1  = 7;
byte buttonPin = 1;

unsigned long  offMillis;
unsigned long  solidMillis;
unsigned long  pulseMillis;
unsigned long  buttonMillis;

int offTiming    = 10000;
int solidTiming  = 10000;
int pulseTiming  = 1;
int buttonTiming = 50;

int value = 0;
boolean directionState = 0;

boolean pushState = HIGH;

int modeCount = 0;

unsigned long k;

void setup()
{
  pinMode(redPin1, OUTPUT);
  pinMode(greenPin1, OUTPUT);
  pinMode(bluePin1, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);

  modeChange();
  pushState = digitalRead(buttonPin);

} //END of setup()

void loop()
{
  checkButton();

  if ((millis() >= offMillis) && (modeCount == 0))
  {
    off();
  }
  
  if ((millis() >= solidMillis) && (modeCount == 1)) 
  {
    solid();
  }
  
  if ((millis() >= pulseMillis) && (modeCount == 2)) 
  {
    pulse();
  }

} //END of loop()

//***********************************************************************
void checkButton()
{
  //************************************
  if (millis() - buttonMillis < buttonTiming)
  {
    //not time yet
    return;
  }

  //reset the timer
  buttonMillis = buttonMillis + buttonTiming;

  //************************************
  boolean reading = digitalRead(buttonPin);

  if (reading != pushState)
  {
    //update to the new state
    pushState = reading;

    //Has the switch gone LOW?
    if (reading == LOW)
    {
      offTiming   = millis();
      solidTiming = millis();
      pulseTiming = millis();

      if (modeCount > 2)
      {
        modeCount = 0;
        modeChange();
      }

      //The switch has gone HIGH
      else
      {
        modeCount++;
        modeChange();
        offMillis = millis();
        solidMillis = millis();
        pulseMillis = millis();
      }

    } //END of    if (reading == LOW)

  } //END of    if (reading != pushState)

} //END of    checkButton()


//***********************************************************************
void solid()
{
  digitalWrite(greenPin1, HIGH);
  solidMillis = millis() + solidTiming;
}



void pulse()
{
  if (directionState == 0)
  {
    analogWrite(greenPin1, value);
    value++;
    if (value == 255)
    {
      directionState = 1;
    }
  }
  if (directionState == 1)
  {
    analogWrite(greenPin1, value);
    value--;
    if (value == 0)
    {
      directionState = 0;
    }
  }
  pulseMillis = millis() + pulseTiming;
}



void off()
{
  digitalWrite(redPin1, LOW);
  digitalWrite(greenPin1, LOW);
  digitalWrite(bluePin1, LOW);

  offMillis = millis() + offTiming;
}

void modeChange()
{
  analogWrite(redPin1, 100);
  delay(80);
  analogWrite(redPin1, 0);
  delay(80);
  analogWrite(greenPin1, 100);
  delay(80);
  analogWrite(greenPin1, 0);
  delay(80);
  analogWrite(bluePin1, 100);
  delay(80);
  analogWrite(bluePin1, 0);
  delay(80);
}

Ok, thank you @MorganS, that was helpful!

First off, I apologize for editing my post/code from before - I now realize why that was dumb on my part and I’ll keep that in mind for future posts. I’ve rarely ever used forums before.

Second, I’m using an arduino Mega, and I’m fairly sure the pins I’m using are PMW pins.

Third, I appreciate your coding suggestions below. Those are the sort of things that are hard to learn when you’re teaching yourself how to code! So I appreciate it.

Lastly, there will be a third mode - lightning(). I have it commented out at the moment so I can understand this much correctly. Then I will add that in and probably rewrite it so it will work with the code I’m working on now. For now, it just cycles through that non-existing mode.

Also, thank you @larryd for the clarification on data types. I knew they all had different attributes, but it’s clear I was using some in ways I shouldn’t have been.

As a relevant aside, I was going off the link above from grumpy_mike to rework my code.

Grumpy_Mike:
Read this

State Machine

However I found the code in your suggested link to be slightly (though mostly the same concept) contradictory to what grumpy_mike has suggested.

larryd:
You also need to ‘fully understand’ how you use millis() in timer creation.
Several things at the same time.
https://forum.arduino.cc/index.php?topic=223286.0

Mostly it was different in the loop() and how the actual millis() timer was treated - whether it was actually in the loop, or whether it was in the functions that the loop pointed to. Which is better? Does it matter? I found the solution in grumpy_mike’s link to be more intuitive, but perhaps not.

In response to your second post that you made recently, I made those changes and the checkButton() loop makes much more sense now - it also works, I just tested it. Also, I still have more code to change, so removing those delay()s is next! Then I have to rewrite a 3rd loop that I have commented out yet.

For anyone following, here is the most current version of the code with all the edits and suggestions so far:

int redPin1 = 5;
int greenPin1 = 6;
int bluePin1 = 7;
int buttonPin = 1;

unsigned long offMillis;
unsigned long solidMillis;
unsigned long pulseMillis;
unsigned long buttonMillis;

int offTiming = 10000;
int solidTiming = 10000;
int pulseTiming = 1;
int buttonTiming = 50;

boolean pushState = HIGH;

int modeCount = 0;

unsigned long k;

void setup() {
  pinMode(redPin1, OUTPUT);
  pinMode(greenPin1, OUTPUT);
  pinMode(bluePin1, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);

  modeChange();
  pushState = digitalRead(buttonPin);
  buttonMillis = millis();
  offMillis = millis();
  solidMillis = millis();
  pulseMillis = millis();
}

void loop() {
  if (millis() >= buttonMillis) checkButton();
  if ((millis() >= offMillis) && (modeCount == 0)) off();
  if ((millis() >= solidMillis) && (modeCount == 1)) solid();
  if ((millis() >= pulseMillis) && (modeCount == 2)) pulse();
}




void checkButton() {
  boolean reading = digitalRead(buttonPin);
  if (reading != pushState) {
    pushState = reading;
    if (reading == LOW) {
      offTiming = millis();
      solidTiming = millis();
      pulseTiming = millis();
      if (modeCount > 2) {
        modeCount = 0;
        modeChange();
      }
      else {
        modeCount++;
        modeChange();
        offMillis = millis();
        solidMillis = millis();
        pulseMillis = millis();
      }
    }
  }
  buttonMillis = millis() + buttonTiming;
}



void solid() {
  digitalWrite(greenPin1, HIGH);
  solidMillis = millis() + solidTiming;
}



void pulse() {
  static boolean directionState = 0;
  static int value = 0;
  if (directionState) {
    analogWrite(greenPin1, value);
    value++;
    if (value >= 255) {
      directionState = 1;
    }
  }
  else {
    analogWrite(greenPin1, value);
    value--;
    if (value <= 0) {
      directionState = 0;
    }
  }
  pulseMillis = millis() + pulseTiming;
}



void off() {
  digitalWrite(redPin1, LOW);
  digitalWrite(greenPin1, LOW);
  digitalWrite(bluePin1, LOW);

  offMillis = millis() + offTiming;
}



void modeChange() {
  analogWrite(redPin1, 100);
  delay(80);
  analogWrite(redPin1, 0);
  delay(80);
  analogWrite(greenPin1, 100);
  delay(80);
  analogWrite(greenPin1, 0);
  delay(80);
  analogWrite(bluePin1, 100);
  delay(80);
  analogWrite(bluePin1, 0);
  delay(80);
}

Everything works, accept for the pulse() loop again. Not sure why it isn’t working, but it’s just sitting with a solid on light. So I’m assuming ‘value’ isn’t being updated, but I’m not sure why. I’ll be working on it, but feel free to leave comments if anyone has some. Thank you all again for being so helpful!

Which is better? Does it matter?
It’s all up to you the programmer, the checkButton() has timing in the function in post #15.

Did you look at the code posted in post #15?

EDIT:

Here is a big hint to getting rid of the delay()s using a state machine solution.

Okay it's more than a hint :wink:

//***********************************************************************
void modeChange()
{
  switch (ledState)
  {
    //*************
    case 0:
      {
        //do nothing
      }
      break;

    //*************
    case 1:
      {
        analogWrite(redPin1, 100);

        ledState = 2;
        commonMillis = millis();
      }
      break;

    //*************
    case 2:
      {
        if (millis() - commonMillis > 80)
        {
          analogWrite(redPin1, 0);

          ledState = 3;
          commonMillis = millis();
        }
      }
      break;

    //*************
    case 3:
      {
        if (millis() - commonMillis > 80)
        {
          analogWrite(greenPin1, 100);

          ledState = 4;
          commonMillis = millis();
        }
      }
      break;

    //*************
    case 4:
      {
        if (millis() - commonMillis > 80)
        {
          analogWrite(greenPin1, 0);

          ledState = 5;
          commonMillis = millis();
        }
      }
      break;

    //*************
    case 5:
      {
        if (millis() - commonMillis > 80)
        {
          analogWrite(bluePin1, 100);

          ledState = 6;
          commonMillis = millis();
        }
      }
      break;

    //*************
    case 6:
      {
        if (millis() - commonMillis > 80)
        {
          analogWrite(bluePin1, 0);

          ledState = 7;
          commonMillis = millis();
        }
      }
      break;

    //*************
    case 7:
      {
        if (millis() - commonMillis > 80)
        {
          //analogWrite(bluePin1, 0);

          //finished, go back to doing nothing
          ledState = 0;
        }
      }
      break;

  } //END of     switch (mState)

} //END of    modeChange()

//***********************************************************************

@larryd, I did look at the code in post 15, and made changes on what I saw... did I miss something? Or are you referring to how I set up the "if (millis() >= buttonMillis) checkButton();" loop in loop()? I only did it that way becuase I felt it wasn't necessary to check the button more than maybe every 50ms or so...

As for the delay solution, thank you! I may not copy your code exactly (because I am trying to learn on my own, after all!) but that will definitely be my guide and reference. Very helpful.

if (millis() >= buttonMillis) checkButton(); this is not a great way of doing the timer.

Stick with the method in post 15.

“I may not copy your code exactly”

Go ahead copy it but make sure you understand how things work.

If there are questions, ask them.