Return not exiting the void loop?

Hello, I am trying to slightly modify the Adafruit Neopixel default routines. I have a button with an interrupt that changes the mode, and several modes that light LEDs in different ways.

This is a rough sketch of my code, for it is very long:

int mode = 1;
void loop(){
  if (mode == 1){
  rainbow(10);
  }

if (mode == 2){
  rainbowCycle(5);
  }
}
void rainbowCycle(uint8_t wait) {
  startMode = mode;
  uint16_t i, j;

  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(wait);
    if (startMode != mode){
      return;
    }
  }
}

void isr(){
  if (digitalRead(2) == HIGH){
    mode += 1;
  }
}

My interrupt to change the mode works fine -- the int for mode changes instantly, but within the rainbow() and rainbowCycle() subroutines, I have checks to see if the mode has changed from when it began. If the state has changed, it returns, but those returns aren't leaving the routine the way I'd expect...

Am I understanding return; incorrectly? Is there any reason why this shouldn't exit the subroutines immediately, if it's checking for a mode change as often as it's displaying the lights' colors?

I think you need "break" instead of "return".

I have just tried replacing all instances of return with break, and am still having the same issue. Thank you for helping suggest solutions :slight_smile:

Mode should be declared as volatile.

it is never called, your condition that is supposed to branch to return statement is false

This fixed the issue. Thank you very much.

In the Arduino IDE, use Ctrl T or CMD T to format your code then copy the complete sketch.

Use the </> icon from the ‘reply menu’ to attach the copied sketch.

and what difference would it make?

You can break from a for or while loop. You should use return to leave a function.

So I was doing it right the first time, just needed the volatile modifier. Good to know. Thanks again, everyone.

That's a hacky way to accomplish what you want. But if it works for you .....

What is hacky about it?

Manual switches connected to interrupts is an extremely poor mix.

Interesting. Is there a better way to do it? I have interrupts on two switches, one to change the mode, and one to turn the brightness to zero / max (essentially turning the sign on and off).

I suppose the brightness button could be made into a switch that toggles the connection altogether... but how could the mode button be anything other than an interrupt?

Thanks for your input :slight_smile:

How would we know this is what you have ?


In the Arduino IDE, use Ctrl T or CMD T to format your code then copy the complete sketch.

Use the </> icon from the ‘reply menu’ to attach the copied sketch.

What happens when mode reaches 10 :thinking:

My description of the project I'm working on was irrelevant to my request for clarification, I just thought it would be interesting additional context. The purpose of the buttons doesn't change my question about your statement:

Why are manual switches connected to interrupts an extremely poor mix?

Complete sketch:

#include <Adafruit_NeoPixel.h>
#define NUM_LEDS 18  // number of LEDs in the strip
#define LED_PIN 4  // pin to which the Neopixel strip is connected
Adafruit_NeoPixel strip = Adafruit_NeoPixel(NUM_LEDS, LED_PIN, NEO_GRB + NEO_KHZ800);
int brightness = 0; // current brightness of the LEDs
int fadeAmount = 5; // how much to fade the LEDs by
int currentPixel = 0;
volatile int mode = 3;
int startMode = 3;
unsigned long timesince;
int nextRando = 0;
int lastRando = 0;
bool bright = true;

uint32_t red = strip.Color(255, 0, 0);
uint32_t green = strip.Color(0, 255, 0);
uint32_t blue = strip.Color(0, 0, 255);
uint32_t yellow = strip.Color(255, 255, 0);
uint32_t white = strip.Color(255, 255, 255);
uint32_t cyan = strip.Color(0, 255, 255);
uint32_t orange = strip.Color(255, 165, 0);
uint32_t purple = strip.Color(200, 3, 255);
uint32_t  colors[] = {red, green, blue, yellow, white, cyan, orange, purple};

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  strip.begin();
  strip.setBrightness(255);
  strip.show();
  attachInterrupt(digitalPinToInterrupt(2), onOff, HIGH);
  attachInterrupt(digitalPinToInterrupt(3), changeMode, HIGH);
  timesince = millis();
}

void loop() {

  if (mode == 1) {
    while (nextRando == lastRando) {
      nextRando = random(8);
    }
    chase(colors[nextRando]);
    lastRando = nextRando;
  }


  else if (mode == 2) {   // SLOW-CHANGE RAINBOW FLASH
    rainbow(40);
  }


  else if (mode == 3) {
    rainbow(5);
  }

  else if (mode == 4) {   // SLOW MOVING RAINBOW WHEEL
    rainbowCycle(20);
  }


  else if (mode == 5) {   // FASTER RAINBOW WHEEL
    rainbowCycle(1);
  }

}


void onOff() {
  if (millis() - timesince > 1000) {
    timesince = millis();
    if (bright == false) {
      strip.setBrightness(255);
      Serial.println("Brightness set to max.");
      bright = true;
    } else if (bright == true) {
      strip.setBrightness(0);
      Serial.println("Brightness set to 0.");
      bright = false;
    }
    strip.show();
  }
}

void changeMode() {
  if (millis() - timesince > 1000) {
    timesince = millis();
    mode += 1;
    if (mode == 9) {
      mode = 1;
    }
    Serial.print("I have just run the changeMode routine. The state is now: ");
    Serial.print(mode);
    Serial.println(".");
  }
}

static void chase(uint32_t c) {
  startMode = mode;
  for (uint16_t i = 0; i < strip.numPixels(); i++) {
    strip.setPixelColor(i  , c); // Draw new pixel
    strip.show();
    delay(50);
    if (startMode != mode){
      break;
    }
  }
}

void rainbow(uint8_t wait) {
  uint16_t i, j;
  startMode = mode;
  for (j = 0; j < 256; j++) {
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel((i + j) & 255));
    }
    strip.show();
    delay(wait);
    if (startMode != mode) {
      break;
    }
  }
}


void rainbowCycle(uint8_t wait) {
  startMode = mode;
  uint16_t i, j;

  for (j = 0; j < 256 * 5; j++) { // 5 cycles of all colors on wheel
    for (i = 0; i < strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(wait);
    if (startMode != mode) {
      break;
    }
  }
}


uint32_t Wheel(byte WheelPos) {
  WheelPos = 255 - WheelPos;
  if (WheelPos < 85) {
    return strip.Color(255 - WheelPos * 3, 0, WheelPos * 3);
  }
  if (WheelPos < 170) {
    WheelPos -= 85;
    return strip.Color(0, WheelPos * 3, 255 - WheelPos * 3);
  }
  WheelPos -= 170;
  return strip.Color(WheelPos * 3, 255 - WheelPos * 3, 0);
}

What happens when mode reaches 10 :thinking:

I have a debouncing routine built in. The reason I didn't include the entire sketch was because I only needed help on one specific part, and my question was answered without needing the entire sketch. Nevertheless, I see the importance of copying the whole thing, I just wanted a quick response and didn't want to overwhelm potential answerers with the 200+ lines, when my question only revolved around maybe 10.

Switches can bounce 10s to 100s of times on a push.

How are your switches wired ?

5V to one side of switch, 5K resistor to ground, and digital pin on the other side. The ISR for changing modes and brightness has a debouncing mechanic built into it; shouldn't be called more than once per second.