Possible Serial Overflow?

I’m writing a program for a project of mine, and I’m trying to read incoming data from a Sony Ericsson T610. It supports text mode, so I thought that this would me easier. I’m able to send messages and calls, but this one is over my head.

Anyways the incoming message I sent myself was:

This is a test message. The quick brown fox jumps over the lazy dog. = /:;()§_DISCOURSE_HOISTED_CODE_0_§@".,?!'

When I run the following code on my arduino:

#include <SoftwareSerial.h>

SoftwareSerial phone(10,11);

void setup()
{
  phone.begin(9600);
  Serial.begin(4800);
  delay(1000);
  
  //sendMessage("Starting Up...");
  
  Serial.println("Going to Read for messages:");
  
  Serial.println(readMessage());
}



void loop()
{

  
}


String readMessage()
{
  phone.println("AT+CMGF=1");
  delay(1000);
  Serial.println(checkPhone());
  phone.println("AT+CPMS=\"ME\"");
  delay(1000);
  Serial.println(checkPhone());
  phone.println("AT+CMGR=1");
  delay(2000);
  
  Serial.println("HELLO WORLD FROM READmESSAGE!");
  phone.println("AT+CMGR=1");
  delay(50);
 
  char inChar;
  String messageString;
    while (phone.available() > 0)      // if the Serial phone is available (Putting something in serial)
  {
    delay(10);                              //wait for a split second (for good measure again)
    inChar =((char) phone.read());      //print out the phone's message (one char at a time)
    messageString += inChar;
  }
  
  Serial.println(messageString);

  
}

  

void sendMessage(String message)
{
  phone.println("AT+CMGF=1");
  delay(700);
  phone.println("AT+CMGS=\"XXXXXXXXXX\"");
  delay(700);
  phone.println(message);
  delay(1000);
  byte ctrlz = 26;
  phone.write(ctrlz);
  
  while(phone.read() != -1 )
  {
    delay(10);
  }
  
  Serial.println("Sent Message: " + message);
}


String checkPhone()
{
  String returnString = ""; 
   
  while(phone.available() > 0)
  {
    returnString += ((char) phone.read());
    delay(100);
  }
  
  return returnString;
}

And here is the output that I’m getting:

æx3xóü?~Ìü??0æGoing to Read for messages:
+CMGF=1

OK

+CPMS="ME"

+CPMS: 1,70,0,30,1,70

OK

HELLO WORLD FROM READmESSAGE!
+CMGR=1

+CMGR: "REC READ","+1XXXXXXXXXX","Mitch","13/02/28,21
+CMGR: "RE"4"1Thisae ml:

OK

=1AT+CMGS="XXXXXXXXXX"Sent Message: AT+CPMS="ME"AT+CMGR=1HELLO WORLD FROM READMESSAGE!Going to Read for messages: ½ó
çÆijN
ó
ä	
õ	>
?è
#%rììé¦ "RE"4"1Thisae ml:

OK
XXXXXXXXXX","Mitch","13/02/28,21
+CMGR:W/?0!&)?èSÅÄÀÁÂÆaÛ½MGR=1

+CMGR: "REC READ","+XXXXXXXXXXX","Mitch","13/02 // phone number omitted for obvious reasons.

Anyways… If I could get ideas / inspiration on how to put the phone number into a string, and the message content into a string, I would be ever grateful!

P.S. If I get info on how to fix this, I’m definitely going to build a library and publish it for everyone to use!

As a general recommendation, the String class at present has some bugs, so using a char[] may be safer. But this is not the problem here. I see two problems:

The problem number one is that you aren't posting the code producing the results: the sendMessage function is never called in your version, so it's hard to tell how the output can contain an AT+CMGS.

The problem number two is that you don't check the response from the phone frequently enough. Yes, an overflow of the input buffer associated to phone is more than likely, but given problem number one I don't know how I can help.

Having no code in loop is silly. but...

  char inChar;
  String messageString;
    while (phone.available() > 0)      // if the Serial phone is available (Putting something in serial)
  {
    delay(10);                              //wait for a split second (for good measure again)
    inChar =((char) phone.read());      //print out the phone's message (one char at a time)
    messageString += inChar;
  }

and this is just daft. Why not just write the char read form phone stright to Serial. There is just no need for the string.

Mark

holmes4: Having no code in loop is silly. but...

I don't have anything in the loop because I don't have any functions that I desire to be repeated right now - the compiler will give me errors otherwise.

holmes4: and this is just daft. Why not just write the char read form phone stright to Serial. There is just no need for the string.

Most of the code works fine if I just write it to Serial, but the purpose of putting it into a string is so that I can use string methods on it to create a string containing the phone number that the SMS was from, and a String to say what the SMS message was.

That way I can create a library that lets me send and receive SMS messages easily. And then I can create events that differ depending on the SMS message and the SMS sender.

spatula: The problem number one is that you aren't posting the code producing the results: the sendMessage function is never called in your version, so it's hard to tell how the output can contain an AT+CMGS.

I posted the whole sketch. I don't know how sendMessage is called. I just didn't need it in my setup while I'm trying to read a message, so I just didn't call it.

spatula: The problem number two is that you don't check the response from the phone frequently enough. Yes, an overflow of the input buffer associated to phone is more than likely, but given problem number one I don't know how I can help.

How can I flush the input using Arduino 1.0.3? I have tried including

while(phone.available > 0);
  1. flush works on the output buffer not the input.

  2. Don’t use Strings. There’s almost a thread a day saying don’t use String.

Mark

I went through my code to comment it and I found some silly errors. Like readMessage() somehow had a return type of String instead of void. Anyways I corrected some code and I’m a little closer to my objective.

Here is what my most current output is:

Going to Read for messages:
+CMGF=1

OK

+CPMS="ME"

+CPMS: 4,70,0,30,4,70

OK

+CMGR=1

+CMGR: "REC READ","+1xxxxxxxxxx","Mitch","13/02/28,21
End of Life

Here is what it Should be:

Going to Read for messages:
+CMGF=1

OK

+CPMS="ME"

+CPMS: 4,70,0,30,4,70

OK

+CMGR=1

+CMGR: "REC READ","+1xxxxxxxxxx","Mitch","13/02/28,21"
This is a test message. The quick brown fox jumps over the lazy dog. = /:;()§_DISCOURSE_HOISTED_CODE_1_§@".,?!'
OK

End of Life

holmes4:
2. Don’t use Strings. There’s almost a thread a day saying don’t use String.

Mark

The point of putting this into one string is so that I could read it and have one String identity = cell sender number, and String smsMessage = the message that the cell phone received. Then I could use those two strings and look at the number for identify the user and then the message to determine what action to take (e.g start the car, roll down windows, shut off, lock doors, etc.) If there could be a easier way than using Strings, I’m all yours. I just don’t know how to make a char ArrayList (if things even exist on arduino) or a char array that would be big enough for the whole output of the “AT+CMGR=1” output, yet small enough that it isn’t wasting space.

Here is my updated code WITH COMMENTS :slight_smile:

#include <SoftwareSerial.h>

SoftwareSerial phone(10,11);        //defines the phone as RX, TX pins

void setup()
{
  phone.begin(9600);               //Starts the  phone at 9600 baud           
  Serial.begin(4800);              //Starts the Serial at 4800 baud
  delay(1000);                     //waits time to get started

  
 Serial.println("Going to Read for messages:"); //Output telling the user that it is going to check for messages
  
  readMessage();             //CALL readMessage
  
  Serial.print("End of Life");   //to tell me (for debugging) when the program has finished
}  



void loop()
{
  //nothing is included here because I don't have anything that I want to loop - I just want to run this stuff once during testing. 
}


void readMessage()
{
  phone.println("AT+CMGF=1");                //Sets the phone to text mode
  delay(1000);                               //waits for time so it can respond
  Serial.println(checkPhone());             //prints available output to Serial (to see for errors)
  phone.println("AT+CPMS=\"ME\"");          //Tells the phone to list SMS memory space
  delay(1000);                              //waits for it to respond
  Serial.println(checkPhone());             //output from phone 
  phone.println("AT+CMGR=1");                //Says READ TEXT 1
  delay(1000);                                //this usually takes a while 
 
  char inChar;                    //defining my char
  String messageString;           //defining my string which will eventually be used for output. 
  
  
  unsigned long bTime = millis();     //marks the beginning time this started
  unsigned long eTime = 5000;         //marks how long I want the phone to be checking for
  
  
    while ((millis()-bTime) < eTime)  //run this chunk of code for 5 seconds. 
  {
    if (phone.available() > 0 )     // if the Serial phone is available (Putting something in serial)
    {
    delay(10);                           //wait for a split second (for good measure again)
    inChar =((char) phone.read());      //put serial char into a char
    messageString += inChar;            //add that char to a string
    }
  }
  
  Serial.println(messageString);        //print the message. 

  
}

String checkPhone()
{
  String returnString = "";           //define return string
   
  while(phone.available() > 0)        //when it is available, 
  {
    returnString += ((char) phone.read());  //put the output in the string
    delay(100);
  }
  
  return returnString;        //return the string
}
    delay(10);                           //wait for a split second (for good measure again)

This is still stupid. If there is data to read, and there must be, of you would not have gotten here, waiting serves no useful purpose. This is a bandaid that sometimes works, and sometimes does. There is some kind of end of packet marker. Read the data as fast as possible, in a nested while loop, looping until the end of packet marker arrives.

Do you have an example of the string sent from the phone? Does the string sent from the phone have a delimiter to indicate the end of the sent string?

You've got loads of delays "that should take care of" problems. But the main problem is not understanding what's going on with Arduino and serial, the mismatch in speeds and asynchronous operations in general.

Arduino is magnitudes faster than serial. Salting in delays is an iffy way to 'fix' that. One char to the next a 4800 baud (480 chars/sec, 8 bits data plus stop and start bits, baud is bit-rate) is to Arduino like 30 mins is to you.

You don't just use loop() to repeat results. Use it to keep on top of your ongoing task in a responsive way as if that task, like capturing serial text, may take hours. Arduino may run through loop() 100's of times between serial chars arriving, or 1000's of times.

Treat serial data capture as Real Time because that's how it arrives, a char at a time over time.

Use loop() to capture your data a char at a time and [u]when it's all really in[/u] set a flag that other code outside of that block will act on.

loop() is for handling Real Time events whether microseconds or days apart. It's how to wait for one or more things while still handling others. It is the opposite of the top-down code. It is far more flexible and tends to be less complex for any but small tasks. It is easier to debug and far less prone to becoming spaghetti than top-down code.

inside loop()....

if (Serial.available()) { get the char and either buffer it or act on it the buffer should be a char array, not a C++ String Object copying itself just to add 1 char when the end of the text marker arrives, set a variable to flag "message complete" }

if (message complete flag is set) { now you do what you do with the complete message }

You add other steps to send messages, etc, all in separate blocks and NONE inside another, unable to act except by passing through multiple levels of if's and while's.

Make sure that each one has a control so that it only runs when it should. That could involve one or more flags that -other- blocks set (like only running if the whole message is in) and/or time (check if an interval is past or a certain time is reached) and/or data available from a device.

If few or no blocks run in one pass through loop() then it will be quicker at getting to the ones that do need attention. If you have a task that needs to wait, break it down to pieces that can run instantly and have each flag the next even as it clears it's own trigger when it's done.

You should never have a part that holds everything else up just to wait.

Stick to that and you can add task blocks to do the extras you didn't plan on when you started writing the sketch without trying to figure out how to jam the new thing to a complex logic structure because you won't need complex logic structures, you replace those with flags and state variables.

Stick to that and your sketch will be flexible and responsive as long as you don't push it hard by say reading a pin millions of times per second or trying to process video. Yes, 8 bits at 16 MHz does have limits and you can exceed them. be reasonable and you won't get more than close.

Okay, I made a sketch that let me communicate with the phone to the Arduino’s Serial!

GoForSmoke:
You’ve got loads of delays “that should take care of” problems. But the main problem is not understanding what’s going on with Arduino and serial, the mismatch in speeds and asynchronous operations in general.

Arduino is magnitudes faster than serial. Salting in delays is an iffy way to ‘fix’ that. One char to the next a 4800 baud (480 chars/sec, 8 bits data plus stop and start bits, baud is bit-rate) is to Arduino like 30 mins is to you.

Thanks for the info about delay(), however in my sketch I found out that I actually needed it in sending the code to the phone, but not reading the code. Here is my sketch:

//Used to talk to Sony Ericsson Phones over AT commands
//Made by Mitch Duncan on Mar. 2, 2013
//Feel free to modify

#include <SoftwareSerial.h>;
String inputString;


SoftwareSerial phone(10,11); // Sets up the software serial 
                             // between the phone and the arduino

void setup()
{
  phone.begin(9600);        //starts the virtual serial at 9600 baud
  delay(100);               //waits for good measure
  Serial.begin(9600);       //begins real serial (for serial monitor) at 4800 baud
  delay(100);               // waits again for good measure
  Serial.println("The connections have been established");    //used to know that the program has started
  
}

void loop()
{
  
  while (phone.available() > 0)      // if the Serial phone is available (Putting something in serial)
  {

    Serial.print((char) phone.read());      //print out the phone's message (one char at a time)
  }
  
  while (Serial.available() > 0) {          //when the user enters something into the Arduino's serial
    delay(10);                              //wait for good measure THIS IS WHERE I NEED DELA
    char letter =  Serial.read();           //read back what it was so the console  can echo it
    inputString += letter;                  // add that to a string to print
  }
  
  if (inputString.length() > 0)            //if there is something in the string (something was printed from the user)
    {  
      if (inputString.equals("26"))        //if the user wants to do "CONTROL-Z"
      {
        byte ctrlz = 26;
        phone.write(ctrlz);
        Serial.println("26 ctrlz");
        inputString = "";
      }
      else
      {
      phone.println(inputString);          //send it to the phone
      Serial.println(inputString);         //send it back (for echoing purposes)
      inputString = "";                    //clear the string
      }
    }
    
    
}

Thanks!

Is it your intention that the user will type in '2' and '6' to request a ctrl-Z, and that any other input goes to the phone directly? This seems like a strange approach. Wouldn't it be better to have the serial client the ctrl-Z character directly, so you can just pass through everything transparently? If you did that, you wouldn't need to do any buffering at all.

Supposing you decide to keep that ctrl-Z behaviour anyway, you really need to stop using the String class, unless you've patched the free() bug. It's giving you no advantage here and leaving you vulnerable to a known memory corruption problem.

Using delay() to slow down serial reading is bad practice. While it's a possible solution to the problem of finding when the input string is complete, it's not a very good solution and I suggest you do the job properly using a delimiter - and then get rid of the delay().

Learn the BlinkWithoutDelay sketch, then you will know how to handle tasks-in-time and generally still have time for more.

You don't need blocking code.

Blocking code kills your code's ability to do more than one thing at the same time. Blocking code kills your code's ability to respond to change quickly.

How that one little, needs no wiring sketch works is the key to not needing blocking code.

From there, learn about state machines.