Why I can't exit this while loop by reading from the Serial?

Question

I'm trying to work with the FastLed libraries and i'm controlling a Led Stip via a WEMOS D1 MINI based on the ESP8266, what I'm trying to achieve is to jump from fastled animation to another one..those animations are inside a While loop as shown in the code, so , i thought to just read again from the serial in order to change an integer to exit those while loop, but for some reason is not working as expected. Anyone can help me to uderstand?

CRGB leds[NUM_LEDS];
int incomingByte = 0;
int exit = 0;
void setup() {
 Serial.begin(115200);
 delay( 1000 ); 
 FastLED.addLeds<LED_TYPE, LED_PIN, COLOR_ORDER>(leds, NUM_LEDS).setCorrection( TypicalLEDStrip );
 FastLED.setBrightness( BRIGHTNESS );
}
void loop() 
{
 while (Serial.available()>0) 
 {
 Serial.write("Serial.available");
 incomingByte = Serial.read();
 switch (incomingByte)
 {
 case '1':
 GreenYellow();
 break;
 case '2':
 Sparkle();
 break;
 }
 }
}
void Sparkle() {
 while(exit = 0){
 int Pixel = random(NUM_LEDS);
 setPixel(Pixel,random(255),random(255),random(255));
 FastLED.show();
 delay(50);
 setPixel(Pixel,0,0,0);
 if(Serial.available()>0){
 incomingByte = Serial.read();
 if(incomingByte != '2'){
 exit = 1;
 }
 }
 }
 }
}
void GreenYellow()
{
 for(int x =0; x< NUM_LEDS/2; x++){
 leds[x]= CRGB::Green;
 }
 
 for(int x =NUM_LEDS/2; x< NUM_LEDS; x++){
 leds[x]= CRGB::Yellow;
 }
 FastLED.show();
}
void setPixel(int Pixel, byte red, byte green, byte blue) 
{
 leds[Pixel].r = red;
 leds[Pixel].g = green;
 leds[Pixel].b = blue;
 FastLED.show();
}
void setAll(byte red, byte green, byte blue) {
 for(int i = 0; i < NUM_LEDS; i++ ) {
 setPixel(i, red, green, blue);
 }
 FastLED.show();
}

Bonus question: I read that there is no thread programming on the ESP2866 via the Arduino IDE, am I right?

Answer 1

The problem is that you check inside the two functions Sparkle and GreenYellow for another serial byte. Then when it's unequal, you move back to the while loop where you wait for the next serial byte, but I think you need to use the same one to decide to go to the correct function instead of waiting for a next serial byte.

Probably the easiest is to not read serial in the while loop but check the incomingByte global variable (containing the last read byte, and e.g. 1 initially), and ONLY in the two functions set that global variable. Then, when returning to the while loop, you check that global variable to jump to the correct function.

So you make a setup function that sets the global variable to 1 (or you do it in the initialization itself):

int incomingByte = 1; // Start with GreenYellow

Then, in the loop function, you use:

void loop() 
{
 switch (incomingByte)
 {
 case '1':
 GreenYellow();
 break;
 case '2':
 Sparkle();
 break;
 }
}

If you don't want to start in either one, then use 3 values (0, 1, 2), and ONLY in the loop function read the Serial byte, but don't read it in the functions. This way it's a bit more cleaner as it separates the LED functionality with the Serial functionality.

The result (using the last method), results in the following code (compiles, not checked).

Comments:

• Used an enumeration for the mode

• Split functionality of serial data, processing the mode and RGB routines

• I added some initialization lines (i.e. constants) to make it compile.

  #include <FastLED.h>
  #define NUM_LEDS 60 // number of led present in your strip
  #define LED_PIN 6
  #define BRIGHTNESS 100
  enum EMode
  {
   ModeNone,
   ModeSparkle,
   ModeGreenYellow
  };
  CRGB leds[NUM_LEDS];
  EMode mode;
  void ParseSerialData();
  void ProcessMode();
  void GreenYellow();
  void Sparkle();
  void setPixel(int Pixel, byte red, byte green, byte blue);
  void setup() 
  {
   Serial.begin(115200);
   delay( 1000 ); 
   FastLED.addLeds<WS2811, LED_PIN, EOrder::RGB>(leds, NUM_LEDS).setCorrection( TypicalLEDStrip );
   FastLED.setBrightness( BRIGHTNESS );
   mode = EMode::ModeNone;
  }
  void loop() 
  {
   ParseSerialData();
   ProcessMode();
  }
  void ParseSerialData()
  {
   while (Serial.available() > 0) 
   {
   Serial.write("Serial.available");
   char incomingByte = Serial.read();
   switch (incomingByte)
   {
   case '1':
   mode = EMode::ModeGreenYellow;
   break;
   case '2':
   mode = EMode::ModeSparkle;
   break;
   default:
   // Ignore
   break;
   }
   }
  }
  void ProcessMode()
  {
   switch (mode)
   {
   case EMode::ModeNone:
   // Do nothing
   break;
   case EMode::ModeGreenYellow:
   GreenYellow();
   break;
   case EMode::ModeSparkle:
   Sparkle();
   break;
   default:
   // Ignore
   break;
   }
  }
  void Sparkle() 
  {
   int Pixel = random(NUM_LEDS);
   setPixel(Pixel,random(255),random(255),random(255));
   FastLED.show();
   delay(50);
   setPixel(Pixel,0,0,0);
  }
  void GreenYellow()
  {
   for(int x =0; x< NUM_LEDS/2; x++)
   {
   leds[x]= CRGB::Green;
   }
   for(int x =NUM_LEDS/2; x< NUM_LEDS; x++)
   {
   leds[x]= CRGB::Yellow;
   }
   FastLED.show();
  }
  void setPixel(int Pixel, byte red, byte green, byte blue) 
  {
   leds[Pixel].r = red;
   leds[Pixel].g = green;
   leds[Pixel].b = blue;
   FastLED.show();
  }
  void setAll(byte red, byte green, byte blue) 
  {
   for(int i = 0; i < NUM_LEDS; i++ ) {
   setPixel(i, red, green, blue);
   }
   FastLED.show();
  }
  

• FabioEnne: thanks for the replay, can you modify my piece of code? I'm have difficulties to understand your solution, so maybe reading the code will help. Thanks.
• Gerben: I'd go the other way. Leave all the serial reads inside the loop function. Remove the while loop inside the Sparkle function. Store the current mode that was selected in a variable. Then, in loop, call the function based on the current mode.
• Michel Keijzers: @Gerben Me too, but the first was the most 'easiest', not the most clean solution.
• Michel Keijzers: I tried to modify the code (it compiles)... I removed the exit code and tried to clean up things.