Pages: [1]   Go Down
Author Topic: Board unexpectedly resetting  (Read 646 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 4
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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  smiley). 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
Logged

'round the world...
Offline Offline
Faraday Member
**
Karma: 42
Posts: 3225
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

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).

Pittsburgh, PA, USA
Offline Offline
Faraday Member
**
Karma: 96
Posts: 4773
I learn a bit every time I visit the forum.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.


Logged

I find it harder to express logic in English than in Code.
Sometimes an example says more than many times as many words.

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 601
Posts: 48556
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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()).
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 4
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 4
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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]); 
}
Logged

United Kingdom
Offline Offline
Tesla Member
***
Karma: 224
Posts: 6593
Hofstadter's Law: It always takes longer than you expect, even when you take into account Hofstadter's Law.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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'.
Logged

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.

Offline Offline
Newbie
*
Karma: 0
Posts: 4
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

United Kingdom
Offline Offline
Tesla Member
***
Karma: 224
Posts: 6593
Hofstadter's Law: It always takes longer than you expect, even when you take into account Hofstadter's Law.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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.

Pages: [1]   Go Up
Jump to: