char* keeps getting overwritten

Projects Programming
1

Good day!

I'm working on reading and writing phonebook entry to sim with a SIM900 gsm shield.
I have done it before but with arduino gsm shield using the gsm library. I had a problem using software serial with the arduino gsm so I switched shield.

Anyway,
I have a problem saving the numbers I have read from the sim. After I have read the second user, the first user's number gets overwritten with the second number.

This is my code:

char Rx_data[50];
unsigned char Rx_index = 0;
int i = 0;
char msg[160];
int sig;

char *adminOneNo;
char *adminTwoNo;

boolean adminOne = false;
boolean adminTwo = false;

int gsm_pin  = 13;
int adm1_pin = 12;
int adm2_pin = 11;


void setup() {
  Serial.begin(9600);
  Serial1.begin(38400);  // GSM
  
  pinMode(gsm_pin, OUTPUT);
  pinMode(adm1_pin, OUTPUT);
  pinMode(adm2_pin, OUTPUT);
  
  digitalWrite(gsm_pin, LOW);
  digitalWrite(adm1_pin, LOW);
  digitalWrite(adm2_pin, LOW);

  Serial.println("GSM(SIM) READ/ADD PB ENTRY");

  initGSM();  
  readSIM();
}

void loop() {

}


void clearSerial(char *data) {
  for( int i = 0; i < sizeof(data);  ++i )
    data[i] = (char)0;   
}

// ----- GSM: TX

void send_msg(char *number, char *msg) {
  char at_cmgs_cmd[30] = {'\0'};
  char msg1[160] = {'\0'};
  char ctl_z = 0x1A;

  sprintf(msg1, "%s%c", msg, ctl_z);
  sprintf(at_cmgs_cmd, "AT+CMGS=\"%s\"\r\n",number);

  sendGSM(at_cmgs_cmd);
  delay(300);
  sendGSM(msg1);
  delay(100);
}

// ----- GSM: COMM

void sendGSM(char *string) { 
  Serial1.write(string);
  delay(90);
}

void clearString(char *strArray) { 
  int j;
  for (j = 100; j > 0; j--)
    strArray[j] = 0x00;
}

void send_cmd(char *at_cmd, char clr) { 
  char *stat = '\0';
  while(!stat) {
    sendGSM(at_cmd);
    delay(90);
    readSerialString(Rx_data);

    stat = strstr(Rx_data, "OK");
  }

  if (clr) {
    clearString(Rx_data);
    delay(200);
    stat = '\0';
  }
}

void readSerialString(char *strArray) { 
  if(!Serial1.available()) {
    return;
  }

  while(Serial1.available()) {
    strArray[i] = Serial1.read();
    //Serial.print(strArray[i]);
    i++;
  }
}

void initGSM(){  
  send_cmd("ATE0\r\n",1);               // Turn off automatic echo of the GSM Module	
  send_cmd("AT\r\n", 1);							
  send_cmd("AT+CMGF=1\r\n", 1);	        // Sucess 
  send_cmd("AT+CNMI=1,2,0,0,0\r\n", 1);	// Set message format to text mode


  Serial.println("Success!\n");	
  digitalWrite(gsm_pin, HIGH);
  delay(2000);
}

// ----- SIM

void readSIM(){  
  Serial.println("Reading SIM...");

  send_cmd("AT+CPBS=\"SM\"\r\n", 1);

  get_no("AT+CPBF=\"User1\"\r\n", 1);
  Serial.println("---");
  Serial.print(adminOneNo);
  Serial.print(", ");
  Serial.println(adminTwoNo);
  Serial.println("---");
  
  get_no("AT+CPBF=\"User2\"\r\n", 2); 
  Serial.println("---");
  Serial.print(adminOneNo);
  Serial.print(", ");
  Serial.println(adminTwoNo);
  Serial.println("---");
  
  Serial.println();  
}

void get_no(char *user, int admin){         
  char gsmData[100]; // Reply from SIM
  int index = 0;

  Serial1.write(user);
  delay(90);

  while(Serial1.available()) {
    delay(3);  //delay to allow buffer to fill 
    if (Serial1.available() > 0) {
      char inChar = Serial1.read();
      //Serial.print(inChar);
      gsmData[index] = inChar;      
      index++;
    }
  }

  String incomingString = String(gsmData);

  if (incomingString.length() > 0) {

    char *atmessage = strtok(gsmData, "\r\n");
    char *reply     = strtok(NULL, "\r\n");

    //Serial.print("Data from gsm: ");
    //Serial.println(incomingString);
  
    if (strcmp(atmessage,"OK") == 0) {
      if (admin == 1) Serial.println("User1 does not exist.");
      else Serial.println("User2 does not exist.");
    } 
    else {

      Serial.print("\nMessage: ");
      Serial.println(atmessage);
      Serial.print("Reply: ");
      Serial.println(reply);
    
      char *atcommand = strtok(atmessage, "\"");
      char *number    = strtok(NULL, "\"");

      //Serial.print("Command: ");
      //Serial.println(atcommand);
      //Serial.print("Number: ");
      //Serial.println(number);

      if (admin == 1) {
        adminOne = true;
        adminOneNo = number;
        digitalWrite(adm1_pin, HIGH);
        Serial.print("User1: ");
        Serial.println(adminOneNo);
      } else if (admin == 2) {
        adminTwo = true;
        adminTwoNo = number;
        digitalWrite(adm2_pin, HIGH);
        Serial.print("User2: ");
        Serial.println(adminTwoNo);
      } 
    }

    clearSerial(gsmData);
    index = 0;
  }
  delay(100);  
}

This is the output:

GSM(SIM) READ/ADD PB ENTRY
Success!

Reading SIM...

Message: +CPBF: 3,"xxxxx9556785",129,"User1"
Reply: OK
User1: xxxxx9556785
---
xxxxx9556785, 
---

Message: +CPBF: 2,"xxxxx5217568",129,"User2"
Reply: OK
User2: xxxxx5217568
---
xxxxx5217568, xxxxx5217568
---

The end result should be xxxxx9556785, xxxxx5217568

Is the problem caused by the adminOneNo and adminTwoNo declaration?

I set it as char* because I'll be using a send function that requires this declaration.

void send_msg(char *number, char *msg)
2

The problem is, adminOneNo is a char *, adminTwoNo is a char * and number is a char *.

You are setting adminOneNo to the pointer when that pointer is valid, you then change that pointer and set it to the adminTwoNo. Now both point to the data from admin two.

3

Try this:

Change these globals at the top to this:

char adminOneNo[64];
char adminTwoNo[64];

Change this block to this, highlighted changed lines:

 if (admin == 1) {
        adminOne = true;
        strcpy( adminOneNo, number );  // changed
        digitalWrite(adm1_pin, HIGH);
        Serial.print("User1: ");
        Serial.println(adminOneNo);
      } else if (admin == 2) {
        adminTwo = true;
        strcpy( adminTwoNo, number );   // changed
        digitalWrite(adm2_pin, HIGH);
        Serial.print("User2: ");
        Serial.println(adminTwoNo);
      }

I'd suggest getting rid of that String class too and use C style strings.

4 
void clearSerial(char *data) {
  for( int i = 0; i < sizeof(data);  ++i )
    data[i] = (char)0;   
}

You STILL don't understand what "NULL terminated array of chars" means, obviously.

If code that is supposed to stop at the stop sign (the NULL) blows past the first stop sign, what makes you think any of the rest will be obeyed?

   delay(3);  //delay to allow buffer to fill

Nonsense. There is NOTHING that ensures that all the data will arrive over a slow serial port in 3 milliseconds.

  String incomingString = String(gsmData);

  if (incomingString.length() > 0) {

What a waste. There IS a strlen() function.

In the get_no() function, adminOneNo and adminTwoNo are pointed to locations in gsmData, which is local to the function. Using those pointers outside the function is not a good idea.

Even is the gsmData array did not go out of scope, pointing two pointers to locations in the array, and then expecting the pointers to point to different things is completely unrealistic.

You can NOT use pointers that way. You MUST create real arrays and COPY the data that you want to save.

5

Thank you for the reply! That's what I'm looking for, a way to get what the char* is pointing. Thank you very much! Everything seems okay now. Will move on to writing on sim.

This is the new code:

char Rx_data[50];
unsigned char Rx_index = 0;
int i = 0;
char msg[160];
int sig;

char adminOneNo[64];
char adminTwoNo[64];

boolean adminOne = false;
boolean adminTwo = false;

int gsm_pin  = 13;
int adm1_pin = 12;
int adm2_pin = 11;


void setup() {
  Serial.begin(9600);
  Serial1.begin(38400);  // GSM
  
  pinMode(gsm_pin, OUTPUT);
  pinMode(adm1_pin, OUTPUT);
  pinMode(adm2_pin, OUTPUT);
  
  digitalWrite(gsm_pin, LOW);
  digitalWrite(adm1_pin, LOW);
  digitalWrite(adm2_pin, LOW);

  Serial.println("GSM(SIM) READ/ADD PB ENTRY");

  initGSM();  
  readSIM();
}

void loop() {

}


//void clearSerial(char *data) {
//  for( int i = 0; i < sizeof(data);  ++i )
//    data[i] = (char)0;   
//}

// ----- GSM: TX

void send_msg(char *number, char *msg) {
  char at_cmgs_cmd[30] = {'\0'};
  char msg1[160] = {'\0'};
  char ctl_z = 0x1A;

  sprintf(msg1, "%s%c", msg, ctl_z);
  sprintf(at_cmgs_cmd, "AT+CMGS=\"%s\"\r\n",number);

  sendGSM(at_cmgs_cmd);
  delay(300);
  sendGSM(msg1);
  delay(100);
}

// ----- GSM: COMM

void sendGSM(char *string) { 
  Serial1.write(string);
  delay(90);
}

void clearString(char *strArray) { 
  int j;
  for (j = 100; j > 0; j--)
    strArray[j] = 0x00;
}

void send_cmd(char *at_cmd, char clr) { 
  char *stat = '\0';
  while(!stat) {
    sendGSM(at_cmd);
    delay(90);
    readSerialString(Rx_data);

    stat = strstr(Rx_data, "OK");
  }

  if (clr) {
    clearString(Rx_data);
    delay(200);
    stat = '\0';
  }
}

void readSerialString(char *strArray) { 
  if(!Serial1.available()) {
    return;
  }

  while(Serial1.available()) {
    strArray[i] = Serial1.read();
    //Serial.print(strArray[i]);
    i++;
  }
}

void initGSM(){  
  send_cmd("ATE0\r\n",1);               // Turn off automatic echo of the GSM Module	
  send_cmd("AT\r\n", 1);							
  send_cmd("AT+CMGF=1\r\n", 1);	        // Sucess 
  send_cmd("AT+CNMI=1,2,0,0,0\r\n", 1);	// Set message format to text mode


  Serial.println("Success!\n");	
  digitalWrite(gsm_pin, HIGH);
  delay(2000);
}

// ----- SIM

void readSIM(){  
  Serial.println("Reading SIM...");

  send_cmd("AT+CPBS=\"SM\"\r\n", 1);

  get_no("AT+CPBF=\"User1\"\r\n", 1);
  Serial.println("---");
  Serial.print(adminOneNo);
  Serial.print(", ");
  Serial.println(adminTwoNo);
  Serial.println("---");
  
  get_no("AT+CPBF=\"User3\"\r\n", 2); 
  Serial.println("---");
  Serial.print(adminOneNo);
  Serial.print(", ");
  Serial.println(adminTwoNo);
  Serial.println("---");
  
  Serial.println();  
}

void get_no(char *user, int admin){         
  char gsmData[100]; // Reply from SIM
  int index = 0;

  Serial1.write(user);
  delay(90);

  while(Serial1.available()) {
    if (Serial1.available() > 0) {
      char inChar = Serial1.read();
      //Serial.print(inChar);
      gsmData[index] = inChar;      
      index++;
    }
  }  

  if (strlen(gsmData) > 0) {

    char *atmessage = strtok(gsmData, "\r\n");
    char *reply     = strtok(NULL, "\r\n");

    //Serial.print("Data from gsm: ");
    //Serial.println(incomingString);
  
    if (strcmp(atmessage,"OK") == 0) {
      if (admin == 1) Serial.println("User1 does not exist.");
      else Serial.println("User2 does not exist.");
    } 
    else {

      Serial.print("\nMessage: ");
      Serial.println(atmessage);
      Serial.print("Reply: ");
      Serial.println(reply);
    
      char *atcommand = strtok(atmessage, "\"");
      char *number    = strtok(NULL, "\"");

      //Serial.print("Command: ");
      //Serial.println(atcommand);
      //Serial.print("Number: ");
      //Serial.println(number);

      if (admin == 1) {
        adminOne = true;
        strcpy(adminOneNo, number);
        digitalWrite(adm1_pin, HIGH);
        Serial.print("User1: ");
        Serial.println(adminOneNo);
      } else if (admin == 2) {
        adminTwo = true;
        strcpy(adminTwoNo, number);
        digitalWrite(adm2_pin, HIGH);
        Serial.print("User2: ");
        Serial.println(adminTwoNo);
      } 
    }

    //clearSerial(gsmData);
    //index = 0;
  }
  delay(100);  
}

I changed adminOneNo = number to strcpy(adminOneNo, number), commented the clearSerial and delay and also replaced string.length() to strlen().

Questions:
How can I clean the string if I don't use clearSerial? Is it ok to not do anything because it's just a local variable?

6

If you want to clear a C style string, just set the first element to 0. However in your case, strcpy will blitz it next time round.

clear like this:

char text[16];
strcpy( text, "DATA" );  // text contains "DATA"
text[0] = 0; // text as far as anyone is concerned is now clear