How to get a COMPLETE string from serial input?

Hi, everyone. I'm writing a small demo program for practice (I'm still quite new to the Arduino). What it does is it lights an LED after a certain number of button presses, and it prints messages to the serial port when the button and/or LED state changes. What I'm trying to do is to enable the user to change the interval at which the LED lights by sending a number to the Arduino via the serial port. I've been partially successful:

Welcome to my State Changer Arduino demo!
LED interval is now 3
down
Number of presses: 1
LED unlit
up
down
Number of presses: 2
LED unlit
up
down
Number of presses: 3
LED lit
up
LED interval is now 2
down
Number of presses: 4
LED lit
up
down
Number of presses: 5
LED unlit
up

However, if I try sending a double-digit number, say 23, I get this:

LED interval is now 2
LED interval is now 3

In other words, it's reading the characters individually, rather than as a string.

Here's the function that I wrote to get a string from the serial port:

char* GetSerialString()
{
    char string[256];
    int index = 0;
    
    while(Serial.available() > 0)
    {
        /*Read a character as it comes in:*/
        char byteBuffer = Serial.read(); 

        if(index < 255)
        {
            /*Place the character in the string buffer:*/
            string[index] = byteBuffer;
            /*Go to the index to place the next character in:*/
            index++;
        }
    }
    
    string[255] = '\0'; //Null-terminate the string;
    
    return string;
}

And the section in loop() where I call it:

if(Serial.available() > 0)
{
    char* serialInput = GetSerialString();
    interval = atoi(serialInput);
    Serial.print("LED interval is now ");
    Serial.println(interval,DEC);
}

I'm guessing the problem lies in the implementation of GetSerialString(), but I don't know for sure. I thought that what it was supposed to do was to read each byte (character) into "string" and null-terminate it. All I know is that I suck at character manipulation :stuck_out_tongue:

I apologize if this forum (as in the whole Arduino forums, not just this sub-forum) is a bad place to have this question; it's probably more of a C problem, not an Arduino API problem...

The problem is that the getSerialString function does not wait for the end of the string. As soon as it has processed all the data in the serial buffer, it returns. There is more data to be written to the serial buffer.

What you need to do is send a terminating character, and make getSerialString not return until it has seen that character.

Carriage return is not a good choice, since the serial monitor does not send a carriage return.

By the way, you should be adding a NULL after every character added to the array, not just one at the very end of the array.

Here is another approach for getting data from the serial monitor.

Send it a numeric value followed by a terminating character (any non digit character) to control the blink delay).

for example: 100# will set a delay of 100 milliseconds (note the # character at the end)

/*
 * Serial  Input (adapated from the arduino Analog Input example)
 */

int ledPin = 13;      // select the pin for the LED
int serialValue = 0;  // variable to store the value coming from serial
int ledDelay = 1000;     // start of with a delay of 1000 ms

void setup() {
  // declare the ledPin as an OUTPUT:
  pinMode(ledPin, OUTPUT);  
  Serial.begin(9600); 
}

void loop() {
  if( Serial.available())
  {
    char ch = Serial.read();

    if(ch >= '0' && ch <= '9')              // is ch a number?  
      serialValue = serialValue * 10 + ch - '0';           // yes, accumulate the value
    else  // its not a digit so terminate the input    
    {
        ledDelay = serialValue;
        serialValue = 0;
    }
  }
  digitalWrite(ledPin, HIGH);  
  delay(ledDelay);             
  digitalWrite(ledPin, LOW);   
  delay(ledDelay);                  
}

One advantage of this approach is that the incoming data is store in an integer (two bytes) instead of requiring a big character array)

The code is based on the analogInput example sketch supplied with arduino, except it uses serial data instead of analog read.

For example, this routine reads integers and terminates the number on any non-numeric character...

unsigned int get_int()
{
  int c;
  unsigned int i = 0;
  while (1) {
    while ((c=Serial.read()) <= 0)
      ; // spin waiting for char
    Serial.print(c);
    if (c < '0' || c > '9')
      break;
    i = (i * 10) + (c - '0');
  }
  return i;
}

The problem is that the getSerialString function does not wait for the end of the string.

...

What you need to do is send a terminating character, and make getSerialString not return until it has seen that character.

I'm not sure I understand what is meant here. Do you mean that I need to add a condition in my if clause to check if the input character isn't, say, "i", or "n", or something else?

i.e.:

if(index < 255 && byteBuffer != 'n')

This compiles, but it doesn't help. :frowning: I guess I really don't know enough about strings and serial parsing to know what I'm doing here...

EDIT: I modified the function (and the call to it) according to mem's example:

int GetNumberString()
{
    int value;
    
    while(Serial.available() > 0)
    {
        /*Read a character as it comes in:*/
        char byteBuffer = Serial.read(); 

        if(byteBuffer >= '0' && byteBuffer <= '9')
            value = (value * 10) + (byteBuffer - '0');
        else
            return value;
    }

    return 0;
}

...and its call:

interval = GetNumberString();

But now it just ouputs zeroes. I really screwed this one up, didn't I?

I'd like to be able to use the return value to assign to a variable, rather than modifying the variable directly within the function (as is done in the example). I assume this isn't the way to do it :stuck_out_tongue:

I'm getting closer...I just needed to initialize "value" to zero, and it's working a bit more like it should:

On inputting "123n":

LED interval is now 0
LED interval is now 23

I'm still analyzing the code...trying to think of it more literally.

I need to think in ASCII, not integers...

Maybe I should learn AVR asm...that's about as literal as you can get (unfortunately it's also as baby-step as you can get :().

EDIT: I'm starting to think the problem now lies outside the implementation of GetStringNumber (renamed for better conveyance of purpose).

Here's the new function (in case it's not):

int GetStringNumber()
{
    int value = 0;
    
    while(1)
    {
        /*Read a character as it comes in:*/
        char byteBuffer = Serial.read(); 

        if(byteBuffer >= '0' && byteBuffer <= '9') //Is the character a digit?
   /*Yes, shift left 1 place (in decimal), and add integer value of character (ASCII value - 48)*/
            value = (value * 10) + (byteBuffer - '0');
        else
        /*No, stop and give us the value we have so far*/
            return value;
    }
}

I think your big problem is that you're failing to understand that "Serial.Read()" returns
immediately EVEN IF THERE IS NO DATA TO BE READ (in this case it returns -1)

You first want to implement a "wait_for_char()" function:

char wait_for_char()
{
  int c;
  do {
     c = Serial.read();
  } while (c == -1);
  return (char) c;
}

Another way of handling the wait_for_char is with the available function you used above.

char wait_for_char()
{
  int c;
  while( Serial.available() < 1)
     ; // wait for at least one  character to be available
  c = Serial.read();
  return (char) c;
}

Westfw's version is slightly more efficient but the one above may be easier to understand

I think your big problem is that you're failing to understand that "Serial.Read()" returns
immediately EVEN IF THERE IS NO DATA TO BE READ (in this case it returns -1)

[smiley=embarassed.gif] ...The sad part about that is that if I had read the Reference in the first place, I would've known that...slaps forehead

I'm trying it with the wait function, but now it doesn't do anything. I guess that means the do-while never escapes, and so something's preventing the first character from being read:

char WaitForChar()
{
    int c;

    do {c = Serial.read();} while(c == -1);

    return (char)c;
}

and the call within GetStringNumber():

char byteBuffer = WaitForChar();

Should I just go ahead and post the whole code? I really think this is a structural thing...either that or I'm just dumb and don't know how to use that function properly. :frowning:

In the wait_for_char function, c can be declared a char. Then, there is no reason to cast it to a char to return it. Not that that is your problem.

So, yes, you should post all your code again.

In the wait_for_char function, c can be declared a char. Then, there is no reason to cast it to a char to return it.

Hmm, I had thought that it was declared as a (signed) int because it needed to be able to hold -1. But whatever, I'll fix it.

Here's the complete source to the sketch:

#define BBOARD_LED 12
#define BUILTIN_LED 13
#define BUTTON 2

bool currentButtonState = false,
        lastButtonState = false;

int bCounter = 0,
    interval = 2;

int GetStringNumber();

void setup()
{
    pinMode(BBOARD_LED,OUTPUT);
    pinMode(BUTTON,INPUT);
    Serial.begin(9600);
    Serial.println("Welcome to my State Changer Arduino demo!");
}

void loop()
{
    if(Serial.available() > 0)
    {
        interval = GetStringNumber();
        Serial.print("LED interval is now ");
        Serial.println(interval,DEC);
    }

    currentButtonState = digitalRead(BUTTON);
  
    if(currentButtonState != lastButtonState) //Is there any change in the button state?
    {
         if(currentButtonState == true)
         {
             bCounter++;
             
             Serial.println("down");
             Serial.print("Number of presses: ");
             Serial.println(bCounter,DEC);
             
             if(bCounter % interval == 0) //Has the button been pressed "interval" times?
             {
                 digitalWrite(BBOARD_LED,1);
                 Serial.println("LED lit");
             }
             else
             {
                 digitalWrite(BBOARD_LED,0);
                 Serial.println("LED unlit");
             }
         }
         else
             Serial.println("up");
    }
    
    lastButtonState = currentButtonState; //Updating button state   
}

char WaitForChar()
{
    char c;

    do {c = Serial.read();} while(c == -1);

    return c;
}

int GetStringNumber()
{
    int value = 0;
        
    while(1)
    {
        /*Wait for a valid character to enter the serial buffer,*
         *store it if we get one...please?                      */
        char byteBuffer = WaitForChar();
        

        if(byteBuffer > -1)
        {
            if(byteBuffer >= '0' && byteBuffer <= '9') //Is the character a digit?
        /*Yes, shift left 1 place (in decimal), and add integer value of character (ASCII value - 48)*/
                value = (value * 10) + (byteBuffer - '0');
            else
            /*No, stop and give us the value we have so far*/
                return value;
        }
    }
}

It's probably just full of beginner's mistakes, but at least it compiles...[smiley=rolleyes.gif]

SOLVED! ;D

It was a simple matter of changing the else branch to break out of the while, then return, rather than just return:

int GetStringNumber()
{
    int value = 0;
        
    while(1)
    {
        /*Read a byte as it comes into the serial buffer*/
        char byteBuffer = Serial.read();
        

        if(byteBuffer > -1) //Is the data a valid character?
        {
            if(byteBuffer >= '0' && byteBuffer <= '9') //Is the character a digit?
        /*Yes, shift left 1 place (in decimal), and add integer value of character (ASCII value - 48)*/
                value = (value * 10) + (byteBuffer - '0');
            else
                /*No, stop*/
                [glow]break;[/glow]
        }
    }

   [glow]return value;[/glow]
}

It now responds correctly when entering a number, then a non-number. Thanks for the help, guys!

Can't believe I didn't see that before, LOL. :smiley: