reading from serial port in c breaks up lines

Question

I'm trying to write a little program in C that will read from the serial port using the select command so that it blocks and waits for input. It's working, except it keeps breaking up lines and I have no idea why. The device is programmed not to break up the lines and works fine with actual terminal programs. I've never done serial communication in C before, and I'm on a Mac, so it's all new to me. I really have no idea where to even look for what's going wrong.

I have some code that finds and lists serial ports. I'll leave that out for simplicity, so if there's a variable that doesn't make sense, that might be why. Here is the code that opens the port, sets attributes, and tries to read from it, complete with copied comments from Apple's site (sorry):

/* this is based on a combination of http://stackoverflow.com/questions/6947413/how-to-open-read-and-write-from-serial-port-in-c
 * and https://developer.apple.com/library/mac/documentation/DeviceDrivers/Conceptual/WorkingWSerial/WWSerial_SerialDevs/SerialDevices.html
 */

static int OpenSerialPort(const char *deviceFilePath, int speed)

{

int         fileDescriptor = -1;
struct termios  options;
memset(&options, 0, sizeof(options)); // init it

// Open the serial port read/write, with no controlling terminal,
// and don't wait for a connection.
// The O_NONBLOCK flag also causes subsequent I/O on the device to
// be non-blocking.
// See open(2) ("man 2 open") for details.
fileDescriptor = open(deviceFilePath, O_RDWR | O_NOCTTY | O_NONBLOCK);
if (fileDescriptor == -1)
{
    printf("Error opening serial port %s - %s(%d).\n", deviceFilePath, strerror(errno), errno);
    goto error;
}

// Note that open() follows POSIX semantics: multiple open() calls to
// the same file will succeed unless the TIOCEXCL ioctl is issued.
// This will prevent additional opens except by root-owned processes.
// See options(4) ("man 4 options") and ioctl(2) ("man 2 ioctl") for details.

if (ioctl(fileDescriptor, TIOCEXCL) == kMyErrReturn)
{
    printf("Error setting TIOCEXCL on %s - %s(%d).\n", deviceFilePath, strerror(errno), errno);
    goto error;
}

// Set raw input (non-canonical) mode, with reads blocking until either
// a single character has been received or a one second timeout expires.
// See tcsetattr(4) ("man 4 tcsetattr") and termios(4) ("man 4 termios")
// for details.

cfmakeraw(&options);
options.c_cc[VMIN] = 1;
options.c_cc[VTIME] = 5;

// The baud rate, word length, and handshake options can be set as follows:
cfsetspeed(&options, speed);   // Set 19200 baud
options.c_cflag = (options.c_cflag & ~CSIZE) | CS8;     // 8-bit chars
// disable IGNBRK for mismatched speed tests; otherwise receive break
// as \000 chars
options.c_iflag &= ~IGNBRK;         // disable break processing
options.c_lflag = 0;                // no signaling chars, no echo,
// no canonical processing
options.c_oflag = 0;                // no remapping, no delays


options.c_iflag &= ~(IXON | IXOFF | IXANY); // shut off xon/xoff ctrl

options.c_cflag |= (CLOCAL | CREAD);// ignore modem controls,
// enable reading
options.c_cflag &= ~(PARENB | PARODD);      // shut off parity
options.c_cflag |= false;
options.c_cflag &= ~CSTOPB;
options.c_cflag &= ~CRTSCTS;

// Cause the new options to take effect immediately.
if (tcsetattr(fileDescriptor, TCSANOW, &options) == kMyErrReturn)
{
    printf("Error setting options attributes %s - %s(%d).\n", deviceFilePath, strerror(errno), errno);
    goto error;
}

// turn on blocking
if (fcntl(fileDescriptor, F_SETFL, 0) == kMyErrReturn)
{
    printf("Error clearing O_NONBLOCK %s - %s(%d).\n", deviceFilePath, strerror(errno), errno);
    goto error;
}


// Success:
return fileDescriptor;
// Failure:
error:
if (fileDescriptor != kMyErrReturn)
{
    close(fileDescriptor);
}
return -1;
}

int main(void)
{
int         fileDescriptor;
kern_return_t   kernResult; // these are Apple-specific
io_iterator_t   serialPortIterator; // Apple
char        deviceFilePath[MAXPATHLEN];
fd_set fdset; // make a file descriptor set
FD_ZERO (&fdset); // init it
char buf[1000]; // some strings are big

kernResult = GetDevices(&serialPortIterator);
printf("Devices on this system:\n");
kernResult = ListDevicePaths(serialPortIterator, deviceFilePath, sizeof(deviceFilePath));

IOObjectRelease(serialPortIterator);    // Release the iterator.

// Open the modem port, initialize the modem, then close it.
if (!deviceFilePath[0])
{
    printf("No modem port found.\n");
    return EX_UNAVAILABLE;
}

fileDescriptor = OpenSerialPort("/dev/cu.usbmodem1d1111", B230400);
FD_SET (fileDescriptor, &fdset); // add to file descriptor set

// now we're going to use select to only read from the file handle when there's data available
while (1)
{
    if (select (FD_SETSIZE, &fdset, NULL, NULL, NULL) < 0) // this will block the program until something is on the line
    {
        printf("select error\n");
    }
    read(fileDescriptor, buf, 1000);
    printf("%s\n", buf);
    memset(buf, '\0', 1000);
}



// let's try to read from the serial port
   /* for (int i = 0; i <= 10; i++)
{
    char buf [100];
    int n = read(fileDescriptor, buf, sizeof buf);
    printf("%s\n", buf);
    //usleep ((7 + 25) * 100);
}*/
close(fileDescriptor);
printf("Modem port closed.\n");

return EX_OK;
}

Expected output:

    This is sample output.
    Hello.

What I actually get in the above program:

    Thi
    s is sam
    ple output.
    Hel
    lo.

Or something like that. It's different each time. Sometimes it works fine. It seems to be random.

So my questions are: What am I doing wrong? What code do I need to work on aside from just a blanket "all of it?" What am I not understanding? I admit I don't really understand how these libraries work, exactly. I am assuming (I know, I know) that they take care of flow control and errors and so on. But then again, the examples I've copied from didn't exactly explain that so I don't know. I just don't really know what's going on.

Answer 1

it keeps breaking up lines and I have no idea why.

If you wants to read lines from the serial terminal, then you have to configure it to do so.
Instead you have configured it to be in non-canonical and non-blocking mode.
The code does not match your stated intentions at all.

Quoting from the Linux termios man page:

In canonical mode:
Input is made available line by line. An input line is available when one of the line delimiters is typed (NL, EOL, EOL2; or EOF at the start of line). Except in the case of EOF, the line delimiter is included in the buffer returned by read(2).

The code is clearly commented that it is using non-canonical mode (i.e. the wrong mode):

// Set raw input (non-canonical) mode, with reads blocking until either
// a single character has been received or a one second timeout expires.
// See tcsetattr(4) ("man 4 tcsetattr") and termios(4) ("man 4 termios")
// for details.

cfmakeraw(&options);
options.c_cc[VMIN] = 1;
options.c_cc[VTIME] = 5;

You need to remove these lines to get canonical mode and read lines instead of raw bytes.

If you expect the read() to return complete lines, then the program will have to wait for input. That means that you need blocking I/O.

// The O_NONBLOCK flag also causes subsequent I/O on the device to
// be non-blocking.
// See open(2) ("man 2 open") for details.
fileDescriptor = open(deviceFilePath, O_RDWR | O_NOCTTY | O_NONBLOCK);

The O_NONBLOCK option needs to be removed from the open() syscall.

In spite of what at least three commenters have written, a Linux serial terminal can be configured to read lines. You are using a real operating system, and not running bare-metal on a microprocessor. All you have to do is activate the line discipline to scan the characters received by the serial terminal.
Full details for programming canonical mode can be found in Serial Programming Guide for POSIX Operating Systems and the termios man page.

There are also several issues with your code that should be corrected:

• Instead of memset(&options, 0, sizeof(options)) the code should be calling tcgetattr() to properly initialize the structure. This can be a serious issue for canonical input, as the existing code will have zeroed out all of the control code specifications instead of having proper definitions.
• Instead of direct assignments, the code should be performing bit-wise operations (in order to preserve existing settings). See Setting Terminal Modes Properly.
• The read(fileDescriptor, buf, 1000) statement needs to be expanded to handle possible errors and to deal with the received data.
• the return code from the read() syscall needs to be checked for any error conditions.
• when no error is detected, then the return code indicates the number of bytes returned in the buffer. Note that the input will not be terminated by a null byte, so string operations should not be applied to the buffer until a null is appended.

The read code should something like:

 rc = read(fileDescriptor, buf, sizeof(buf) - 1);
 if (rc < 0) {
     /* handle error condition */
 } else {
     buf[rc] = '\0';
     printf("%s", buf);
 }

Since buf[] is allocated for 1000 bytes, the read() request can return a line up to 999 characters long.

Answer 2

The problem is that you are reading an arbitrary number of bytes, and then outputing them separated by a newline:

read(fileDescriptor, buf, 1000);
printf("%s\n", buf);

You opened the descriptor O_NONBLOCK and I'm not sure your fcntl call is sufficient to clear it. The result is that read pulls out however many characters happen to be buffered that that moment, and then you print them followed by a newline.

You probably do not want to read in blocking mode, as then it may not return until 1000 characters are read. This may be closer to what you want:

amt = read(fileDescriptor, buf, 1000);
if (amt > 0)
    write(1,buff,amt);
else
    break;

Of course, there should be a lot more error handling.