Code hangs at random

I am using the following code to stream RGB Addressable light strip (5M). Aside from setting parameters, the overall process is simple. Because AdaFruit disables interupts, there is a software handshaking, where the controllers asks for input data from the com port. The code receives a command from the PC to turn on light 1 and then it moves the light down the strand at a predetermined speed.

The issue I have is that the code works flawlessly for a period of time and then suddenly the controller hangs and wont respond. Pressing the hardware reset button, doesn't help, I have to unplug the controller from the USB port. The code might run without fail for hours or just minutes... Does anyone see anything I might be missing that could cause random hangs?

#include <Adafruit_NeoPixel.h>
#include <avr/wdt.h>
//  // Serial Input
//  // L:1:1:20:0:0    (Lane:count:red:green:blue)
//  // D:100    (Delay time)
//  // C:600  (Total number of lights per lane)

#include <Adafruit_NeoPixel.h>
Adafruit_NeoPixel lane1 = Adafruit_NeoPixel(600, 2, NEO_GRB + NEO_KHZ800);
Adafruit_NeoPixel lane2 = Adafruit_NeoPixel(600, 3, NEO_GRB + NEO_KHZ800);
Adafruit_NeoPixel lane3 = Adafruit_NeoPixel(600, 4, NEO_GRB + NEO_KHZ800);


int lightcount = 100;  //initialize maximum number of lights.
int lightdelay = 1000;  //initialize delay for light movement.
int serialdelay = 100;  //Check serial port every 500 miliseconds
byte lanetotal = 3;  //Total number of lanes to process in the loop
byte lightrefresh = 5;

void setup() {
  Serial.begin(9600);
  Serial.println("Serial Light Sequencer 10.2");
  Serial.println("System Ready");

  lane1.begin();
  lane1.show();  //Set all lights to off
  lane2.begin();
  lane2.show();  //Set all lights to off
  lane3.begin();
  lane3.show();  //Set all lights to off
}

void loop() {
  static unsigned long nextLightUpdate = millis();
  static unsigned long nextSerialUpdate = millis();
  static byte nextLightRefresh = 1;
  if (millis() > nextLightUpdate) {

    for (int i = lightcount; i >= 1 ; i--) {
      lane1.setPixelColor(i, lane1.getPixelColor(i - 1));
      lane1.setPixelColor(i - 1, 0);
      if (lanetotal >= 2) {
        lane2.setPixelColor(i, lane2.getPixelColor(i - 1));
        lane2.setPixelColor(i - 1, 0);
      }
      if (lanetotal >= 3) {
        lane3.setPixelColor(i, lane3.getPixelColor(i - 1));
        lane3.setPixelColor(i - 1, 0);
      }
    }
    nextLightRefresh ++;
    if (nextLightRefresh >= lightrefresh) {
      lane1.show();
      if (lanetotal >= 2) {
        lane2.show();
      }
      if (lanetotal >= 3) {
        lane3.show();
      }
      nextLightRefresh = 1;
    }
    //Serial.println("A");
    //delay (lightdelay);
    nextLightUpdate += lightdelay;
  }
  //Get Serial Data on interval
  if (millis() > nextSerialUpdate) {
    Serial.println("A");
    GetSerial();
    nextSerialUpdate += serialdelay;
  }
}

//void serialEvent() {
//  GetSerial();
//}

void SoftwareReboot()
{
  wdt_enable(WDTO_15MS);
  while (1)
  {
  }
}

//Read Serial Data
void GetSerial () {
  //Serial.flush();

  if (Serial.available()) {
    char *list[7];
    char buf[20] ;
    byte i;

    i = Serial.readBytesUntil('\n', buf, 20);
    buf[i] = '\0';  //terminate string with null
    //Serial.flush();
    //    Serial.println(buf);
    //    return;
    char *token = strtok(buf, ":");
    i = 0;
    list[i] = token;
    i++;
    while (token)
    {
      token =  strtok(NULL, ":"); // Use NULL as the 1st argument to keep parsing the same string
      list[i] = token;
      //Serial.println(token);
      i++;
    }
    char tmpproc;
    strncpy (&tmpproc, (list[0]), 1); //Copy Char* to Char * convert to INT for Switch comparison
    switch (tmpproc) {  //Check process
      case 'L':  //Set light
        switch (atoi(list[1])) {
          case 1:
            for (int i = 1; i <= byte(atoi(list[2])) ; i++) {
              lane1.setPixelColor(i, byte(atoi(list[3])), byte(atoi(list[4])), byte(atoi(list[5])));  //(light1,r,g,b)
            }
            break;
          case 2:
            if (lanetotal >= 2) {
              for (int i = 1; i <= byte(atoi(list[2])) ; i++) {
                lane2.setPixelColor(i, byte(atoi(list[3])), byte(atoi(list[4])), byte(atoi(list[5])));  //(light1,r,g,b)
              }
            }
            else
            {
              Serial.print("Invalid Lane:");
            }

            break;
          case 3:
            if (lanetotal >= 3) {
              for (int i = 1; i <= byte(atoi(list[2])) ; i++) {
                lane3.setPixelColor(i, byte(atoi(list[3])), byte(atoi(list[4])), byte(atoi(list[5])));  //(light1,r,g,b)
              }
            }
            else
            {
              Serial.print("Invalid Lane:");
            }

            break;
        }
        Serial.print((list[0]));
        Serial.print (":");
        Serial.print((list[1]));
        Serial.print(":");
        Serial.print((list[2]));
        Serial.print(":");
        Serial.print((list[3]));
        Serial.print (":");
        Serial.print((list[4]));
        Serial.print (":");
        Serial.println((list[5]));
        break;

      case 'D':  //Set Delay
        lightdelay = atoi(list[1]);
        Serial.print ("D:");
        Serial.println((list[1]));
        break;

      case 'C':  //Set Light Count
        lightcount = atoi(list[1]);
        Serial.print ("C:");
        Serial.println((list[1]));
        break;

      case 'S':  //Set Serial Delay
        serialdelay = atoi(list[1]);
        Serial.print ("S:");
        Serial.println((list[1]));
        break;
        //      case 'N':  //Set Light Number
        //        lightnumber = atoi(list[1]);
        //        Serial.print ("N:");
        //        Serial.println((list[1]));
        //        break;
      case 'R':  //Set Light Refresh Frequency (Strand)
        lightrefresh = atoi(list[1]);
        Serial.print ("R:");
        Serial.println((list[1]));
        break;
      case 'T':  //Set Total Lanes
        lanetotal = atoi(list[1]);
        Serial.print ("T:");
        Serial.println((list[1]));
        break;
      case 'X':  //Reset device
        Serial.println("System Reboot");
        SoftwareReboot();
        break;
    }
  }
}

bishop0114:
then suddenly the controller hangs and wont respond. Pressing the hardware reset button, doesn't help, I have to unplug the controller from the USB port.

There is too much code for lazy ol' me to study.

My wild guess is that there is too much data flowing and some buffer is getting full up. If pressing RESET won't work I am inclined to think the problem is on the PC.

Another (almost opposite) thought is that your use of readBytesUntil() may be causing the problem - what happens if it times out after only 1 character is received - or some other strange situation? But I would expect that to be solved by pressing RESET.

Edit to add ...

And another thought - is it possible the Arduino and the PC get out of step - the PC is waiting for a request while the Arduino is still waiting for a reply?

...R

    char *list[7];

These don't point to allocated memory. They point to random garbage. You need to EXPLICITLY initialize these to point to NULL.

Assigning them all to point to the same place makes little sense. But, that is what you are doing. You are NOT making them point to where token points to at the time. You'd need to use strdup() to do that, and you'd need to free the space when you were done with it.

    char tmpproc;
    strncpy (&tmpproc, (list[0]), 1); //Copy Char* to Char * convert to INT for Switch comparison

Is this somehow simpler to understand than

   char tmpproc = list[0][0];
        switch (atoi(list[1])) {

Dereferencing a pointer that might still point to random garbage hardly makes sense.

Great information. I found examples online of how to do the strncpy. I am pretty new to the Arduino code so any help is greatly appreciated! I will take a look and see if this helps.

Here is an update to my code. I am still getting the lockup. It seems to run fine for a couple hours and then it wont respond. I have tracked it down to what appears to be an issue with the com port.

I set the loop to turn the on-board LED on and off, to see if the code was still running and the light continues to blink but I cant get it to respond to the serial port and I do not get any output from the port. I can disconnect and reconnect the port but that is it.

#include <Adafruit_NeoPixel.h>
#include <avr/wdt.h>  //Used to software reset
#include <MemoryFree.h>  //Used to get free memory for error reporting.
//  // Serial Input
//  // L:1:1:20:0:0    (Lane:count:red:green:blue)

#include <Adafruit_NeoPixel.h>
Adafruit_NeoPixel lane1 = Adafruit_NeoPixel(600, 2, NEO_GRB + NEO_KHZ800);
Adafruit_NeoPixel lane2 = Adafruit_NeoPixel(600, 3, NEO_GRB + NEO_KHZ800);
Adafruit_NeoPixel lane3 = Adafruit_NeoPixel(600, 4, NEO_GRB + NEO_KHZ800);

int lightcount = 100;  //initialize maximum number of lights.
int lightdelay = 1000;  //initialize delay for light movement.
int serialdelay = 100;  //Check serial port every 500 miliseconds
int onboardled = 13;  //pin for onboard led Error Reporting
byte lanetotal = 3;  //Total number of lanes to process in the loop
byte lightrefresh = 5;  //Frequency the light strands are refreshed (lightrefresh * lightdelay)

void setup() {
  Serial.begin(9600);
  Serial.println("Serial Light Sequencer 11.0");
  Serial.println("System Ready");

  lane1.begin();
  lane1.show();  //Set all lights to off
  lane2.begin();
  lane2.show();  //Set all lights to off
  lane3.begin();
  lane3.show();  //Set all lights to off
  pinMode(onboardled, OUTPUT);
}

void loop() {
  static unsigned long nextLightUpdate = millis();
  static unsigned long nextSerialUpdate = millis();
  static unsigned long lastMillis = millis();
  static byte nextLightRefresh = 1;



  if (millis() > nextLightUpdate) {
    for (int i = lightcount; i >= 1 ; i--) {
      lane1.setPixelColor(i, lane1.getPixelColor(i - 1));
      lane1.setPixelColor(i - 1, 0);
      if (lanetotal >= 2) {
        lane2.setPixelColor(i, lane2.getPixelColor(i - 1));
        lane2.setPixelColor(i - 1, 0);
      }
      if (lanetotal >= 3) {
        lane3.setPixelColor(i, lane3.getPixelColor(i - 1));
        lane3.setPixelColor(i - 1, 0);
      }
    }
    nextLightRefresh ++;
    if (nextLightRefresh >= lightrefresh) {
      lane1.show();
      if (lanetotal >= 2) {
        lane2.show();
      }
      if (lanetotal >= 3) {
        lane3.show();
      }
      nextLightRefresh = 1;
    }
    nextLightUpdate += lightdelay;
  }
  //Get Serial Data on interval
  if (millis() > nextSerialUpdate) {
    Serial.println("A");
    // GetSerial();
    nextSerialUpdate += serialdelay;
    if (errtrap == 1) {
      Serial.print("millis=");
      Serial.println(millis());
    }
  }
  if (millis() < lastMillis) {  //Handles millis timer reset (rollover)
    nextSerialUpdate = millis() + serialdelay;
    nextLightUpdate = millis() + lightdelay;
  }
  lastMillis = millis();
}

void serialEvent() {
  GetSerial();
}

void SoftwareReboot()
{
  wdt_enable(WDTO_15MS);
  while (1)
  {
  }
}

//Read Serial Data
void GetSerial () {
  //Serial.flush();

  if (Serial.available()) {
    char *list[7];
    char buf[31] ;
    byte i = 0;
    //Return error information (free memory)


    i = Serial.readBytesUntil('\n', buf, 30);
    buf[i] = '\0';  //terminate string with null

    char *token = strtok(buf, ":");
    i = 0;
    list[i] = token;
    i++;
    while (token)
    {
      token =  strtok(NULL, ":"); // Use NULL as the 1st argument to keep parsing the same string
      list[i] = token;
      i++;
    }
    char tmpproc = list[0][0];
    switch (tmpproc) {  //Check process
      case 'L':  //Set light
        switch (atoi(list[1])) {
          case 1:
            for (int i = 1; i <= byte(atoi(list[2])) ; i++) {  //Turn on number of lights
              lane1.setPixelColor(i, byte(atoi(list[3])), byte(atoi(list[4])), byte(atoi(list[5])));  //(light1,r,g,b
           }
           break;
        }
        Serial.print((list[0]));
        Serial.print (":");
        Serial.print((list[1]));
        Serial.print(":");
        Serial.print((list[2]));
        Serial.print(":");
        Serial.print((list[3]));
        Serial.print (":");
        Serial.print((list[4]));
        Serial.print (":");
        Serial.println((list[5]));
        break;
    }
  }
}

I have an issue like this with windows 8.1, the com port becomes unresponsive and I have to reset the arduino to reconnect... Not sure if it's related to this though

bishop0114:
Here is an update to my code.

I suggest you get rid of serialEvent() and Serial.readBytesUntil() and use your own non-blocking code to read the data. There is an example in this demo. If you always recieve the same number of bytes you can simplify things by starting with if (Serial.available() >= numBytes).

I also suggest you read all the data into an array before trying to figure out any of the received values.

...R