RGB Cycle Stop Works On Simulation But Not On Breadboard

Projects Programming
1

So I have a code to cycle colors on an RGB LED and then added a button to pause the RGB cycle on a color when it is pressed. The code works on the Tinkercad Simulator, but when I transfer it to my breadboard all it does is cycle the colors and stop on green every time once the button is pressed. Why is this working in the simulator but not on the breadboard? Below is the code and a short video to better show what I'm talking about.

const int redPin = 11;
const int greenPin = 10;
const int bluePin = 9;

unsigned int rgbColour[3];

int buttonPin = 2;
int temp = 1;

void setup() {
  // Start off with the LED off.
  setColourRgb(0, 0, 0);
  pinMode(buttonPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(buttonPin), Pause, FALLING);
}

void loop() {

  if (temp == 1) {
    // Start off with red.
    rgbColour[0] = 255;
    rgbColour[1] = 0;
    rgbColour[2] = 0;

    // Choose the colours to increment and decrement.
    for (int decColour = 0; decColour < 3; decColour += 1) {
      int incColour = decColour == 2 ? 0 : decColour + 1;

      // cross-fade the two colours.
      for (int i = 0; i < 255; i += 1) {
        rgbColour[decColour] -= 1;
        rgbColour[incColour] += 1;
        setColourRgb(rgbColour[0], rgbColour[1], rgbColour[2]);
        if (temp == 0) {
          return;
        }
        delay(5);
      }
    }
  }
  else {
    // Do nothing pause
  }
}

void setColourRgb(unsigned int red, unsigned int green, unsigned int blue)
{
  analogWrite(redPin, red);
  analogWrite(greenPin, green);
  analogWrite(bluePin, blue);
}

void Pause()
{
  if (temp == 1) {
    temp = 0;
  }
  else {
    temp = 1;
  }
}

Here is the video: RGB Cycling Video (YouTube)

2

There is no switch debounce in your ISR. It will be called multiple times for several milliseconds when ever it is pressed, because of the rapid bouncing of the switch contacts. It's like spinning a roulette wheel, since there are an unknown even or odd number of pulses, it can either change the state, or not, depending on chance.

Your "pause" logic doesn't seem to do what you say it should. When you invoke 'return;', what will happen is the loop() will begin again from the start. Then your whole cross fade for loop will run again from the beginning. That behaviour will continue until a switch press changes 'temp' again. It's hard to imagine how this can work, even if you say it works in the simulator. It doesn't seem to fit the definition of a "pause".

By the way, don't you think there is a better name for "temp"? To speak to what it actually does?

I would imagine a "pause" to look something like:

  if (temp == 1) {
      for (int i = 0; i < 255; ;) {
        rgbColour[decColour] -= 1;
        rgbColour[incColour] += 1;
        setColourRgb(rgbColour[0], rgbColour[1], rgbColour[2]);
          i++;
        }
   }

But it will not remember the current colour and start from there when cross fade is enabled. Instead it will start from the beginning colour. To do that requires more complicated logic.

You can delete this, it does nothing:

  else {
    // Do nothing pause
  }
3

Thanks a lot for your detailed response, I will have to go in a different direction with this then.

4

So I made the changes you suggested and ended up the error "setColourRgb' was not declared in this scope".

5

That was just an untested snippet. Probably something about the context. If you post the entire revised sketch, we can have a look. It's not a complete solution anyway... I feel that there may be a fix that someone can throw in. It's too late in the day for me. :slight_smile:

6

I completely understand, you guys have been so helpful. Everything I know about this has been from YouTube or from here so I'm learning as I go as I don't know much right now. Just trying to make an RGB light box for my kids and using it as a learning project for myself. Here's the entire code I'm using.

const int redPin = 11;
const int greenPin = 10;
const int bluePin = 9;

unsigned int rgbColour[3];

int buttonPin = 2;
int temp = 1;

void setup() {
  // Start off with the LED off.
  setColourRgb(0, 0, 0);
  pinMode(buttonPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(buttonPin), Pause, FALLING);
}

void loop() {

  if (temp == 1) {
      for (int i = 0; i < 255; ;) {
        rgbColour[decColour] -= 1;
        rgbColour[incColour] += 1;
        setColourRgb(rgbColour[0], rgbColour[1], rgbColour[2]);
          i++;
        }
   }

    // Choose the colours to increment and decrement.
    for (int decColour = 0; decColour < 3; decColour += 1) {
      int incColour = decColour == 2 ? 0 : decColour + 1;

      // cross-fade the two colours.
      for (int i = 0; i < 255; i += 1) {
        rgbColour[decColour] -= 1;
        rgbColour[incColour] += 1;
        setColourRgb(rgbColour[0], rgbColour[1], rgbColour[2]);
        if (temp == 0) {
          return;
        }
        delay(5);
      }
    }
  }
}

void setColourRgb(unsigned int red, unsigned int green, unsigned int blue)
{
  analogWrite(redPin, red);
  analogWrite(greenPin, green);
  analogWrite(bluePin, blue);
}

void Pause()
{
  if (temp == 1) {
    temp = 0;
  }
  else {
    temp = 1;
  }
}
7

It's just one too many brackets here:

        delay(5);
      }
    }
  }
}
8

I removed one and then go this error code:

Arduino: 1.8.12 (Windows 10), Board: "Arduino Uno"

C:\Users\coffe\Desktop\sketch_jun20a\sketch_jun20a.ino: In function 'void loop()':

sketch_jun20a:20:32: error: expected primary-expression before ';' token

for (int i = 0; i < 255; :wink: {

^

sketch_jun20a:20:32: error: expected ')' before ';' token

sketch_jun20a:20:33: error: expected primary-expression before ')' token

for (int i = 0; i < 255; :wink: {

^

exit status 1
expected primary-expression before ';' token

This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.

9

Oh, I see what happened. You added my code, it was supposed to replace a section:

const int redPin = 11;
const int greenPin = 10;
const int bluePin = 9;

unsigned int rgbColour[3];

int buttonPin = 2;
int runState = 1;

void setup() {
  // Start off with the LED off.
  setColourRgb(0, 0, 0);
  pinMode(buttonPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(buttonPin), Pause, FALLING);
}

void loop() {
  // nothing happens in loop unless runState is asserted
  if (runState == 1) {

    // Choose the colours to increment and decrement.
    for (int decColour = 0; decColour < 3; decColour += 1) {
      int incColour = decColour == 2 ? 0 : decColour + 1;

      // cross-fade the two colours.
      for (int i = 0; i < 255; i += 1) {
        rgbColour[decColour] -= 1;
        rgbColour[incColour] += 1;
        setColourRgb(rgbColour[0], rgbColour[1], rgbColour[2]);
        if (runState == 0) {
          return;
        }
        delay(5);
      }
    }
  }
}

void setColourRgb(unsigned int red, unsigned int green, unsigned int blue)
{
  analogWrite(redPin, red);
  analogWrite(greenPin, green);
  analogWrite(bluePin, blue);
}

void Pause()
{
  if (runState == 1) {
    runState = 0;
  }
  else {
    runState = 1;
  }
}
10

Oh, I'm sorry about that. So now the RGB runs its cycle and when the button is pressed, it finishes the cycle and then turns off. It turns back on if I press the button again and just cycles thru again.

11

I shouldn't be trying to do this now, I hate rushing. The problem here is that the initialization of the for loop variables also has to be made conditional on 'runState'.

const int redPin = 11;
const int greenPin = 10;
const int bluePin = 9;

unsigned int rgbColour[3];

int buttonPin = 2;
int runState = 1;

void setup() {
  // Start off with the LED off.
  setColourRgb(0, 0, 0);
  pinMode(buttonPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(buttonPin), Pause, FALLING);
}

void loop() {

  // Choose the colours to increment and decrement.
  for (int decColour = 0; decColour < 3;) {
    int incColour = decColour == 2 ? 0 : decColour + 1;
    // cross-fade the two colours.
    for (int i = 0; i < 255;) {
      rgbColour[decColour] -= 1;
      rgbColour[incColour] += 1;
      setColourRgb(rgbColour[0], rgbColour[1], rgbColour[2]);
      delay(5);
      if (runState == 1) {
        i++;
      }
    }
    if (runState == 1) {
      decColour++;
    }
  }
}

void setColourRgb(unsigned int red, unsigned int green, unsigned int blue)
{
  analogWrite(redPin, red);
  analogWrite(greenPin, green);
  analogWrite(bluePin, blue);
}

void Pause()
{
  if (runState == 1) {
    runState = 0;
  }
  else {
    runState = 1;
  }
}
12

You don't even have to help me with this, let alone rush to help. I'm in no hurry, just grateful to have help. I tried the code but once I press the button the LED now just starts going a bit crazy switching colors without an actual smooth cycle.

13

So here is the problem. You've succeeded (except for debounce) to register the button event asynchronously with the main code, which is looping colours to the LEDs. But you need to use that event to control the main code, so it can do something different. But the inherent structure of the main code is non multi tasking. You have to make your own. Any control structure that repeats, such as while{} and for{}, must be translated to a non-blocking, cooperatively multitasking approach. Unfortunately, for{} statements are slightly painful, but a general gist of it is like:

for(i=3; i<6; i++) {<somecode>}

decomposes as the sequence

i=3;
while (i<6) {
<somecode>
i++;
}

For the state machine you would do something like

if (state == INITIALIZE) {
 i=3;
 state = RUN;
}
else if (state == RUN and i<6) {
 <somecode>
 i++;
}
else {
 state = IDLE;
}

To activate it, you just assert:

state = INITIALIZE;

Or, you can use a switch case statement:

switch (state) {

case:INITIALIZE
 i=3;
 state = RUN;
 break;

case:RUN
 if (i<6) {
 <somecode>
 i++;
 break;

 state = IDLE;
}

So not only that, but you would need to merge the run/stop logic into the above scheme. The trick is that you can check run/stop input from the button ISR variable, in the RUN state. You can keep skipping the RUN code until the runState becomes true. That way, the initialization and comparison states won't be affected and it will appear to run the same as the original for{} loops.
The thing is that since there is no central task switcher as in an RTOS, the main code has to actually actively monitor for asynchronous events that it needs input from.

So you might have something like:

if (state == INITIALIZE) {
 i=3;
 state = RUN;
}
else if (state == RUN and i<6 and runState == 1) {
 <somecode>
 i++;
}
else {
 state = IDLE;
}

Also if you want to keep repeating the for{} sequences continously, you can just go directly from IDLE to INITIALIZE instead.

14

I understood your explanation. Unfortunately, it seems I have tried adding a feature to my project that is way more complicated than I thought it would be and I'm way in over my skills for this one for now. I really appreciate your explanation though, I did learn a lot.