Exiting a loop or making it repeat (Neopixel)

Hello,

it is one of my first animations with neopixels. I want to make a raibow flow that goes in both directions of the strip and be able co control it using serial commands.

I took as a starting point the rainbow in the strandtest of AdafruitNeopixel library.
For this purspose I adapted it for the 2 directions.

Now, learning about “break”, I managed to be able to stop a loop and to switch between them.

The problem is that the loops doesn’t repeat for an infinite time ad also, I’m not totally sure if this is the correct method of stopping the loops and easily switch between them.

Thanks

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
 #include <avr/power.h>
#endif

#define LED_PIN    12

#define LED_COUNT 30

Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

void setup() {

  Serial.begin(9600);

  strip.begin();
  strip.show();
  strip.setBrightness(30);
}

void loop() {
  char e = Serial.read();

if(e == 'l'){
counterClockwise(1);
}

else if(e == 'r'){
clockWise(1);
}

else if(e == 'n'){
 strip.show();
}

}

void counterClockwise(int wait){
     for(long  colorFirstPixel= 0; colorFirstPixel < 5*65536; colorFirstPixel += 256) {
      for(int i=0; i<strip.numPixels(); i++) {
      
      int pixelHue = colorFirstPixel + (i * 65536L / strip.numPixels() );
     
      strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
    }
    strip.show();
    delay(35);

char e = Serial.read(); 
    if(e == 'n'){
      break;
    }
   }
  }

void clockWise(int wait){
  for(long  colorFirstPixel= 5*65536; colorFirstPixel > 0; colorFirstPixel -= 256) {
    for(int i=0; i<strip.numPixels(); i++) {
      
      int pixelHue = colorFirstPixel + (i * 65536L / strip.numPixels());
     
      strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
    }
    strip.show();
    delay(35);

char e = Serial.read(); 
    if(e == 'n'){
      break;
    }
   }
  }

You forgot to check Serial.available() before doing your Serial.read(). You’re reading zeroes because when you read without checking, most of the time there is nothing there.

I'm not totally sure if this is the correct method of stopping the loops and easily switch between them.

In my opinion your whole concept is wrong. That sounds a bit harsh, and you have definitely got something that works, but to use a 'delay()' based method, and then to exit the loop is not really the way. If you want to stick to what you are doing now you can however create a function that tests for 'Serial.available()' (as Aarg pointed out correctly) and then tests for possible inputs, and returns a value according to them, which you can use to exit the loop and possibly enter a different one. There are better ways, let me leave it at that.

Ok so I made some changes with the Serial.available() and 2 more if to try make it loop for an infinite time. But…it still doesn’t loop for an infinite amount of time…

I’m aware that probably this isn’t the best way of doing this but I don’t know how to do it in an another way. I just took pieces of the Adafruit strandtest

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
 #include <avr/power.h>
#endif

#define LED_PIN    12

#define LED_COUNT 30

Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

int c = 2;

void setup() {

  Serial.begin(9600);

  strip.begin();
  strip.show();
  strip.setBrightness(30);
}

void loop() {
  if(Serial.available()){
  char e = Serial.read();

if(e == 'l'){
counterClockwise(c);
  if(c <2 ){
  c++;
}
}

else if(e == 'r'){

clockWise(c);
  if(c < 2){
  c++;
}
}

else if(e == 'n'){
 strip.show();
}
}
}

void counterClockwise(int wait){
     for(long  colorFirstPixel= 0; colorFirstPixel < 5*65536; colorFirstPixel += 256) {
      for(int i=0; i<strip.numPixels(); i++) {
      
      int pixelHue = colorFirstPixel + (i * 65536L / strip.numPixels() );
     
      strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
    }
    strip.show();
    delay(15);

char e = Serial.read(); 
    if(e == 'n'){
      break;
    }
   }
  }

void clockWise(int wait){
  for(long  colorFirstPixel= 5*65536; colorFirstPixel > 0; colorFirstPixel -= 256) {
    for(int i=0; i<strip.numPixels(); i++) {
      
      int pixelHue = colorFirstPixel + (i * 65536L / strip.numPixels());
     
      strip.setPixelColor(i, strip.gamma32(strip.ColorHSV(pixelHue)));
    }
    strip.show();
    delay(15);

char e = Serial.read(); 
    if(e == 'n'){
      break;
    }
   }
  }

Check out the “state machines” and “Blink without delay” examples in the tutorials sticky.

To do more than one thing at once you need to get out of the “do this, wait, do that”, ‘linear’ style of programming.

Think more like cooking a meal, for example a stir fry and rice....
Do you weigh the rice, put the water on to boil, wait for it to boil, put the rice in, wait for it to cook while stirring continuously, drain and put it on the plate. Before finally startIng on the stir fry?
Or do you do a bit of each task (e.g. rice, then stir fry...) so it looks like you are doing both at the same time?

So instead of doing loops to get your effects (i.e. doing the complete “cook rice” task) you need to do one step of it, then check the serial port, etc.

On your first version you incorrectly wrote over the value of e with zeros because you did not check Serial.available(). On your second version you correctly checked Serial.available() but then only call the clockwise / counter clockwise functions if you read a character. If you just rearrange the code so that you keep the value of the e variable when there is nothing to read it should work.

You should get in the habit of printing messages to the serial port to figure out what’s going on in the code.

void loop() {
  if (Serial.available()) {
    char e = Serial.read();
  }
  if (e == 'l') {
    counterClockwise(c);
    if (c < 2 ) {
      c++;
    }
  }

  else if (e == 'r') {

    clockWise(c);
    if (c < 2) {
      c++;
    }
  }

  else if (e == 'n') {
    strip.show();
  }

}

Oook now it works almost as initially intended.

Thanks to everyone that helped