Problems with parsing text from serial

First off, what I’m using in my build (so far):
Common-anode RGB LED, 3 resistors, USB cable for Serial

Here’s my code:

const int RED = 11;
const int GRN = 10;
const int BLU = 9;

String inputString = "";
boolean stringComplete = false;

void setup() {
  Serial.begin(9600);
  pinMode(RED, OUTPUT);
  pinMode(GRN, OUTPUT);
  pinMode(BLU, OUTPUT);
  inputString.reserve(256);
  
  // I'm using a common-anode led, 
  //so HIGH and LOW works in reverse
  digitalWrite(RED, HIGH);
  digitalWrite(GRN, HIGH);
  digitalWrite(BLU, HIGH);
}

void loop(){
  if(stringComplete) {
    inputString.toLowerCase();
    Serial.println("received " + inputString);
    
    matchString(inputString, 500);
    
    inputString = "";
    stringComplete = false;
  }
}

// serial string read based on example on
// http://arduino.cc/en/Tutorial/SerialEvent
void serialEvent() {
  while(Serial.available()) {
    char inChar = (char) Serial.read();   
    inputString += inChar;
    if(inChar == '\n')
      stringComplete = true;
  }
}

void matchString(String input, int duration) {
  if(containsString(input,"red")) {
    colour(RED, duration);
    Serial.write("input matched red\n"); // debug print
  }
  else if(containsString(input, "green")) {
    colour(GRN, duration);
    Serial.write("input matched green\n");
  }
  else if(containsString(input, "blue")) {
    colour(BLU, duration);
    Serial.write("input matched blue\n");
  }
  else {
    delay(duration);
    Serial.write("input didn't match anything\n");
  }
}

void colour(int pin, int duration) {
  digitalWrite(pin, LOW);
  delay(duration);
  digitalWrite(pin, HIGH);
}

// will be used to implemet cyan magenta and yellow
void colour(int pin1, int pin2, int duration) {
  digitalWrite(pin1, LOW);
  digitalWrite(pin2, LOW);
  delay(duration);
  digitalWrite(pin1, HIGH);
  digitalWrite(pin2, HIGH);
}

// Attempt to mimic String.contains
boolean containsString(String input, String search) {
  int max = input.length() - search.length();
  
  for(int i = 0; i <= max; i++) {
    if(input.substring(i) == search)
      return true;
  }
  return false;
}

I’m not quite sure what goes wrong here
it always prints “input didn’t match anything” whenever I write “red” “green” or “blue”, rather than any of the strings I’ve asked it to match… Anyone got an idea what goes wrong here?

void serialEvent() {
  while(Serial.available()) {
    char inChar = (char) Serial.read();   
    inputString += inChar;
    if(inChar == '\n')
      stringComplete = true;
  }
}

You probably want to break out of the while loop if a \n arrives.

  for(int i = 0; i <= max; i++) {
    if(input.substring(i) == search)
      return true;
  }
  return false;

This does not make sense. Why are you not simply checking that the two strings are the same?

PaulS:

void serialEvent() {

while(Serial.available()) {
   char inChar = (char) Serial.read();  
   inputString += inChar;
   if(inChar == ‘\n’)
     stringComplete = true;
 }
}



You probably want to break out of the while loop if a \n arrives.

the loop breaks itself when there’s nothing new in the buffer, the newline just determines whether it should make a string of the buffer or not

PaulS:

  for(int i = 0; i <= max; i++) {

if(input.substring(i) == search)
     return true;
 }
 return false;



This does not make sense. Why are you not simply checking that the two strings are the same?

Because if I get “red\n” and “red” they aren’t the same, checking to see if they contain the same would be a way to go, and that loop keeps going until the substring of the input matches the string I want to match, if it never matches, then the loop simply terminates and returns false, which is exactly how String.Contains works in C# and Java

Because if I get "red\n" and "red" they aren't the same,

Then, don't store the \n in the String.

and as for breaking the loop, well the loop breaks itself when there's nothing new in the buffer

If there is some delay, and the buffer contains "red\ngr", do you really want to store all those characters in inputString, just because they are all in the buffer?

What happens when "een\n" arrives?

PaulS:

Because if I get "red\n" and "red" they aren't the same,

Then, don't store the \n in the String.

and as for breaking the loop, well the loop breaks itself when there's nothing new in the buffer

If there is some delay, and the buffer contains "red\ngr", do you really want to store all those characters in inputString, just because they are all in the buffer?

What happens when "een\n" arrives?

that never happens, it takes too long for each individual message to arrive

and even after adding a "break;" after the newline check, it doesn't change much I mean surely it should be able to compare the strings correctly, now that I've made sure it can only receive one string at a time, and not combine them

Adding some Serial.print() statements in your code should tell you where the problem is. What, exactly, is in inputString when serialEvent() sets stringComplete to true. And, I mean exactly.

Serial.print("inputString contains: [");
Serial.print(inputString);
Serial.println("]");

What, exactly, is in the substring? What, exactly, is in the String to be searched for?

PaulS:
Adding some Serial.print() statements in your code should tell you where the problem is. What, exactly, is in inputString when serialEvent() sets stringComplete to true. And, I mean exactly.

Serial.print("inputString contains: [");

Serial.print(inputString);
Serial.println("]");




What, exactly, is in the substring? What, exactly, is in the String to be searched for?
received red

inputString contains: [red
]
input didn't match anything

it contains a newline... as expected... it should still be able to filter it out using the containsString function

received red

Why isn't the received string/String in delimiters, too?

PaulS:

received red

Why isn't the received string/String in delimiters, too?

why would it be? the string delimiters are only there for us so we can separate strings from code

    Serial.write("input didn't match anything\n");

Why not println?

    Serial.println("input didn't match anything");

Saves you typing a whole lot of backslashes.

why would it be?

To assure yourself, and us, that what was received was >red<, not >red
<.

PaulS:

why would it be?

To assure yourself, and us, that what was received was >red<, not >red
<.

but what difference would that make if the containsString function filters it out anyway?

And how would it even work? I mean if I type: >red< return, then I’ll get >red< and not >red
< anyway

but what difference would that make if the containsString function filters it out anyway?

If? That's what we're trying to validate.

// Attempt to mimic String.contains
boolean containsString(String input, String search) {
  int max = input.length() - search.length();

  for(int i = 0; i <= max; i++) {
    if(input.substring(i) == search)
      return true;
  }
  return false;
}

What’s wrong with strstr?

And if I may suggest, stop using String.

http://www.gammon.com.au/forum/?id=12153#trap23

PaulS:

but what difference would that make if the containsString function filters it out anyway?

If? That's what we're trying to validate.

I edited the previous post

void setup ()
  {
  Serial.begin (115200);
  Serial.println ();
  String s = "But does it get goat's blood out?";
  Serial.println (strstr (s.c_str (), "goat") != NULL);
  Serial.println (strstr (s.c_str (), "dragon") != NULL);
  }  // end of setup

void loop () { }

Result:

1
0

And how would it even work? I mean if I type: >red< return, then I’ll get >red< and not >red
< anyway

What are you typing red in? The Serial Monitor? If so, what do you have the line ending set to?

Interesting... I'll have a look at that

PaulS:

And how would it even work? I mean if I type: >red< return, then I’ll get >red< and not >red
< anyway

What are you typing red in? The Serial Monitor? If so, what do you have the line ending set to?

it’s set to newline

it’s set to newline

Then this:

if I type: >red< return, then I’ll get >red< and not >red
< anyway

is wrong.