what's wrong with my code??

Projects Programming
1

Hello! I am trying to retrieve phone numbers from the gsm module and store it in an array which is working. But there is just a little problem, after uploading, when I hit '1' from the keypad, it wouldn't show the number. But after hitting another key, that will be the time the previous number will be shown.Hence, the output is delayed.

#include <Keypad.h>

//SMS
char incoming_char=0;       //Initiate a variable that will hold the incoming character from the Serial Port.
char numberIn[12];

//Keypad
char key3;
const byte ROWS = 4; 
const byte COLS = 3; 
// Define the Keymap
char keys[ROWS][COLS] = {
{'1','2','3',},
{'4','5','6',},
{'7','8','9',},
{'*','0','#',}
};
// Connect keypad ROW0, ROW1, ROW2 and ROW3 to these Arduino pins.
byte rowPins[ROWS] = {24,34,32,28}; 
byte colPins[COLS] = {26,22,30};
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );

void setup()
{
  Serial.begin(9600);
  Serial1.begin(9600);
}
void loop()
{
  key3 = keypad.getKey();
  if (key3=='1')
  {        
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();
    
   
  }
  if (key3=='2')
  {
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();
   
  }
  if (key3=='3')
  {
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();
 
  }
  if (key3=='4')
  {
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();
   
  }
  if (key3=='5')
  {
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();           
  }
}

void readPBentry()
{  
  if(Serial1.available() >0)
  {
    incoming_char=Serial1.read();
    Serial.print(incoming_char);
    while(incoming_char != '+')     //Wait for the start of the communication from
    {                               //   the shield
      incoming_char=Serial1.read();
      delay(10);
    }
    incoming_char=Serial1.read();
    Serial.print(incoming_char);
    if(incoming_char == 'C')        //Wait for the character CMGR
    {
      incoming_char=Serial1.read();
      Serial.print(incoming_char);
      delay(10);
      if(incoming_char == 'P')
      {
        incoming_char=Serial1.read();
        Serial.print(incoming_char);
        delay(10);
        if(incoming_char == 'B')
        {
          incoming_char=Serial1.read();
          Serial.print(incoming_char);
          delay(10);
          if(incoming_char == 'R')
          {
            while(incoming_char != ',')      
            { 
              incoming_char=Serial1.read();
              Serial.print(incoming_char);
            }
            incoming_char=Serial1.read();
            Serial.println(incoming_char);
            delay(10);
                                    
            for (int readNumber=0; readNumber<=10; readNumber++)
            {
              incoming_char=Serial1.read();
              Serial.print(incoming_char);                          
              numberIn[readNumber]=incoming_char;
              numberIn[readNumber+1]='\0';  
            }            
            Serial.println();
            Serial.print("num: ");
            Serial.println(numberIn); 
            Serial.print(numberIn);           
          }
        }
      }
    }
  }
   //If a character is coming from the terminal to the Arduino...
  if(Serial.available() >0)
  {
    incoming_char=Serial.read();  //Get the character coming from the terminal
    Serial1.print(incoming_char);    //Send the character to the cellular module.
  }
}
2

Shown? Where? On the serial output, or the LCD?

Would you mind explaining why you do exactly the same thing for all 5 "if" tests?

  if (key3=='1')
  {        
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();
  }
  if (key3=='2')
  {
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();
  }

...

Wouldn't it be easier to say:

  if (key3 >= '1' && key3 <= '5')
  {        
    Serial.print("keypress: ");
    Serial.println(key3);
    Serial1.print("AT+CPBR=");
    Serial1.println(key3);
    readPBentry();
  }

?

3

oh sorry! it should be displayed in the serial monitor, I have removed the lcd.begin() from the code. And for the "if's", you just make my life easy. I always make long codes as I am not good in programming. Thanks for that!

4

here's my the serial output so you can see the problem:

keypress: 1     //this is when I pressed key '1', the data will not be shown until I press another key
keypress: 2             //which is in this case, '2'
ACPBR=1

+CPBR: 1,"              //so here, the index is 1 instead of 2 since I already pressed key '2', 
09496700737
num: 09496700737
09496700737keypress: 3     //same thing happens here.. 
"CPBR=2

+CPBR: 2,"           
09157553672
num: 09157553672
09157553672keypress: 4
"CPBR=3

+CPBR: 3,"
09152765809
num: 09152765809
09152765809keypress: 5
"CPBR=4

+CPBR: 4,"
09155685398
num: 09155685398
09155685398
5 
void readPBentry()
{  
  if(Serial1.available() >0)
  {
    incoming_char=Serial1.read();
    Serial.print(incoming_char);
    while(incoming_char != '+')     //Wait for the start of the communication from
    {                               //   the shield
      incoming_char=Serial1.read();
      delay(10);
    }
    incoming_char=Serial1.read();
    Serial.print(incoming_char);
    if(incoming_char == 'C')        //Wait for the character CMGR
    {
      incoming_char=Serial1.read();
      Serial.print(incoming_char);
      delay(10);
      if(incoming_char == 'P')
      {
        incoming_char=Serial1.read();
        Serial.print(incoming_char);
        delay(10);
        if(incoming_char == 'B')
        {
          incoming_char=Serial1.read();
          Serial.print(incoming_char);
          delay(10);
          if(incoming_char == 'R')
          {
            while(incoming_char != ',')      
            { 
              incoming_char=Serial1.read();
              Serial.print(incoming_char);
            }
            incoming_char=Serial1.read();
            Serial.println(incoming_char);
            delay(10);
                                    
            for (int readNumber=0; readNumber<=10; readNumber++)
            {
              incoming_char=Serial1.read();
              Serial.print(incoming_char);                          
              numberIn[readNumber]=incoming_char;
              numberIn[readNumber+1]='\0';  
            }            
            Serial.println();
            Serial.print("num: ");
            Serial.println(numberIn); 
            Serial.print(numberIn);           
          }
        }
      }
    }
  }
   //If a character is coming from the terminal to the Arduino...
  if(Serial.available() >0)
  {
    incoming_char=Serial.read();  //Get the character coming from the terminal
    Serial1.print(incoming_char);    //Send the character to the cellular module.
  }
}

Just my guess, you call readPBentry() before the serial1 data has been sent, it hits that first if (if(Serial1.available() >0) and shoots right through. Next time you hit a keypad character the data from the first time is all queued up and ready.

You might want to start the function with:
while (Serial1.available() < 1);
Then it will hang until the data comes in, which ain't much better or worse than what you've got anyway.

readPBentry() is entirely a mess. One missed or mis-transmitted character or just a read made before the next serial character is available (and in some places you don't check at all but what the hey, you have delays to make it work) blows the whole scheme and probably many that follow in unpredictable ways making debugging an even bigger mess. You need to code for errors always. Murphy was an Optimist.

6

If I may suggest:

That shows how you can read serial input, without delays and without a lot of separate reads. There is also a demonstration state machine.