odd behivor of loop function

Hi All
I observed an odd behavior of loop() function. If anyone could help me to understand it.
Here is the code below. In the monitor window, when I send a long string, say 32 chars in one try,
I am expecting that ReadComString() will read in all chars before return since there is a while loop checking Serial.available()
but when delay time MS set to 10 or less, often the incoming string is broken in to several pieces. But when MS set to 500, whole string will be read correctly.
When the string is very long, say 100 chars, it always breaks into couple of pieces. Question is why this while loop can not hold when delay is set less than 10 ms?

#include <arduino.h>
#include <Time.h>
#include <string.h>
#include <memory.h>
#include <stdio.h>
#include <stdlib.h>
#include <typedef.h>

String ComStr="";
int MS = 10;
int loopindex=0;
int ReadComString()
{
byte s=0;
ComStr="";
int n=0;
while ( Serial.available())
{

s = Serial.read();
if(s== (byte)'\r' )
{
//discard
}
else if( s == (byte)'\n')
{
ComStr+='\0';
}
else
{
ComStr+= (char)s;
n++;
}

}

return n;
}

void setup()
{
Serial.begin(9600);
Serial.flush();
}

void loop()
{
if(loopindex==0)
{
Serial.println("start");
loopindex++;
}

int n = ReadComString();
if(n>0)
{
Serial.println(ComStr);
}
delay(MS);
}

A few points here.

  1. You don't have to do both serial.available and serial.read. serial.read will return -1 if no data is available.
  2. The handling of \n looks wrong to me. Putting a \0 will terminate the string, but you don't return
    or increment the character count. That may be your problem right there.
  3. Since your delay is outside of ReadComStr, it doesn't effect the function.
  4. it's entirely possible that if the sending side is very slow, there will be gaps between the characters when serial.available will say there's no data. It is more reliable to wait for the end of string marker, which you said was a return (\n) in this case.
  5. There's already a function that does this, (except for skipping the \r).
    see: Serial.readBytesUntil() - Arduino Reference
  1. Serial.available() returns the number of characters already in the serial input buffer. It doesn't wait for characters to arrive on the serial interface or anything like that.

  2. The Serial.flush() is completely unnecessary. It waits till all characters in the outgoing buffer are sent. As you haven't sent anything, it does nothing.

  3. Don't use a delay() if it's not necessary.

  4. Don't use the String class, it clobbers your memory and the current IDE has a bug that produces a memory leak.

ckiick:

  1. You don't have to do both serial.available and serial.read. serial.read will return -1 if no data is available.

I don't think that's good advice. You should be in the habit of checking for bytes being available before you read them.

I expect the original problem was caused by the fact that the Arduino can read received characters from the UART quicker than the UART receives them over the serial line. From the Arduino's point of view the characters are received extremely slowly and you would not expect to receive a lot of characters in one go unless you had waited for them all to be received.

Thanks all for the reply. Some suggestion are very good

I find the possible reason:

The while loop in the function GetComString() might be interrupted by hardware COM. Now I put the while loop inside the loop(), instead of using a function returning a string, when the interrupt happened, it will continue the while loop since it is in loop(). What I need to do is the break of while loop does not necessary means a complete new line has been read in, I need check if it read in a newline token and flag it.

The function GetComString() always return a string, even it is interrupted by other interruption. That causes the string been broken into small pieces. Now I do not need any delay.

Thanks