Linux Serial Port Buffer Not Empty When Opening Device

Linux serial port buffer not empty when opening device

The Linux terminal driver buffers input even if it is not opened. This can be a useful feature, especially if the speed/parity/etc. are set appropriately.

To replicate the behavior of lesser operating systems, read all pending input from the port as soon as it is open:

...
int fd = open ("/dev/ttyS0", O_RDWR | O_NOCTTY | O_SYNC);
if (fd < 0)
        exit (1);

set_blocking (fd, 0);   // disable reads blocked when no input ready

char buf [10000];
int n;
do {
        n = read (fd, buf, sizeof buf);
} while (n > 0);

set_blocking (fd, 1);  // enable read blocking (if desired)

...  // now there is no pending input

void set_blocking (int fd, int should_block)
{
        struct termios tty;
        memset (&tty, 0, sizeof tty);
        if (tcgetattr (fd, &tty) != 0)
        {
                error ("error %d getting term settings set_blocking", errno);
                return;
        }

        tty.c_cc[VMIN]  = should_block ? 1 : 0;
        tty.c_cc[VTIME] = should_block ? 5 : 0; // 0.5 seconds read timeout

        if (tcsetattr (fd, TCSANOW, &tty) != 0)
                error ("error setting term %sblocking", should_block ? "" : "no");
}

Reading from serial port fails

read() returning 0 indicates the end-of-file condition. You should check for that and break out of the loop if it occurs.

As to what's causing that - end-of-file on a serial port indicates it has detected a hangup, meaning that the DCD line has been dropped.

You can set the CLOCAL flag in options.c_cflag to ignore the modem control lines, if your device doesn't set them properly.

USB Serial port programming has disastrous results

from within Code::Blocks the debugger is also hosed - once the program is started it cannot be paused or stopped

It is far more likely that you do not understand your tools than that you have created an unkillable program.

It's easy enough to figure this out: divide and conquer. You've got a whole pile of unrelated components here. Start separating them and find out which pieces work fine in isolation and which continue to behave badly when disconnected from everything else. Then you'll have your culprit.

Specifically here, that means try running your program outside the IDE, then under command line gdb instead of GDB via the IDE.

Also, it should be possible to run your program without starting the web server piece, so that you can run the serial part of the app in isolation. This is not only good for debugging by minimizing confounding variables, it also encourages a loosely-coupled program design, which is a good thing in its own right.

In the end, you may find that the thing keeping your program from stopping is the web framework, Code::Blocks, or the way GDB operates on the Pi under Code::Blocks, rather than anything to do with the USB to serial adapter.

once the problem happens the only solution seems to be to reboot the Pi

If your program is still running in the background, then of course your next instance will fail if it tries to open the same USB port.

Don't guess, find out:

$ sudo lsof | grep ttyACM

or:

$ lsof -p $(pidof myprogram)

(Substitute pgrep if your system doesn't have pidof.)

I've done things like this before using a scripting language (Tcl) but this time around I'm looking for a performance boost from a non-interpreted language

Your serial port is running at 115,200 bps. Divide that by 10 to account for the stop and start bits, then flip the fraction to get seconds per byte, and you come to 87 microseconds per byte. And you only achieve that when the serial port is running flat-out, sending or receiving 11,500 bytes per second. Wanna take a guess at how many lines of code Tcl can interpret in 87 microseconds? Tcl isn't super-fast, but 87 microseconds is an eternity even in Tcl land.

Then on the other side of the connection, you have HTTP and a [W]LAN, likely adding another hundred milliseconds or so of delay per transaction.

Your need for speed is an illusion.

Now come back and talk to me again when you need to talk to 100 of these asynchronously, and then maybe we can start to justify C over Tcl.

(And I say this as one whose day job involves maintaining a large C++ program that does a lot of serial and network I/O.)

Now lets get to the many problems with this code:

system("ls -l /dev/serial/by-path > usbSerial\n");  // get current port list
ifd = fopen("usbSerial", "r");

Don't use a temporary where a pipe will suffice; use popen() here instead.

while(1) {

This is simply wrong. Say while (!feof(ifd)) { here, else you will attempt to read past the end of the file.

This, plus the next error, is likely the key to your major symptoms.

if (fgets(line, 127, ifd) == NULL) { 
    break;

There are several problems here:

1. You're assuming things about the meaning of the return value that do not follow from the documentation. The Linux fopen(3) man page isn't super clear on this; the BSD version is better:

   The fgets() and gets() functions do not distinguish between end-of-file and error, and callers must use feof(3) and ferror(3) to determine which occurred.

   Because fgets() is Standard C, and not Linux- or BSD-specific, it is generally safe to consult other systems' manual pages. Even better, consult a good generic C reference, such as Harbison & Steele. (I found that much more useful than K&R back when I was doing more pure C than C++.)

   Bottom line, simply checking for NULL doesn't tell you everything you need to know here.

2. Secondarily, the hard-coded 127 constant is a code bomb waiting to go off, should you ever shrink the size of the line buffer. Say sizeof(line) here.

   (No, not sizeof(line) - 1: fgets() leaves space for the trailing null character when reading. Again, RTFM carefully.)

3. The break is also a problem, but we'll have to get further down in the code to see why.

Moving on:

strcat(port, ptr);              // append the "ttyACMn" part of the line

Two problems here:

1. You're blindly assuming that strlen(ptr) <= sizeof(port) - 6. Use strncat(3) instead.

   (The prior line's strcpy() (as opposed to strncpy()) is justifiable because you're copying a string literal, so you can see that you're not overrunning the buffer, but you should get into the habit of pretending that the old C string functions that don't check lengths don't even exist. Some compilers will actually issue warnings when you use them, if you crank the warning level up.)

   Or, better, give up on C strings, and start using std::string. I can see that you're trying to stick to C, but there really are things in C++ that are worth using, even if you mostly use C. C++'s automatic memory management facilities (not just string, but also auto_ptr/unique_ptr and more) fall into this category.

   Plus, C++ strings operate more like Tcl strings, so you'll probably be more comfortable with them.

2. Factual assertions in comments must always be true, or they are likely mislead you later, potentially hazardously so. Your particular USB to serial adapter may use /dev/ttyACMx, but not all do. There's another common USB device class used by some serial-to-USB adapters that causes them to show up under Linux as ttyUSBx. More generally, a future change may change the device name in some other way; you might port to BSD, for example, and now your USB to serial device is called /dev/cu.usbserial, blowing your 15-byte port buffer. Don't assume.

   Even with the BSD case aside, your port buffer should not be smaller than your line buffer, since you are concatenating the latter onto the former. At minimum, sizeof(port) should be sizeof(line) + strlen("/dev/"), just in case. If that seems excessive, it is only because 128 bytes for the line buffer is unnecessarily large. (Not that I'm trying to twist your arm to change it. RAM is cheap; programmer debugging time is expensive.)

Next:

fcntl(cfd, F_SETFL, 0);                                 // blocking mode

File handles are blocking by default in Unix. You have to ask for a nonblocking file handle. Anyway, blasting all the flags is bad style; you don't know what other flags you're changing here. Proper style is to get, modify, then set, much like the way you're doing with tcsetattr():

int flags;
fcntl(cfd, F_GETFL, &flags);
flags &= ~O_NONBLOCK;
fcntl(cfd, F_SETFL, flags);

Well, you're kind of using tcsetattr() correctly:

tcsetattr(cfd, TCSANOW, &options); 

...followed by further modifications to options without a second call to tcsetattr(). Oops!

You weren't under the impression that modifications to the options structure affect the serial port immediately, were you?

if (write(cfd, msgOut, 3) < 3) {
    logIt(fprintf(lfd, "Sending of output message failed\n"));
    close(cfd);
    continue; 
}

Piles of wrong here:

1. You're collapsing the short-write and error cases. Handle them separately:

   int bytes = write(cfd, msgOut, 3);
   if (bytes == 0) {
       // can't happen with USB, but you may later change to a
       // serial-to-Ethernet bridge (e.g. Digi One SP), and then
       // it *can* happen under TCP.
       //
       // complain, close, etc.
   }
   else if (bytes < 0) {
       // plain failure case; could collapse this with the == 0 case
       // close, etc
   }
   else if (bytes < 3) {
       // short write case
   }
   else {
       // success case
   }
   

2. You aren't logging errno or its string equivalent, so when (!) you get an error, you won't know which error:

   logIt(fprintf(lfd, "Sending of output message failed: %s (code %d)\n",
              strerror(errno), errno));
   

   Modify to taste. Just realize that write(2), like most other Unix system calls, has a whole bunch of possible error codes. You probably don't want to handle all of them the same way. (e.g. EINTR)

3. After closing the FD, you're leaving it set to a valid FD value, so that on EOF after reading one line, you leave the function with a valid but closed FD value! (This is the problem with break above: it can implicitly return a closed FD to its caller.) Say cfd = -1 after every close(cfd) call.

Everything written above about write() also applies to the following read() call, but also:

if (read(cfd, msgIn, 4) != 4) {

There's nothing in POSIX that tells you that if the serial device sends 4 bytes that you will get all 4 bytes in a single read(), even with a blocking FD. You are especially unlikely to get more than one byte per read() with slow serial ports, simply because your program is lightning fast compared to the serial port. You need to call read() in a loop here, exiting only on error or completion.

And just in case it isn't obvious:

remove("usbSerial");

You don't need that if you switch to popen() above. Don't scatter temporary working files around the file system where a pipe will do.

Why does Linux C serial read only work with high CPU?

As your code doesn't work in a common condition that you can't prevent from happening, the why of "Why [it] only work[s] with high CPU?" doesn't really matter. It's probably interesting to spend a lot of time and effort finding out the "Why?", but I'd think you're going to have to change your code because anything that stops working when CPU load goes down is, IMO, waaaaay too fragile to trust to work for any time.

First, is threading even useful on your system? If there's only one CPU that can run only one thread at a time, creating multiple threads will be counterproductive. Have you tried a simple single-threaded solution and actually found that it doesn't work?

If you have tried a single-threaded solution and it doesn't work, the first thing I note is that your currently posted code is doing a tremendous amount of extra work it doesn't need to do, and it's likely contending over a single lock when that doesn't help much at all.

So eliminate your extraneous copying of data along with all the unnecessary bookkeeping you're doing.

You also probably have a lot of contention with just a single mutex and condition variable. There's no need to not read because the logging thread is doing something, or the logging thread not processing because the read thread is doing some bookkeeping. You'd almost certainly benefit from finer lock granularity.

I'd do something like this:

#define CHUNK_SIZE ( 4 * 1024 )
#define NUM_BUFFERS 16

struct dataStruct
{
    sem_t full;
    sem_t empty;
    ssize_t count;
    char data[ CHUNK_SIZE ]
};

struct dataStruct dataArray[ NUM_BUFFERS ];

void initDataArray( void )
{
    for ( int ii = 0; ii < NUM_BUFFERS; ii++ )
    {
        // buffers all start empty
        sem_init( &( dataArray[ ii ].full ), 0, 0 );
        sem_init( &( dataArray[ ii ].empty ), 0, 1 );
    }
}

void *readPortThread( void *arg )
{
    unsigned currBuffer = 0;

    // get portFD from arg
    int portFD = ???
    for ( ;; )
    {
        sem_wait( &( dataArray[ currBuffer  ].empty ) );

        // probably should loop and read more, and don't
        // infinite loop on any error (hint...)
        dataArray[ currBuffer  ].count = read( portFD, 
            dataArray[ currBuffer  ].data,
            sizeof( dataArray[ currBuffer  ].data ) );
        sem_post( &( dataArray[ currBuffer  ].full ) );
        currBuffer++;
        currBuffer  %= NUM_BUFFERS;
    }
    return( NULL );
}

void *processData( char *data, ssize_t count )
{
    ...
}

void *logDataThread( void *arg )
{
    for ( ;; )
    {
        sem_wait( &( dataArray[ currBuffer  ].full ) );

        processData( dataArray[ currBuffer  ].data,
            dataArray[ currBuffer  ].count );

        sem_post( &( dataArray[ currBuffer  ].empty ) );
        currBuffer++;
        currBuffer  %= NUM_BUFFERS;
    }
    return( NULL );
}

Note the much finer locking granularity, and the complete lack of extraneous copying of data. Proper headers, all error checking, and full implementation are left as an exercise...

You'll have to test to find optimum values for CHUNK_SIZE and NUM_BUFFERS. Setting the number of buffers dynamically would be a good improvement also.

OT: There's no need for any indirection in your int open_port(int flags, int *fd) function. Simply return the fd value - it's good enough for open() itself:

int open_port(int flags )
{
    struct termios options;
    int fd = open("/dev/ttyUSB0", flags | O_NOCTTY);

    if(fd < 0){
        printf("Error opening port\n");
        return fd;
    }

    // no need for else - the above if statement returns
    printf("Port handle is %d\n", fd);

    // did you set **ALL** the fields???
    memset( options, 0, sizeof( options ) );

    options.c_cflag = BAUDRATE | CS8 | CLOCAL | CREAD;
    options.c_iflag = 0;
    options.c_oflag = 0;
    options.c_lflag = 0;
    options.c_cc[VTIME] = 0;
    options.c_cc[VMIN] = 200;
    tcflush(fd, TCIFLUSH);
    tcsetattr(fd, TCSANOW, &options);

    return fd;
}

---

Related Topics