Strcmp not working as expected, many tries

//declared like this at top of source
//const char cmdOn[]  = {"on"};
//const char cmdOff[] = {"off"};
void processBuffer() {
    int len        = strlen(readBuf) + 1;
	char copy        [len];
	snprintf         (copy,len, "%s", readBuf);
	//values look good when printed
	Serial.printlnf  ("[%s] [%s] [%s] Received from Particle", copy, cmdOn, cmdOff);
	//none of these strcmp tests work
	if (strcmp(copy,"on") == 0) {
    	digitalWrite(LEDD7,HIGH);
	}
	if (strcmp(copy,"off") == 0) {
    	digitalWrite(LEDD7,LOW);
	}
    if(0==strcmp(copy,cmdOn)){
        digitalWrite(LEDD7,HIGH);
    }
    if(0==strcmp(copy,cmdOff)){
        digitalWrite(LEDD7,LOW);
    }
    //but this does
    if(copy[0]=='0'){
        digitalWrite(LEDD7,LOW);
    }
    if(copy[0]=='1'){
        digitalWrite(LEDD7,HIGH);
    }
}

Your code has a lot of if, but no else. Could it be that you are accidentally overwriting the result of other if-cases than you’re expecting?

I would write this code like this:

if (strcmp(copy,"on") == 0) {
	digitalWrite(LEDD7,HIGH);
} else if (strcmp(copy,"off") == 0) {
	digitalWrite(LEDD7,LOW);
} else if(0==strcmp(copy,cmdOn)){
	digitalWrite(LEDD7,HIGH);
} else if(0==strcmp(copy,cmdOff)){
	digitalWrite(LEDD7,LOW);
} else if(copy[0]=='0'){
	digitalWrite(LEDD7,LOW);
} else if(copy[0]=='1'){
	digitalWrite(LEDD7,HIGH);
} else {
	Serial.println("should not happen, unless all above failed");
}

Also, have you tried testing if the string also contains a non-visual ASCII character like \n, \r or \0? That’s likely what is causing the problem.

Just add this snippet to type out the ASCII character number of your string:

Serial.println("String start:");
for( int i=0; i<len;i++)
{
	Serial.println( copy[i], DEC);
}
Serial.println("String end!");

J

Checking now Thanks!..

[on
][3] [on][2] [off][3] Received from Particle
Your Right there is an extra char the \r or \n

Answered! Fixed thanks to you jenschr. I test the len and if ok I do a strncmp and now it works. I now have a serial cmd parser.

1 Like

You can also use strncmp() like this

  if (strncmp(copy, "someString", strlen("someString") == 0)

and to save yourself some writing you can define a macro for that

#define STRINGCOMPARE(str, literal) strncpy(str, literal, strlen(literal))
...
  if (!STRINGCOMPARE(copy, "someString)) 
2 Likes