String parsing from Serial input

I am trying to parse some serial data that is coming from a programme written in C#.
I haven’t written the programme yet, but I intend the data being transmitted to be in the below format:

1234,56789\n
1235,56790\n
1236,56791\n

et cetera.

The numbers are of course illustrative, but they will be incrementing, and have no fixed length as they are, in effect, totalising counters.

I wish to parse the text by delimiting it at the “,” (comma) and the “\n” (carriage return), and put both numbers into ints for some further processing.

For example, after the last line of code above, the values of
dayN = 1236 and
allN = 56791

My question is, given that the string will be a rapidly changing stream of data, what is the best parsing method? I have seen both very positive and very negative opinions of strtok, for example, and like the look of things like this Regular Expression library.

I have attempted to begin writing the sketch using strtok, but was working from an example shown here, and could not work out what did and did not apply to my situation. The sketch as it is is below (warning: unfinished and holey).

#include <string.h>

int dayN = 0; //Day Users
int dayO = 0; //Old Day Users
int dayX = 0; //Difference in Day's Users
int allN = 0; //Total Users
int allO = 0; //Old Total Users
int allX = 0; //Difference in Total Users
int stringy;

char inData[80];
char users[80];
byte index = 0;

void setup()
{
  pinMode(12, OUTPUT); //Sets Pin 12 as Day User Output 
  pinMode(13, OUTPUT); //Sets Pin 13 as Total User Output
  Serial.begin(9600);
}  // end of setup

void loop()
{
  while(Serial.available() > 0)
  {
    char aChar = Serial.read();
    if(aChar == '\n'){
      //Parse here
      int counter = 0; 
      char * p = inData; //p points to inData
      char * str; //declare str
      while (str = strtok_r(p, ",", &p)) //comma delimiter
      {
        users[counter] = *str ;
        counter++;
        p=NULL;
      }

      //End Parse
      index = 0;
      inData[index] = NULL;
    }
    else
    {
      inData[index] = aChar;
      index++;
      inData[index] = '\0'; //Keep NULL Terminated
    }
  }

  dayX = dayN - dayO; //Current Day Users - Old Day Users = Change in Day's Users
  allX = allN - allO; //Current Total Users - Old Total Users = Change in Total Users

  for (; dayX > 0; dayX--)
  {
    digitalWrite(12,HIGH);
  }
  dayO = dayN;

  for (; allX > 0; allX--)
  {
    digitalWrite(13,HIGH);
  }
  allO = allN;

}

Any help would be much appreciated, thank you.

My question is, given that the string will be a rapidly changing stream of data,

No, it won't. The string will, at any given point in time, contain data read from the serial port.

I have attempted to begin writing the sketch using strtok

I don't see any calls to strtok() in that sketch. I see a comment that shows where the calls should be made.

but was working from an example shown here, and could not work out what did and did not apply to my situation.

Well, the part about NULL termination of the array applies, but you are already doing that.

The part about needing to write some code, and see how it works applies.

What other part(s) are you not sure about?

By the way, stick to strtok(), not strtok_r(). The strtok() function is simpler. The strtok_r() version is the thread safe version. Hardly a requirement on a system with a single thread.

My question is, given that the string will be a rapidly changing stream of data, what is the best parsing method? I have seen both very positive and very negative opinions of strtok, for example, and like the look of things like this Regular Expression library.

The example streams you provided are by no means rapidly changing with respect to their layout/format. For that situation, something like strtok is far more appropriate. It handles rapidly changing values in a standard layout just fine. Regular Expressions would work as well, but would be considerable overkill. Those are more useful in scenarios where there's no predefined layout of data, thus requiring more flexible parsing capabilities.

Another option would be sscanf as well. Likely easier to implement than strtok, but also likely to result in a larger binary size (1-2k).

PaulS: I don't see any calls to strtok() in that sketch. I see a comment that shows where the calls should be made.

Sorry, uploaded an outmoded sketch accidentally, have edited it now to show what I actually meant.

The bit I'm stuck with is how entering data into an array that keeps incrementing will solve my problem. My guess is that it won't, and that I need to rather than just reading and rewriting.

I basically need the characters between "\n" and "," to go into dayN, and those between "," and "\n" to go into allN.

Or, perhaps there is an easier way to accomplish what I'm trying to do altogether?

PaulS: By the way, stick to strtok(), not strtok_r(). The strtok() function is simpler. The strtok_r() version is the thread safe version.

Have changed this now, thank you.

The bit I'm stuck with is how entering data into an array that keeps incrementing will solve my problem. My guess is that it won't, and that I need to rather than just reading and rewriting.

The thing that is incrementing is the index into the array, not the array. You set the index back to 0 when a carriage return is received, after parsing the string. This solves the problem.

I basically need the characters between "\n" and "," to go into dayN, and those between "," and "\n" to go into allN.

There is no "\n" at the beginning of the array. There is no "\n" at the end, either. What you want is this:

char *token = NULL;
token = strtok(inData, ",");
if(token)
{
   dayN = atoi(token);
   token = strtok(NULL, "\0"); // Use NULL to continue parsing the same string
   if(token)
   {
      allN = atoi(token);
   }
}

Oh wow, that’s the whole parsing? That’s nice and succinct. Thank you!
Am I right in thinking that the below sketch ought to work, then? Or can some things now be omitted/need to be added?

#include <string.h>

int dayN = 0; //Day Users
int dayO = 0; //Old Day Users
int dayX = 0; //Difference in Day's Users
int allN = 0; //Total Users
int allO = 0; //Old Total Users
int allX = 0; //Difference in Total Users

char inData[80];
char users[80];
byte index = 0;

void setup()
{
  pinMode(12, OUTPUT); //Sets Pin 12 as Day User Output 
  pinMode(13, OUTPUT); //Sets Pin 13 as Total User Output
  Serial.begin(9600);
}  // end of setup

void loop()
{
  while(Serial.available() > 0)
  {
    char aChar = Serial.read();

    if(aChar == '\n'){
      //Parse here
      char *token = NULL;
      token = strtok(inData, ",");
      if(token)
      {
        dayN = atoi(token);
        token = strtok(NULL, "\0"); // Use NULL to continue parsing the same string
        if(token)
        {
          allN = atoi(token);
        }
      }
      //End Parse
      index = 0;
      inData[index] = NULL;
    }
    else
    {
      inData[index] = aChar;
      index++;
      inData[index] = '\0'; //Keep NULL Terminated
    }
  }

  dayX = dayN - dayO; //Current Day Users - Old Day Users = Change in Day's Users
  allX = allN - allO; //Current Total Users - Old Total Users = Change in Total Users

  for (; dayX > 0; dayX--)
  {
    digitalWrite(12,HIGH);
  }
  dayO = dayN;

  for (; allX > 0; allX--)
  {
    digitalWrite(13,HIGH);
  }
  allO = allN;

}
  for (; dayX > 0; dayX--)
  {
    digitalWrite(12,HIGH);
  }

There is no need to set a pin HIGH more than once. Presumably you wanted to do something different here.

  for (; allX > 0; allX--)
  {
    digitalWrite(13,HIGH);
  }

And here.

Ah yes, it's an electromechanical counter that requires a pulse for each incremental count.

Amended to:

 for (; dayX > 0; dayX--)
  {
    digitalWrite(12,HIGH);
    digitalWrite(12, LOW);
  }

  for (; allX > 0; allX--)
  {
    digitalWrite(13,HIGH);
    digitalWrite(13,LOW);
  }

Ah yes, it's an electromechanical counter that requires a pulse for each incremental count.

Is that going to generate a long enough pulse? Sometimes, you need a delay between the HIGH and the LOW to generate a long enough pulse. Sometimes you need a delay between the LOW and the next HIGH, too.

PaulS:

Ah yes, it's an electromechanical counter that requires a pulse for each incremental count.

Is that going to generate a long enough pulse? Sometimes, you need a delay between the HIGH and the LOW to generate a long enough pulse. Sometimes you need a delay between the LOW and the next HIGH, too.

I included a delay of (75) since that's how long they require, thanks for pointing that out too.