Strings from serial are not equal

Question

In my code I have that string:String code = rs1 + rs2 + rs3; //rs1=034, rs2=000, rs3=017 It means that code = "034000017". But if I run if (code.equals("034000017")) Serial.println("OK"); There is nothing in serial monitor. Here is full code:

String code, rs1, rs2, rs3, readString;
void setup() {
 Serial.begin(9600);
 Serial.println("Serial ready!");
}
void loop() {
 if (Serial.available()) {
 while (Serial.available()) {
 delay(10); 
 if (Serial.available() >0) {
 char c = Serial.read();
 readString += c;
 rs1 = readString.substring(0, 3); //034
 rs2 = readString.substring(3, 6); //000
 rs3 = readString.substring(6, 9); //017
 code = rs1 + rs2 + rs3;
 Serial.println(code);
 if (code.equals("034000017")) Serial.println("OK");
 } 
 }
 }
}

• Majenko: Why do you read just one character and add it to the string then split the string up as if it had at least 9 characters in it? You need to read the entire string first before splitting it.
• Lowder: @Majenko Why one character? I write 034000017 to serial.
• Majenko: No, you write "0" to the serial. Then you write "3" to the serial. Etc.
• Majenko: This may be of use to you: hackingmajenkoblog.wordpress.com/2016/02/01/…
• gre_gor: What's the point in spitting the string and then combing it back again into the same string?

Answer 1

Lots of errors in your code. Mine is basically the same that @frarugi87's answer.

I only have to add: when reading serial, wait for the end of line ('\r' and/or '\n') to know when you have all your data, so you don't depend on length (which can be corrupted).

Edit (on Majeko's comment)

String code, rs1, rs2, rs3, readString;
void setup()
{
 Serial.begin(9600);
 Serial.println("Serial ready!");
}
void loop()
{
 static bool inData = true;
 while (Serial.available()) {
 char c = (char) Serial.read();
 if (c != '\r' && c != '\n') {
 readString += c;
 inData = true;
 }
 else {
 if (inData) {
 rs1 = readString.substring(0, 3); //034
 rs2 = readString.substring(3, 6); //000
 rs3 = readString.substring(6, 9); //017
 code = rs1 + rs2 + rs3;
 Serial.println(code);
 if (code.equals("034000017")) {
 Serial.println("OK");
 }
 readString = "";
 inData = false;
 }
 }
 }
}

This code works for any configuration of line ending ('\r', '\n' or '\r\n').

It doesn't test for length (or validity or timeout); it's left as an exercise to the reader :-)

I hope it shows how code grows when making it robust in all scenarios.

• Majenko: The only failing with your code is if the string is terminated with \r\n - in that case, the \r will trigger a parsing, and then straight after the \n will trigger another with an empty string. You should decide on just one to use, not either/or. Discard the one you're not using. A switch/case can be good for that. Also checking the length of the string can be useful to make sure you got all the data.
• user31481: @Majenko. You are right; I edited it.
• Chris Stratton: ...but why are you retaining the absurd splitting of the String into parts only to expensively concatenate them right back together in the same order?
• user31481: @ChrisStratton. Because that is not what the OP asked. I took his code and made it better. He will learn by comparing his version with mine. If I were to post the way I program, he will understand nothing and my answer will be useless to him. If you are so brilliant, go and post a better (in your opinion) answer.
• Chris Stratton: The point you are making in your text isn't an important one - it's not actually necessary to wait for the end of the line, you can keep checking and failing. It is wasteful to do that, but it's also wasteful to keep in the pointless things you kept in. The only thing you're proposing that actually has a chance of helping with the actual problem is clearing the accumulated string, but you call no attention to that fact.

Answer 2

In your code there is a trivial error and a conceptual error.

The trivial error is in line 16, you wrote

rs2 = readString.substring(6, 9); //017

but it should have been

rs3 = readString.substring(6, 9); //017

(3 instead of 2)

The conceptual one is that you do not have to check the string every time; if you know that the string must be 9 chars long, check for it:

void loop() {
 while (Serial.available()) {
 readString += (char)Serial.read();
 if (readString.length() >= 9) {
 rs1 = readString.substring(0, 3); //034
 rs2 = readString.substring(3, 6); //000
 rs3 = readString.substring(6, 9); //017
 code = rs1 + rs2 + rs3;
 Serial.println(code);
 if (code.equals("034000017")) Serial.println("OK");
 readString = "";
 }
 }
}

Note that in this code I cleaned up a bit your code, and MOST IMPORTANT I reset the readstring to nothing after the check (otherwise only the first string is accepted)

I left a lot of things as they were (for instance in my opinion using a string in this case is pointless - better use a byte array; or splitting the string in three for joining it again later), since I think this is part of a larger program

• Chris Stratton: This is much closer to being a correct answer.