Serial Interface Refinement

[bigbytes]

Hello, I'm still quite new to all of this and wondered if anyone wanted to help refine my code to make it better, e.g. I use both *char and String which seems wrong to me but that is where I am with coding. Also which variables should I make static or global, or any other optimisations.

char rx_byte = 0;
String rx_str = "";
String rx_str_prompt = "OK";

void setup() {
  
  //  SERIAL: init
  //Serial.begin(9600);
  Serial.begin(115200);
  while (!Serial)
  {
    ; //  SERIAL: wait USB port
  }
  
  
  Serial.println("init");
  // SERIAL: send ready prompt
  Serial.println(rx_str_prompt);
  
}

void loop() {
  
  cmd_main();

}

void cmd_main()
{
  if (Serial.available() > 0)
  {
    rx_byte = Serial.read();
    
    if (rx_byte != '\n')
    {
      rx_str += rx_byte;
    }
    else
    {
      //  PARSE
      rx_str.trim();
      if(rx_str.length() > 0)
      {
        cmd_parse();
      }

      //  RESET
      rx_str = "";
      Serial.println(rx_str_prompt);
    }
  }
}

void cmd_parse()
{
  const int max_index = 4;
  char *strings[max_index];
  byte index = 0;
  const char s[2] = " ";
  char *token;
  
  //  COPY
  char str[rx_str.length()+1];
  rx_str.toCharArray(str, rx_str.length()+1);
  
  //  SPLIT
  token = strtok(str, s);
  while( token != NULL && (index <= max_index-1) ) {
    strings[index] = token;
    index++;
    token = strtok(NULL, s);
  }

  /*
  //  DUMP
  for(int n = 0; n < index; n++)
  {
    Serial.println(strings[n]);
  }
  */
  //  PARSE
  if(strcmp(strings[0], "set") == 0)
  {
    
  }
  else
  if(strcmp(strings[0], "home") == 0)
  {
    Serial.println("house");

    if(index >= 2){
      //int value2 = rx_str.toInt();    //  string to int
      int value = atoi(strings[1]);     /// char* to int
      Serial.println(value);
    }

    
  }
  else
  {
    Serial.print("Unknown command: ");
    Serial.println(strings[0]);
  }
  
}

Thankyou

[J-M-L]

I would suggest to study Serial Input Basics as a starting point to avoid the String class

[bigbytes]

Thankyou

//  https://forum.arduino.cc/t/serial-input-basics-updated/382007

const byte numChars = 32;       //  max input
char receivedChars[numChars];   // an array to store the received data

void setup() {
    Serial.begin(9600);
    Serial.println("INIT");
}

void loop() {
    recvWithEndMarker();
}

void recvWithEndMarker() {
    static byte ndx = 0;
    //char endMarker = '\n';
    char rc;
    
    while (Serial.available() > 0)
    {
        rc = Serial.read();

        //if (rc != endMarker) {
        if (rc != '\n') {
            receivedChars[ndx] = rc;
            ndx++;
            if (ndx >= numChars) {
                ndx = numChars - 1;
            }
        }
        else {
            receivedChars[ndx] = '\0'; // terminate the string
            ndx = 0;
            cmd_parse();
        }
    }
}

void cmd_parse()
{
  const int max_index = 4;
  char *strings[max_index];
  byte index = 0;
  const char s[2] = " ";
  char *token;
  
  
  //  SPLIT
  token = strtok(receivedChars, s);
  while( token != NULL && (index <= max_index-1) ) {
    strings[index] = token;
    index++;
    token = strtok(NULL, s);
  }

  /*
  //  DUMP
  for(int n = 0; n < index; n++)
  {
    Serial.println(strings[n]);
  }
  */
  //  PARSE
  if(strcmp(strings[0], "set") == 0)
  {
    
  }
  else
  if(strcmp(strings[0], "home") == 0)
  {
    Serial.println("house");

    if(index >= 2){
      //int value2 = rx_str.toInt();    //  string to int
      int value = atoi(strings[1]);     /// char* to int
      Serial.println(value);
    }
  }
  else
  {
    Serial.print("Unknown command: ");
    Serial.println(strings[0]);
  }
}

[johnwasser]

@bigbytes

which variables should I make static or global,

Use global variables where the same variable is used in more than one function (like in both setup() and loop()). Use local variables where the variable is used in only one function. Make the local variable 'static' if its value needs to stay set between calls to the function.

If your sketch has more than one tab (source file) and some of those are .cpp files, global variables that are only used within one source file should be marked 'static'. This is particularly important in libraries where you want to minimize the chances of two files using the same name for a global and getting a redefinition error.