Parse next part of comma delimited Char array

Hi, go easy on me I’m very very new to C and C++ programming - I’ve done a fair bit of VB and VBA over the years, self tought and very lazy with memory usage, programming efficiency, variable declarations etc, so I’m finding this lark a bit of a batism of fire!

Anyhow, I’m doing a project to read some NMEA data into the serial ports on a mega, and datalog some of the data to an SD card and possibly ‘do stuff’ with some of the data too.

I’ve got a fair bit of it working, this is the main loop that reads data from 2 serial ports and then when a complete sentence is received copies it to the char array buffer and calls the readline function.

void loop() 
{    
  
// read1:
  while(SerPort1.available()) {
      c1=SerPort1.read();
      
      if (c1 == -1) continue;
      if (c1 == '\n') continue;
      if ((buff1idx == BUFFSIZ-1) || (c1 == '\r')) {
        buff1[buff1idx] = 0; 
        strcpy(buffer, buff1); 
        buffidx = buff1idx;  
        readline();
        buff1idx=0; continue;}
      buff1[buff1idx++]= c1;
    }

// read2:  
  while(SerPort2.available()) {
      c2=SerPort2.read();
      
      if (c2 == -1) continue;
      if (c2 == '\n') continue;
      if ((buff2idx == BUFFSIZ-1) || (c2 == '\r')) {
        buff2[buff2idx] = 0; 
        strcpy(buffer, buff2); 
        buffidx = buff2idx; 
        readline();
        buff2idx=0; continue;}
      buff2[buff2idx++]= c2;    
    } 
    
    unsigned long currentMillis = millis();
    
    if(currentMillis - previousMillis > interval) {
        logdata();
        previousMillis = currentMillis; 
        }
 
  
}

Then every so often calls the logdata function to write the data to the SD Card. What I am planning to do is to strip the various required data (Lat, long, speed, depth and a few other bits) from the relevent NMEA sentences in the readline function, and then keep updating a bunch of global char arrays with the latest data. Then every 5 or 10 seconds (or whatever gives sensible results when I test it) the data is written to the SD card.

This is where I am at so far:

void readline()
{
  
  uint8_t i;
  char *p;
  
  // strip 3 letter sentence ID and null terminate 
  for (i=0; i<4; i++){ sentence[i]=buffer[i+3];}
     sentence[3]=0;
      
  if(strcmp(sentence,"GGA")==0){
     
  }   
  else if(strcmp(sentence,"RMC")==0){
      char *p = buffer;
        p = strchr(p, ',')+1; // go to next element
       strcpy (fixtime, parsechar(p));
        p = strchr(p, ',')+1; // go to next element
        strcpy (latitude, parsechar(p));
        // and so on
  }
  else if(strcmp(sentence,"HDG")==0){
    
  }
  else if(strcmp(sentence,"HDM")==0){
   // and so on for other sentance types
              

}

where fixtime and latitude are the char arrays that I’m keeping track of the latest data in.

So what I need is this function parsechar() which needs to copy the characters in the input char array, into an output char array until it encounters a comma. It sounds fairly easy but I’ve tried a few methods and am getting nowhere.

I’ve tried a few permutations on the following

char* parsechar(char *str)
  {
   int i = 0;
   
  char* strout;
  while (str[i] !=0)  
    {
    if (str[i] == ','){
       i++; 
      strout[i] = '/0';
      return strout;
      }
      
        strout[i] = str[i];
        i++;
     
      }  
      
}

which I INTEND to go through the input array character by character, and then copy that character to an output char array and keep going until I hit a comma. I’ve obviously done something really stupid because it isn’t working! And just spews out a load of rubbish if I print the output to the serial port and then the serial port monitor locks up on my lapop!

Before anyone suggests it, i’ve looked at using strtok_r but that will skip consecutive delimters and the input sentences are all of a set format and could contain no data in certain fields so for example they could look like.
for example with this input - 11111,2222,6666,8888
AFAIK strtok would pick up “6666” as the 3rd field in, wheras I need to always get 6666 as the 6th field and if I want the 3rd field then I need to know its blank, which is why I’m using strchr to get to the correct comma and then grabbing the next piece of data in the string. The other benefit (in my mind) of doing it this way is that I can also write “parseDecimal”, “parsefloat” etc procedures if I need that next element out in another format to do something with (during future expansion of the project)

Help please!

What are SerPort1 and SerPort2?

  char* strout;

strout is a pointer, but it doesn't point to any memory. You should ALWAYS initialize local variables. You should ALWAYS make pointers point to some place that you can actually store/read data.

      return strout;

You should NEVER return a pointer to memory allocated on the stack. Not that that is what you are doing here; just make sure that you don't do that.

Why aren't you using (or at least looking at) TinyGPS?

SerPort1 and SerPort2 are serial ports

Someone else did a similar project and I’ve pinched the serial port part of the code from them and they had buffer over-run issues so used this library to solve that problem:

#include <SerialPort.h>

// Setup serial ports
SerialPort<0, 63, 63> SerPort;
SerialPort<1, 255, 255> SerPort1;     // Serial port buffers 256 bytes for send and receive
SerialPort<2, 255, 255> SerPort2;     // Serial port buffers 256 bytes for send and receive
SerialPort<3, 255, 255> SerPort3;     // Serial port buffers 256 bytes for send and receive

Why aren’t you using (or at least looking at) TinyGPS?

I’ve looked at tinyGPS and an NMEA library and I can see a couple of issues - firstly they only deal with GPS NMEA sentances whereas I’ve got a whole boat’s worth and also want to data-log stuff like depth, windspeed etc which are in other sentences. Also (and I may be wrong here) but it apears that tinyGPS and other similar libraries parse the data on the fly - i.e you send them each character than comes in the serial port and then they retun the relevent bits of information once a sentence is received. I want to get the whole sentence in from 2 or 3 ports - and send them back out again unchanged from a single port- but then also strip the data from the completed sentance.

Is my function correct in principle (i.e i’m attempting to do the right thing, but doing it wrong)

should I declare something like this at the start instead?

char strout[];

Also (and I may be wrong here) but it apears that tinyGPS and other similar libraries parse the data on the fly - i.e you send them each character than comes in the serial port and then they retun the relevent bits of information once a sentence is received.

TinyGPS collects the data. You can call the functions that parse the last collected sentence. Generally, you do this when they report that they have a sentence.

It's worth at least looking at how it parses the data.

should I declare something like this at the start instead?

That won't do much good. It's an array, without a specified size, so the compiler counts the number of initializers to size the array. You don't have any, so you will get an array that can hold zero elements.

That probably won't be big enough for most uses.

That won't do much good. It's an array, without a specified size, so the compiler counts the number of initializers to size the array. You don't have any, so you will get an array that can hold zero elements

That probably won't be big enough for most uses..

XD XD

Ok - the largest that strout can be is the same size as the input string (if there was no comma at all), so I could find the size of the input string and use that to size the strout array?

I'll have another read of tinyGPS and see what it does.

PArt of the reasoning behind not using tinyGPS and similar is because I also want to learn how to program as part of this project.

What are SerPort1 and SerPort2?

Alyne:
SerPort1 and SerPort2 are serial ports

I don't speak for PaulS, but I think he was suggesting that it would be helpful if you gave them more meaningful names, such as "gps" or "console" or whatever.

Alyne:
PArt of the reasoning behind not using tinyGPS and similar is because I also want to learn how to program as part of this project.

I'd think you'd get plenty of learning just writing the rest of the code needed but maybe a better approach would be to add support to TinyGPS for the "other" sentence types you mentioned earlier?

Just a thought.

Regards,

Brad
KF7FER

Ok learning as I go here:

char* parsechar(char *str)
  {
   int i = 0;
   char strout[sizeof(str)];
   while (str[i] !=0)  
    {
    if (str[i] == ','){
      strout[i] = '/0';
      return strout;
      }
        strout[i] = str[i];
        i++;
      }    
}

Not tested - but is this correct in principle (up to the point I'm going to make in a minute)?
so while the current character is not null
if the current char is a comma, then replace the "," with a null terminator and return (a pointer to the array?)
if not a comma then copy str[index] to strout[index], increment i and continue the while loop.

I'm still trying to get my head around this, but the strout that's returned is a pointer to the strout array? which is local to the procedure and is destroyed upon exit. so basically a pointer is returned to a block of memory that is currently indertimate?

So what i really need to do is to declare Strout as a global variable, using a size that I reckon will realistically be the largest size of a single term, with maybe a couple of extra bytes for luck?

char strout[20];

char* parsechar(char *str)
  {
   int i = 0;
   
   while (str[i] !=0)  
    {
    if (str[i] == ','){
       strout[i] = '/0';
      return strout;
      }
        strout[i] = str[i];
        i++;
      }    
}
   char strout[sizeof(str)];

A pointer is two bytes. You may want to use strlen(), not sizeof().

      return strout;

I've already told you NOT to do this.

Make strout a global array, or pass strout to the function, after allocating it before the call to the function.

  return strout;
I've already told you NOT to do this.

Out of interest why not?

I can see that as strout is now a global array so there is no need to return anything from the function, but at the moment returning strout it works as expected.

I'm not doubting you and I'm sure there is a good reason for not doing this, but I don't understand what it is!

Out of interest why not?

The stack space is no longer yours when the function ends. Any other function can trash the whole stack before you get around to using the data. How would you use it, anyway? Serial.print() it? Oops, that’s a function call. Maybe, copy it? Like strcat()? Oops, that’s a function call.

but at the moment returning strout it works as expected.

Returning a pointer to a global variable, while useless, is not the same as returning a pointer to a local variable that is no longer in scope.

@Alyne,

You might try the new TinyGPS++ (TinyGPS++ | Arduiniana), which allows you to parse custom sentences in addition to the "standard" ones that are parsed by default.