Data parsing question

I have a program in Visual C++ that will send a string of serial data to my Arduino. There will be two numbers, separated by a symbol. I can change the symbol in the code if it's necessary. The numbers are each 0-100. For example, a few strings could look like
0X0
100X100
15X25
35X95
You get the idea.
I need to read this data into two separate variables in the Arduino so I can display each individually, as well as perform calculations on them.

Here is some example code I wrote, but I realized quickly I'm in over my head on this one.

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

void setup(){

  lcd.begin(2, 24);

  Serial.begin(115200);

}

void loop()
{
int i=0;
int j=0;
char var1 [3];
char var2 [3];
  
  if (Serial.available()) {      
        delay(100);                  
          
        i = 0; 
        j = 0;   

        while (Serial.available()){  
          if (Serial.read()!="X"){
              var1[i] = Serial.read();
              i++;}
           else{
              var2[j] = Serial.read();
              j++;}
          }
       }

  lcd.clear(); 
  lcd.write(var1[]);
  setCursor(0,1);
  lcd.write(var2[]); 

  }

I'm sure this isn't anywhere close to correct, it obviously won't compile due to me not having a clue what I'm doing with character arrays, but am I at least on the right track?

Any help is greatly appreciated, just a push in the right direction would be wonderful.

Thanks,
Steve

Push in the right direction:

  1. Accumulate characters in a buffer until you get a newline:
if (Serial.available()) {
  buf[index] = Serial.read();
  if (buf[index] == '\n') {
    buf[index] = 0;
    interpret(buf);
    index=0;
  } else {
    index++;  // Check for overflow!
  }
  1. Split up the buffer using 'X' as a separator:
void interpret(char *buf)
{
   char *p;
   int var1, var2;

   p = strtok(buf, 'X');
   var1 = atoi(p);
   p = strtok(0, 'X');
   var2 = atoi(p);

   lcd.clear();
   lcd.write(var1);
   setCursor(0,1);
   lcd.write(var2);
 ...
}

Might not work exactly as written, but hopefully it's the right direction :slight_smile:

--
Check out our new shield: http://www.ruggedcircuits.com/html/gadget_shield.html

Below is some simple test code I made for receiving and parcing serial data that might be of interest.

//zoomkat 8-12-10 roborealm serial servo parce test


#include <WString.h> //provides easy string handling
String readString = String(100);
String parce1 = String(10);
String parce2 = String(10);
int pos = 0;
int ind1 = 0;
int ind2 = 0;

 
#include <Servo.h> 
Servo myservo;  // create servo object to control a servo 

void setup() {
      Serial.begin(9600);
        myservo.attach(9);
        }

void loop() {

        //expect a string like wer,qwe rty,123 456,hyre kjhg,
        while (Serial.available()) {
        delay(10);  
          if (Serial.available() >0) {
        char c = Serial.read();  //gets one byte from serial buffer
        if (c == ',') { goto parce;}
        readString.append(c); } //makes the string readString
        }
      
      
      parce:  
      if (readString.length() >0) {
      Serial.println(readString);
      pos = readString.length(); //capture string length
      ind1 = readString.indexOf(' '); //position of the space
      parce1 = readString.substring(0, ind1); //first part of string
      parce2 = readString.substring(ind1+1, pos); //second part of string
      Serial.println(parce1);
      Serial.println(parce2);
      Serial.println();
      
      //int n;
      //n = atoi(readString); //convert string to number
      //myservo.writeMicroseconds(n);
      //myservo.write(n);
      readString="";
      } 
   }
goto parce;

Bad boy, zoomkat. The break command will get out of the while loop just as well, without the need for spaghetti code.

Bad boy, zoomkat. The break command will get out of the while loop just as well, without the need for spaghetti code.

Indeed, however in this case it's functionally identical, in fact GCC will generate exactly the same output in this type of construct (jump to the next instruction outside a loop), and it may be argued that "break" is more opaque than "goto" in this case. No spaghetti code in sight, IMHO.

in fact GCC will generate exactly the same output

By this argument, do you want to plead for using assembly language in the first place :o

Thanks for all the help everyone. I was able to get zoomkat's code to work, seems pretty nifty. The only interesting thing I found is it won't print 0's to the LCD, just blanks. All the other numbers are fine. Any idea?

(a bit off topic, however...)

if(c == ',') {goto parse;}

It is a big "no-no" to use "goto" in C based code, or at least frond upon. The reason is that the C language was built to be a structured language and 'goto' is not the common form of program control (as in unstructured languages such as FORTRAN or BASIC).

Since C uses the statements "break" and "continue", there is little need for "goto". However, there is a few narrow set of programming situations that you may use them and not be frond upon. One of them is jumping outside a set of deeply nested loops.

In this code, using the statement "break" would be more in the spirit of C programming, as it is only a single nested loop.


It may be good to include a serial.flush() after ',' has been received.


De-bouncing on serial input is also not recommended, as serial data received is usually considered good. (while serial.availiable()... if serial.availiable())

It may be good to include a serial.flush() after ',' has been received.

NO! NO! NO! NEVER!

Something is sending the Arduino serial data, with the expectation that ALL of the serial data being sent will be processed. Serial.flush() throws away all data that has been received and stored in the serial buffer, but not yet processed. How much data gets thrown away? There is no way of knowing. That's why dumping random amounts of data is NOT a good idea.

If there is a problem with processing the amount of data that is being sent, it is a much better idea to implement hand-shaking, so the Arduino tells the sender that it is ready for more data.

Unfortunately in this case handshaking would make the project considerably more difficult. The other portion of the program, written in Visual C++, receives data from a Neurosky Mindset and does the parsing and error checking, then transmits the data to the Arduino. I was barely able to get the code working as is, adding a handshaking routine to it would be well beyond my realm. I do agree, it would be the more proper way to do things.

Bad boy, zoomkat. The break command will get out of the while loop just as well, without the need for spaghetti code.

Well, I don't think what I wrote would be generally considered "spaghetti code". "Spaghetti code" is usually a term used by those when they can't follow the logic flow in a program for whatever reason. Computers have no problem with goto when properly used. Just to see how "appropriate" code would look, please post up the same code using break instead of goto so they can be directly compared.

De-bouncing on serial input is also not recommended, as serial data received is usually considered good. (while serial.availiable()... if serial.availiable())

I've tried getting rid of the double checking, but have issues. Please post up the code without the double check so I can see what works.

I've tried getting rid of the double checking, but have issues

What issues do you have with serial.available?


And as for avoiding the goto, you should always try to "hide your methods from main", so I'd suggest that you make a function that loops around until ',' is found.

ie

uint8_t get_substring() {

  //wait for input
  while(!Serial.available()) {/*do nothing*/
  }
  
  //for a maximum of 10 charactors
  for(uint8_t i = 0; i <10; i++) {
    
    //is there input left?
    if (Serial.available()) {
       
      //if so, get it
      char c = Serial.read();  //gets one byte from serial buffer

      //if end of sub-string found, return success
      if (c == ',')  return(1);

      //if not found, append char to string
      readString.append(c); 
    } 

    //if the input string has come to an end without
    // ',' char found
    else {

      // stop looking for inputs
      break;
    }
  }
  
  //failures: termination failure or string is full
  return(0);
}

This does not rely on the non-structured approach and can detect errors in input.

What issues do you have with serial.available?

None the way my code is written. if I do as you suggest and remove one of the conditional statements, there is a repeating spew of data.

"hide your methods from main"

Not sure why that would apply to code that is correctly written that compiles and runs. Is there a technical reason or perhaps a coding superstitious behavior from the old days?

If you have an arduino, why is it not tested? If it works, then post up the complete parcing code like I did (It should not much more complex than what I posted). I'm trying to see code that actually works so I can study it. That is one of the problems I've found in the forum, 80%+ of the solutions posted for us nubies is non functional code "snipits" which is often of little use to a nubie.

This does not rely on the non-structured approach and can detect errors in input.

I've not seen the interpertaion of what constitutes structured vs. nonstructured, so I'm clueless there. What errors is your code catching? If it is the "termination failure", considering how fast the arduino loops, that failure may happen every time if some loop delay is not provided.

Is there a technical reason or perhaps a coding superstitious behaviour from the old days?

You should always try and make the main part of your program appear more like a high level language for readability and ease of maintenance. I suppose it is a good habit to get into as you start making larger programs.


I've not seen the interpertaion of what constitutes structured vs. nonstructured, so I'm clueless there.

"The distinguishing feature of a structured language is compartmentalization of code and data. This is the ability of a language to section off and hide from the rest of the program all infomation and instructions necessary to perform a specific task"

"In a structured language, the use of goto is either prohibited or discouraged..."

(Schildt H., (2003), 'The Complete Reference: C++')


I concede that the code would not wait for the full string to come in.

This can be solved by changing the function to

uint8_t get_substring() {

  //for a maximum of 10 charactors
  for(uint8_t i = 0; i <10; i++) {
    
    //is there input left?
    if (Serial.available()) {
      
      //if so, get it
      char c = Serial.read();  //gets one byte from serial buffer

      //if end of sub-string found, return success
      if (c == ',')  return(1);

      //if not found, append char to string
      readString.append(c);
    }

    //if the input string has come to an end without
    // ',' char found
    else {

      // stop looking for inputs
      break;
    }
  }
  
  //failures: termination failure or string is full
  return(0);
}

and including another function called wait_for_input_string

void wait_for_input_string() {
  
  //wait for serial input...
  while(!Serial.available()) {
    /*do nothing*/
  }
  
  //wait for full message to arrive
  delay(100);
}

The code in the loop would look more like this:

//wait for user to enter a string
wait_for_input_string();

//retreve substring into 'readString'
if(get_substring()) {
  /*no errors, so you may use it*/
}

Notice that if you needed to reuse 'wait_for_input_string', you could.


It's fair enough that you wrote a program that works to help someone: All I'm doing is telling you why you shouldn't use "goto" (and offering you some code that is untested, but an alternative to using it).

Please don't be angry that I disagree with the way you wrote your code.


Just to see how "appropriate" code would look, please post up the same code using break instead of goto so they can be directly compared.

With goto:

while (Serial.available()) {
        delay(10);  
          if (Serial.available() >0) {
        char c = Serial.read();  //gets one byte from serial buffer
        if (c == ',') { goto parce;}
        readString.append(c); } //makes the string readString
        }
      parce:
// We get here when there is no more serial data to read
// or when the serial data contained a comma

With break:

while (Serial.available())
{
   delay(10);  
   if (Serial.available() > 0)
   {
     char c = Serial.read();  //gets one byte from serial buffer
     if (c == ',')
        break;
     readString.append(c); //makes the string readString
    }
}
// We get here when there is no more serial data to read
// or when the serial data contained a comma

I think that break is better because you don't have to look for "parce", you just look to the end of the while statement.

There may be circumstances where goto is appropriate. I've been writing C/C++ programs for 25 years, and I've never used it. But, there might be circumstances where it is needed.

Proper use of functions and structured programming techniques have all but eliminated the need for it in C/C++.

Now, in FORTRAN, it was an absolutely essential statement, so I'm well aware of how to use it.

I've been writing C/C++ programs for 25 years, and I've never used it

Yes, applications programmers will rarely see a "goto" these days, but they're still to be seen occasionally over on the systems side of things.
They're often the best way out of deeply nested error conditions in drivers for high-speed devices like SCSI or Fibre Channel.

I think that break is better because you don't have to look for "parce", you just look to the end of the while statement.

Trust me, as a nubie it is easier for me to find "parce" than the appropriate } } } } "}" }... when looking at code. :slight_smile: