Thread: Serial port buffer

Hello,
I'm having trouble parsing a continuous stream of serial data on a linux machine. Each packet of data consists of 75 bytes, and the packets are transmitted at a rate of 3hz. The packets are further broken down into frames, consisting of 5 bytes each, with each frame starting with a byte of value '1' and one out of every twenty five frames has a sync bit (the LSB of the second byte is set to one once every 25 frames).

My general approach is to read in all available serial data, find the first sync bit and the following sync bit so that I can verify the entire packet before disseminating any data that it contains. The code in its current state is part of a pre-existing program (that I didn't write) that reads serial data and streams it, without any processing, through a TCP socket. My code below is my attempt at parsing the serial data.

The code below sort of works. The serial device that I'm using is a pulse oximeter and when my finger is not in it all of the data bits are set to high (as per the manufacturers specifications). This is correctly displayed by my code. If I stick my finger in it, I get a segmentation fault. What appears to happen is that the code locates the first sync byte, but then is unable to locate a second sync bit, and a strcat is attempted that overflows buf[1024]. I know the device is working because I've hooked it up to a windows machine and looked at the output using the manufactures software, so there's a problem with my code, not the device.

Code:

unsigned char buf[1024];
unsigned char buf_tmp[1024]= "";
unsigned char *buf_pointer;
unsigned char *buf_pntr_end;

int RCV_Flag;
unsigned int frm_chksum;
unsigned int frm_chksum2;
int i;

... TCP/serial opening stuff

while(1)

...

if ((csize = read(sd2, buf, 100)) >= 1) {
    RCV_Flag = 0;                      
    strcat(buf_tmp,buf);                
    memset(&buf[0],'\0',strlen(buf));
    strcpy(buf,buf_tmp);               

     end_pos=(strlen(buf)-1);   
   
     memset(&buf_tmp[0],'\0',strlen(buf_tmp));     

    buf_pointer=&buf[0];             // using two buffers to frame an entire packet.
    buf_pntr_end=&buf[0];

    /* cycle through the entire buffer. In order to check the validity of a given frame you need all five bytes, so if the buf contains 
     fewer than 5 bytes write the existing bytes into buf_tmp and leave the for loop. Next time the while loop passes through this 'if' statement
     buf_tmp will be added to buffer */
    for(;buf_pointer<= &buf[end_pos];buf_pointer++,buf_pntr_end++){
          if(*buf_pointer == 1 && buf_pointer>&buf[(end_pos-5)]){						
		for(i=0;buf_pointer<=&buf[end_pos];buf_pointer++,i++){
			buf_tmp[i]=*(buf_pointer);						
		}					
		memset(&buf[0],'\0',strlen(buf));
		break;
           }	

	    // if I detect the beginning of a new frame ( *buf_pointer ==1) and there is enough data left in the buffer such that I can verify the checksum 
           // I enter this if statement	
           if(*(buf_pointer) == 1 && buf_pointer<=&buf[(end_pos-5)]){
		frm_chksum = 0;
		frm_chksum = (unsigned int)*(buf_pointer) + (unsigned int)*(buf_pointer+1) + (unsigned int)*(buf_pointer+2) + (unsigned int)*(buf_pointer+3);
                     if (*(buf_pointer+1) > 127 && *(buf_pointer+1) & 0x01 && *(buf_pointer+4) == (frm_chksum%256)){
                          
                          /* If I make it to this point buf_pointer is now pointing to the beginning of a new frame.
                           now I do essentially the same process as above, but only increment buf_pntr_end
                           so when I eventually break from this loop I have two pointers indicating the begging and end of the packet. */

                          /* I know that I shouldn't be seeing the next sync bit for a while so I just skip over an arbitrary number of bytes.
                          I know I'm not really skipping over them, and I'm pretty inefficiently rescanning data that I have already scanned, 
                          but I would like to get this working before i address inefficiencies. */
                          
                          if(buf_pntr_end>=&buf[(end_pos-20)]){
				for(i=0;buf_pointer<=&buf[(end_pos)];buf_pointer++,i++){
					buf_tmp[i]=*(buf_pointer);
				}
				memset(&buf[0],'\0',strlen(buf));
				break; 
                          }
                          // this for loop is the same as before, but only buf_pntr_end is being incremented
                          for(buf_pntr_end++;buf_pntr_end<=&buf[(end_pos)];buf_pntr_end++){
                                if(buf_pntr_end>&buf[(end_pos-4)]){						
					for(i=0;buf_pointer<=&buf[end_pos];buf_pointer++,i++){
						buf_tmp[i]=*(buf_pointer);
					}
					memset(&buf[0],'\0',strlen(buf));
					break;
				}
                                 if(*(buf_pntr_end) == 1 && buf_pntr_end<=&buf[end_pos-4]){
					frm_chksum2=0;
					 frm_chksum2=(unsigned int)*(buf_pntr_end)+(unsigned int)*(buf_pntr_end+1)+(unsigned int)*(buf_pntr_end+2)+(unsigned int)*(buf_pntr_end+3);
								
					 if (*(buf_pntr_end+1) > 127 && *(buf_pntr_end+1) & 0x01 && *(buf_pntr_end+4) == (frm_chksum2%256)){			
						RCV_Flag = 1;
						break;
					}
                                   }
                               } end of for(buf_pntr_end++;buf_pntr_end<=&buf[(end_pos)];buf_pntr_end++)

                               break;

                         } // end of if (*(buf_pointer+1) > 127 && *(buf_pointer+1) & 0x01 && *(buf_pointer+4) == (frm_chksum%256))
              } //end of if(*(buf_pointer) == 1 && buf_pointer<=&buf[(end_pos-5)])
     } //end of for(;buf_pointer<= &buf[end_pos];buf_pointer++,buf_pntr_end++)

If anyone can point out any (pertinent) problems with code, or just tell me that I'm going about this the wrong way, it would be greatly appreciated.
thanks for your time!
-robert

Thanks for taking the time to look at the code Salem!

I had put the memset calls on a whim while trying to get the code working. I removed them and the "functionality" of the code remained the same.

I added the null character to the end of buf, but left the 100 as is because sizeof(buf) would return 1024 and I need to append buf_tmp to buf anyway, so I'd be in trouble if I managed to read 1023 bytes from the serial port in one call (also a printf("%d\n",strlen(buf)); called right after buf is populated by read indicates that it's only pulling around 20 bytes per call). Really buf should never have more than 75-80 bytes in it because once I receive that I should have the contents of one of the packet I'm interested in and could flush buf up to the &buf_pntr_end.

The problem is still that I'm not finding the second sync bit. Do you think strcat/strcpy, or any of my assignments to buf_tmp could be adding/removing random bytes from my data stream? In order to fill up buf, it should contain ~15 (1024/75) complete packets, and presumably as many sync bits, how am I missing all of them and filling up buf?

I agree, I take care of any trailing data after the second sync bit further down in an 'if(RCV_Flag)' statement.

Code:

if(RCV_Flag){
				RCV_Flag=0;

				sprintf(packet,"-1,%u,%u,",*(buf_pointer+3),*(buf_pointer+8));
				write(sd1,packet,100);
				
				for(i=0,buf_pntr_end++;   buf_pntr_end<=&buf[end_pos];  buf_pntr_end++,i++){             
					buf_tmp[i]=*(buf_pntr_end);
				}
				
				
			}

I may have found a possible source of error. The program has no problem when the sensor is not connected, because all of the measurement bits are set to high. For instance the heart rate is contained in 2 bits of the 4th byte of the first frame and 7 bits of the 4th byte of the second frame (so the max value of HR is 111111111 = 511). When I put my finger in the device it will only be measuring a value of ~60 so the two bits from the 4th byte in the first frame are set to zero so I get a byte value of zero. Is this zero being interpreted at a null byte, or being ignored for some reason by the read() command? If this is a problem how do I work around it? Do I need to stop using read(), or should I convert my buf value to a hex array, prior to the strcat/strcpy.

Thanks, again.

I've started using memmove and memcpy, and I can now get actual readings when I have my finger plugged in! The program hangs after a couple of readings, but I'll hopefully be able to manage from here.

Is passing zero values over the serial port just a terrible design on the part of the device manufacturer? or is it standard to have to manipulate char arrays containing 'reserved' characters? Should I have just converted these values to hex/dec 4byte/value array so I could be using string manipulation?

Not terrible design at all. What would you expect the sensor to return on a corpse? If you saw anything other than a zero, congratulations! You're experiencing a resurrection .

Perhaps your confusion comes from C's use of the word 'char' to represent single-byte values. Don't think of every char array as a string, think of it as an array of bytes (like ints but with a smaller range) . Only if you want it to work with the string.h functions do you need the magic null (zero) byte at the end, but strings should generally contain human-readable text. You're getting raw binary data, byte by byte. Arrays of bytes are exactly what you want for this.