Read/write seem to be working on linux, need to test on OS X.

examples/serial_example.cc

@@ -1,5 +1,6 @@
 #include <string>
 #include <iostream>
+#include <cstdio>
 
 // OS Specific sleep
 #ifdef __WIN32__

include/serial/impl/unix.h

@@ -75,7 +75,7 @@ public:
   available ();
 
   size_t
-  read (char* buf, size_t size = 1);
+  read (unsigned char* buf, size_t size = 1);
 
   size_t
   write (const string &data);
@@ -174,7 +174,7 @@ private:
   string port_;               // Path to the file descriptor
   int fd_;                    // The current file descriptor
 
-  bool isOpen_;
+  bool is_open_;
   bool xonxoff_;
   bool rtscts_;
 

include/serial/serial.h

@@ -118,9 +118,6 @@ public:
    * FLOWCONTROL_NONE, possible values are: FLOWCONTROL_NONE,
    * FLOWCONTROL_SOFTWARE, FLOWCONTROL_HARDWARE
    *
-  * \param buffer_size The maximum size of the internal buffer, defaults
-  * to 256 bytes (2^8).
-  *
    * \throw PortNotOpenedException
    */
   Serial (const std::string &port = "",
@@ -175,6 +172,12 @@ public:
    *
    * \return A std::string containing the data read.
    */
+  size_t
+  read (unsigned char *buffer, size_t size);
+  size_t
+  read (std::vector<unsigned char> &buffer, size_t size = 1);
+  size_t
+  read (std::string &buffer, size_t size = 1);
   std::string
   read (size_t size = 1);
 
@@ -182,13 +185,17 @@ public:
    *
    * Reads from the serial port until a single line has been read.
    *
-  * \param size A maximum length of a line defaults to size_t::max()
+   * \param size A maximum length of a line, defaults to 65536 (2^16)
    * \param eol A string to match against for the EOL.
    *
    * \return A std::string containing the line.
    */
+  size_t
+  readline (std::string &buffer,
+            size_t size = 65536,
+            std::string eol = "\n");
   std::string
-  readline(size_t size = std::numeric_limits<std::size_t>::max(),
+  readline (size_t size = 65536,
             std::string eol = "\n");
 
   /*! Reads in multiple lines until the serail port times out.
@@ -196,22 +203,28 @@ public:
    * This requires a timeout > 0 before it can be run. It will read until a
    * timeout occurs and return a list of strings.
    *
+   * \param size A maximum length of combined lines, defaults to 65536 (2^16)
+   * 
    * \param eol A string to match against for the EOL.
    *
    * \return A vector<string> containing the lines.
    */
   std::vector<std::string>
-  readlines(std::string eol = "\n");
+  readlines (size_t size = 65536, std::string eol = "\n");
 
   /*! Write a string to the serial port.
    *
-  * \param data A const std::string reference containg the data to be written
+   * \param data A const reference containg the data to be written
    * to the serial port.
    *
    * \return A size_t representing the number of bytes actually written to
    * the serial port.
    */
   size_t
+  write (const unsigned char *data, size_t size);
+  size_t
+  write (const std::vector<unsigned char> &data);
+  size_t
   write (const std::string &data);
 
   /*! Sets the serial port identifier.
@@ -410,6 +423,10 @@ private:
   class ScopedReadLock;
   class ScopedWriteLock;
 
+  // Read common function
+  size_t
+  read_ (unsigned char *buffer, size_t size);
+
 };
 
 class SerialExecption : public std::exception

src/impl/unix.cc

@@ -10,15 +10,20 @@
 #include <sysexits.h>
 #include <termios.h>
 #include <sys/param.h>
-#include <sys/select.h>
-#include <sys/time.h>
-#include <time.h>
 #include <pthread.h>
 
 #if defined(__linux__)
 #include <linux/serial.h>
 #endif
 
+#include <sys/select.h>
+#include <sys/time.h>
+#include <time.h>
+#ifdef __MACH__
+#include <mach/clock.h>
+#include <mach/mach.h>
+#endif
+
 #include "serial/impl/unix.h"
 
 #ifndef TIOCINQ
@@ -41,7 +46,7 @@ Serial::SerialImpl::SerialImpl (const string &port, unsigned long baudrate,
                                 long timeout, bytesize_t bytesize,
                                 parity_t parity, stopbits_t stopbits,
                                 flowcontrol_t flowcontrol)
-: port_ (port), fd_ (-1), isOpen_ (false), xonxoff_ (true), rtscts_ (false),
+: port_ (port), fd_ (-1), is_open_ (false), xonxoff_ (true), rtscts_ (false),
   timeout_ (timeout), baudrate_ (baudrate), parity_ (parity),
   bytesize_ (bytesize), stopbits_ (stopbits), flowcontrol_ (flowcontrol)
 {
@@ -63,11 +68,11 @@ Serial::SerialImpl::open ()
 {
   if (port_.empty ())
   {
-    throw invalid_argument ("bad port specified");
+    throw invalid_argument ("Empty port is invalid.");
   }
-  if (isOpen_ == true)
+  if (is_open_ == true)
   {
-    throw SerialExecption ("port already open");
+    throw SerialExecption ("Serial port already open.");
   }
 
   fd_ = ::open (port_.c_str(), O_RDWR | O_NOCTTY | O_NONBLOCK);
@@ -82,7 +87,7 @@ Serial::SerialImpl::open ()
         return;
       case ENFILE:
       case EMFILE:
-        throw IOException ("to many file handles open");
+        throw IOException ("Too many file handles open.");
         break;
       default:
         throw IOException (errno);
@@ -90,7 +95,7 @@ Serial::SerialImpl::open ()
   }
 
   reconfigurePort();
-  isOpen_ = true;
+  is_open_ = true;
 }
 
 void
@@ -99,7 +104,7 @@ Serial::SerialImpl::reconfigurePort ()
   if (fd_ == -1)
   {
     // Can only operate on a valid file descriptor
-    throw IOException ("invalid file descriptor");
+    throw IOException ("Invalid file descriptor, is the serial port open?");
   }
 
   struct termios options; // The options for the file descriptor
@@ -266,7 +271,8 @@ Serial::SerialImpl::reconfigurePort ()
   if (stopbits_ == STOPBITS_ONE)
       options.c_cflag &= (unsigned long) ~(CSTOPB);
   else if (stopbits_ == STOPBITS_ONE_POINT_FIVE)
-      options.c_cflag |=  (CSTOPB);  // XXX same as TWO.. there is no POSIX support for 1.5
+  // ONE POINT FIVE same as TWO.. there is no POSIX support for 1.5
+      options.c_cflag |=  (CSTOPB);
   else if (stopbits_ == STOPBITS_TWO)
       options.c_cflag |=  (CSTOPB);
   else
@@ -318,8 +324,12 @@ Serial::SerialImpl::reconfigurePort ()
 #error "OS Support seems wrong."
 #endif
 
-  options.c_cc[VMIN] = 1; // Minimum of 1 character in the buffer
-  options.c_cc[VTIME] = 0; // timeout on waiting for new data
+  // http://www.unixwiz.net/techtips/termios-vmin-vtime.html
+  // this basically sets the read call up to be a polling read, 
+  // but we are using select to ensure there is data available 
+  // to read before each call, so we should never needlessly poll
+  options.c_cc[VMIN] = 0;
+  options.c_cc[VTIME] = 0;
 
   // activate settings
   ::tcsetattr (fd_, TCSANOW, &options);
@@ -328,27 +338,27 @@ Serial::SerialImpl::reconfigurePort ()
 void
 Serial::SerialImpl::close ()
 {
-  if (isOpen_ == true)
+  if (is_open_ == true)
   {
     if (fd_ != -1)
     {
       ::close (fd_); // Ignoring the outcome
       fd_ = -1;
     }
-    isOpen_ = false;
+    is_open_ = false;
   }
 }
 
 bool
 Serial::SerialImpl::isOpen () const
 {
-  return isOpen_;
+  return is_open_;
 }
 
 size_t
 Serial::SerialImpl::available ()
 {
-  if (!isOpen_)
+  if (!is_open_)
   {
     return 0;
   }
@@ -364,44 +374,90 @@ Serial::SerialImpl::available ()
   }
 }
 
+inline void get_time_now(struct timespec &time) {
+# ifdef __MACH__ // OS X does not have clock_gettime, use clock_get_time
+  clock_serv_t cclock;
+  mach_timespec_t mts;
+  host_get_clock_service(mach_host_self(), CALENDAR_CLOCK, &cclock);
+  clock_get_time(cclock, &mts);
+  mach_port_deallocate(mach_task_self(), cclock);
+  time.tv_sec = mts.tv_sec;
+  time.tv_nsec = mts.tv_nsec;
+# else
+  clock_gettime(CLOCK_REALTIME, &time);
+# endif
+}
+
 size_t
-Serial::SerialImpl::read (char* buf, size_t size)
+Serial::SerialImpl::read (unsigned char* buf, size_t size)
 {
-  if (!isOpen_)
+  if (!is_open_)
   {
     throw PortNotOpenedException ("Serial::read");
   }
   fd_set readfds;
-  ssize_t bytes_read = 0;
-  int count = 0;
-  while (true)
-  {
-    count++;
-    // printf("Counting: %u\n", count);
-    if (timeout_ != -1)
-    {
-      FD_ZERO (&readfds);
-      FD_SET (fd_, &readfds);
+  size_t bytes_read = 0;
   struct timeval timeout;
   timeout.tv_sec =                    timeout_ / 1000;
   timeout.tv_usec = static_cast<int> (timeout_ % 1000) * 1000;
-      int r = select (fd_ + 1, &readfds, NULL, NULL, &timeout);
-
-      if (r == -1 && errno == EINTR)
-        continue;
-
-      if (r == -1)
+  while (bytes_read < size)
   {
+    FD_ZERO (&readfds);
+    FD_SET (fd_, &readfds);
+    // On Linux the timeout struct is updated by select to contain the time 
+    // left on the timeout to make looping easier, but on other platforms this 
+    // does not occur.
+#if !defined(__linux__)
+    // Begin timing select
+    struct timespec start, end;
+    get_time_now(start);
+#endif
+    // Do the select
+    int r = select (fd_ + 1, &readfds, NULL, NULL, &timeout);
+#if !defined(__linux__)
+    // Calculate difference and update the structure
+    get_time_now(end);
+    // Calculate the time select took
+    struct timeval diff;
+    diff.tv_sec = end.tv_sec-start.tv_sec;
+    diff.tv_usec = (end.tv_nsec-start.tv_nsec)/1000;
+    // Update the timeout
+    if (timeout.tv_sec <= diff.tv_sec) {
+      timeout.tv_sec = 0;
+    } else {
+      timeout.tv_sec -= diff.tv_sec;
+    }
+    if (timeout.tv_usec <= diff.tv_usec) {
+      timeout.tv_usec = 0;
+    } else {
+      timeout.tv_usec -= diff.tv_usec;
+    }
+#endif
+
+    // Figure out what happened by looking at select's response 'r'
+/** Error **/
+    if (r < 0) {
+      // Select was interrupted, try again
+      if (errno == EINTR) {
+        continue;
+      }
+      // Otherwise there was some error
       throw IOException (errno);
     }
+/** Timeout **/
+    if (r == 0) {
+      break;
     }
-
-    if (timeout_ == -1 || FD_ISSET (fd_, &readfds))
-    {
-      bytes_read = ::read (fd_, buf, size);
+/** Something ready to read **/
+    if (r > 0) {
+      // Make sure our file descriptor is in the ready to read list
+      if (FD_ISSET (fd_, &readfds)) {
+        // This should be non-blocking returning only what is avaialble now
+        //  Then returning so that select can block again.
+        ssize_t bytes_read_now = ::read (fd_, buf, size-bytes_read);
         // read should always return some data as select reported it was
         // ready to read when we get to this point.
-      if (bytes_read < 1)
+        if (bytes_read_now < 1)
         {
           // Disconnected devices, at least on Linux, show the
           // behavior that they are always ready to read immediately
@@ -409,68 +465,40 @@ Serial::SerialImpl::read (char* buf, size_t size)
           throw SerialExecption ("device reports readiness to read but "
                                  "returned no data (device disconnected?)");
         }
+        // Update bytes_read
+        bytes_read += static_cast<size_t> (bytes_read_now);
+        // If bytes_read == size then we have read everything we need
+        if (bytes_read == size) {
           break;
         }
-    else
-    {
-      break;
+        // If bytes_read < size then we have more to read
+        if (bytes_read < size) {
+          continue;
+        }
+        // If bytes_read > size then we have over read, which shouldn't happen
+        if (bytes_read > size) {
+          throw SerialExecption ("read over read, too many bytes where "
+                                 "read, this shouldn't happen, might be "
+                                 "a logical error!");
         }
       }
-  return static_cast<size_t> (bytes_read);
+      // This shouldn't happen, if r > 0 our fd has to be in the list!
+      throw IOException ("select reports ready to read, but our fd isn't"
+                         " in the list, this shouldn't happen!");
+    }
+  }
+  return bytes_read;
 }
 
 size_t
 Serial::SerialImpl::write (const string &data)
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::write");
   }
 
-  fd_set writefds;
-  ssize_t bytes_written = 0;
-  while (true)
-  {
-    if (timeout_ != -1)
-    {
-      FD_ZERO (&writefds);
-      FD_SET (fd_, &writefds);
-      struct timeval timeout;
-      timeout.tv_sec =                    timeout_ / 1000;
-      timeout.tv_usec = static_cast<int> (timeout_ % 1000) * 1000;
-      int r = select (fd_ + 1, NULL, &writefds, NULL, &timeout);
-
-      if (r == -1 && errno == EINTR)
-        continue;
-
-      if (r == -1)
-      {
-        throw IOException (errno);
-      }
-    }
-
-    if (timeout_ == -1 || FD_ISSET (fd_, &writefds))
-    {
-      bytes_written = ::write (fd_, data.c_str (), data.length ());
-      // read should always return some data as select reported it was
-      // ready to read when we get to this point.
-      if (bytes_written < 1)
-      {
-        // Disconnected devices, at least on Linux, show the
-        // behavior that they are always ready to read immediately
-        // but reading returns nothing.
-        throw SerialExecption ("device reports readiness to read but "
-                               "returned no data (device disconnected?)");
-      }
-      break;
-    }
-    else
-    {
-      break;
-    }
-  }
-
-  return static_cast<size_t> (bytes_written);
+  return static_cast<size_t> (::write (fd_, data.c_str (), data.length ()));
 }
 
 void
@@ -501,7 +529,7 @@ void
 Serial::SerialImpl::setBaudrate (unsigned long baudrate)
 {
   baudrate_ = baudrate;
-  if (isOpen_)
+  if (is_open_)
     reconfigurePort ();
 }
 
@@ -515,7 +543,7 @@ void
 Serial::SerialImpl::setBytesize (serial::bytesize_t bytesize)
 {
   bytesize_ = bytesize;
-  if (isOpen_)
+  if (is_open_)
     reconfigurePort ();
 }
 
@@ -529,7 +557,7 @@ void
 Serial::SerialImpl::setParity (serial::parity_t parity)
 {
   parity_ = parity;
-  if (isOpen_)
+  if (is_open_)
     reconfigurePort ();
 }
 
@@ -543,7 +571,7 @@ void
 Serial::SerialImpl::setStopbits (serial::stopbits_t stopbits)
 {
   stopbits_ = stopbits;
-  if (isOpen_)
+  if (is_open_)
     reconfigurePort ();
 }
 
@@ -557,7 +585,7 @@ void
 Serial::SerialImpl::setFlowcontrol (serial::flowcontrol_t flowcontrol)
 {
   flowcontrol_ = flowcontrol;
-  if (isOpen_)
+  if (is_open_)
     reconfigurePort ();
 }
 
@@ -570,7 +598,7 @@ Serial::SerialImpl::getFlowcontrol () const
 void
 Serial::SerialImpl::flush ()
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::flush");
   }
@@ -580,7 +608,7 @@ Serial::SerialImpl::flush ()
 void
 Serial::SerialImpl::flushInput ()
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::flushInput");
   }
@@ -590,7 +618,7 @@ Serial::SerialImpl::flushInput ()
 void
 Serial::SerialImpl::flushOutput ()
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::flushOutput");
   }
@@ -600,7 +628,7 @@ Serial::SerialImpl::flushOutput ()
 void
 Serial::SerialImpl::sendBreak (int duration)
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::sendBreak");
   }
@@ -610,7 +638,7 @@ Serial::SerialImpl::sendBreak (int duration)
 void
 Serial::SerialImpl::setBreak (bool level)
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::setBreak");
   }
@@ -626,7 +654,7 @@ Serial::SerialImpl::setBreak (bool level)
 void
 Serial::SerialImpl::setRTS (bool level)
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::setRTS");
   }
@@ -642,7 +670,7 @@ Serial::SerialImpl::setRTS (bool level)
 void
 Serial::SerialImpl::setDTR (bool level)
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::setDTR");
   }
@@ -659,7 +687,7 @@ Serial::SerialImpl::setDTR (bool level)
 bool
 Serial::SerialImpl::getCTS ()
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::getCTS");
   }
@@ -670,7 +698,7 @@ Serial::SerialImpl::getCTS ()
 bool
 Serial::SerialImpl::getDSR()
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::getDSR");
   }
@@ -681,7 +709,7 @@ Serial::SerialImpl::getDSR()
 bool
 Serial::SerialImpl::getRI()
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::getRI");
   }
@@ -692,7 +720,7 @@ Serial::SerialImpl::getRI()
 bool
 Serial::SerialImpl::getCD()
 {
-  if (isOpen_ == false)
+  if (is_open_ == false)
   {
     throw PortNotOpenedException ("Serial::getCD");
   }

src/serial.cc

@@ -9,7 +9,6 @@
 #endif
 
 using std::invalid_argument;
-using std::memset;
 using std::min;
 using std::numeric_limits;
 using std::vector;
@@ -86,98 +85,113 @@ Serial::available ()
   return pimpl_->available ();
 }
 
+size_t
+Serial::read_ (unsigned char *buffer, size_t size)
+{
+  return this->pimpl_->read (buffer, size);
+}
+
+size_t
+Serial::read (unsigned char *buffer, size_t size)
+{
+  ScopedReadLock (this->pimpl_);
+  return this->pimpl_->read (buffer, size);
+}
+
+size_t
+Serial::read (std::vector<unsigned char> &buffer, size_t size)
+{
+  ScopedReadLock (this->pimpl_);
+  unsigned char *buffer_ = new unsigned char[size];
+  size_t bytes_read = this->pimpl_->read (buffer_, size);
+  buffer.insert (buffer.end (), buffer_, buffer_+bytes_read);
+  delete[] buffer_;
+  return bytes_read;
+}
+
+size_t
+Serial::read (std::string &buffer, size_t size)
+{
+  ScopedReadLock (this->pimpl_);
+  unsigned char *buffer_ = new unsigned char[size];
+  size_t bytes_read = this->pimpl_->read (buffer_, size);
+  buffer.append (reinterpret_cast<const char*>(buffer_), bytes_read);
+  delete[] buffer_;
+  return bytes_read;
+}
+
 string
 Serial::read (size_t size)
 {
-  ScopedReadLock(this->pimpl_);
-  if (read_cache_.size() >= size)
-  {
-    // Don't need to do a new read.
-    string result = read_cache_.substr (0, size);
-    read_cache_ = read_cache_.substr (size, read_cache_.size ());
-    return result;
+  std::string buffer;
+  this->read (buffer, size);
+  return buffer;
 }
-  else
-  {
-    // Needs to read, loop until we have read enough or timeout
-    string result (read_cache_.substr (0, size));
-    read_cache_.clear ();
 
+size_t
+Serial::readline (string &buffer, size_t size, string eol)
+{
+  ScopedReadLock (this->pimpl_);
+  size_t eol_len = eol.length();
+  unsigned char buffer_[size];
+  size_t read_so_far = 0;
   while (true)
   {
-      char buf[256];
-      size_t chars_read = pimpl_->read (buf, 256);
-      if (chars_read > 0)
-      {
-        read_cache_.append(buf, chars_read);
+    size_t bytes_read = this->read_ (buffer_+read_so_far, 1);
+    read_so_far += bytes_read;
+    if (bytes_read == 0) {
+      break; // Timeout occured on reading 1 byte
     }
-      else
-        break; // Timeout occured
-      
-      if (chars_read > size)
-      {
-        result.append (read_cache_.substr (0, size));
-        read_cache_ = read_cache_.substr (size, read_cache_.size ());
-        break;
+    if (string(buffer_[read_so_far-eol_len], eol_len) == eol) {
+      break; // EOL found
     }
-      else
-      {
-        result.append (read_cache_.substr (0, size));
-        read_cache_.clear ();
-        size -= chars_read;
+    if (read_so_far == size) {
+      break; // Reached the maximum read length
     }
   }
-    return result;
-  }
+  return read_so_far;
 }
 
 string
 Serial::readline (size_t size, string eol)
 {
-  size_t leneol = eol.length ();
-  string line = "";
-  while (true)
-  {
-    string c = read (1);
-    if (!c.empty ())
-    {
-      line.append (c);
-      if (line.length () > leneol &&
-          line.substr (line.length () - leneol, leneol) == eol)
-        break;
-      if (line.length () >= size)
-      {
-        break;
-      }
-    }
-    else
-      // Timeout
-      break;
-  }
-  return line;
+  std::string buffer;
+  this->readline (buffer, size, eol);
+  return buffer;
 }
 
 vector<string>
-Serial::readlines(string eol)
+Serial::readlines (size_t size, string eol)
 {
-  if (pimpl_->getTimeout () < 0)
-  {
-    throw invalid_argument ("Error, must be set for readlines");
+  ScopedReadLock (this->pimpl_);
+  std::vector<std::string> lines;
+  size_t eol_len = eol.length();
+  unsigned char buffer_[size];
+  size_t read_so_far = 0;
+  size_t start_of_line = 0;
+  while (read_so_far < size) {
+    size_t bytes_read = this->read_ (buffer_+read_so_far, 1);
+    read_so_far += bytes_read;
+    if (bytes_read == 0) {
+      if (start_of_line != read_so_far) {
+        lines.push_back(
+          std::string(buffer_[start_of_line], read_so_far-start_of_line));
       }
-  size_t leneol = eol.length ();
-  vector<string> lines;
-  while (true)
-  {
-    string line = readline (numeric_limits<size_t>::max (), eol);
-    if (!line.empty ())
-    {
-      lines.push_back (line);
-      if (line.substr (line.length () - leneol, leneol) == eol)
-        break;
+      break; // Timeout occured on reading 1 byte
+    }
+    if (string(buffer_[read_so_far-eol_len], eol_len) == eol) {
+      // EOL found
+      lines.push_back(
+        std::string(buffer_[start_of_line], read_so_far-start_of_line));
+      start_of_line = read_so_far;
+    }
+    if (read_so_far == size) {
+      if (start_of_line != read_so_far) {
+        lines.push_back(
+          std::string(buffer_[start_of_line], read_so_far-start_of_line));
+      }
+      break; // Reached the maximum read length
     }
-    else
-      // Timeout
-      break;
   }
   return lines;
 }