1

I am trying to write a function that reads a chunk of data sent through UART. I am using Raspbian Jessie running on a RaspberryPi model B but I wanted to use this C code (with any necessary revisions) on openwrt. So far, this is what I wrote.

Header:

#ifndef __UART_LIB__
#define __UART_LIB__

#include <stdlib.h>     //Errors, etc
#include <unistd.h>     //Used for UART
#include <fcntl.h>      //Used for UART
#include <termios.h>    //Used for UART

#include <sys/types.h>  //These includes are for timeout
#include <sys/stat.h>   
#include <fcntl.h>
#include <sys/select.h> //
#include <sys/ioctl.h>

#define BITS_PER_PACKAGE_ 11
#define WAIT_PROLONGATION_CONSTANT_ 1.1f

//Some values used by default, left for the user to change if needed
unsigned int BAUD_ ;
unsigned int NUM_BITS_  ;
char *UART_PATH_ ;
unsigned int MAX_SIZE_ ;
unsigned int OPEN_FLAG_ ;
time_t TIMEOUT_SEC_ ;
suseconds_t TIMEOUT_USEC_ ;
struct timeval WAIT_CONSTANT_ ;

int open_conf_UART_() ;
int read_UART_(int uart_filestream, char** dest, int max_len) ;

#endif

.c file:

#include "uartlib.h"

unsigned int BAUD_ = B115200 ;
unsigned int NUM_BITS_ = CS8 ;
char *UART_PATH_ = "/dev/ttyAMA0" ;
unsigned int MAX_SIZE_ = 128 ;
unsigned int OPEN_FLAG_ = O_RDWR ;
time_t TIMEOUT_SEC_ = 5 ;
suseconds_t TIMEOUT_USEC_ = 0 ;


int open_conf_UART_()
{
    int indicator, old_fl;
    int uart_filestream ;
    struct termios options ;

    // Opening the port in a read/write mode
    uart_filestream = open(UART_PATH_, OPEN_FLAG_ | O_NOCTTY );
    if (uart_filestream < 0)
    {
        // Unable to open the serial port, so produce an error and halt
        return -1;
    }

    // Configuring the options for UART

    // Retrieve the options and modify them. 
    indicator = tcgetattr(uart_filestream, &options);
    if(indicator < 0)
    {   
        // Unable to get the attributes
        close(uart_filestream);
        return -1;
    }

    // I found a question on stackoverlow where the answer said that VTIME and VMIN will be ignored unless I 
    // switch the FNDELAY flag off
    old_fl = fcntl(uart_filestream, F_GETFL);
    if(old_fl < 0)
    {
        return -1;
    }
    old_fl &= ~FNDELAY;
    fcntl(uart_filestream, old_fl);

    //Setting the options
    options.c_cflag = CRTSCTS | BAUD_ | NUM_BITS_ | CLOCAL | CREAD ;
    options.c_iflag = 0;
    options.c_oflag = 0;
    options.c_lflag = 0;

    //I want the uart to wait 1/10 of a second between bytes at most
    options.c_cc[VTIME] = 1;
    options.c_cc[VMIN] = 0;

    // Flushing the file stream (the input and the output area)
    indicator = tcflush(uart_filestream, TCIOFLUSH);
    if(indicator < 0)
    {   
        // Unable to flush
        close(uart_filestream);
        return -1;
    }


    // Setting the options for the file stream. 
    indicator = tcsetattr(uart_filestream, TCSANOW, &options);
    if(indicator < 0)
    {   
        // Unable to set the attributes
        close(uart_filestream);
        return -1;
    }
    return uart_filestream;
}

int read_UART_(int uart_filestream, char** dest, int max_len)
{
    int indicator;
    int buffer_length;

    indicator = tcflush(uart_filestream, TCIFLUSH);
    if(indicator < 0)
    {   
        // Unable to flush
        return -1;
    }

    //Do the actual reading
    buffer_length = read(uart_filestream, (void*)(*dest), max_len);
    if(indicator < 0)
    {
        return -1;
    }
    else
    {
        // Returning number of read bytes
        return buffer_length;
    }   
    // Both branches of the if statement above have return, so this will not be reached
}

So, when I try to read more than 8 bytes, the message gets truncated to 8 bytes. As I read, setting VTIME to a certain value allows the time interval between two bytes to be at most that long. I am not certain what is going on but I suspect that the read() call reads the buffer before the receiving of the data is complete. My wish is to read a chunk of data of undefined size. I also used select() with a timeout before the read to make sure the program won't block entirely. I read a lot of forum topics, stackoverflow questions, guides, etc. on this topic and none seem to help me with my solution. So, can anyone explain what is going on here? Is it possible to do what I want?

Note that I removed some of the code (I also wrote a function that writes to a UART port) so here might be some redundant includes, global variables, etc.

NMilev
  • 77
  • 2
  • 11
  • `So, when I try to read more than 8 bytes, the message gets truncated to 8 bytes` The UART has a fixed-size recieve buffer(and flow control, if you want) . You should do your own buffering in your program. – wildplasser Aug 03 '16 at 11:48
  • 1
    Posted code shows no call to `int read_UART_(int uart_filestream, char** dest, int max_len)`, so no evidence that is called correctly. – chux - Reinstate Monica Aug 03 '16 at 11:50
  • Are you sure that data are sent in a whole chunk? Did you test the transmission with an oscilloscope? Side note: what is the `max_len` value passed to function? – LPs Aug 03 '16 at 11:52
  • 2nd `if(indicator < 0)` in `read_UART_()` serves no purpose. – chux - Reinstate Monica Aug 03 '16 at 11:52
  • IIRC the raspberryPi has a 3-wire (emulated) uart, so there can be no hardware flow controll. (XON/XOFF might be possible, though) – wildplasser Aug 03 '16 at 11:54
  • BTW: the VTIME setting has to do with canonical mode (has to do with line-editing: user at a terminal). I suppose you want non-canonical mode. – wildplasser Aug 03 '16 at 11:59
  • @wildplasser So, if I do a reading of, say, 20 bytes, I should have 3 subsequent calls to read()? I also do not want to wait a long time in a blocking call, this should be as fast as possible. I also think I found some pieces of code online which use VTIME with non canonical mode. Not even sure if I could use the canonical mode, I just need the bytes, unaltered. – NMilev Aug 03 '16 at 12:25
  • @chux I am pretty sure it is a correct call. I send the return value from open_conf_UART_(), an allocated buffer and its size as arguments. – NMilev Aug 03 '16 at 12:26
  • @LPs Yes, it was measured. A #defined value is sent as max_len argument. – NMilev Aug 03 '16 at 12:26
  • Also, I have tried calculating some time interval in which two subsequent bytes should be received (using the BOAD and the number of bits needed to send 8 bits of data), expanding it a little and waiting (with a select()) that amount of time for the next byte every time, at most. The result was unpredictable as, with two subsequent calls, once receive the full message and the second time something like a half. I also tried expanding the time interval as much as 5 times to have the same result, just for the larger chunks of data. – NMilev Aug 03 '16 at 12:29
  • @NMilev : Yes. serial I/O is not different from socket programming: your program basically sits in a select() or poll() loop and fetches what needs to be fetched from the file descriptor(s); and stores it into a (larger) buffer, including the necessary bookkeeping. And *since it sits in a select loop* there will be no blocking reads. (and the "reverse" method should be used for writing) – wildplasser Aug 03 '16 at 12:37
  • Well, per my experience its faster and better to design the code to receive the bytes available and design your communication with a protocol that implement a length byte into header of message. So you can avoid to take care of that data instead of counts bytes received as unique information. – LPs Aug 03 '16 at 12:40
  • @wildplasser So, you are suggesting that I do something like a while loop with select and nonblocking read, store it in a buffer and exit only when select times out? And, afterwards, pick my timeout carefully? – NMilev Aug 03 '16 at 13:10
  • @LPs I need to design the code to be as general as possible so I would first like to try something like I described and, if I don't succeed, I will use a protocol like that. – NMilev Aug 03 '16 at 13:13
  • Inside the select loop you can check if you have collected enough data. (check for '\n', or just check the amount of collected characters) BTW: `#ifndef __UART_LIB__` identifiers starting with an underscore are reserved. – wildplasser Aug 03 '16 at 13:15
  • @wildplasser The problem is that I do not know the amount of data I am to receive, nor do I have a footer or a terminating symbol. Thanks for the underscore info. – NMilev Aug 03 '16 at 13:20
  • (1) The tcflush() in read_UART_() is wrong. Your application has no direct access to the UART. The read() syscall is a simple copy from the system buffer to your buffer. Discarding data from this buffer is wrong. (2) The termios configuration is improper. See [Setting Terminal Modes Properly](http://www.chemie.fu-berlin.de/chemnet/use/info/libc/libc_12.html#SEC237) and [Serial Programming Guide for POSIX Operating Systems](http://www.cmrr.umn.edu/~strupp/serial.html). If you find conflicting advice, then read the **man** page for termios. – sawdust Aug 03 '16 at 16:41
  • (3) Using select() and nonblocking reads is just adding program complexity if there's just one file descriptor and you don't need to detect timeouts. When using non-canonical mode there is no efficient method to get the read() syscall to reliably detect the message frames for you. So call read() repeatedly, and buffer and parse the input to get all the data you want. You're not going to gain any "improvement" by using non-blocking mode. See http://stackoverflow.com/questions/25996171/linux-blocking-vs-non-blocking-serial-read/26006680#26006680 – sawdust Aug 03 '16 at 16:53
  • @sawdust I will follow the advice on the links you sent and also remove tcflush() calls. I actually copied a lot of code from an online source and changed it so my needs so this remained for some reason. Haven't looked into it much. – NMilev Aug 04 '16 at 07:25
  • I actually solved the problem by introducing a timeout between smaller bursts that make up the one I want to read and using select to wait on it if necessary. Once I follow up on the flags advice, I will post this as answer here. Any advice is appreciated. I also have a write function, since that is what I will also need, but haven't posted it here to focus on the read first. Thanks, guys! – NMilev Aug 04 '16 at 07:28
  • After reading the links, my initial settings were: `options.c_cflag |= BAUD_ | NUM_BITS_ | CLOCAL | CREAD ;` `options.c_cflag &= ~(HUPCL | CSTOPB | PARENB);` `options.c_iflag &= ~(INPCK | ISTRIP | IGNBRK | BRKINT | IGNCR |` `ICRNL | INLCR | IXOFF | IXON | IXANY | IMAXBEL);` `options.c_oflag &= ~(OPOST | ONLCR | OLCUC | OCRNL | ONOCR |` `ONLRET | OFILL | FFDLY);` `options.c_lflag &= ~(ICANON | ECHO | ISIG | IEXTEN | NOFLSH |` `TOSTOP);` And I found that for no processing on the output or the input characters, it is enough to use cfmakeraw(); Also, can someone explain CLOCAL? – NMilev Aug 04 '16 at 08:41

1 Answers1

0

So, I solved my problem. I still can't flush (when the system is freshly booted up and there was a signal before I turned my program on, there is some old content in the buffer, as it seems to me). I used this assumption: The package will arrive in smaller bursts and they will be separated by TIMEOUT_BYTE_ amount of time. If that expires, I assume that the package is over. Also, I have a timeout for initial waiting for the data but I reckon that is situational.

Header file:

#ifndef UART_LIB_
#define UART_LIB_

#include <stdlib.h>     //Errors, etc
#include <unistd.h>     //Used for UART
#include <fcntl.h>      //Used for UART
#include <termios.h>    //Used for UART
#include <sys/types.h>  //These includes are for timeout
#include <sys/select.h> //Used for select(), etc

//Some values used by default, left for the user to change if needed

//Used to set up the baud rate
unsigned int BAUD_ ;

//Used to indicate number of bits in one backage 
unsigned int NUM_BITS_  ;

//Path to the UART device
char *UART_PATH_ ;

//Flag for opening the device
unsigned int OPEN_FLAG_ ;

//Timeout for answer from the other side
time_t TIMEOUT_SEC_ ;
suseconds_t TIMEOUT_USEC_ ;

//Time interval between two bursts of data inside the package
suseconds_t TIMEOUT_BYTE_ ;

int open_conf_UART_(void) ;
int read_UART_(int uart_filestream, char* dest, int max_len) ;

#endif

Source file:

#include <errno.h>
#include "uartlib.h"

unsigned int BAUD_ = B38400 ;
unsigned int NUM_BITS_ = CS8 ;
char *UART_PATH_ = "/dev/ttyAMA0" ;
unsigned int OPEN_FLAG_ = O_RDWR ;
time_t TIMEOUT_SEC_ = 2 ;
suseconds_t TIMEOUT_USEC_ = 0 ;

// This needs to be finely tuned
suseconds_t TIMEOUT_BYTE_ = 5000;


int open_conf_UART_()
{
    // Variable section
    int indicator;
    int uart_filestream ;
    struct termios options ;

    // Opening the port in a read/write mode
    uart_filestream = open(UART_PATH_, OPEN_FLAG_ | O_NOCTTY | O_NONBLOCK);
    if (uart_filestream < 0)
    {
        // Unable to open the serial port, so produce an error and halt
        return -1;
    }

    // Configuring the options for UART

    // Flushing the file stream (the input and the output area)
    indicator = tcflush(uart_filestream, TCIOFLUSH);
    if(indicator < 0)
    {   
        // Unable to flush
        close(uart_filestream);
        return -1;
    }

    // Retrieve the options and modify them. 
    indicator = tcgetattr(uart_filestream, &options);
    if(indicator < 0)
    {   
        // Unable to get the attributes
        close(uart_filestream);
        return -1;
    }

    // Setting the options
    cfmakeraw(&options);
    options.c_cflag |= BAUD_ | NUM_BITS_ | CREAD;


    // Setting the options for the file stream. 
    indicator = tcsetattr(uart_filestream, TCSANOW, &options);
    if(indicator < 0)
    {   
        // Unable to set the attributes
        close(uart_filestream);
        return -1;
    }
    return uart_filestream;
}

int read_UART_(int uart_filestream, char* dest, int max_len)
{
    // Variable section
    int indicator;
    int buffer_length;
    char *tmp_dest;
    fd_set set;
    struct timeval timeout, init_timeout;

    while(1)
    {
        // Reseting the set and inserting the uart_filestream in it
        FD_ZERO(&set);
        FD_SET(uart_filestream, &set);

        // Setting the time for initial contact
        init_timeout.tv_sec = TIMEOUT_SEC_ ;
        init_timeout.tv_usec = TIMEOUT_USEC_ ;

        // Waiting for the first contact. If this times out, we assume no contact.
        indicator = select(uart_filestream + 1, &set, NULL, NULL, &init_timeout);
        if(indicator < 0)
        {
            if(errno == EINTR)
            {
                // Try again
                continue;
            }
            return -1;
        }
        else if(indicator == 0)
        {   // Timeout has occurred
            return -2;
        }
        else
        {
            break;
        }
    }

    // This section means that there is something to be read in the file descriptor
    buffer_length = 0 ;
    tmp_dest = dest ;

    // The first select is redundant but it is easier to loop this way.
    while(buffer_length < max_len)
    {
        // select changes the timeval structure so it is reset here
        timeout.tv_sec = 0;
        timeout.tv_usec = TIMEOUT_BYTE_;

        // Reinitialize the sets for reading
        FD_ZERO(&set);
        FD_SET(uart_filestream, &set);

        // Wait for the file descriptor to be available or for timeout
        indicator = select(uart_filestream+1, &set, NULL, NULL, &timeout);

        if(indicator < 0)
        {   
            if(errno == EINTR)
            {
                // Try again
                continue;
            }

            // This indicates an error
            return -1;
        }
        else if(indicator == 0)
        {
            // This indicates a timeout; We assume that the transmission is over once first timeout is reached
            return buffer_length;
        }

        // There's been a select that didn't time out before this read
        indicator = read(uart_filestream, (void*)tmp_dest, max_len - buffer_length);
        if(indicator < 0)
        {
            if(errno == EINTR)
            {
                // If the call was interrupted, try again
                continue;
            }

            // If it was any other condition, the read is corrupt.
            return -1;
        }
        else if(indicator == 0)
        {
            // If, somehow, EOF was reached
            break;
        }

        // Change the necessary values
        buffer_length += indicator ;
        tmp_dest += indicator; 

    }
    // Both branches of the if statement above have return, so this will not be reached
    // but a warning is generated 
    return buffer_length;
}

void flush_buffer_UART_(int uart_filestream)
{
    char c;
    while(read(uart_filestream, &c, 1) > 0);
}

I know it is not the topic here, but if someone knows how to solve the flush issue, please respond. Also, any constructive criticism is very welcome.

P.S. I also have a write_UART() function but I did not deem necessary to post it as it represented no problem (measured with an oscilloscope and later, tried with echo. Echo wouldn't be able to give me the same message).

EDIT: Flush has been introduced and then merged with the source file. Still haven't figured out is it working or not.

NMilev
  • 77
  • 2
  • 11
  • `if(indicator < 0) { return -1; }` a -1 return from select() is not *necessarily* an error, it could be an interrupted system call, check errno (just like you did in read()) to choose if you want to exit or retry. – joop Aug 05 '16 at 10:22
  • `while(buffer_length < max_len)` this loop should be **outside** the select(). If select() indicates that the fd is readable, you are allowed to read() **once**. – joop Aug 05 '16 at 10:28
  • @joop I agree with the comment about the indicator, will fix that! I am not sure what you are saying about the loop. The first select returns >0 if there has been an "initial contact" on the port and, from there on, I read the bursts. I cannot do only one read. Tried, the first select returns immediately after the UART buffer is full (in my case, 8 bytes) and doesn't even try to read the rest that arrives. That is why this is in a loop. – NMilev Aug 05 '16 at 12:03
  • You should again initialise the fd set `FD_ZERO(&set); FD_SET(uart_filestream, &set);` before the second select. (this is one of the reasons people use loops around select) – joop Aug 05 '16 at 12:21
  • @joop Thanks, sort of forgotten about it. I will edit the post soon to introduce all the changes – NMilev Aug 05 '16 at 12:51
  • One of the things that stuck out to me in the code you posted in the question, was that you zero-out all other flags in the `termios` struct except for `c_cflags` – tijko Aug 05 '16 at 20:27
  • @tijko Yes, after a bit of additional reading, I realised that it is not a good approach so I kept all the flags, changing only the ones I needed to change, thus ensuring I do not turn off some necessary options. Not sure if it made any difference in my case but I suppose it could greatly affect portability/reusability of the code. – NMilev Aug 08 '16 at 11:41