Help me this NMEA parser written in C

[Herath]

Hi,
I am writing a NMEA parser in C these days. To be specific, this is for ATMega16. This NMEA parser is a unit of a system I am trying to build. I just need some ideas and need to verify my approach (if it is good considering the embedded systems). I hope advices from people.
nmea.h header file

Code: Select all

//#include "buffer.h"
#include<stdio.h>

/*void init_NMEA_Engine();*/

void parseNMEA();
void processGGA();
void processGLL();
void processVTG();
char readFromBuffer();
void readNextParameter(char* store);
void zeroFillArray(char* array,short size);
char tempBuffer[15];//Temporarily hold parameters
char longitude[10];
char lattitude[10];
char lon_; // N, S
char lat_;// E,W
char utcTime[10];
short fixMode; //false- no fix, true-valid fix
short satsUsed;
float MSL_Altitude;
short course;
short speed;
short day;
short month;
short year;

nmea.c file

Code: Select all

#include "nmea.h"
#include<stdlib.h>
#include<string.h>

char nmea1[]="$GPGGA,161229.487,8.425838,N,82.538363,E,1,07,1.0,9.0,M, , , ,0000*18\r\n";
int j=0;
//Just for debugging the parser
char readFromBuffer(){
      return nmea1[j++];

}
void readNextParameter(char* store){
       char temp;
       while((temp=readFromBuffer())!=',') *store++=temp;
}
void parseNMEA(){
        char temp;
        char sentenceType[5];//MSG ID is exactly 5 characters long
        while((temp=readFromBuffer())!='$');//Seek $ then exit loop and execute code below

               //Read sentence type identifier
               readNextParameter(&sentenceType);

               if(strcmp(sentenceType,"GPGGA")){
                        processGGA();
               }else if(strcmp(sentenceType,"GPGLL")){
                        processGLL();
               }else if(strcmp(sentenceType,"GPVTG")){
                       processVTG();
               }

}

void zeroFillArray(char* array,short size){
       for(int i=0;i<size;i++) *array++=0;
}

void processGGA(){
	char temp;
	short i=0;

	zeroFillArray(&utcTime,15);
	readNextParameter(&utcTime);
	
	zeroFillArray(&lattitude,10);
	readNextParameter(&lattitude);
	
	readNextParameter(&lat_);//N,S indicator

	zeroFillArray(&longitude,10);
	readNextParameter(&longitude);

	readNextParameter(&lon_);// E,W indicator
	
	readNextParameter(&temp);
	fixMode=atoi(temp);	//Position validity	


	readNextParameter(&temp);
	satsUsed=atoi(temp);


	readNextParameter(&temp); //skip HDOP

	zeroFillArray(&tempBuffer,15);
	readNextParameter(&tempBuffer);
	MSL_Altitude=atof(tempBuffer);

	//discard all the others for now
		
}


void processGLL(){
}
void processVTG(){
}

I have defined a "readFromBuffer" inside the parser to debug it. in the actual system, it will read from a circular buffer.

Thanks

[Herath]

Also I get warnings for passing a pointer to a char array where the function is expecting a char* . I need some help on this too. . But it works fine.

[Herath]

Seems like I can use the string tokenizing too. . I think that is going to be a better way. Anyway still expecting for ideas. I can find ones that are already there. But most of them are heavy for my job.

[Neo]

Few things for your note.

1. You need to define all variables in the source file before the function implementations
2. What you need to define in header is only prototypes to functions and references to variables. Remember you only need to define the ones that are only used by including that header.
   
   Examples:
   MySrc.c

   Code: Select all

   #include <stdio.h>
   
   int myGlobalWay = 0;
   int mySecondGlobalWay = 0;
   
   void myFunc(){
   	mySecondGlobalWay++;
   }
   
   

   MySrc.h

   Code: Select all

   #ifndef MY_SRC_HDR_
   #define MY_SRC_HDR_
   
   extern int myGlobalWay;
   extern void myFunc();
   
   #endif
   

   
   Another source file. Say MyNewFile.c

   Code: Select all

   #include "MySrc.h" // See I have included the header
   
   void myTestFunc(){
   	myFunc(); // Call to a function defined by the header
   	myGlobalWay++; // Access a variable defined by the header
   
   	mySecondGlobalWay++; // This will generate a warning or error since this is not known to this source file (not defined in header)
   }
   
   int main(){
   	return myGlobalWay;
   }
   

3. Circular buffer is the right way. Always chose the array length to be a power of two. This will make wraparound easy handle.
   Ex:

   Code: Select all

   unsigned char myCircularBuf[512]; // Length is power of two
   int readPoniter = 0;
   int writePointer = 0;
   
   unsigned char getNextByte(){
   	unsigned char ret;
   	ret = myCircularBuf[readPoniter];
   	readPoniter = (readPoniter + 1) & 511; // This is the useful point is choosing the array length to be a power of two
   }
   
   void putNextByte(unsigned char newByte){
   
   	myCircularBuf[writePointer] = newByte;
   	writePointer = (writePointer + 1) & 511; // This is the useful point is choosing the array length to be a power of two
   }

4. Instead of zeroFillArray function, you can use memset(buf, 0, size) straight away.
5. I see a problem here.

   Code: Select all

   char temp;
   readNextParameter(&temp);

   We can't expect to find a , character in case the string is corrupted. So always read to a buffer with enough space.

Rest of the coding seems fine to me.

Also I get warnings for passing a pointer to a char array where the function is expecting a char*

If you can tell me the line you get the warning, I can have a look.

Seems like I can use the string tokenizing too.

Since you are going to work on a circular buffer, it won't possible.

[Neo]

Herath, what's the decoder you use? I mean the brand and type. Sorry I can't remember.

[Herath]

Thanks for the tips. I haven't touched C and C++ in about 3 years. Haven't done any real world think with them.
Seems like writing this system is going to make my C better.

Writing procedural programs seems to be very hard. It easily becomes a mess.

I figured out some more problems. In the way I have used strcmp and issues arising because of the "null terminating strings". I know that it is very easy for you to catch those errors in my source.

If you mean the GPS Module by decoder, that is going to be a SiRFIILP based one.

[Neo]

SiRF II e LP I guess? What's the brand?

[Herath]

It is made by NAVIUS. I bought it from ebay. It has been pulled out from a working NSA-C3 GPS reciever. I only have the module. Do not have the housing with the PS2 connector. I tested the module by connecting it to PC through a PL2303.
http://dl.dropbox.com/u/959225/NSA-C3-Brochure.pdf

About the pointer warning I'm getting,

I get it with

Code: Select all

void readNextParameter(char* store)

when i user readNextParameter(&lattitude) where lattitude is a character array. It warns about incompatible pointer type. Is there any type of pointers for arrays. I guess not.

[Neo]

Ohhh I understand the problem.

You have,

Code: Select all

char longitude[10];

longitude is a pointer to a string array.

So when you pass this to a function that accepts a string array, you will have to pass as follows.

Code: Select all

readNextParameter(longitude);

[Herath]

I made the corrections. And read (and still reading) about extern. I have not used it before. I hope that you could have a look.

Header -- nmea.h

Code: Select all

//#include "buffer.h"
#include<stdio.h>
#ifndef NMEA_H
#define NMEA_H
	extern char longitude[10];
	extern char lattitude[10];
	extern char lon_;
	extern char lat_;
	extern char utcTime[10];
	extern short fixMode;
	extern short satsUsed;
	extern float MSL_Altitude;
	extern float course;
	extern float speed;
	extern short day;
	extern short month;
	extern short year;
#endif


/*void init_NMEA_Engine();*/
void parseNMEA();
void processGGA();
void processGLL();
void processVTG();
char readFromBuffer();
void readNextParameter(char* store);

Source nmea.c

Code: Select all

#include "nmea.h"
#include<stdlib.h>
#include<string.h>

char tempBuffer[15];//Temporarily hold parameters
char longitude[10];
char lattitude[10];
char lon_; // N, S
char lat_;// E,W
char utcTime[10];
short fixMode=0; //false- no fix, true-valid fix
short satsUsed=0;
float MSL_Altitude;
float course=0;
float speed=0;
short day;
short month;
short year;

char nmea1[]="$GPGGA,161229.487,8.425838,N,82.538363,E,1,07,1.0,9.0,M, , , ,0000*18\r\n";
int j=0;
//Just for debugging the parser
char readFromBuffer(){
	return nmea1[j++];

}
void readNextParameter(char* store){
	char temp;
	while((temp=readFromBuffer())!=',') *store++=temp;
}
void parseNMEA(){
	char temp;
	char szSentenceType[6];//MSG ID is exactly 5 characters long
	while((temp=readFromBuffer())!='$');//Seek $ then exit loop and execute code below

		//Read sentence type identifier
		readNextParameter(szSentenceType);
		szSentenceType[5]='\0';// Make sentenceType a null terminating one
		
		
		if(strcmp(szSentenceType,"GPGGA")==0){
			processGGA();
		}else if(strcmp(szSentenceType,"GPGLL")==0){
			processGLL();
		}else if(strcmp(szSentenceType,"GPVTG")==0){
			processVTG();
		}

}

void processGGA(){
	char temp;

	memset(utcTime,0,15);
	readNextParameter(utcTime);
	
	memset(lattitude,0,10);
	readNextParameter(lattitude);
	
	readNextParameter(&lat_);//N,S indicator

	memset(longitude,0,10);
	readNextParameter(longitude);

	readNextParameter(&lon_);// E,W indicator
	
	readNextParameter(&temp);
	fixMode=atoi(&temp);	//Position validity	


	readNextParameter(&temp);
	satsUsed=atoi(&temp);


	readNextParameter(tempBuffer); //skip HDOP

	memset(tempBuffer,0,15);
	readNextParameter(tempBuffer);
	MSL_Altitude=atof(tempBuffer);

	//discard all the others for now
		
}


void processGLL(){
}
void processVTG(){
}

I will read more about extern. Thanks.