Issue with code execution at strcmp

Current setup uses a Parralax RFID reader, a servo, and a button. When the button is pushed the servo rotates like it should. When attempting to swipe the correct RFID card that was defined earlier, it doesn’t activate the servo. I tested using the Serial connection to see if the codes matched, and they did. Another interesting note: when I replace the == 1 to == 0 after the strcmp, it still wouldn’t activate the servo, but like usual, the rest of the code worked as usual. Anyone might know what is going on and/or how I fix this?

#include <Servo.h>

Servo Servo1;
int  val = 0; 
char code[10]; 
int bytesread = 0;
int ServoPin = 9;
int ButtonPin = 8;
char Card[] = {'0','4','1','5','E','D','3','F','4','7'};

void setup() { 

  Serial.begin(2400);                      // Start serial connection
  pinMode(2,OUTPUT);                       // RFID Enable pin
  pinMode(ButtonPin,INPUT);
  pinMode(ServoPin,OUTPUT);
  digitalWrite(2, LOW);                    // Enable the RFID reader
  Servo1.attach(ServoPin);
  Servo1.write(0);

}  


 void loop() { 

  if(Serial.available() > 0) {                   // if data available from reader 
    if((val = Serial.read()) == 10) {            // check for header 
      bytesread = 0; 
      while(bytesread<10) {                      // read 10 digit code 
        if( Serial.available() > 0) { 
          val = Serial.read(); 
          if((val == 10)||(val == 13)) {        // if header or stop bytes before the 10 digit reading 
            break;                              // stop reading 
          } 
          code[bytesread] = val;                // add the digit           
          bytesread++;                          // ready to read next digit  
        } 
      } 
      if(bytesread == 10) {                    // if 10 digit read is complete 
        Serial.println(code);
        Serial.println(Card);
        String Code = String(code);
        if ( strcmp(code,Card) == 1 ) { ServoCycle; }
      } 
      bytesread = 0; 
      digitalWrite(2, HIGH);                  // deactivate the RFID reader for a moment so it will not flood
           delay(3000);                       // wait for a bit 
           digitalWrite(2, LOW);              // Activate the RFID reader
    } 
  }
  if ( digitalRead(ButtonPin) == HIGH ) { ServoCycle(); }
}

void ServoCycle()
{
  Servo1.write(100);
  delay(3000);
  Servo1.write(0);
  loop();
}

I did use someone’s example RFID reading code as my base and added onto it. Any help would be greatly appreciated!

strcmp expects null-terminated strings, add a zero to the end of your Card array.

I added a zero to my card array with no avail, but if what you say is true, do I need to also add a 0 to the code array?

I don't think so, but I'll check what the String method does.

Also, you probably don't want to be checking if strcmp returns one - it returns zero if they're equal.

e: I don't think you really want to be using the String () function there. Just make the code array one char longer, and the last index to be zero.

I tried to make it go for returning 0 too which still doesn't work, but I am also questioning my own IDE since none of the string functions beyond String are actually highlighted.

I agree with Yes about == 0. The specs I read on strcmp() say it returns 0 if the strings are equal, > 0 if one is greater, and < 0 if the other one is greater. You should probably be using == 0 or != 0.

Also, you might need to declare your code array size 11, to include the null terminator. If you put a null terminator on code, you won’t have to even use Code. Which is good, because you’re using code in the strcmp() and not Code.

Code was from earlier debugging and I meant to delete it earlier xD. Anyways, I made code[11] and everything else moving ==1 to ==0. Only issue I am having is I am apparently unable to force code[11] = 0;

You want code[10] = 0. Remember array indexing starts at zero (so 10 is the 11'th slot). When you declare the array though, you use the size you want starting from one (so you would declare it with code[11]).

I'm curious, what error did you get when you tried assigning code[11] = 0?

there was no error, it's just when I ran it on the Arduino and was watching the serial logs, the code was coming back with 10 chars instead of 11, but what you just said makes sense and explains the whole thing. xD Let me see if this works.

I just can’t figure this out myself, sorry. xD
This is what I have now.
What do I need to edit in this?

#include <Servo.h>

Servo Servo1;
int  val = 0; 
char code[11]; 
int bytesread = 0;
int ServoPin = 9;
int ButtonPin = 8;
char Card[11] = {'0','4','1','5','E','D','3','F','4','7','0'};

void setup() { 

  Serial.begin(2400);                      // Start serial connection
  pinMode(2,OUTPUT);                       // RFID Enable pin
  pinMode(ButtonPin,INPUT);
  pinMode(ServoPin,OUTPUT);
  digitalWrite(2, LOW);                    // Enable the RFID reader
  Servo1.attach(ServoPin);
  Servo1.write(0);

}  

 void loop() { 

  if(Serial.available() > 0) {                   // if data available from reader 
    if((val = Serial.read()) == 10) {            // check for header 
      bytesread = 0; 
      while(bytesread<10) {                      // read 10 digit code 
        if( Serial.available() > 0) { 
          val = Serial.read(); 
          if((val == 10)||(val == 13)) {        // if header or stop bytes before the 10 digit reading 
            break;                              // stop reading 
          } 
          code[bytesread] = val;                // add the digit           
          bytesread++;                          // ready to read next digit  
        } 
      } 
      if(bytesread == 10) {                    // if 10 digit read is complete 
        code[10] = 0;
        Serial.println(code);
        Serial.println(Card);
        if ( strcmp(code,Card) == 0 ) { ServoCycle; }
      } 
      bytesread = 0; 
      digitalWrite(2, HIGH);                  // deactivate the RFID reader for a moment so it will not flood
           delay(3000);                       // wait for a bit 
           digitalWrite(2, LOW);              // Activate the RFID reader
    } 
  }
  if ( digitalRead(ButtonPin) == HIGH ) { ServoCycle(); }
}

void ServoCycle()
{
  Servo1.write(100);
  delay(3000);
  Servo1.write(0);
  loop();
}
char Card[11] = {'0','4','1','5','E','D','3','F','4','7','0'};

Should be:

char Card[11] = {'0','4','1','5','E','D','3','F','4','7',0};

ChewyJoe:
I just can't figure this out myself, sorry. xD
This is what I have now.
What do I need to edit in this?

        if ( strcmp(code,Card) == 0 ) { ServoCycle; }

I didn't read the whole program, but this just takes the address of the ServoCycle function, and then discards the address, because it wasn't used. To call a function do:

        if ( strcmp(code,Card) == 0 ) { ServoCycle (); }

Yes didn't actually mean add a zero to the array. What he meant to say was add a null string termination character to your array, so the last character in the ID is: '\0'

The X3J11 C Standard does not require the null termination character to be a binary zero. The committee purposely left the implementation of the null termination character up to the compiler vendor. They did this in case future technologies may have a reason to use something other than binary zero for null. To my knowledge, however, all compiler vendor do use binary zero, but it's much more portable to use '\0' than assuming its value is always zero.

FFFFFFF
I forgot to re-add the parenthesis earlier :Facepalm:.
sorry for making you go through all that. It works great now.

The statement:

char Card[11] = {'0','4','1','5','E','D','3','F','4','7',0};

is actually doing a silent cast on the last element of the array because the compiler will default the zero value to an int. However, because it's part of a character array, it will cast the int to a char "behind your back". You could use:

char Card[11] = {'0','4','1','5','E','D','3','F','4','7',(char) 0};

and the compiler would be happy but more importantly, you've told the rest of the world reading your code what your intention is. However, the clearest method is:

char Card[] = {'0','4','1','5','E','D','3','F','4','7', '\0'};

because the last element in the array is the standard definition of a null termination character. Also note that I left out the array size in the definition. Compilers are very good at counting initializer lists. Also, if you do add another element somewhere at a later date, you don't have to remember to change the element size.

Thank you all, I did learn a lot though through this process. :smiley:

econjack:
Yes didn't actually mean add a zero to the array. What he meant to say was add a null string termination character to your array, so the last character in the ID is: '\0'

The X3J11 C Standard does not require the null termination character to be a binary zero. The committee purposely left the implementation of the null termination character up to the compiler vendor. They did this in case future technologies may have a reason to use something other than binary zero for null. To my knowledge, however, all compiler vendor do use binary zero, but it's much more portable to use '\0' than assuming its value is always zero.

That is completely wrong for character strings. I was on the original X3J11 committee from the first meeting through the release of the 1989 ANSI standard (American), and the 1990 ISO standard (world wide). The string terminator has always been all 0's, even in K&R C (and earlier versions).

Now, what you may be thinking of, is the standard does not require that 0.0 or a NULL pointer be all 0 bits.

Since the strings being compared are always both 10 characters long, it would make more sense to compare them using memcmp, rather than adding terminating nulls so that strcmp can be used.

the last character in your array needs to be the number zero and not the char '0'