Serial input into Byte array

[NaokiS]

Seems like you are overcomplicating things, look at this:

Code: [Select]

#define MAX_LINE_LEN 25

bool readCommand(char* cmd, byte *rgb)
{
  char line[MAX_LINE_LEN]; //One line of data
  char idx = 0, cc; //Buffer index
  while ((idx < MAX_LINE_LEN - 1) && Serial.available())
  {
    cc = Serial.read();
    if (cc == '\n')
    {
      line[idx] = 0;
      break;
    }
    else line[idx++] = cc;
  }
  if (idx >= MAX_LINE_LEN) return false; //Newline was not found
  char* token = strtok(line, ",");
  idx = 0;
  while (token && (idx < 4))
  {
    if (idx == 0) memcpy(cmd, token, strlen(token) + 1); //+1 to include \0
    else rgb[idx-1] = atoi(token);
    token = strtok(NULL, ",");
    idx++;
  }
  return (idx == 4); //Success if 4 tokens was received
}

void loop()
{
  char cmd[20];
  byte rgb[3];
  if (readCommand(cmd, rgb))
  {
    Serial.print("Command: ");
    Serial.print(cmd);
    Serial.print(" R=");
    Serial.print(rgb[0]);
    Serial.print(" G=");
    Serial.print(rgb[1]);
    Serial.print(" B=");
    Serial.println(rgb[2]);
  }
}

Sorry to bring this topic back up, only recently got a chance to try this. I've tried this in my sketch and it fails to read anything. Making sure it wasn't my own code causing it, I put it in a blank sketch with only Serial.begin(9600); in the setup and still it is playing up.

What I find is that if I run it as it, I get no output on Arduino 1.8.7. It doesn't run the if in the main loop. I've tried "L1S,255,255,255" and "L1S,255,255,255," but it doesn't seem to register it correctly. If I change idx to a byte and have the serial monitor print idx's value during the while(token  && (idx < 4)) loop, it will print indefinitely. The loop repeats forever except in some cases of re-entering the same command it will stop after "processing it" but still it will not parse.

Doing a little extra digging, it seems to loop as token is set to 0xFF from strtok and thus the second while loop is always repeating.

Edit: I had set idx = 0; at the start, removing it fixes the loop but the code still has no proper output. In fact reverting it back to a char causes the serial monitor to scroll endlessly without any command. Code below:

Code: [Select]


#define MAX_LINE_LEN 25

bool readCommand(char* cmd, byte *rgb)
{
  char line[MAX_LINE_LEN]; //One line of data
  char cc, idx; //Buffer index
  while ((idx < MAX_LINE_LEN - 1) && Serial.available())
  {
    cc = Serial.read();
    if (cc == '\n')
    {
      line[idx] = 0;
      break;
    }
    else line[idx++] = cc;
  }
  if (idx >= MAX_LINE_LEN) return false; //Newline was not found
  char* token = strtok(line, ",");
  idx = 0;
  while (token && (idx < 4))
  {
    if (idx == 0) {
      memcpy(cmd, token, strlen(token) + 1);  //+1 to include \0
    }
    else {
      rgb[idx - 1] = atoi(token);
    }
    token = strtok(NULL, ",");
    Serial.println(idx);
    idx++;
  }
  return (idx == 4); //Success if 4 tokens was received
}

void loop()
{
  char cmd[20];
  byte rgb[3];
  if (readCommand(cmd, rgb))
  {
    Serial.print("Command: ");
    Serial.print(cmd);
    Serial.print(" R=");
    Serial.print(rgb[0]);
    Serial.print(" G=");
    Serial.print(rgb[1]);
    Serial.print(" B=");
    Serial.println(rgb[2]);
  }
}

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

 

[NaokiS]

Further digging, it turns out that it was reading from serial far too quickly and resetting itself before the whole command was even registered in the serial buffer. Can confirm this by adding delayMicroseconds(100) at the end of the Serial.available() while loop and it will function properly. Will probably add a timeout routine which will wait for a valid start character, a valid end code and a timer to make sure the total transmission is fully read before it times out and voids the entire command in case of a rogue or misformed command. Hopefully it will also mean it will can track a command and know when to proceed.

EDIT:

I have done the above, posted below for future reference and also maybe someone has some optimisation. Changed the command set to <CMD, DATA, DATA, DATA>\n. Also changed so sending less
or more than 4 is accepted even though parsing into the RGB array doesn't account for this (will change later, tired as now lol). Sleep is for the weak. Code is in my eyes 100% what I need now. Thanks guys

Code: [Select]


#define MAX_LINE_LEN 25
#define COMM_TIMEOUT 500

bool readCommand(char* cmd, byte *data)
{
  static char line[MAX_LINE_LEN] = {}; //One line of data
  char cc;
  static byte idx = 0, numOfBytes = 0; //Buffer index
  static long commandTimer;
  static bool startChar, validCommand;

  while ((idx < MAX_LINE_LEN - 1) && Serial.available()) {
    // Read command in and look for newline. If not found, store char in array.
    if (startChar) commandTimer = millis();
    cc = Serial.read();
    //Serial.println(cc);
    if (cc == '\n')
    {
      line[idx] = 0;
      break;
    }
    else line[idx++] = cc;

    if (cc == ',') numOfBytes++;

    // Look for valid start of command. If not received, cancel the command
    if (idx - 1 == 0 && line[idx - 1] == '<' && !startChar) {
      //Serial.println("Start char");
      startChar = 1;
      commandTimer = millis();
      idx = 0;
    } else if (idx - 1 == 0 && line[idx - 1] != '<' && !startChar) {
      //Serial.println("Bad start");
      commandTimer = 0; startChar = 0; validCommand = 0; idx = 0; numOfBytes = 0;
      return false;
    }

    // Look for valid end of command.
    if (line[idx - 1] == '>' && startChar) {
      validCommand = 1;
      line[idx - 1] = 0;
      commandTimer = 0; startChar = 0; idx = 0;
      Serial.println("<OK>");
    }
  }

  // If timer expires since last command and it's not been valid
  if (startChar && (millis() - commandTimer) >= COMM_TIMEOUT && !validCommand) {
    commandTimer = 0; startChar = 0; validCommand = 0; idx = 0; numOfBytes = 0;
    //Serial.println("Comm Timeout");
    return false;
  }

  // If too many chars are detected, cancel
  if (idx >= MAX_LINE_LEN) {
    commandTimer = 0; startChar = 0; validCommand = 0; idx = 0; numOfBytes = 0;
    return false; //Newline was not found
  }

  if (validCommand) {
    //Serial.println("Decode");
    // Set up stringtoken with the received command and the denominator.
    char* token = strtok(line, ",");
    idx = 0;
    // Whilst there is a valid token and number of data bytes, repeat.
    while (token && (idx < numOfBytes+1)) {
      if (idx == 0) {
        // Copy the entire first command into the cmd with memory copy.
        memcpy(cmd, token, strlen(token) + 1);  //+1 to include \0
      }
      else {
        // Convert the current command into an integer and store in the RGB array.
        data[idx - 1] = atoi(token);
      }
      token = strtok(NULL, ","); // I dont know what this bit is but it seems to help so I'll leave it be.
      idx++;
    }
    commandTimer = 0; startChar = 0; validCommand = 0; idx = 0; numOfBytes = 0;
    return 1; // Success
  }

  // Nothing happened, cancel
  return false;
}

void loop()
{
  char cmd[20];
  byte data[4];   // Most can be received is 4 bytes of data
  if (readCommand(cmd, data))
  {
    Serial.print("Command: ");
    Serial.print(cmd);
    if (cmd[0] == 'L') {
      Serial.print(" R=");
      Serial.print(data[0]);
      Serial.print(" G=");
      Serial.print(data[1]);
      Serial.print(" B=");
      Serial.println(data[2]);
    }
    else Serial.println(" Not RGB");
  }
}

void setup() {
  Serial.begin(115200);
}