How can I simplify this simple code? (Again...)

Ok, so I've really appreciated everyone's advice on this forum for simplifying code, making it more efficient, etc... So far, I've removed 3000 bytes from my autopilot base code and counting.

Here is some code which reads an incoming data stream from the IMU via the Serial port. It starts with the '*' character and ends with '#'. Is there a way I can make this simpler/faster?

float Plane::current(byte input)
{
  char x;
  char inData[512];
  String inDataString = "*";
  
  while (Serial.available() == 0) { }
  delay(0.5); // need delay to prevent gibberish
  while (1) { if(Serial.read() == '*') { break; } }
  while (1) { /// if statement used to be below...
      x = Serial.read();
      inDataString.concat(x);
      if (x == '#') {break;}
  }
 
  inDataString.toCharArray(inData, 512);
  int returnVal = sscanf(inData, "*%[^,],%[^,],%[^,],%c,%c,%[^,],%[^,],%[^,]#", &initRollReading, &initPitchReading, &initYawReading, &rollDirection, &pitchDirection, &initRollSpeed, &initPitchSpeed, &initAltitude);
  if (input == ROLL) {
    rollReading = atof(*&initRollReading);
    return rollReading;
  }
  else if (input == PITCH) {
    pitchReading = atof(*&initPitchReading);
    return pitchReading;
  }
  else if (input == YAW) {
    yawReading = atof(*&initYawReading);
    return yawReading;
  }
  else if (input == ROLLSPEED) {
    rollSpeed = atof(*&initRollSpeed);
    return rollSpeed;
  }
  else if (input == PITCHSPEED) {
    pitchSpeed = atof(*&initPitchSpeed);
    return pitchSpeed;
  }
  else if (input == ALT) {
    alt = atof(*&initAltitude);
    return alt;
  }
  else if (input == SPEED) {
    currentSpeed = atof(*&initSpeed);
    return currentSpeed;
  }
}

Thanks in advance,
Zachary

Hello,

  • you could use char arrays instead of the String class
  • Serial buffer's size is 64 bytes only (for Arduino Mega, for some other boards it's 16 I believe)
  • do not use delay() or blocking code such as your while loops, especially if the thing is a plane autopilot :slight_smile:

For non blocking Serial reads you can do something like that:

void loop()
{
  if ( Serial.available() > 0 ) 
  {
    static char input[64];
    static uint8_t i;
    char c = Serial.read();

    if ( c != '#' && i < 63)
      input[i++] = c;
    
    else
    {
      input[i] = '\0';
      i = 0;

      //process the input string here
      Serial.println( input );
    }
  }
}
delay(0.5); // need delay to prevent gibberish

Sadly, delay takes an integer parameter

guix:
For non blocking Serial reads you can do something like that:

void loop()

{
 if ( Serial.available() > 0 )
 {
   static char input[64];
   static uint8_t i;
   char c = Serial.read();

if ( c != '#' && i < 63)
     input[i++] = c;
   
   else
   {
     input[i] = '\0';
     i = 0;

//process the input string here
     Serial.println( input );
   }
 }
}

Note that I will be read and can be incremented before being initialized with this code.

Thanks so much guix. I need to wait for the '*' character, so this needs to precede your code:

while (Serial.read() != '*') { ; }

AWOL:

delay(0.5); // need delay to prevent gibberish

Sadly, delay takes an integer parameter

If OP really wants half a millisecond delay, they look into the delayMicrosecond.