Troublle interpreting string from serial.read

Hi All

I have tried to make a small program which recieves a string from serial.read splits it into to an array of results for each ; but it returns jibborish… the code is

void setup()
{
  // start serial port at 9600 bps:
  Serial.begin(9600);
  pinMode(2, INPUT);   // digital sensor is on digital pin 2
}

void loop()
{
 int i=0;
 char buf[7];
  char serialString[20];
// recieves string from serial when available
  if(Serial.available()){
    delay(100);
    while( Serial.available() && i< 19) {
       serialString[i++] = Serial.read();
    }
    serialString[i++]='\0';
    
    char *result[20];
    char *p = serialString;
    char *str;
    while ((str = strtok_r(p, ";", &p)) != NULL){ // delimiter is the semicolon
      result[i++] = strdup(str);
    }
    free(str);
    str = NULL;
    Serial.flush();
    Serial.println("You wrote ");
    Serial.println(result[0]);
    Serial.println(result[1]);
  }
}

And the reply to “1;2;3;” is this…

You wrote
ûPSI RVA?â$
X

What did I miss??? I guess the string I read from serial isn’t a string like char string=(“1;2;3;”); but what can I do instead?

Thanks in advance

    serialString[i++]='\0';

So, what is in serialString now?

    while ((str = strtok_r(p, ";", &p)) != NULL){ // delimiter is the semicolon
      result[i++] = strdup(str);
    }

The strtok_r() function is the thread-safe version of strtok(). Since the Arduino is not running a multi-threaded operating system, why are you using the more complicated version of the function?

Where did you reset i?

    free(str);

You are making many copies of data in str, then freeing the last one. Wouldn't want to leak that last little bit of memory now would we. The oceans of data lost prior to this are OK?

    Serial.flush();

Why?

    Serial.println("You wrote ");
    Serial.println(result[0]);
    Serial.println(result[1]);

But, you didn't put anything in result[0] or result[1], so why are you printing them?

Hi Paul
Thanks for a quick reply :slight_smile: The answer for most of Your questions are that I am a noob…
What I have done is to use other peoples code test them a bit and then combine them without knowing everything about the functions…
after

serialString[i++]='\0'; 
Serial.print(serialString);

I get 1;2;3;

I used strtok_r from a thread You helped another guy… so I stole the idea tested it and it worked alone…

The free(str) was also from this previously mentioned thread:) I would of course like to clear all the wasted memory.

the Serial.flush(); was a leftover from some old code no need for it…

I tried to devide the serialString for each ; into the result array I know it is a pointer but it worked for me in this

void loop(){
  char *result[16];
  char *p = buf;
  char *str;
  while ((str = strtok_r(p, ";", &p)) != NULL){ // delimiter is the semicolon
    result[i++] = strdup(str);
  }
  for (i=0; i<=6; i++) Serial.println(result[i],bin);
while(1);
}

so I thought it would work here as well.

I get 1;2;3;

Excellent.

I used strtok_r from a thread You helped another guy.... so I stole the idea tested it and it worked alone...

I've often recommended strtok(). I have never recommended strtok_r(), but a number of people have tried to use that version anyway. Why is a mystery when strtok() is much easier to use.

The free(str) was also from this previously mentioned thread:) I would of course like to clear all the wasted memory.

Quit using strtok_r() and you won't have that need.

I tried to devide the serialString for each ; into the result array I know it is a pointer but it worked for me in this

In that situation, i apparently defaults to 0.

The code you posted would probably work if you added

i = 0;

before the while loop that parses the data.

The way that you use Serial.available isn't correct and your declaration of the variables as local to loop() will cause untold misery :-). If there's only one character available when you enter loop(), you will store it in serialString[0] and then exit the loop function (there's no guarantee that the delay will be long enough for the whole string to be in the buffer). The next time the loop function is entered and there's a character available you will store it in serialString[0] overwriting the previous character because i is reset to zero again. Also, when you leave the loop function, the variables "i", "buf" and "serialString" which were created on the stack upon entry to loop, will be destroyed and recreated on reentering loop. Those three variables need to be declared outside the loop function, i.e. as globals. Second, there are any number of ways of storing and parsing the input but they depend upon what you are going to do with the three values when you've successfully read the string. If you need them as three separate strings the parsing is somewhat different than if you need to convert and store the strings as integers.

Pete

@Paul when I was searching for a ref for strtok I read Your reply to http://arduino.cc/forum/index.php/topic,79966.0.html and got the idea of making my own code for it… and it works :0D.

#define SEP ';'

int i=0,i2=0;
char inChar,serialString[10][4];
      int temp[10];

void setup()
{
  // start serial port at 9600 bps:
  Serial.begin(9600);
}

void loop()
{
// recieves string from serial when available
  if(Serial.available()){
    delay(100);
    for (i=0; i<10; i++) serialString[i][0]='\0';
    i=i2=0;
    while( Serial.available() && (i2<10) && (i<2)) 
    {
      inChar = Serial.read();
      if (inChar==SEP) 
      {
      serialString[i2][i++]='\0';
      i2++;
      i=0;
    }
    else serialString[i2][i++]=inChar;
    }
    serialString[i2][i++]='\0';
    Serial.flush();
    for (i=0; serialString[i][0]!= '\0'; i++){
      temp[i] = atoi(serialString[i]);
      if (temp[0]==10) Serial.println(serialString[i]);
    }
  }
}

@el_supremo the definitions were there due to my lazyness… I need to convert them to int but I thought I would learn to spilt them first :slight_smile:

but now this part of the code works as I need it to :smiley:
Thanks

If you actually need them as integers there is no need to “split” them. You can just read in the string and then convert the integers - you could even convert them as they are read in if you’re feeling adventurous. But you are still using Serial.available incorrectly. Even if it works for you right now, you will eventually use this same reading technique and someday it will bite you in very mysterious ways.

This is untested but should work:

// semicolon counter
int sccount = 0;
// index of next character in string
int sindex = 0;
// the string
char c_string[20];
// the three converted integers
int a_int[3];
loop()
{
  while(Serial.available() > 0) {
    // store the next character
    c_string[sindex] = Serial.read();
    // If it is a semicolon, count it
    if(c_string[sindex++] == ';')sccount++;
    // Put a null at the end of the current string
    c_string[sindex] = 0;
    // If we have three semicolons, we're done
    if(sccount == 3)break;
  }
  if(sccount == 3) {
    // Now parse the string into three integers
    s_index = 0;
    for(int i = 0;i < 3;i++) {
      a_int[i] = atoi(&c_string[s_index]);
      // skip to the next semicolon
      while(c_string[s_index] != ';')s_index++;
      // skip over the semicolon
      s_index++;
    }
    // zero the counts just in case there's a next time
    s_index = 0;
    ccount = 0;
  }
}

Pete