Reading from a serial port

I'm receiving data from a serial port in C, using Serial Programming Guide for POSIX Operating Systems as a guide.

The data I receive should be always 10 bytes in length but I want to be sure that, if there is any error (more or less bytes received), reading will clear buffer before the next data arrive, so that there is always proper data in the buffer to be processed. I'm using select to monitor serial file descriptor and local socket. I figured out that I can clean serial buffer in case there was something left from previous transmission, when device is not sending for some period of time.

Question is: is this the right solution?

main.c:

loop_num=0;
si_processed=0;
while(TRUE) {
    /* copy fd_set for select */
    tmp_input=input;

    n = select(max_fd,&tmp_input,NULL,NULL,NULL/*&timeout*/);

    /* See if there was an error */
    if (n10) {
        /* clear buffer */
        si_processed=0;
        loop_num=0;
    }
    fflush(stdout);
    usleep(20000);
}

serial_port.c

char serial_buffer[256];

int process_serial(int serial_fd) {
    int bytes;
    int n,i;
    char tmp_buffer[32];

    ioctl(serial_fd, FIONREAD, &bytes);
    if(!bytes)
        return 0;

    n=read(serial_fd, tmp_buffer, sizeof(tmp_buffer));
    for(i=0;i=INPUT_BYTES_NUM/* defined as 10 */) {
        serial_buffer[si_processed]='\0';
        printf("read:%s\n",serial_buffer);
        si_processed=0;
        fflush(stdout);
    }
    return 1;
}

Some more info. I'm querying device every 5 seconds for data, and it should always respond with 10 bytes packets: so data should come fairly quickly into serial buffer, and then a few seconds of silence; sometimes however the device can send data on its own, I don't know when, but also as a 10 bytes packets.

I was doing some test with my program and minicom connected on the other side.

Until I was sending data like

1234567890

1234567890

program was reading what it supposed to read:

1234567890

Problem ari

The code you posted contains no error-recovery at all. It's a bit off-topic on this site to ask how to implement a new feature (error-recovery); but I'll try.

It's not clear what your communication protocol looks like. It might be:

• A continuous stream of bytes, to be split into packets of 10

• 10-byte packets, with a measurable delay between packets, no reply expected

• 10-byte packets, after which silence until there is a reply, perhaps with a retry if there no reply

I'll assume it isn't 1. because that has little opportunity for error-recovery: if a byte were ever lost then you wouldn't know where the boundary is between "packets", if it's a continuous stream of bytes, unless you use 'framing' and 'escape sequences' in the data.

So I'll assume that it's 2. or 3.. In either case, what's important is:

• There are 10 bytes received (probably received quickly with little or no delay between bytes)

• There is then a delay (no bytes received) after those 10 bytes.

Setting Read Timeouts looks interesting. It may be your best algorithm: set VMIN to say that you want to receive no more or less than exactly 10 bytes when you read.

However that may have a problem:

However, the timeout only applies to the first character read, so if for some reason the driver misses one character inside the N byte packet then the read call could block forever waiting for additional input characters.

You don't say why you want error-recovery after losing a character: perhaps you sometimes get overruns in the driver, or bit parity errors from the serial port?

"Setting Input Parity Options" talks about handling the parity errors:

-
Replace/mark the byte with an error byte

If you do this then you can be more sure that you will get 10 bytes; but you can only do it if the protocol allows you to recognize an illegal byte value.

-
Or remove the error byte

If you do this then you must be able to recover from getting 9 bytes sometimes.

The data I receive should be always 10 bytes in length but I want to be sure that, if there is any error (more or less bytes received), reading will clear buffer before the next data arrive, so that there is always proper data in the buffer to be processed.

Reading should empty the buffer, if your sizeof(tmp_buffer) is bigger is the number of bytes queued in the driver. However:

• It depends on the read mode (e.g. a read may block forever instead of clearing the buffer, if VMIN is set to non-zero)

• If there are, somehow, very many received bytes enqueued in the driver, then you may have to read repeatedly until the driver is empty.

• Immediately after you read, there may be more bytes in the driver: for example if the serial port is flow-controlled, reading from the driver allows the device to send again / send more.

Whether you can clear the buffer "before the next data arrive" is difficult to say: I don't know when the next data is supposed to arrive.

Depending on how you handle parity errors, maybe bytes are never lost. If bytes are sometimes lost then you may want to implement logic like:

read_10_bytes:
  read (waiting forever) until some bytes are returned
  if 10 bytes were read then return
  if less than 10 bytes were read, then:
    set an expiry timer
    loop doing the following
  read more bytes
  if you read 10 bytes before the timer expires, then clear the timer and return
  if the timer expires before you read 10 bytes, then discard the bad bytes and return

Instead of actually setting a timer, above, perhaps loop doing a blocking-read-with-timeout (which either reads the bytes, or expires).

As well as handling too few bytes (above), you could modify the above to check for receiving more than 10 bytes: after you receive 10 bytes, do another blocking-read-with-small-timeout to verify that there are no more bytes to receive. If there are extra bytes, then read them all before returning to the 'waiting-for-next-10-bytes' state (otherwise, these extra bytes will mess up your next 10-byte packet).

I think there's an obvious bug in the code. After you read 16 bytes, you completely clear your serial_buffer by zeroing si_processed, and you then think "success!" after each next 10 bytes you read.

Instead you should:

• If you read too much data, don't discard the extra bytes: instead use memmove to move them to the beginning of the buffer (assuming they're the start of the next packet), and then read the subsequent bytes into the vacant space after them.

Something like:

// after reading and processing a 10-byte packet
if (si_processed > 10) // already have the start of the next packet
{
     // move start of next packet to start of buffer
     memmove(serial_buffer, serial_buffer+10, si_processed - 10);
     // not 0: remember how many start bytes we already have
     si_processed -= 10;
}

• After you read any bytes, verify whether the start byte is your 0xA5 value, and discard the bytes if not.

Something like:

```
if (si_processed > 0) /

StackExchange Code Review Q#39752, answer score: 8