next topic »

[on:]

Hello,

I'm new to arduino and programming in c++ in general but have some experience in java. I'm attempting to make a function that would parse an incoming string and place the resulting (int)'s into an array (input looks like "123,52,152," or "1,412,54," each number can be 1,2 or 3 digits... output is an int[]).

However even on the most simple testbed, every time my function is called, the board resets. It seems to output 2 of the three expected numbers correctly, but the first number always comes back as a much larger one. I've been reading about memory leaks in c++ (still don't have a real good handle on it), and thought that might be an issue (i guess i'm pretty lazy, coming form java  ). Is that a valid concern given the limited size of my program?

Any suggestions about my ineptitude or a more efficient way of parsing this data would be greatly appreciated!

(ps. probably doesn't matter, but there is nothing connected to the board, i'm just running the program)

Code:

void setup(){
  Serial.begin(9600);
  Serial.println("setup");
  delay(10);
}

//loops once every second.
void loop (){
  Serial.println("looping");
  delay(10);
  //This string is would come over a serial port.
  String output = "123,152,35,";
  parseData(output);
  delay(3000);
}

//Takes the string from the input and parses it into an int array. using ',' as a delimiter Finally prints out the vale
void parseData(String input){
  int currentBrightnessArray[] = {0,0,0};
  int returnCounter = 0;
  int currentNum = 0;
 
  //the loop looks for the delimiter and adds each digit (currently type char) into a substring,
  //which is turned into an int. then the space in the string is set to ' ' and trimmed off.
  while(input.length() > 0){
   input.trim();
   int nextCommaPosition = input.indexOf(',');
   input.setCharAt(nextCommaPosition, ' ');
   char currentSub[nextCommaPosition-1];
   
   for(int counter = nextCommaPosition-1; counter >= 0; counter--){
      currentSub[counter] = input.charAt(counter);
      input.setCharAt(counter, ' ');
   }
   int temp = atoi(currentSub);
   currentBrightnessArray[returnCounter] = atoi(currentSub);
   returnCounter++;
  }   
 
  //values of array are printed out.
  Serial.print("Output =");
  Serial.print(currentBrightnessArray[0]);
  Serial.print(", ");
  Serial.print(currentBrightnessArray[1]);
  Serial.print(", ");
  Serial.println(currentBrightnessArray[2]); 
}

Thanks!
~Jester

[Reply #1 on:]

Do you really really need to use String?

Wouldn't it be simpler to use a char array, strtok and atoi? 

I think the problem you're having is calling atoi without an end delimiter and also your while loop looks funny. What happens to input the second time you run it?

[Reply #2 on:]

You don't even need strtok or atoi, or a char array if you get the bytes from serial and convert as they come in. But you will do as you want and if it works then wth?

You could write some debug printing into the parse routine to get some idea what it is doing.

[Reply #3 on:]

Code:

   char currentSub[nextCommaPosition-1];

No, you can't do this. You can't dynamically allocate space in a while loop like this.

As the others have mentioned, ditch the stupid String class. It is horribly inefficient. Use a char array, and some professionally written parsing and conversion functions (strtok() - NOT strtok_r() - and atoi()).

[Reply #4 on:]

Thanks for all the feedback, I've changed the code to use both strtok and atoi rather than the mess I was using before. Unfortunately  setup() is still being called after loop() is done executing.  I assume this means the board is crashing for some reason.

Code:

void setup(){
  Serial.begin(9600);
  Serial.println("setup");
  delay(10);
}

//loops once every second.
void loop (){
  Serial.println("looping");
  delay(10);
  //This string is would come over a serial port.
  char output [12]= {'1','2','3',',','2','2','6',',','1','0','0',','};
  parseData(output);
  delay(3000);
}

void parseData(char input []){
  int currentBrightnessArray[] = {0,0,0};
  int returnCounter = 0;
  int currentNum = 0;

  for (int counter = 0; counter < 3; counter++){
   
   char *currentSub = NULL;
   currentSub = strtok (input, ",");

   while (currentSub != NULL){
     currentBrightnessArray[returnCounter] = atoi(currentSub);
     currentSub = strtok (NULL, ",");
     returnCounter++;
   }
  }   
 
  //values of array are printed out.
  Serial.print("Output =");
  Serial.print(currentBrightnessArray[0]);
  Serial.print(", ");
  Serial.print(currentBrightnessArray[1]);
  Serial.print(", ");
  Serial.println(currentBrightnessArray[2]); 
}

Thanks again,
Jester

[Reply #5 on:]

realized I had an unnecessary for loop in there, but the result is the same.

Code:

void parseData(char input []){
  int currentBrightnessArray[] = {0,0,0};
  int returnCounter = 0;
  int currentNum = 0;
 
  //the loop looks for the delimiter and adds each digit (currently type char) into a substring,
  //which is turned into an int. then the space in the string is set to ' ' and trimmed off.
   
   char *currentSub = NULL;
   currentSub = strtok (input, ",");

   while (currentSub != NULL){
     currentBrightnessArray[returnCounter] = atoi(currentSub);
     currentSub = strtok (NULL, ",");
     returnCounter++;
   }
 
 
  //values of array are printed out.
  Serial.print("Output =");
  Serial.print(currentBrightnessArray[0]);
  Serial.print(", ");
  Serial.print(currentBrightnessArray[1]);
  Serial.print(", ");
  Serial.println(currentBrightnessArray[2]); 
}

[Reply #6 on:]

strtok() expects its first argument to be a null-terminated string. Therefore, you need to add an extra null character at the end of 'output', or alternatively replace the final comma with a null character i.e. '\0'.

[Reply #7 on:]

Ah ha! That solved the problem. Thank you. One final quick question: My output looks like this

se26, 100
setup
looping
Output =123, 226, 100
looping
Output =123, 226, 100
looping
Output =123, 226, 100

What is that being printed out at the top? Is that old data in the serial port or something (does that statement even make sense?  ).

Thanks,
Jester

[Reply #8 on:]

Looks like old data in the PC serial buffer to me. What gets printed if you press the reset button on the Arduino?