Arduino DigiSpark NeoPixel BlueTooth C++ Buffer Overflow

156 views Asked by At

i was able to make the following code after about a week of desk head banging, it works, kind of. the problem is that it's not that responsive, most of the times i have to spam the buttons on my phone to send the same command over and over again until it catches up.

Can you help me clean the code a bit?

As you will see, at times it seems i over complicated things but that is only cause i found it works better this way then what it seems to be a more "logical" simpler version. i will lay the code and then i will explain and ask my questions.

#include <Adafruit_NeoPixel.h> // NeoPixel Lib
#include <SoftSerial.h>  // Serial Lib
#define LED_PIN    2
#define LED_COUNT 30

SoftSerial bluetooth(4, 5); // RX TX
Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

const byte numChars = 32;
char receivedChars[numChars];
char tempChars[numChars];
boolean newData = false;
boolean goLED0 = false;
boolean goLED1 = true;
boolean goLED2 = false;
boolean goLED3 = false;
boolean goLED4 = false;


int eFx = 1;
int rC1 = 255;
int gC1 = 0;
int bC1 = 0;
int xS = 20;
int xB = 125;


void setup() {

  bluetooth.begin (9600);
  strip.begin();           // INITIALIZE NeoPixel strip object (REQUIRED)
  strip.show();            // Turn OFF all pixels ASAP

}


void loop() {
  checkLedData();
  delay(50);
  runLED();
  delay(50);
}


void recvWithStartEndMarkers() {
    static boolean recvInProgress = false;
    static byte ndx = 0;
    char startMarker = '<';
    char endMarker = '>';
    char rc;
 
    while (bluetooth.available() > 0 && newData == false) {
        rc = bluetooth.read();

        if (recvInProgress == true) {
            if (rc != endMarker) {
                receivedChars[ndx] = rc;
                ndx++;
                if (ndx >= numChars) {
                    ndx = numChars - 1;
                }
            } else {
                receivedChars[ndx] = '\0'; // terminate the string
                recvInProgress = false;
                ndx = 0;
                newData = true;
            }
        }   else if (rc == startMarker) {
            recvInProgress = true;
        }
    }
}


void parseData() {      // split the data into its parts

    char * strtokIndx; // this is used by strtok() as an index

    strtokIndx = strtok(tempChars, ",");      // get the first part - the string
    eFx = atoi(strtokIndx);     // convert this part to an integer

    strtokIndx = strtok(NULL, ",");      // get the first part - the string
    rC1 = atoi(strtokIndx);     // convert this part to an integer
 
    strtokIndx = strtok(NULL, ","); // this continues where the previous call left off
    gC1 = atoi(strtokIndx);     // convert this part to an integer

    strtokIndx = strtok(NULL, ","); // this continues where the previous call left off
    bC1 = atoi(strtokIndx);     // convert this part to an integer

    strtokIndx = strtok(NULL, ",");
    xS = atoi(strtokIndx);     // convert this part to an integer

    strtokIndx = strtok(NULL, ",");
    xB = atoi(strtokIndx);     // convert this part to an integer

    strtokIndx = strtok(NULL, NULL);
}



void checkLedData() {
  recvWithStartEndMarkers();
  if (newData == true) {
    strcpy(tempChars, receivedChars);
    parseData();
    newData = false;
    strip.setBrightness(xB);
   if (eFx == 0) { 
     goLED0 = true;
     goLED1 = false;
     goLED2 = false;
     goLED3 = false;
     goLED4 = false;
    }
   if (eFx == 1) { 
     goLED0 = false;
     goLED1 = true;
     goLED2 = false;
     goLED3 = false;
     goLED4 = false;
    }
   if (eFx == 2) { 
     goLED0 = false;
     goLED1 = false;
     goLED2 = true;
     goLED3 = false;
     goLED4 = false;
    }
   if (eFx == 3) { 
     goLED0 = false;
     goLED1 = false;
     goLED2 = false;
     goLED3 = true;
     goLED4 = false;
    }  
   if (eFx == 4) { 
     goLED0 = false;
     goLED1 = false;
     goLED2 = false;
     goLED3 = false;
     goLED4 = true;
    }}}
    

void runLED() {
  if (goLED0 == true) {
            
  } 
    if (goLED1 == true) {
          colorWipe(strip.Color(rC1, gC1, bC1), xS);
          delay(50);
          recvWithStartEndMarkers();
              
  } 
    if (goLED2 == true) {
          colorWipe(strip.Color(bC1,rC1,gC1), xS);
          colorWipe2(strip.Color(rC1, gC1, bC1), xS);
          delay(50);
          recvWithStartEndMarkers();
              
  } 
    if (goLED3 == true) {
          colorWipe(strip.Color(rC1, gC1, bC1), xS);
          colorWipe2(strip.Color(rC1/2, gC1/2, bC1/2), xS);
          colorWipe(strip.Color(rC1/5, gC1/5, bC1/5), xS); 
          colorWipe2(strip.Color(rC1/10, gC1/10, bC1/10), xS);
          delay(50);
          recvWithStartEndMarkers();  
  } 
      if (goLED4 == true) {
          colorWipe(strip.Color(gC1,rC1,bC1), xS);
          colorWipe2(strip.Color(bC1,gC1,rC1), xS);
          colorWipe(strip.Color(bC1,rC1, gC1), xS);
          colorWipe2(strip.Color(rC1, gC1, bC1), xS);
          delay(50);
          recvWithStartEndMarkers();
             
  }
  
  
}



void colorWipe(uint32_t color, int wait) {
  
  for(int i=0; i<strip.numPixels(); i++) { // For each pixel in strip...
    strip.setPixelColor(i, color);         //  Set pixel's color (in RAM)
    strip.show();                          //  Update strip to match
    delay(wait);                           //  Pause for a moment
  }
}

void colorWipe2(uint32_t color, int wait) {
  
  for(int i=29; i<strip.numPixels(); i--) { // For each pixel in strip...
    strip.setPixelColor(i, color);         //  Set pixel's color (in RAM)
    strip.show();                          //  Update strip to match
    delay(wait);                           //  Pause for a moment
  }
}

So what i'm doing is i send a code from phone, eg: <1,255,255,255,100,100> it gets picked up by void recvWithStartEndMarkers() parsed then all values get stored as INTs by void parseData()

inside the loop i got void checkLedData() that is calling the 2 functions above and depending on the first INT it activates or deactivates those booleans, then i have void runLED() that checks what booleans are true and starts blinking the LEDS

At first i had the switches activate by the INT eFx but for some reason it was working very poorly so i decided to use that part of the code to flip the booleans, in the runLED() you'll notice that i keep calling over and over this function recvWithStartEndMarkers(); as it was the only way to actually make the board respond.

I'm not sure what is going on, i believe that it works because of a bufferoverflow issue, it crashes then it can take a new command , the first version after it took 1 command it just got stuck with it, the void colorWipe was working as the Leds were going on and off, switching colors and so on, but when i was trying to change the effect or colors it didn't responded at all.

This next code i used before all this:

void loop() {
  recvWithStartEndMarkers();
  if (newData == true) {
        strcpy(tempChars, receivedChars);
            // this temporary copy is necessary to protect the original data
            //   because strtok() used in parseData() replaces the commas with \0
  parseData();
  strip.setBrightness(xbrithness);
  controlLed();
  newData = false;
  }
}

void controlLed() {
         colorWipe(strip.Color(redColor, greenColor, blueColor), xSpeed);
         colorWipe(strip.Color( greenColor, redColor, blueColor), xSpeed);
}

now this was very responsive but the problem was that it would go through void controlLed() once and it stopped while if i was calling the same function outside recvWithStartEndMarkers(); it would go in loop, like i wanted to cause i was trying to make a looping effect.

Does anyone have any idea what i could do to make it responsive but still loop the functions to make a "light show"?

Now that i posted all this i was thinking, not sure if arduino is multitasking i wonder if ATtiny85 is multi tasking, so that might be the issue, it's so busy processing the codes that it won't listen on serial what is coming in, any way to go around that?

1

There are 1 answers

21
Delta_G On BEST ANSWER

I'm bored and feeling generous so here you go. See if you can follow this. It's uncompiled and untested so I can't guarantee it does exactly what you want, but see if you can understand what I'm trying to do here. Nothing ever stops and waits for anything to happen. There are no delay calls. There is no waiting. Just cycling through and see if it is time to do something.

One change I made that doesn't affect this, just makes typing easier was to take all your goLED variables and make an array out of them. Anytime you catch yourself putting numbers on variable names, use an array instead so the compiler has access to those numbers and you don't have to repeat yourself. See how much simpler the checkLedData function got.

Then I made the colorWipe functions so that they will return a boolean, true if they've completed and false if they haven't. Then the runLED function can just check that to see if it is time to move on to the next step. For the first case it is easy, just set the goLED variable to whatever the colorWipe returns. As long as it is still working it returns true and goLED[1] stays true and you keep calling the same colorWipe function. For the others we have to make them state machines, so I added a state variable. When any of them get finished they set their goLED variable back to false. When all of those are false, meaning there's no effect currently running, then runLED will fall all the way through to the last else statement and go to see if there is another command.

Like I said, there may be a mistake or two in there. But see if you can understand how I am writing a checklist to see what needs to happen instead of a story to tell one thing right after another.

#include <Adafruit_NeoPixel.h> // NeoPixel Lib
#include <SoftSerial.h>  // Serial Lib
#define LED_PIN    2
#define LED_COUNT 30

SoftSerial bluetooth(4, 5); // RX TX
Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

const byte numChars = 32;
char receivedChars[numChars];
char tempChars[numChars];
boolean newData = false;

boolean goLED[5];
    
int eFx = 1;
int rC1 = 255;
int gC1 = 0;
int bC1 = 0;
int xS = 20;
int xB = 125;


void setup() {

  bluetooth.begin (9600);
  strip.begin();           // INITIALIZE NeoPixel strip object (REQUIRED)
  strip.show();            // Turn OFF all pixels ASAP

}


void loop() {
  recvWithStartEndMarkers();
  runLED();
}


void recvWithStartEndMarkers() {
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char startMarker = '<';
  char endMarker = '>';
  char rc;

  while (bluetooth.available() > 0 && newData == false) {
    rc = bluetooth.read();

    if (recvInProgress == true) {
      if (rc != endMarker) {
        receivedChars[ndx] = rc;
        ndx++;
        if (ndx >= numChars) {
          ndx = numChars - 1;
        }
      } else {
        receivedChars[ndx] = '\0'; // terminate the string
        recvInProgress = false;
        ndx = 0;
        newData = true;
      }
    }   else if (rc == startMarker) {
      recvInProgress = true;
    }
  }
}


void parseData() {      // split the data into its parts

  char * strtokIndx; // this is used by strtok() as an index

  strtokIndx = strtok(tempChars, ",");      // get the first part - the string
  eFx = atoi(strtokIndx);     // convert this part to an integer

  strtokIndx = strtok(NULL, ",");      // get the first part - the string
  rC1 = atoi(strtokIndx);     // convert this part to an integer

  strtokIndx = strtok(NULL, ","); // this continues where the previous call left off
  gC1 = atoi(strtokIndx);     // convert this part to an integer

  strtokIndx = strtok(NULL, ","); // this continues where the previous call left off
  bC1 = atoi(strtokIndx);     // convert this part to an integer

  strtokIndx = strtok(NULL, ",");
  xS = atoi(strtokIndx);     // convert this part to an integer

  strtokIndx = strtok(NULL, ",");
  xB = atoi(strtokIndx);     // convert this part to an integer

  strtokIndx = strtok(NULL, NULL);
}



void checkLedData() {
  if (newData == true) {
    strcpy(tempChars, receivedChars);
    parseData();
    newData = false;
    strip.setBrightness(xB);
    for (int i = 0; i < 5; i++) {
      if (i == eFx) {
        goLED[i] = true;
      } else {
        goLED[i] = false;
      }
    }
  }
}





void runLED() {
  static int whichStep = 0;
  if (goLED[0] == true) {
    goLED[0] = false;
  }
  else if (goLED[1] == true) {
    goLED[1] = !colorWipe(strip.Color(rC1, gC1, bC1), xS)
  }

  else if (goLED[2] == true) {
    if (whichStep == 0) {
      if (colorWipe(strip.Color(bC1, rC1, gC1), xS)) {
        whichStep = 1;
      }
    }
    else if (whichStep == 1) {
      goLED[2] = !colorWipe2(strip.Color(rC1, gC1, bC1), xS));

    }
  }

  else if (goLED[3] == true) {

    if (whichStep == 0) {
      if ((colorWipe(strip.Color(rC1, gC1, bC1), xS)) {
      WhichStep = 1;
      }
    }
    else if (whichStep == 1) {
      if ((colorWipe2(strip.Color(rC1 / 2, gC1 / 2, bC1 / 2), xS)) {
        whichStep = 2;
      }
    }
    else if (whichStep == 2) {
      if ((colorWipe(strip.Color(rC1 / 5, gC1 / 5, bC1 / 5), xS)) {
        whichStep = 3;
      }
    }
    else if (whichStep == 3) {
      if ((colorWipe2(strip.Color(rC1 / 10, gC1 / 10, bC1 / 10), xS)) {
        goLED[3] = false;
      }
    }
  }
  
  else if (goLED[4] == true) {
    // You write this one
    goLED[4] = false;
  }
  else {
    checkLedData();  // get next command
    whichStep = 0;
  }

}



boolean colorWipe(uint32_t colot, int wait) {
  static unsigned long lastMillis = millis();
  static int state = 0;
  static int ledIndex = 0;

  if (state == 0) {
    lastMillis = millis();
    ledIndex = 0;
    strip.setPixelColor(ledIndex, color);
    strip.show();
    state = 1;
  }
  if (state == 1) {
    if (millis() - lastMillis >= wait) {
      lastMillis = millis();
      ledIndex++;
      strip.setPixelColor(ledIndex, color);
      strip.show();
      if (ledIndex == strip.numPixels() - 1) {
        state = 0;
        return true;
      }
    }
  }
  return false;
}


boolean colorWipe2(uint32_t colot, int wait) {
  static unsigned long lastMillis = millis();
  static int state = 0;
  static int ledIndex = 0;

  if (state == 0) {
    lastMillis = millis();
    ledIndex = strip.numPixels() - 1;
    strip.setPixelColor(ledIndex, color);
    strip.show();
    state = 1;
  }
  if (state == 1) {
    if (millis() - lastMillis >= wait) {
      lastMillis = millis();
      ledIndex--;
      strip.setPixelColor(ledIndex, color);
      strip.show();
      if (ledIndex == 0) {
        state = 0;
        return true;
      }
    }
  }
  return false;
}