Help Spliting a serial string using strcmp strtok etc.

[system]

Hi,

A while back I needed some help with my SDCard project and a really nice guy on here helped me to resolve the issues I was having using the strcmp strtok and atoi commands.

I thought I understood how to carry this forward to my next project... how wrong was I. I'll try and put as much info here as possible but if I leave anything important out, please accept my apologies and I'll add that information.

What I want to do:
I have an Arduino that sends a text string message (via a Bluetooth module) when a setting is changed to another Arduino with a small 8x2 LCD connected to it. So I have a Controller and remote

So on the first Arduino (Controller) I have the following lines.

    if (line == 1) { Serial.print("Speed   ,"); Serial.println(T_degree[tilDegree]); } //should result in "Speed" on lcd(0,0) and "1 - 5/8" for example on lcd(0,1)
    if (line == 2 && tilDirec==2)
                   { Serial.print("Direc   ,"); Serial.println("Up"); }
    if (line == 2 && tilDirec==3)
                   { Serial.print("Direc   ,"); Serial.println("Down"); }  //this can be streamlined with else

Basically 'line' refers to a menu line number - there are 7 menus each with at least 2 lines. The current line sends a two string statement with a comma delimiter to separate it into the two lines to be displayed on my LCD.

On the second Arduino (remote) I put the following code

if (Serial.available() > 0)  
  {
         co_data = Serial.readStringUntil('\n');
         int commaIndex = co_data.indexOf(',');
         String firstLine = co_data.substring(0,commaIndex);
         String secondLine = co_data.substring(commaIndex+1); 
             lcd.setCursor(0,0); lcd.print(firstLine); 
             lcd.setCursor(0,1); lcd.print(secondLine);
             co_data = ""; firstLine =""; secondLine="";
   }

The problem with this is that although it works, on the second line of the LCD I get a random looking character after the secondLine string followed by a repeating of the firstline. string So I am guessing my code is repeating itself. and not clearing the buffer(?) plus this doesn't look like a neat way of doing it.

So what I thought was I would try the code given to me by PaulS who previously helped me out.

I figured that using the strcmp strtok and atoi commands must be a better way of doing this, but my knowledge of how to implement this is not what I thought it was and so I can't make it work. and then accidentally closed the wrong window and lost what I had been trying. - need to press ctrl-s more often (then I shorted out one of the Arduinos and fried it. Spare now in its place.)

Can anyone clue me in / show me code replacement for the above on how to:

Serial.print a string i.e "Speed,100%" and then receive it on the other Arduino and lcd.print it out to show
Speed
100%
using the strcmp etc commands.

Many thanks

Steve (in the UK)

[michinyon]

plus this doesn't look like a neat way of doing it.

If all you want to do, is to split a single String into two Strings at the comma, then this is the neatest way to do it. I can't see that using strtok( ) would be any neater.

Perhaps you need to check out how to clear the lcd before you write to it. I think that would be a more productive line of endeavour.

[michinyon]

The other problem you may have, is that lcd.print( ) may be expecting a character array with a nul ( byte with the value 0 ) , at the end of it.

If you call the function with a String object, it might appear to work, but if there is no zero byte at the end of a String object, the lcd will keep on writing.

If the first String object happens to be allocated in memory right after the second String object, and there is no null byte between them, that would be a plausible reason why you see what you do see.

You should probably learn how to use char arrays. I never use Strings on the arduino, only with java.

[system]

Perhaps you need to check out how to clear the lcd before you write to it. I think that would be a more productive line of endeavour.

Thanks, Michinyon,

The characters are still there even if I clear the display or write out a line of blank spaces. So I am pretty sure the garbage character is being added to the end of the second line string.

I'll add a null (is it \O) to the sending string and see if that will make a difference, can't remembered if I had tried that or not. and also to the receiving string.

Yes I have used the char arrays before as a way of creating meaningful displays but thought it must be so easy to do it this way. You think I should set-up some arrays then - ok I'll give that a go also.

[econjack]

First, get rid of the Strings. They are an H-bomb-to-kill-an-ant. I don't have an LCD display hand, but try this:

char co_data[50];
char firstLine[21];
char secondLine[21];

 void setup() {
  // put your setup code here, to run once:
 Serial.begin(115200);
}
void loop() { 
  if (Serial.available() > 0)
  {
    Serial.readBytesUntil('\n', co_data, 50);
    int commaIndex = strcspn(co_data, ",");       // Find the comma...
    strncpy(firstLine, co_data, commaIndex);      // Copy the first part...
    strcpy(secondLine, &co_data[commaIndex + 1]); // The address of operator (&) is necessary to 
                                                  // pass in the correct substring address
//    lcd.setCursor(0, 0); lcd.print(firstLine);
//    lcd.setCursor(0, 1); lcd.print(secondLine);
    Serial.println(firstLine);
    Serial.println(secondLine);
   // co_data = ""; firstLine = ""; secondLine = "";
  }
}

[system]

I agree with the suggestion to get rid of the String class.

Also note that your incoming strings are terminated by \r\n not just \n, so the second part of your string would include a trailing \r. I don't know what your LCD will make of that.

I suggest you separate out buffering of the serial stream from parsing the received message, and don't rely on the Serial object to do that for you. Here is a code fragment which will accumulate input lines and call a function to handle them when a complete line has been received. You would need to implement the parsing logic inside handleReceivedMessage():

void handleSerial()
{
    const int BUFF_SIZE = 32; // make it big enough to hold your longest command
    static char buffer[BUFF_SIZE+1]; // +1 allows space for the null terminator
    static int length = 0; // number of characters currently in the buffer

    if(Serial.available())
    {
        char c = Serial.read();
        if((c == '\r') || (c == '\n'))
        {
            // end-of-line received
            if(length > 0)
            {
                handleReceivedMessage(buffer);
            }
            length = 0;
        }
        else
        {
            if(length < BUFF_SIZE)
            {
                buffer[length++] = c; // append the received character to the array
                buffer[length] = 0; // append the null terminator
            }
            else
            {
                // buffer full - discard the received character
            }
        }
    }
}

void handleReceivedMessage(char *msg)
{
     your parsing code goes here
}

You can separate the left and right halves of your message by two calls to strtok().

char *left = strtok(msg, ",");
char *right = strtok(NULL, ",");

Since you're only dividing it into two tokens, it would be equally easily to call strchr() to locate the comma and then split the string at that point.

[system]

Hi guys,

Thanks for your input so far. I don't like to bother you but I am really stuck.

I have tried so many combinations from the code posted above by others and read loads of stuff on-line in an attempt to understand how Serial and the array command structure work and whilst I am getting there absorbing this information it proving, at least for me, a steep learning curve. I have been at this pretty much all week so you can imagine my confidence level in my ability is now at rock bottom.

I am still seeing an odd character on the LCD which looks like a 1 and | ( " 1| " ) at the end of each line of text on the LCD. plus the data (firstline/secondline) is echoed after the initial printout with random start points so what should be displayed is:
Speed
100%
what actually displays is:
Speed 1| eed 1| or Speed 1| ,Spp 1|
100% 1| 00% 1| or 100% 1| ,10 1|

I would point out that this is meant to display on a 8,2 display but I have put a 16,2 on the breadboard to see more of the data being printed.

This is the code being Serial.printed via blue tooth to the remote:

    if (line == 1) {Serial.print("Speed   ,"); Serial.println(T_SPD);  }
    if (line == 2) {Serial.print("Operate ,"); Serial.println(R_MTR);  }
    if (line == 3) {Serial.print("Direc   ,"); Serial.println(T_DIR);  }

The T_SPD, R_MTR and T_DIR are all the parts of the transmitted data that change based on which ever instruction is sent to the controller - so when I press a button on the remote for instance T_SPD will increase or decrease (depending on whether the up or down button was pressed)

This is the code on the remote device.

#include <Bounce.h>
#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 7,6,5,4);

    const int ack = 2;                          //Acknowledgement LED pin
    char co_data[50];                           //controller sent this
    char firstLine[11];                         //first line of LCD shows
    char secondLine[11];                        //second line of LCD shows
    char cd_return[50];                         //compare recieved to current data to stop refresh

//Buttons 
    unsigned int ui_Data;                         //The user input Data to Send to controller
    const int incPIN = A2;                        //Incremental button
    const int decPIN = A4;                        //decremental PIN
    const int selPIN = A3;                        //Select button
    int select = 0;                               //temp just for testing
    const int YaxPIN = A0;                        //Y axis PIN 
    const int XaxPIN = A1;                        //X axis PIN
    int flag = 0;                                 //Control flag
    Bounce increment = Bounce(incPIN, 125);       //deblounce increment (button, milliseconds)
    Bounce decrement = Bounce(decPIN, 125);       //deblounce decrement (button, milliseconds)
    Bounce selected  = Bounce(selPIN, 125);       //deblounce select    (button, milliseconds)
        
void setup()
{
//LCD Display setup. and test text
      pinMode (9,OUTPUT);                       //contrast pin via a 100uf cap to gnd
      analogWrite(9,70);                        //CONTRAST
      lcd.begin(16,2);
      lcd.setCursor(0,0);
      lcd.print(" TEST  ");
      lcd.setCursor(0,1);
      lcd.print(" TEXT  ");
      delay(1000); lcd.clear();
      
//Remote button setup  
  Serial.begin(9600);
      pinMode (ack,OUTPUT);                     //Activity LED
      pinMode (incPIN,INPUT);
      pinMode (decPIN,INPUT);
      pinMode (selPIN,INPUT);
      pinMode (YaxPIN,INPUT);
      pinMode (XaxPIN,INPUT);
}

void loop()
{   //if previous millis is less than current millis  - don't read else do the next block ?
    selected.update();     decrement.update();     increment.update();
  
        int Sel; Sel = selected.read();                                //Select button
        if (Sel == LOW) { ui_Data = 7; flag = 1; select = !select;
        }
        int Dec; Dec = decrement.read();                               //Decrease Value
        if (Dec == LOW) { ui_Data = 2; flag = 1; 
        }
        int Inc; Inc = increment.read();                               //Increase Value
        if (Inc == LOW) { ui_Data = 1; flag = 1; 
        }
        int xaxis; xaxis = map(analogRead(YaxPIN), 0, 1023, 1, 10);    //Slider control
        if (xaxis > 7) { ui_Data = 4 ; flag = 1; delay(100);  }        //DOWN    delay as its too fast need a better way 
        if (xaxis < 4) { ui_Data = 3 ; flag = 1; delay(100);  }        //UP      to do this such as millis to slow reading
        
        int yaxis; yaxis = map(analogRead(XaxPIN), 0, 1023, 1, 10);    //Slider control
        if (yaxis > 7) { ui_Data = 5 ; flag = 1; delay(100);  }        //LEFT    delay as its too fast need a better way 
        if (yaxis < 3) { ui_Data = 6 ; flag = 1; delay(100);  }        //RIGHT   to do this such as millis to slow reading
        
        if (ui_Data < 1) flag = 0; //atempt to stop sending
     
    if (flag == 1)  { communicate();  }                                 // Button pressed so send the data

    if (Serial.available())
      {
          Serial.readBytesUntil('\r\n', co_data, 50);
          if(co_data != cd_return) 
          {
          int commaIndex = strcspn(co_data, ",");          // Find the comma...
            strncpy(firstLine, co_data, commaIndex);       // Copy the first part...
            strcpy(secondLine, &co_data[commaIndex + 1]);  // The address of operator (&) is necessary to pass in the correct substring address
            
            strncpy(cd_return, co_data, 50);               //set cd_return to  = co_Data to act as a flag to stop screen re-write
   
            lcd.setCursor(0, 0); lcd.print(firstLine);     //print first line to LCD
            lcd.setCursor(0, 1); lcd.print(secondLine);    //print second line to LCD
           }
      }
}

void communicate()
{
  Serial.println(ui_Data);   flag = 0;   ui_Data = 0; Serial.flush();      //send User Input Data from buttons
  digitalWrite(ack,HIGH); delay(100); digitalWrite(ack,LOW); delay(50);    //Flash acknowledge LED - need to get rid of delays

}

I tried setting a var to equal what was received and if this did not change then stop a rewrite of the data to the LCD and although both lines look the same (I printed co_Data to 0,0 and cd_return to 0,1 on the LCD it is still performing the block of code so I have got something wrong there.

Effectively I have two issues; the repeating printout and the if compare block not working. I apologise in advance for the code it's been messed about a bit so there are some untidy bits in there like the delays which I will rewrite to something more efficient once this is working.

Any help gratefully received

[system]

UPDATE: So as I could not figure out how my variable handling was going so wrong and I knew I had to get rid of the delay(); statements I set about working on those. I noticed as I worked through this that the issue was get less and less obvious. Then THE LIGHT BULB Moment. of course delay would halt the processor as the data was being written out - would that be it???? I recall reading something by PaulS about assuming the timings happening in sequence. Anyways I think it is fixed.

I just need to work out the acknowledgement LED and how to clear the display before writing the new data (as its lengths are different). Many thanks for sticking with me on this.....

For reference:
Oh I forgot to mention - although the above code uses econjacks code I did try it with PeterHs code as well. I would just like to add that Peters code does not have the funny ' 1| ' bit which is great but I still get this ghosting effect of random characters but handling the Serial out side as suggested is the way to go I feel (IMO). I have a sketch for ether version and with Peters code is as follows

#include <Bounce.h>
#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 7,6,5,4);

    const int ack = 2;                          //Acknowledgement LED pin
    unsigned long currentMillis;
    unsigned long previousMillis;

//Buttons 
    unsigned int ui_Data;                         //The user input Data to Send to controller
    const int incPIN = A2;                        //Incremental button
    const int decPIN = A4;                        //decremental PIN
    const int selPIN = A3;                        //Select button
    int select = 0;                               //temp just for testing
    const int YaxPIN = A0;                        //Y axis PIN 
    const int XaxPIN = A1;                        //X axis PIN
    int flag = 0;                                 //Control flag
    Bounce increment = Bounce(incPIN, 125);       //deblounce increment (button, milliseconds)
    Bounce decrement = Bounce(decPIN, 125);       //deblounce decrement (button, milliseconds)
    Bounce selected  = Bounce(selPIN, 125);       //deblounce select    (button, milliseconds)
        
void setup()
{
//LCD Display setup. and test text
      pinMode (9,OUTPUT);                       //contrast pin via a 100uf cap to gnd
      analogWrite(9,70);                        //CONTRAST
      lcd.begin(16,2);
      lcd.setCursor(0,0);
      lcd.print(" TEST  ");
      lcd.setCursor(0,1);
      lcd.print(" TEXT  ");
      delay(1000); lcd.clear();
      
//Remote button setup  
  Serial.begin(9600);
      pinMode (ack,OUTPUT);                     //Activity LED
      pinMode (incPIN,INPUT);
      pinMode (decPIN,INPUT);
      pinMode (selPIN,INPUT);
      pinMode (YaxPIN,INPUT);
      pinMode (XaxPIN,INPUT);
}

void loop()
{   currentMillis = millis();
    if(currentMillis >= (previousMillis + 100))
    {  
    previousMillis = currentMillis;                                    // Update previousMillis
    selected.update();     decrement.update();     increment.update();
  
        int Sel; Sel = selected.read();                                //Select button
        if (Sel == LOW) { ui_Data = 7; flag = 1; select = !select;
        }
        int Dec; Dec = decrement.read();                               //Decrease Value
        if (Dec == LOW) { ui_Data = 2; flag = 1; 
        }
        int Inc; Inc = increment.read();                               //Increase Value
        if (Inc == LOW) { ui_Data = 1; flag = 1; 
        }
        int xaxis; xaxis = map(analogRead(YaxPIN), 0, 1023, 1, 10);    //Slider control
        if (xaxis > 7) { ui_Data = 4 ; flag = 1;    }        //DOWN    delay as its too fast need a better way 
        if (xaxis < 4) { ui_Data = 3 ; flag = 1;    }        //UP      to do this such as millis to slow reading
        
        int yaxis; yaxis = map(analogRead(XaxPIN), 0, 1023, 1, 10);    //Slider control
        if (yaxis > 7) { ui_Data = 5 ; flag = 1;    }        //LEFT    delay as its too fast need a better way 
        if (yaxis < 3) { ui_Data = 6 ; flag = 1;    }        //RIGHT   to do this such as millis to slow reading
   }     
        if (ui_Data < 1) flag = 0; //atempt to stop sending
        if (flag == 1)  { communicate();  }                                 // Button pressed so send the data
       handleSerial();
}

void communicate()
{
  Serial.println(ui_Data);   flag = 0;   ui_Data = 0; Serial.flush();      //send User Input Data from buttons
 // digitalWrite(ack,HIGH); delay(100); digitalWrite(ack,LOW); delay(50);    //Flash acknowledge LED - need to get rid of delays
}      

void handleSerial()
{
    const int BUFF_SIZE = 32; // make it big enough to hold your longest command
    static char buffer[BUFF_SIZE+1]; // +1 allows space for the null terminator
    static int length = 0; // number of characters currently in the buffer

    if(Serial.available())
    {
        char c = Serial.read();
        if((c == '\r') || (c == '\n'))
        {
            // end-of-line received
            if(length > 0)
            {
                handleReceivedMessage(buffer);
            }
            length = 0;
        }
        else
        {
            if(length < BUFF_SIZE)
            {
                buffer[length++] = c; // append the received character to the array
                buffer[length] = 0; // append the null terminator
            }
            else
            {
                // buffer full - discard the received character
            }
        }
    }
}

void handleReceivedMessage(char *msg)
{
     //your parsing code goes here
     char *left = strtok(msg, ",");
     char *right = strtok(NULL, ",");
            lcd.setCursor(0, 0); lcd.print(left);     //print first line to LCD
            lcd.setCursor(0, 1); lcd.print(right);    //print second line to LCD
}

Thanks Peter - this looks really promising as well as econjacks input

Steve