Will you review my sketch & give some advice? It's not working.

I am a newbie, so I paid someone to make a sketch, but it’s not working (it doesn’t respond to my SMS text commands of ON or OFF)
I’m using Uno R3 & Itead 3G shield:
Note: I am able to load a different sketch & it sends a message, so there IS communication.

The sketch is supposed to do this:
When Uno/3G shield receives SMS text “On”: relay 1 will activate for only 5 seconds
When it receives SMS text “Off”, relay 2 will activate for 5 seconds.
It will then send a SMS “Confirmed ON” or “Confirmed OFF”.
This should work from any phone, (not from a list of approved phone numbers). There should also be no passwords.
Also note: when I put in a sketch to send me a test SMS message: it only works if I do NOT have a “+” in front of my phone number. (note: I am in the USA).

Thanks!

Uno.zip (2.31 KB)

It's going to be hard to review your sketch if you don't post it.

Please explain exactly what you mean by "it's not working".

Jim_01.ino

#include <SoftwareSerial.h>
#include "setup.h"
 
SoftwareSerial GSM(GSM_RX_PIN, GSM_TX_PIN);
String gsmSerialBufferString;
String tempString;
String messageString;
String phoneNumber;

 
void setup(){
  GSM.begin(115200); 
  Serial.begin(115200);
  
  pinMode(GSM_RESET_PIN, OUTPUT);
  
  pinMode(RELAY_1_PIN, OUTPUT);
  pinMode(RELAY_2_PIN, OUTPUT);

  gsmSerialBufferString.reserve(GSM_BUFFER_SIZE+2);
  tempString.reserve(GSM_BUFFER_SIZE+2);
  messageString.reserve(GSM_BUFFER_SIZE+2);
  phoneNumber.reserve(48);
  
  digitalWrite(RELAY_1_PIN, RELAY_OFF);
  digitalWrite(RELAY_2_PIN, RELAY_OFF);
  
  Serial.print("Reseting GSM module");
  while (gsmReset() != GSM_STATUS_OK){
    Serial.print(".");
  }
  Serial.println("Done!");
  
  gsmSendCommand("AT+CLIP=1");
  gsmSendCommand("AT+CMGF=1");
  gsmSendCommand("AT+CSCS= \"GSM\"");
  gsmSendCommand("AT+CMGDA=\"DEL ALL\"");
}//setup()
 
 

////// 
void loop(){
if(GSM.available()){
    messageString = "";
    int event = gsmGetIncomingEvent();
    Serial.print("event: ");
    Serial.println(event);
    Serial.println("messageString: ");
    Serial.println(messageString);
    
      if (event == GSM_EVENT_SMS){
        phoneNumber = gsmGetSmsSendersNumber();
        Serial.print("incoming sms from number: ");
        Serial.println(phoneNumber);
//          if(gsmGetPasswordFromSms() == true){
            int command = gsmGetCommandFromSms();  
            
            switch (command){
              case COMMAND_NONE:
                messageString = "Unknown or incorrect command";
              break;
              
              case COMMAND_ON:
                messageString = "Confirmed ON";
                digitalWrite(RELAY_1_PIN, RELAY_ON);
                delay(RELAY_PAUSE);
                digitalWrite(RELAY_1_PIN, RELAY_OFF);                
              break;

              case COMMAND_OFF:
                messageString = "Confirmed OFF";
                digitalWrite(RELAY_2_PIN, RELAY_ON);
                delay(RELAY_PAUSE);
                digitalWrite(RELAY_2_PIN, RELAY_OFF);
              break;

            }
           
          Serial.println(messageString);
          gsmSendSms(phoneNumber, messageString);
          gsmReadSerial();
          Serial.println(gsmSerialBufferString);
 /*       }else{
          messageString = "wrong PIN";
          gsmSendSms(phoneNumber, messageString);
          Serial.println(messageString);
        }
 */
      }
      gsmPurgeSerial();
  }


}//loop()

gsm.ino

//////
int gsmReset(){
  Serial.print("reseting GSM module...");
  digitalWrite(GSM_RESET_PIN, LOW);
  delay(GSM_SERIAL_TIMEOUT/6);
  digitalWrite(GSM_RESET_PIN, HIGH);
  delay(GSM_SERIAL_TIMEOUT/6);
  digitalWrite(GSM_RESET_PIN, LOW);
  delay(GSM_SERIAL_TIMEOUT/2);
  if(gsmSendCommand("AT") == GSM_STATUS_OK){
    //Serial.println(" done!");
    return(GSM_STATUS_OK);
  }else{
    //Serial.println("ERROR");
    return(GSM_STATUS_ERROR);
  }
}//gsmReset()



//////
int gsmSendCommand(String command){
gsmPurgeSerial();
gsmSerialBufferString = "";
GSM.println(command);

int i = 0;
char c;
unsigned long gsmSerialMillis = millis();

  while (!GSM.available()){
  if (millis() - gsmSerialMillis > GSM_SERIAL_TIMEOUT){
        return(GSM_STATUS_SERIAL_TIMEOUT);
        }      
  }
  
  gsmSerialMillis = millis();
  while (i < GSM_BUFFER_SIZE && millis() - gsmSerialMillis < GSM_SERIAL_TIMEOUT){
        if(GSM.available()){
          c = GSM.read();
          delay(1);
          gsmSerialBufferString = gsmSerialBufferString + c;
          i++;
        }
  }
  
if(gsmSerialBufferString.indexOf("OK")){  
return (GSM_STATUS_OK);
}else{
  return (GSM_STATUS_ERROR);
}
  
}//gsmSendCommand()




//////
void gsmSendSms(String number, String message){
  char endChar[2];
  endChar[0]=0x1a;
  endChar[1]='\0';
  gsmSendCommand("AT+CMGS=\""+number+"\"");
  gsmSendCommand(message);
  gsmSendCommand(endChar);
  gsmSendCommand("AT+CMGDA=\"DEL ALL\"");
}//gsmSendSms()




//////
void gsmReadSerial(){
  int i = 0;
  char c;
  unsigned long gsmSerialMillis = millis();
  gsmSerialBufferString = "";
  
  while (!GSM.available() && millis() - gsmSerialMillis < GSM_SERIAL_TIMEOUT){
        }

  gsmSerialMillis = millis();
  
  while (i < GSM_BUFFER_SIZE && millis() - gsmSerialMillis < GSM_SERIAL_TIMEOUT){
        if(GSM.available()){
          c = GSM.read();
         // Serial.write(c);
          delay(1);
          gsmSerialBufferString = gsmSerialBufferString + c;
          i++;
        }
  }
}//gsmReadSerial()



//////
void gsmPurgeSerial(){
  while (GSM.available()){
    GSM.read();
    delay(1);
  }
}//gsmPurgeSerial()



//////
int gsmGetIncomingEvent(){
  gsmReadSerial();
  String res = gsmSerialBufferString;
  int clipIndex = res.indexOf("+CLIP: \"");
  int smIndex = res.indexOf("\"SM\",");
  int endIndex;

  if(clipIndex > -1){
    gsmSendCommand("ATH0");  
    gsmPurgeSerial();
    endIndex = res.indexOf("\",");
    gsmSerialBufferString = res.substring(clipIndex + 8, endIndex);
    return (GSM_EVENT_CALL);
    }
  if(smIndex > -1){
    gsmPurgeSerial();
    tempString = res.substring(smIndex + 5);

    gsmSerialBufferString = "AT+CMGR=" + tempString;
    gsmSendCommand(gsmSerialBufferString);  //
    res = gsmSerialBufferString;

    gsmSerialBufferString = "AT+CMGD=" + tempString;
    gsmSendCommand(gsmSerialBufferString);  //
    gsmSerialBufferString = res;
    return (GSM_EVENT_SMS);
  }
return (GSM_EVENT_NONE);
}//gsmGetIncomingNumber()




//////
String gsmGetSmsSendersNumber(){
  int separatorIndex = gsmSerialBufferString.indexOf("\",\"");
  if(separatorIndex > -1){
    tempString = gsmSerialBufferString.substring(separatorIndex + 3);
    separatorIndex = tempString.indexOf("\",\"");
    return(tempString.substring(0, separatorIndex));
    }  
    return ("");
}//gsmGetSmsSendersNumber()




//////
bool gsmGetPasswordFromSms(){
  if(gsmSerialBufferString.indexOf(PIN) > -1){
    return (true);
    }else{
    return (false);
  } 
}//gsmGetPasswordFromSms()



//////
int gsmGetCommandFromSms(){
  if(gsmSerialBufferString.indexOf("On") > 0 
    || gsmSerialBufferString.indexOf("ON") > 0 
    || gsmSerialBufferString.indexOf("on") > 0){
    return (COMMAND_ON);
    }
 if(gsmSerialBufferString.indexOf("Off") > 0 
    || gsmSerialBufferString.indexOf("OFF") > 0 
    || gsmSerialBufferString.indexOf("off") > 0){
    return (COMMAND_OFF);
    }
  
  return (COMMAND_NONE);
}//gsmGetCommandFromSms()

setup.h

#define PIN "password"

#define GSM_TX_PIN 7
#define GSM_RX_PIN 6
#define GSM_RESET_PIN 8

#define RELAY_1_PIN 3
#define RELAY_2_PIN 2

#define RELAY_PAUSE 6000

#define GSM_BUFFER_SIZE 256
#define GSM_SERIAL_TIMEOUT 6000

#define GSM_EVENT_NONE 0
#define GSM_EVENT_SMS 1
#define GSM_EVENT_CALL 2

#define GSM_STATUS_ERROR 0
#define GSM_STATUS_OK 1
#define GSM_STATUS_SERIAL_TIMEOUT 2
#define GSM_STATUS_NO_CONNECTION 3

#define RELAY_ON LOW
#define RELAY_OFF HIGH


//////
typedef enum {
COMMAND_NONE = 0,
COMMAND_ON, 
COMMAND_OFF
} GSM_COMMAND;

See how much better that is? Don't you think you'll get more help the easier you make it to see the code?

Sop you read from the GSM software serial, but you've got no way to know if the whole message has arrived. You might only read part of it on one pass, serial data is slow you read it a lot faster than it comes in. If your gsmReadSerial method doesn't read all the string in one pass it's OK, you can pick the rest up on the next pass EXCEPT that you do quite possibly the stupidest thing possible to do and dump all the rest of what's on the serial line without even looking at it in the gsmPurgeSerial function. What is the purpose of doing that?

Delta_G:
Sop you read from the GSM software serial, but you’ve got no way to know if the whole message has arrived. You might only read part of it on one pass, serial data is slow you read it a lot faster than it comes in. If your gsmReadSerial method doesn’t read all the string in one pass it’s OK, you can pick the rest up on the next pass EXCEPT that you do quite possibly the stupidest thing possible to do and dump all the rest of what’s on the serial line without even looking at it in the gsmPurgeSerial function. What is the purpose of doing that?

I am guessing: I told the guy who wrote the sketch that I don’t want the Sim Card to fill up (so maybe that is why he put in the gsmPurgeSerial function?). Again, I am new at this.
What I mean by not working: It does not respond to my ON or OFF SMS texts.

Delta_G:
See how much better that is? Don't you think you'll get more help the easier you make it to see the code?

Thanks!

Hansen70:
I am a newbie, so I paid someone to make a sketch, but it's not working

Seems like you should be pursuing this with that **someone ** >:(

Or pay someone who actually knows what they’re doing. When you see them dumping data indiscriminately like that then that’s a good sign that this isn’t someone you want to pay for coding.

gfvalvo:
Seems like you should be pursuing this with that **someone ** >:(

He’s not responding very well, that’s why I’m on this forum to find some good advice. Do you think it’s an easy fix?

Hansen70:
He's not responding very well, that's why I'm on this forum to find some good advice. Do you think it's an easy fix?

Is he going to give your money back?

Not really an easy fix, significant parts of the code may need to be restructured.

Delta_G:
Is he going to give your money back?

Not really an easy fix, significant parts of the code may need to be restructured.

Now that I know that significant parts of the code need to be restructured, I'll ask for my $ back.
Are you interested in making it work?