Is the following packet processing correct?

**tbatra18** · 09-03-2012

Code:

I had the following instruction regarding data:-

All data isASCII character code without checksumand CR.
・ Range ofMachineNumber start from0000 to 9999
・ If there is no top digit at data part, itwill be zero suppression and put space to fill it.
・ Calculate sum total from top till before checksum (1 bite unit), then recognize this
complement of 1 as checksum.
・ Finish of format is CR(Hexadecimal: OD)


  2   2      4       128           1        1   BYTES
+---+----+--------+-----------+----------+----+
|RE | 00 | M/c_NO | DATA_PART | Checksum | CR |
+---+----+--------+-----------+----------+----+

This is the packet that i will receive from the machine on the com port so i made the following program:-

Code:

 char data[6]="CM00";
struct parse_data{


char Response[2];
char oo[2];
char mc_no[4];
char data_part[128];
char checksum;
char cr;
};
struct stored_parse_data{
char	Status_ofmachine;
char	Rinsemode;
char	Bypass;
int	Dialysate_flow;
int	Blood_pump_flow;
float  Treated_blood_volume;
float	W_B_conductivity;
//HERE More fields are also there but for making code less i am showing only these fields

};

int main()
{
int i, n,
      cport_nr=0,        /* /dev/ttyS0 (COM1 on windows) */
      bdrate=9600;       /* 9600 baud */


  unsigned char buf[4096];
  int m;
  for(int k=0;k<=10;k++)
  {
  if(OpenComport(cport_nr, bdrate))
  {
	  char buf[30];
	  //sprintf(buf,"Can not open comport %d\n",cport_nr);
	//  wLog->WriteDebugLog(buf);
    printf("Can not open comport\n");
	cport_nr++;
	continue;
    return(0);
  }

char *b;
 char buf_temp[10];
 int chkvalue=calculatechecksum();
 b = itoa(chkvalue,buf_temp,10);


	data[4]= ~(*b);
 data[5]=0x0D;

 if(SendBuf1(cport_nr,data,6) > -1)
 {
	 //wLog->WriteDebugLog("Data Sent Successfully\n");
	 printf("Send succesfuly\n");
 }
 else
 {
	 printf("\nn\failed\n\n");
	 //wLog->WriteErrorLog("failed in sending data\n");
 
 }
 while(1)
  {
    n = PollComport(cport_nr, buf, 4095);


    if(n > 0)
    {
      buf[n] = 0;   /* always put a "null" at the end of a string! */


      for(i=0; i < n; i++)
      {
        if(buf[i] < 32)  /* replace unreadable control-codes by dots */
        {
          buf[i] = '.';
        }
      }
	  char buf11[70];


      printf("received %i bytes: %s\n", n, (char *)buf);
	  sprintf(buf11,"received %i bytes: %s\n", n, (char *)buf);
	  wLog->WriteDebugLog(buf11);
	  parse_data curretnt_data;


	  if(n==138 || n >138)
		int i=0;
	  
		{
			curretnt_data.Response[0] = buf[i++];
			curretnt_data.Response[1] = buf[i++];
			curretnt_data.oo[0] = buf[i++];
			curretnt_data.oo[1] = buf[i++];
			
			for(int j =0; j<3; j++)
				curretnt_data.mc_no[j]=buf[i++];
		
			for(int k =0; k<128; k++)
				curretnt_data.data_part[k] = buf[i++];


			curretnt_data.checksum = buf[i++];
			curretnt_data.cr = buf[i++];
		}
	  
		stored_parse_data fill;
	
		fill.Status_ofmachine = curretnt_data.data_part[0];
		fill.Rinsemode = curretnt_data.data_part[1];
		fill.Bypass = curretnt_data.data_part[2];


		char temp[3],j=3;
		for(int i =0; i < 3;i++)
			temp[i] = curretnt_data.data_part[j++];


		fill.Dialysate_flow =(int)atoi(temp);
		printf("Dialysate_flow is =%d\n",fill.Dialysate_flow); 
		char buf_d12[500];
		sprintf(buf_d12,"Dialysate_flow is =%d\n",fill.Dialysate_flow); 
		WriteToDataFile(buf_d12);


		j--;
		for(int i =0; i < 3;i++)
			temp[i] = curretnt_data.data_part[j++];


		fill.Blood_pump_flow =(int)atoi(temp);
		printf("Blood_pump_flow is  =%d\n",fill.Blood_pump_flow); 
		char buf_d13[500];
		sprintf(buf_d13,"Blood_pump_flow is  =%d\n",fill.Blood_pump_flow);
		WriteToDataFile(buf_d13);




		char temp_5[5];
		j--;
		for(int i =0; i < 5;i++)
			temp_5[i] = curretnt_data.data_part[j++];
		fill.Treated_blood_volume = (float)atof(temp_5);
		printf("Treated_blood_volume is =%f\n",fill.Treated_blood_volume); 
		char buf_d14[500];
		sprintf(buf_d14,"Treated_blood_volume is =%f\n",fill.Treated_blood_volume); 
		WriteToDataFile(buf_d14);
		j--;




		char temp_4[4];
		for(int i=0;i<4;i++)
			temp_4[i]=curretnt_data.data_part[j++];
		fill.W_B_conductivity=(float)atof(temp_4);
		printf("W_B_conductivity is =%f\n",fill.W_B_conductivity); 
		char buf_d15[500];
		sprintf(buf_d15,"W_B_conductivity is =%f\n",fill.W_B_conductivity);  
		WriteToDataFile(buf_d15);
		j--;

Can anyone tell is there any problem in the code ?
I have not done this work before i am new to C and C++,i am putting this code here because i will get only 2 chance to run the code on machine so its better that it may be checked by experience guys.
Thanks

**root4** · 09-03-2012

Originally Posted by tbatra18

Can anyone tell is there any problem in the code ?

Be specific. do you get compilation errors? runtime errors? or are you just looking for programming advices?

Besides, 160+ lines of an unindented, uncommented code sample... try to make things understandable quickly for people reading.

**tbatra18** · 09-03-2012

Sir as i said that i will be able to run the above code only 2 times on a machine so according following instruction:-

All data isASCII character code without checksumand CR.・ Range ofMachineNumber start from0000 to 9999・ If there is no top digit at data part, itwill be zero suppression and put space to fill it.・ Calculate sum total from top till before checksum (1 bite unit), then recognize thiscomplement of 1 as checksum.・ Finish of format is CR(Hexadecimal: OD)will my code work?

**root4** · 09-03-2012

Maybe I'm asking too much, but try to isolate the serial port mechanics from the packet parsing so that you can test it independently. Plus it will allow us to compile the code and test it.

**Salem** · 09-03-2012

> I have not done this work before i am new to C and C++,i am putting this code here because i will get only 2 chance to run the code on machine
Unless this is a quiz of some sort, this is a totally unrealistic expectation.
Even pros would be hard pushed to guarantee "success" in only two attempts.

root4 is right, make some functions.
In particular, start with something like this.

Code:

int main ( ) {
    char test[] = "RE001234.......xx\r";  // a valid message
    parse(test);
    return 0;
}

Then do this

Code:

int main ( ) {
    char test[140];
    FILE *fp = fopen("tests.txt","r");
    while ( fgets( test, sizeof(test), fp ) != NULL  ) {
        // replace the \n with \r
        parse(test);
    }
    fclose( fp );
    return 0;
}

When you're really happy that you can parse everything OK, then you can invent your simple com port reader which calls parse() for each complete packet.

**iMalc** · 09-03-2012

This appears to be business related, i.e. it looks like you are employed to write this code. However, code like this

Code:

      if(n==138 || n >138)
         int i=0;

indicates being very much unqualified for the job. It shows a lack of experience with any C-like programming languages i.e. C, C++, C#, Java, and possibly a lack of programming skills in general.

My advice would be to tell your employer that you are not up to the task right now, or otherwise start to set the expectation that you are not going to get it right. Make sure they either sort out a good contingency plan, or get someone else to do it. If you had told them that you were up to it, then now is the time to come clean, not afterwards.
Otherwise from the look of it, this is going to end very badly for you. As much as your employer will be a little unhappy to hear it now, they would be a lot less happy when it turns to custard later.

Thread: Is the following packet processing correct?

Thread Tools

Search Thread

Display

Is the following packet processing correct?

Similar Threads

How to get packet IP?

Packet processing in Socket programming, please help

pcap "packet" processing and storage

Correct post processing in multi-threaded server?

TCP/IP packet run-together-ing

Tags for this Thread