next topic »

[on:]

hi all,

My code constructs string commands from serial input. However, the command is not picked up when the total length is longer than 9 characters.

How do i extend this?

Code:

String cmd;

void setup(){
  Serial.begin(9600);
  cmd = "";
}

void loop()
{
  serialReader();
}

void serialReader(){
  int makeSerialStringPosition;
  int inByte;
  char serialReadString[300];
  const int terminatingChar = 13; //Terminate lines with CR
 
  inByte = Serial.read();
  makeSerialStringPosition=0;
 
  if (inByte > 0 && inByte != terminatingChar) { //If we see data (inByte > 0) and that data isn't a carriage return
    delay(10); //Allow serial data time to collect (I think. All I know is it doesn't work without this.)

    while (inByte != terminatingChar && Serial.available() > 0){ // As long as EOL not found and there's more to read, keep reading
      serialReadString[makeSerialStringPosition] = (char) inByte; // Save the data in a character array
      makeSerialStringPosition++; //Increment position in array
      //if (inByte > 0) Serial.println(inByte); // Debug line that prints the charcodes one per line for everything recieved over serial
      inByte = Serial.read(); // Read next byte
    }

    if (inByte == terminatingChar) //If we terminated properly
    {
      Serial.println("command received");
      serialReadString[makeSerialStringPosition] = 0; //Null terminate the serialReadString (Overwrites last position char (terminating char) with 0
      cmd = serialReadString;
      Serial.println(cmd);
    }
  }
}

Thank you!

[Reply #1 on:]

How do i extend this?

Begin with getting rid of the String class and all instances of it.

Code:

void loop()
{
  serialReader();
}

Unless this is the start of a larger project, or trimmed down code to illustrate a problem, calling just one function from loop() is unnecessary overhead.

Code:

  inByte = Serial.read();

You don't know that there is anything to read.

Code:

  if (inByte > 0 && inByte != terminatingChar) { //If we see data (inByte > 0) and that data isn't a carriage return

I'd recommend NOT using compound statements in this if statement. Nested ifs, in this case, are far easier to understand.

Code:

    delay(10); //Allow serial data time to collect (I think. All I know is it doesn't work without this.)

You really need to understand why, and get rid of this. Waiting around for each character is a waste of time.

Looping UNTIL a specific character arrives is a whole different story.

This code will read all the serial data and store it in an array, without a single String or delay:

Code:

#define SOP '<'
#define EOP '>'

bool started = false;
bool ended = false;

char inData[80];
byte index;

void setup()
{
   Serial.begin(57600);
   // Other stuff...
}

void loop()
{
  // Read all serial data available, as fast as possible
  while(Serial.available() > 0)
  {
    char inChar = Serial.read();
    if(inChar == SOP)
    {
       index = 0;
       inData[index] = '\0';
       started = true;
       ended = false;
    }
    else if(inChar == EOP)
    {
       ended = true;
       break;
    }
    else
    {
      if(index < 79)
      {
        inData[index] = inChar;
        index++;
        inData[index] = '\0';
      }
    }
  }

  // We are here either because all pending serial
  // data has been read OR because an end of
  // packet marker arrived. Which is it?
  if(started && ended)
  {
    // The end of packet marker arrived. Process the packet

    // Reset for the next packet
    started = false;
    ended = false;
    index = 0;
    inData[index] = '\0';
  }
}

You can change the SOP and EOP values (or remove the SOP and started stuff),

You can move the while code to a separate function, in a while(!ended) loop, and return when ended is true.

[Reply #2 on:]

You're right with your comments, instead of correcting it I went for yours.

Thanks!