Program functions correctly on first run-through, but not afterwards

This is a new problem occurring after a previous solution. See previous thread here: Communication between .ino and .cpp with functions [SOLVED] - Programming Questions - Arduino Forum
(Merge these threads?)

The first time the program is run, the servo moves to the correct position after a card is read from. If another card is read after that, the servo does not move to the new position supplied by "card 2". I've added code to clear the previous contents of the variable that passes values to the servo, but no effect.

//Servo
#include <Servo.h>

//Writing
#include <SPI.h>
#include <MFRC522.h>

#define RST_PIN 9
#define SS_PIN 10

MFRC522 mfrc522(SS_PIN, RST_PIN);   //Create MFRC522 instance

Servo servo1; //create servo object to control a servo

const int buttonPin = 2; //the number of the pushbutton pin

int potpin1 = 0; //analog pin used to connect the potentiometer
int analog1; //variable to read the value from the analog pin
int fromAnalog_servo1; //analog value scaled for servo
int buttonState = 0; //variable for reading the pushbutton status
int servodataInt;

byte buffer[34];
byte block;
byte len;
byte servodata[18];

MFRC522::StatusCode status;
MFRC522::MIFARE_Key key;

String fromAnalog_servo1_string;
char* servo1_string;

char message;

void setup() {
  Serial.begin(9600); //Initialize serial communications with the PC
  SPI.begin(); //Init SPI bus
  mfrc522.PCD_Init(); //Init MFRC522 card
  servo1.attach(9); //attaches the servo on pin 9 to the servo object
  pinMode(buttonPin, INPUT); //initialize the pushbutton pin as an input:
}

void loop() {
  buttonState = digitalRead(buttonPin); //read the state of the pushbutton value:
  Serial.setTimeout(20000L);     // wait until 20 seconds for input from serial
  //WRITE MODE
  if (buttonState == HIGH) { //check if the pushbutton is pressed.
    //write to tag
    if (message != 'w')
    {
      Serial.println("Scan a PICC to write...");
      message = 'w';
    }
    // Prepare key - all keys are set to FFFFFFFFFFFFh at chip delivery from the factory.

    for (byte i = 0; i < 6; i++) key.keyByte[i] = 0xFF;

    // Look for new cards
    if ( ! mfrc522.PICC_IsNewCardPresent()) {
      return;
    }

    // Select one of the cards
    if ( ! mfrc522.PICC_ReadCardSerial())    return;

    Serial.print(F("Card UID:"));    //Dump UID
    for (byte i = 0; i < mfrc522.uid.size; i++) {
      Serial.print(mfrc522.uid.uidByte[i] < 0x10 ? " 0" : " ");
      Serial.print(mfrc522.uid.uidByte[i], HEX);
    }
    Serial.print(F(" PICC type: "));   // Dump PICC type
    MFRC522::PICC_Type piccType = mfrc522.PICC_GetType(mfrc522.uid.sak);
    Serial.println(mfrc522.PICC_GetTypeName(piccType));


    //Get sensor1 readings ==================================================================
    analog1 = analogRead(potpin1); //read sensor positions
    fromAnalog_servo1 = map(analog1, 0, 1023, 0, 180); //scale sensor positions (0 to 1023) to be usable by servo (0 to 180)
    fromAnalog_servo1_string = String(fromAnalog_servo1);
    len = sizeof(fromAnalog_servo1_string);
    for (byte fillbuffer = 0; fillbuffer < len; fillbuffer++)
    {
      buffer[fillbuffer] = fromAnalog_servo1_string[fillbuffer];
      Serial.print(F("buffer["));
      Serial.println(fillbuffer);
      Serial.print(F("] = "));
      Serial.println(buffer[fillbuffer]);
    }
    Serial.print(F("len = "));
    Serial.println(len);
    Serial.print(F(" analog1 = "));
    Serial.println(analog1);
    Serial.print(F(" fromAnalog_servo1 = "));
    Serial.println(fromAnalog_servo1);
    Serial.print(F(" fromAnalog_servo1_string = "));
    Serial.println(fromAnalog_servo1_string);
    for (byte i = len; i < 3; i++) {
      buffer[i] = ' '; // pad with spaces
    }
    block = 1;
    Serial.println(F("Authenticating using key A..."));
    status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }
    else Serial.println(F("PCD_Authenticate() success: "));

    // Write block
    status = mfrc522.MIFARE_Write(block, buffer, 16);
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }
    else Serial.println(F("MIFARE_Write() success: "));

    block = 2;
    //Serial.println(F("Authenticating using key A..."));
    status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }

    // Write block
    status = mfrc522.MIFARE_Write(block, &buffer[16], 16);
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }
    else Serial.println(F("MIFARE_Write() success: "));

    //Get sensor2 readings ================================================================
    /*
      data2 = Serial.readBytesUntil('#', (char *) buffer, 20); // read first name from serial
      for (byte i = data2; i < 20; i++) buffer[i] = ' ';     // pad with spaces

      block = 4;
      //Serial.println(F("Authenticating using key A..."));
      status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }

      // Write block
      status = mfrc522.MIFARE_Write(block, buffer, 16);
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }
      else Serial.println(F("MIFARE_Write() success: "));

      block = 5;
      //Serial.println(F("Authenticating using key A..."));
      status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }

      // Write block
      status = mfrc522.MIFARE_Write(block, &buffer[16], 16);
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }
      else Serial.println(F("MIFARE_Write() success: "));
    */

    Serial.println(" ");
    mfrc522.PICC_HaltA(); // Halt PICC
    mfrc522.PCD_StopCrypto1();  // Stop encryption on PCD

    //READ MODE
  } else {
    //read from tag
    if (message != 'r')
    {
      Serial.println("Scan a PICC to read...");
      message = 'r';
    }
    // Look for new cards
    if ( ! mfrc522.PICC_IsNewCardPresent()) {
      return;//go to start of loop if there is no card present
    }

    // Select one of the cards
    if ( ! mfrc522.PICC_ReadCardSerial()) {
      return;//if ReadCardSerial returns 1, the "uid" struct (see MFRC522.h lines 238-45)) contains the ID of the read card.
    }

    // Dump debug info about the card. PICC_HaltA() is automatically called.
    mfrc522.PICC_DumpToSerial(&(mfrc522.uid), servodata);
    for (int extract = 0; extract < 3; extract++)
    {
      if ((servodata[extract] > 47) & (servodata[extract] < 58))
      {
        servo1_string += servodata[extract] - '0';
        Serial.print(F("servodata: "));
        Serial.println(servodata[extract] - '0');
        Serial.print(F("servo1_string: "));
        Serial.println(atoi(servo1_string));
      }
    }
    servo1.write(atoi(servo1_string));
    for (int clrSD = 0; clrSD < 18; clrSD++)
    {
      Serial.print(F("Clearing...\n"));
      servodata[clrSD] = 0;
    }
  }
}

MFRC522.cpp (70.7 KB)

MFRC522.h (23.3 KB)

Create a new program where you get rid of everything beside the skeleton of what you are trying to achieve: Something that reads a card and move a servo accordingly. And get that to work. Then build upon this

Give this a try:

//Servo
#include <Servo.h>

//Writing
#include <SPI.h>
#include <MFRC522.h>

#define RST_PIN 9
#define SS_PIN 10

MFRC522 mfrc522(SS_PIN, RST_PIN);   //Create MFRC522 instance

Servo servo1; //create servo object to control a servo

const int buttonPin = 2; //the number of the pushbutton pin

int potpin1 = 0; //analog pin used to connect the potentiometer
int analog1; //variable to read the value from the analog pin
int fromAnalog_servo1; //analog value scaled for servo
int buttonState = 0; //variable for reading the pushbutton status
int servodataInt;

byte buffer[34];
byte block;
byte len;
byte servodata[18];

MFRC522::StatusCode status;
MFRC522::MIFARE_Key key;

String fromAnalog_servo1_string;
const byte servoValueDigits = 3;
char servo1_string[servoValueDigits + 1]; //+1 for the terminator

char message;

void setup() {
  Serial.begin(9600); //Initialize serial communications with the PC
  SPI.begin(); //Init SPI bus
  mfrc522.PCD_Init(); //Init MFRC522 card
  servo1.attach(9); //attaches the servo on pin 9 to the servo object
  pinMode(buttonPin, INPUT); //initialize the pushbutton pin as an input:
}

void loop() {
  buttonState = digitalRead(buttonPin); //read the state of the pushbutton value:
  Serial.setTimeout(20000L);     // wait until 20 seconds for input from serial
  //WRITE MODE
  if (buttonState == HIGH) { //check if the pushbutton is pressed.
    //write to tag
    if (message != 'w')
    {
      Serial.println("Scan a PICC to write...");
      message = 'w';
    }
    // Prepare key - all keys are set to FFFFFFFFFFFFh at chip delivery from the factory.

    for (byte i = 0; i < 6; i++) key.keyByte[i] = 0xFF;

    // Look for new cards
    if ( ! mfrc522.PICC_IsNewCardPresent()) {
      return;
    }

    // Select one of the cards
    if ( ! mfrc522.PICC_ReadCardSerial())    return;

    Serial.print(F("Card UID:"));    //Dump UID
    for (byte i = 0; i < mfrc522.uid.size; i++) {
      Serial.print(mfrc522.uid.uidByte[i] < 0x10 ? " 0" : " ");
      Serial.print(mfrc522.uid.uidByte[i], HEX);
    }
    Serial.print(F(" PICC type: "));   // Dump PICC type
    MFRC522::PICC_Type piccType = mfrc522.PICC_GetType(mfrc522.uid.sak);
    Serial.println(mfrc522.PICC_GetTypeName(piccType));


    //Get sensor1 readings ==================================================================
    analog1 = analogRead(potpin1); //read sensor positions
    fromAnalog_servo1 = map(analog1, 0, 1023, 0, 180); //scale sensor positions (0 to 1023) to be usable by servo (0 to 180)
    fromAnalog_servo1_string = String(fromAnalog_servo1);
    len = sizeof(fromAnalog_servo1_string);
    for (byte fillbuffer = 0; fillbuffer < len; fillbuffer++)
    {
      buffer[fillbuffer] = fromAnalog_servo1_string[fillbuffer];
      Serial.print(F("buffer["));
      Serial.println(fillbuffer);
      Serial.print(F("] = "));
      Serial.println(buffer[fillbuffer]);
    }
    Serial.print(F("len = "));
    Serial.println(len);
    Serial.print(F(" analog1 = "));
    Serial.println(analog1);
    Serial.print(F(" fromAnalog_servo1 = "));
    Serial.println(fromAnalog_servo1);
    Serial.print(F(" fromAnalog_servo1_string = "));
    Serial.println(fromAnalog_servo1_string);
    for (byte i = len; i < 3; i++) {
      buffer[i] = ' '; // pad with spaces
    }
    block = 1;
    Serial.println(F("Authenticating using key A..."));
    status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }
    else Serial.println(F("PCD_Authenticate() success: "));

    // Write block
    status = mfrc522.MIFARE_Write(block, buffer, 16);
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }
    else Serial.println(F("MIFARE_Write() success: "));

    block = 2;
    //Serial.println(F("Authenticating using key A..."));
    status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }

    // Write block
    status = mfrc522.MIFARE_Write(block, &buffer[16], 16);
    if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
    }
    else Serial.println(F("MIFARE_Write() success: "));

    //Get sensor2 readings ================================================================
    /*
      data2 = Serial.readBytesUntil('#', (char *) buffer, 20); // read first name from serial
      for (byte i = data2; i < 20; i++) buffer[i] = ' ';     // pad with spaces

      block = 4;
      //Serial.println(F("Authenticating using key A..."));
      status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }

      // Write block
      status = mfrc522.MIFARE_Write(block, buffer, 16);
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }
      else Serial.println(F("MIFARE_Write() success: "));

      block = 5;
      //Serial.println(F("Authenticating using key A..."));
      status = mfrc522.PCD_Authenticate(MFRC522::PICC_CMD_MF_AUTH_KEY_A, block, &key, &(mfrc522.uid));
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("PCD_Authenticate() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }

      // Write block
      status = mfrc522.MIFARE_Write(block, &buffer[16], 16);
      if (status != MFRC522::STATUS_OK) {
      Serial.print(F("MIFARE_Write() failed: "));
      Serial.println(mfrc522.GetStatusCodeName(status));
      return;
      }
      else Serial.println(F("MIFARE_Write() success: "));
    */

    Serial.println(" ");
    mfrc522.PICC_HaltA(); // Halt PICC
    mfrc522.PCD_StopCrypto1();  // Stop encryption on PCD

    //READ MODE
  } else {
    //read from tag
    if (message != 'r')
    {
      Serial.println("Scan a PICC to read...");
      message = 'r';
    }
    // Look for new cards
    if ( ! mfrc522.PICC_IsNewCardPresent()) {
      return;//go to start of loop if there is no card present
    }

    // Select one of the cards
    if ( ! mfrc522.PICC_ReadCardSerial()) {
      return;//if ReadCardSerial returns 1, the "uid" struct (see MFRC522.h lines 238-45)) contains the ID of the read card.
    }

    // Dump debug info about the card. PICC_HaltA() is automatically called.
    mfrc522.PICC_DumpToSerial(&(mfrc522.uid), servodata);
    for (int extract = 0; extract < servoValueDigits; extract++)
    {
      if ((servodata[extract] > 47) & (servodata[extract] < 58))
      {
        servo1_string[extract] = servodata[extract];
        Serial.print(F("servodata: "));
        Serial.write(servodata[extract]);
        Serial.print(F(", servo1_string: "));
        Serial.println(servo1_string[extract]);
      }
    }
    Serial.print(F("atoi(servo1_string): "));
    Serial.println(atoi(servo1_string));
    servo1.write(atoi(servo1_string));
    for (int clrSD = 0; clrSD < 18; clrSD++)
    {
      Serial.print(F("Clearing...\n"));
      servodata[clrSD] = 0;
    }
  }
}

It just copies the first 3 numbers from the sensordata array to the servo1_string char array. Usually you want to add a null terminator to the end of the string but since globals are zero initialized servo1_string[3] will already be set to 0.

XORduino:

      if ((servodata[extract] > 47) & (servodata[extract] < 58))

I don't like the way this check is done. If servodata[extract] is not in that range it indicates bad data right? If so you should probably just skip the whole operation instead of continuing on and still doing the servo.write().

As J-M-L said, making a simplified test sketch is a good troubleshooting technique. In this case you could have even started with code that just sets sets servodata to arbitrary values and does the conversion so that all the MFRC522 library code could be eliminated. This would also make it easier for others to help since it's likely we don't have the hardware to actually run your full sketch. Once that's working you add in the Servo code, then the MFRC522 code, testing at each step to make sure it's still working correctly.

Adding in lots of Serial.print()s also helps to narrow down exactly where things are going wrong.

don't like the way this check is done. If servodata[extract] is not in that range it indicates bad data right? If so you should probably just skip the whole operation instead of continuing on and still doing the servo.write().

Not to mention that && would be conceptually more appropriate (although in that case it works)

Alright, thanks for the input J-M-L and pert! I will consider all of your points and report back when I have done so.

(Ah yes, && is logical, not &. I always have trouble remembering which is which >.<)

XORduino:
(Ah yes, && is logical, not &. I always have trouble remembering which is which >.<)

your pseudo hints that you prefer bitwise operations :slight_smile:

"You're a wizard, Harry pert"
Thank's for that magic, pert! What was I doing wrong exactly that was preventing later cards' data from being applied/used?

I completely agree with the reasoning behind your advice on removing the check for integer values. I had it there as a weak attempt at detecting the end of the "intelligent" data, since the data passed will be from 0 to 180 (degrees) and I wanted to filter data in the event that a "47" somehow manages to get followed by garbage and ends up looking like: "47<ØOóß&çÂð^Ÿeü"

Thanks J-M-L for your reminder about logical vs. bitwise oprations!

One last side thing:
Every time the program is started (or the Reset button on the board is pushed), the servo seems to get set to a default position. Is there a way of preventing this? If it's something being done internally by the servo itself, then I guess not lol

Not a big issue, I'd just prefer if it didn't for practicality's sake.

For the default position of the servo, Read Robin's answer here

XORduino:
"You're a wizard, Harry pert"
Thank's for that magic, pert!

Glad I was able to help!

XORduino:
What was I doing wrong exactly that was preventing later cards' data from being applied/used?

To be honest I have no clue. I just gave it a quick test and then decided to simplify the code as much as possible and that seemed to fix it. I guess I'm one of those "Pointers make my head spin" guys. Actually I'm not afraid to use them when I have to but tend to avoid it when possible. I'm sure your code using char* could have been fixed with some minor changes but your byte to char conversion code was definitely more complex than necessary.

The switch from your previous version using String for servo1_string had me confused for a second when I first looked at this code, "huh, you can't do atoi() on a String". I'm sure there's a time and place for String but in the end I find char arrays to be pretty much as easy to work with and much more of a standard way of doing things so easier to find example code for any common operation.

XORduino:
I completely agree with the reasoning behind your advice on removing the check for integer values.

I was actually saying that you need to handle things differently if the check is not passed rather than removing the check. Sanity checks on external data inputs are usually a good idea, I'm not sure how necessary in this case because I don't have any experience with the RFID hardware. My suggestion is to cancel the the servo write if any of the servodata values are outside the valid range. You should add some sort of output to indicate that bad data was encountered, in this case it would probably just be a Serial.println() but depending on your device there could be other ways to indicate to the user that something went wrong(buzzer/LED/LCD message, etc.).

I understand pointers for a short time after learning about them, but then the understanding starts to fade quickly if/when I stop using them lol. I'm not surprised that my code is more complex than it needs to be as a lot of this stuff is new to me and/or stuff I very rarely use. This, combined with a very strict time constraint, results in me just sticking with anything that works and moving on, haha. This time constraint is also why I haven't bothered to put in a proper error handling system for the servo data.

Thanks again!