Go Down

Topic: Board unexpectedly resetting (Read 720 times) previous topic - next topic

jestermaximus

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: [Select]
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

bubulindo

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?
Eu não sou o teu criado. Se respondo no fórum é para ajudar todos mediante a minha disponibilidade e disposição. Responder por mensagem pessoal iria contra o propósito do fórum e por isso evito-o.
Se realmente pretendes que eu te ajude por mensagem pessoal, então podemos chegar a um acordo e contrato onde me pagas pela ajuda que eu fornecer e poderás então definir os termos de confidencialidade do meu serviço. De forma contrária toda e qualquer ajuda que eu der tem de ser visível a todos os participantes do fórum (será boa ideia, veres o significado da palavra fórum).
Nota também que eu não me responsabilizo por parvoíces escritas neste espaço pelo que se vais seguir algo dito por mim, entende que o farás por tua conta e risco.

Dito isto, mensagens pessoais só se forem pessoais, ou seja, se já interagimos de alguma forma no passado ou se me pretendes convidar para uma churrascada com cerveja (paga por ti, obviamente).

GoForSmoke

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.


Nick Gammon on multitasking Arduinos:
1) http://gammon.com.au/blink
2) http://gammon.com.au/serial
3) http://gammon.com.au/interrupts

PaulS

Code: [Select]
   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()).

jestermaximus

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: [Select]
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

jestermaximus

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

Code: [Select]
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]); 
}

dc42

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'.
Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

jestermaximus

Ah ha! That solved the problem. Thank you. One final quick question: My output looks like this
Quote
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?  :smiley-roll-blue:).

Thanks,
Jester

dc42

Looks like old data in the PC serial buffer to me. What gets printed if you press the reset button on the Arduino?
Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

Go Up