From 16a024691b1f9b978620e9c6f5c861df8badd324 Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Mon, 28 Oct 2013 19:48:01 -0400
Subject: [PATCH 01/13] Add MillisecondTimer class.

---
 include/serial/impl/unix.h | 12 +++++++
 src/impl/unix.cc           | 71 ++++++++++++++++++++++----------------
 2 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/include/serial/impl/unix.h b/include/serial/impl/unix.h
index a7985da3..ffb79afb 100644
--- a/include/serial/impl/unix.h
+++ b/include/serial/impl/unix.h
@@ -53,6 +53,18 @@ using std::invalid_argument;
 using serial::SerialException;
 using serial::IOException;
 
+class TimerExpiredException : public std::exception {};
+
+class MillisecondTimer {
+public:
+  MillisecondTimer(const uint32_t millis);         
+  uint32_t remaining();
+
+private:
+  static timespec now();
+  timespec timeout_time;
+};
+
 class serial::Serial::SerialImpl {
 public:
   SerialImpl (const string &port,
diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index 321995a7..4276e9ff 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -48,12 +48,54 @@
 using std::string;
 using std::stringstream;
 using std::invalid_argument;
+using serial::MillisecondTimer;
 using serial::Serial;
+using serial::TimerExpiredException;
 using serial::SerialException;
 using serial::PortNotOpenedException;
 using serial::IOException;
 
 
+MillisecondTimer::MillisecondTimer (const uint32_t millis)
+  : timeout_time(now())
+{
+  int64_t tv_nsec = timeout_time.tv_nsec + (millis * 1e6);
+  if (tv_nsec > 1e9) {
+    timeout_time.tv_nsec = tv_nsec % (int)1e6;
+    timeout_time.tv_sec += tv_nsec / (int)1e6;
+  }
+}
+
+uint32_t
+MillisecondTimer::remaining ()
+{
+  timespec now_time(now());
+  int64_t millis = (timeout_time.tv_sec - now_time.tv_sec) * 1e3;
+  millis += (timeout_time.tv_nsec - now_time.tv_nsec) / 1e6;
+  if (millis <= 0) {
+    throw TimerExpiredException();
+  }
+  return millis;
+}
+
+timespec
+MillisecondTimer::now ()
+{
+  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
+  return time;
+}
+
 Serial::SerialImpl::SerialImpl (const string &port, unsigned long baudrate,
                                 bytesize_t bytesize,
                                 parity_t parity, stopbits_t stopbits,
@@ -404,35 +446,6 @@ 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
-}
-
-inline void
-diff_timespec (timespec &start, timespec &end, timespec &result) {
-  if (start.tv_sec > end.tv_sec) {
-    throw SerialException ("Timetravel, start time later than end time.");
-  }
-  result.tv_sec = end.tv_sec - start.tv_sec;
-  result.tv_nsec = end.tv_nsec - start.tv_nsec;
-  if (result.tv_nsec < 0) {
-    result.tv_nsec = 1e9 - result.tv_nsec;
-    result.tv_sec -= 1;
-  }
-}
-
 size_t
 Serial::SerialImpl::read (uint8_t *buf, size_t size)
 {

From 6747711632e903496a98a6c1f6a12d32b2ea2685 Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Mon, 28 Oct 2013 20:17:07 -0400
Subject: [PATCH 02/13] Implement MillisecondTimer in the unix read() and
 write() functions.

---
 src/impl/unix.cc | 116 ++++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 77 deletions(-)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index 4276e9ff..b7921ffb 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -96,6 +96,14 @@ MillisecondTimer::now ()
   return time;
 }
 
+timespec
+timespec_from_ms (const uint32_t millis) 
+{
+  timespec time;
+  time.tv_sec = millis / 1e3;
+  time.tv_nsec = (millis % (int)1e3) * 1e6;
+}
+
 Serial::SerialImpl::SerialImpl (const string &port, unsigned long baudrate,
                                 bytesize_t bytesize,
                                 parity_t parity, stopbits_t stopbits,
@@ -455,59 +463,29 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size)
   }
   fd_set readfds;
   size_t bytes_read = 0;
-  // Setup the total_timeout timeval
-  //  This timeout is maximum time before a timeout after read is called
-  struct timeval total_timeout;
+  
   // Calculate total timeout in milliseconds t_c + (t_m * N)
   long total_timeout_ms = timeout_.read_timeout_constant;
   total_timeout_ms += timeout_.read_timeout_multiplier*static_cast<long>(size);
-  total_timeout.tv_sec = total_timeout_ms / 1000;
-  total_timeout.tv_usec = static_cast<int>(total_timeout_ms % 1000);
-  total_timeout.tv_usec *= 1000; // To convert to micro seconds
-  // Setup the inter byte timeout
-  struct timeval inter_byte_timeout;
-  inter_byte_timeout.tv_sec = timeout_.inter_byte_timeout / 1000;
-  inter_byte_timeout.tv_usec =
-    static_cast<int> (timeout_.inter_byte_timeout % 1000);
-  inter_byte_timeout.tv_usec *= 1000; // To convert to micro seconds
+  MillisecondTimer total_timeout(total_timeout_ms);
+
   while (bytes_read < size) {
-    // Setup the select timeout timeval
-    struct timeval timeout;
-    // If the total_timeout is less than the inter_byte_timeout
-    if (total_timeout.tv_sec < inter_byte_timeout.tv_sec
-     || (total_timeout.tv_sec == inter_byte_timeout.tv_sec
-      && total_timeout.tv_usec < inter_byte_timeout.tv_sec))
-    {
-      // Then set the select timeout to use the total time
-      timeout = total_timeout;
-    } else {
-      // Else set the select timeout to use the inter byte time
-      timeout = inter_byte_timeout;
+    // Timeout for the next select is whichever is less of the total read
+    // timeout and the inter-byte timeout.
+    timespec timeout;
+    try {
+      timeout = timespec_from_ms(std::min(total_timeout.remaining(), 
+                                          timeout_.inter_byte_timeout));
+    } catch (TimerExpiredException) {
+      break;
     }
+
     FD_ZERO (&readfds);
     FD_SET (fd_, &readfds);
-    // Begin timing select
-    struct timespec start, end;
-    get_time_now (start);
+    
     // Call select to block for serial data or a timeout
-    int r = select (fd_ + 1, &readfds, NULL, NULL, &timeout);
-    // Calculate difference and update the structure
-    get_time_now (end);
-    // Calculate the time select took
-    struct timespec diff;
-    diff_timespec (start, end, diff);
-    // Update the timeout
-    if (total_timeout.tv_sec <= diff.tv_sec) {
-      total_timeout.tv_sec = 0;
-    } else {
-      total_timeout.tv_sec -= diff.tv_sec;
-    }
-    if (total_timeout.tv_usec <= (diff.tv_nsec / 1000)) {
-      total_timeout.tv_usec = 0;
-    } else {
-      total_timeout.tv_usec -= (diff.tv_nsec / 1000);
-    }
-
+    int r = pselect (fd_ + 1, &readfds, NULL, NULL, &timeout, NULL);
+    
     // Figure out what happened by looking at select's response 'r'
     /** Error **/
     if (r < 0) {
@@ -572,41 +550,25 @@ Serial::SerialImpl::write (const uint8_t *data, size_t length)
   }
   fd_set writefds;
   size_t bytes_written = 0;
-  struct timeval timeout;
-  timeout.tv_sec =                    timeout_.write_timeout_constant / 1000;
-  timeout.tv_usec = static_cast<int> (timeout_.write_timeout_multiplier % 1000);
-  timeout.tv_usec *= 1000; // To convert to micro seconds
-  while (bytes_written < length) {
+
+  // Calculate total timeout in milliseconds t_c + (t_m * N)
+  long total_timeout_ms = timeout_.write_timeout_constant;
+  total_timeout_ms += timeout_.write_timeout_multiplier*static_cast<long>(length);
+  MillisecondTimer total_timeout(total_timeout_ms);
+
+  while (bytes_written < length) { 
+    timespec timeout;
+    try {
+      timeout = timespec_from_ms(total_timeout.remaining());
+    } catch (TimerExpiredException) {
+      break;
+    }
+
     FD_ZERO (&writefds);
     FD_SET (fd_, &writefds);
-    // 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, NULL, &writefds, NULL, &timeout);
-#if !defined(__linux__)
-    // Calculate difference and update the structure
-    get_time_now(end);
-    // Calculate the time select took
-    struct timespec diff;
-    diff_timespec(start, end, diff);
-    // 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_nsec / 1000)) {
-      timeout.tv_usec = 0;
-    } else {
-      timeout.tv_usec -= (diff.tv_nsec / 1000);
-    }
-#endif
+    int r = pselect (fd_ + 1, NULL, &writefds, NULL, &timeout, NULL);
 
     // Figure out what happened by looking at select's response 'r'
     /** Error **/

From d8a1ef4ecfa115d11ba37184d3244ccf61532eea Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Mon, 28 Oct 2013 20:22:55 -0400
Subject: [PATCH 03/13] Rename now function to timespec_now and timeout_time to
 expiry, for greater clarity.

---
 include/serial/impl/unix.h |  4 ++--
 src/impl/unix.cc           | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/serial/impl/unix.h b/include/serial/impl/unix.h
index ffb79afb..9be0361d 100644
--- a/include/serial/impl/unix.h
+++ b/include/serial/impl/unix.h
@@ -61,8 +61,8 @@ class MillisecondTimer {
   uint32_t remaining();
 
 private:
-  static timespec now();
-  timespec timeout_time;
+  static timespec timespec_now();
+  timespec expiry;
 };
 
 class serial::Serial::SerialImpl {
diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index b7921ffb..d15d6e22 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -57,21 +57,21 @@ using serial::IOException;
 
 
 MillisecondTimer::MillisecondTimer (const uint32_t millis)
-  : timeout_time(now())
+  : expiry(timespec_now())
 {
-  int64_t tv_nsec = timeout_time.tv_nsec + (millis * 1e6);
+  int64_t tv_nsec = expiry.tv_nsec + (millis * 1e6);
   if (tv_nsec > 1e9) {
-    timeout_time.tv_nsec = tv_nsec % (int)1e6;
-    timeout_time.tv_sec += tv_nsec / (int)1e6;
+    expiry.tv_nsec = tv_nsec % (int)1e6;
+    expiry.tv_sec += tv_nsec / (int)1e6;
   }
 }
 
 uint32_t
 MillisecondTimer::remaining ()
 {
-  timespec now_time(now());
-  int64_t millis = (timeout_time.tv_sec - now_time.tv_sec) * 1e3;
-  millis += (timeout_time.tv_nsec - now_time.tv_nsec) / 1e6;
+  timespec now(timespec_now());
+  int64_t millis = (expiry.tv_sec - now.tv_sec) * 1e3;
+  millis += (expiry.tv_nsec - now.tv_nsec) / 1e6;
   if (millis <= 0) {
     throw TimerExpiredException();
   }
@@ -79,7 +79,7 @@ MillisecondTimer::remaining ()
 }
 
 timespec
-MillisecondTimer::now ()
+MillisecondTimer::timespec_now ()
 {
   timespec time;
 # ifdef __MACH__ // OS X does not have clock_gettime, use clock_get_time

From 589e7b9a3ba8c8c3f72a5c3943eb62c19fcd0275 Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Thu, 31 Oct 2013 00:22:13 -0400
Subject: [PATCH 04/13] Eliminate modulus operator.

---
 src/impl/unix.cc | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index d15d6e22..971459ef 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -61,8 +61,9 @@ MillisecondTimer::MillisecondTimer (const uint32_t millis)
 {
   int64_t tv_nsec = expiry.tv_nsec + (millis * 1e6);
   if (tv_nsec > 1e9) {
-    expiry.tv_nsec = tv_nsec % (int)1e6;
-    expiry.tv_sec += tv_nsec / (int)1e6;
+    int64_t sec_diff = tv_nsec / (int)1e6;
+    expiry.tv_nsec = tv_nsec - (int)(1e6 * sec_diff);
+    expiry.tv_sec += sec_diff;
   }
 }
 
@@ -101,7 +102,7 @@ timespec_from_ms (const uint32_t millis)
 {
   timespec time;
   time.tv_sec = millis / 1e3;
-  time.tv_nsec = (millis % (int)1e3) * 1e6;
+  time.tv_nsec = (millis - (time.tv_sec * 1e3)) * 1e6;
 }
 
 Serial::SerialImpl::SerialImpl (const string &port, unsigned long baudrate,

From 5820056aef7c82e7fb3ceae0416f8686c54d2eba Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Thu, 31 Oct 2013 00:29:39 -0400
Subject: [PATCH 05/13] Remove TimerExpiredException.

---
 include/serial/impl/unix.h |  4 +---
 src/impl/unix.cc           | 29 +++++++++++++----------------
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/include/serial/impl/unix.h b/include/serial/impl/unix.h
index 9be0361d..df73e2d5 100644
--- a/include/serial/impl/unix.h
+++ b/include/serial/impl/unix.h
@@ -53,12 +53,10 @@ using std::invalid_argument;
 using serial::SerialException;
 using serial::IOException;
 
-class TimerExpiredException : public std::exception {};
-
 class MillisecondTimer {
 public:
   MillisecondTimer(const uint32_t millis);         
-  uint32_t remaining();
+  int64_t remaining();
 
 private:
   static timespec timespec_now();
diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index 971459ef..c7cf798e 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -50,7 +50,6 @@ using std::stringstream;
 using std::invalid_argument;
 using serial::MillisecondTimer;
 using serial::Serial;
-using serial::TimerExpiredException;
 using serial::SerialException;
 using serial::PortNotOpenedException;
 using serial::IOException;
@@ -67,15 +66,12 @@ MillisecondTimer::MillisecondTimer (const uint32_t millis)
   }
 }
 
-uint32_t
+int64_t
 MillisecondTimer::remaining ()
 {
   timespec now(timespec_now());
   int64_t millis = (expiry.tv_sec - now.tv_sec) * 1e3;
   millis += (expiry.tv_nsec - now.tv_nsec) / 1e6;
-  if (millis <= 0) {
-    throw TimerExpiredException();
-  }
   return millis;
 }
 
@@ -471,16 +467,17 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size)
   MillisecondTimer total_timeout(total_timeout_ms);
 
   while (bytes_read < size) {
-    // Timeout for the next select is whichever is less of the total read
-    // timeout and the inter-byte timeout.
-    timespec timeout;
-    try {
-      timeout = timespec_from_ms(std::min(total_timeout.remaining(), 
-                                          timeout_.inter_byte_timeout));
-    } catch (TimerExpiredException) {
+    int64_t timeout_remaining_ms = total_timeout.remaining();
+    if (timeout_remaining_ms <= 0) {
+      // Timed out
       break;
     }
 
+    // Timeout for the next select is whichever is less of the remaining
+    // total read timeout and the inter-byte timeout.
+    timespec timeout(timespec_from_ms(std::min((uint32_t)timeout_remaining_ms, 
+                                               timeout_.inter_byte_timeout)));
+
     FD_ZERO (&readfds);
     FD_SET (fd_, &readfds);
     
@@ -558,12 +555,12 @@ Serial::SerialImpl::write (const uint8_t *data, size_t length)
   MillisecondTimer total_timeout(total_timeout_ms);
 
   while (bytes_written < length) { 
-    timespec timeout;
-    try {
-      timeout = timespec_from_ms(total_timeout.remaining());
-    } catch (TimerExpiredException) {
+    int64_t timeout_remaining_ms = total_timeout.remaining();
+    if (timeout_remaining_ms <= 0) {
+      // Timed out
       break;
     }
+    timespec timeout(timespec_from_ms(timeout_remaining_ms));
 
     FD_ZERO (&writefds);
     FD_SET (fd_, &writefds);

From 3f2ed36849cf63d26798865540e06e4dd9a8ef52 Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Thu, 31 Oct 2013 01:14:04 -0400
Subject: [PATCH 06/13] Of course, the timespec_from_ms function must return
 its result.

---
 src/impl/unix.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index c7cf798e..2bfb548a 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -99,6 +99,7 @@ timespec_from_ms (const uint32_t millis)
   timespec time;
   time.tv_sec = millis / 1e3;
   time.tv_nsec = (millis - (time.tv_sec * 1e3)) * 1e6;
+  return time;
 }
 
 Serial::SerialImpl::SerialImpl (const string &port, unsigned long baudrate,

From cfac5bbcc9246525d67eaa81ba0560cb4d853730 Mon Sep 17 00:00:00 2001
From: William Woodall <william@osrfoundation.org>
Date: Thu, 31 Oct 2013 14:47:10 -0700
Subject: [PATCH 07/13] use static casts rather than C-style casting

C-style casting can result in undesired reinterpret_casts
So we should avoid them, see:
http://stackoverflow.com/questions/332030/when-should-static-cast-dynamic-cast-and-reinterpret-cast-be-used
---
 src/impl/unix.cc | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index 2bfb548a..e4ddc295 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -60,8 +60,8 @@ MillisecondTimer::MillisecondTimer (const uint32_t millis)
 {
   int64_t tv_nsec = expiry.tv_nsec + (millis * 1e6);
   if (tv_nsec > 1e9) {
-    int64_t sec_diff = tv_nsec / (int)1e6;
-    expiry.tv_nsec = tv_nsec - (int)(1e6 * sec_diff);
+    int64_t sec_diff = tv_nsec / static_cast<int> (1e6);
+    expiry.tv_nsec = tv_nsec - static_cast<int> (1e6 * sec_diff);
     expiry.tv_sec += sec_diff;
   }
 }
@@ -94,7 +94,7 @@ MillisecondTimer::timespec_now ()
 }
 
 timespec
-timespec_from_ms (const uint32_t millis) 
+timespec_from_ms (const uint32_t millis)
 {
   timespec time;
   time.tv_sec = millis / 1e3;
@@ -301,7 +301,7 @@ Serial::SerialImpl::reconfigurePort ()
     // other than those specified by POSIX. The driver for the underlying serial hardware
     // ultimately determines which baud rates can be used. This ioctl sets both the input
     // and output speed.
-    speed_t new_baud = static_cast<speed_t>(baudrate_);
+    speed_t new_baud = static_cast<speed_t> (baudrate_);
     if (-1 == ioctl (fd_, IOSSIOSPEED, &new_baud, 1)) {
       THROW (IOException, errno);
     }
@@ -314,7 +314,7 @@ Serial::SerialImpl::reconfigurePort ()
     }
 
     // set custom divisor
-    ser.custom_divisor = ser.baud_base / (int) baudrate_;
+    ser.custom_divisor = ser.baud_base / static_cast<int> (baudrate_);
     // update flags
     ser.flags &= ~ASYNC_SPD_MASK;
     ser.flags |= ASYNC_SPD_CUST;
@@ -461,10 +461,10 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size)
   }
   fd_set readfds;
   size_t bytes_read = 0;
-  
+
   // Calculate total timeout in milliseconds t_c + (t_m * N)
   long total_timeout_ms = timeout_.read_timeout_constant;
-  total_timeout_ms += timeout_.read_timeout_multiplier*static_cast<long>(size);
+  total_timeout_ms += timeout_.read_timeout_multiplier * static_cast<long> (size);
   MillisecondTimer total_timeout(total_timeout_ms);
 
   while (bytes_read < size) {
@@ -476,15 +476,15 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size)
 
     // Timeout for the next select is whichever is less of the remaining
     // total read timeout and the inter-byte timeout.
-    timespec timeout(timespec_from_ms(std::min((uint32_t)timeout_remaining_ms, 
+    timespec timeout(timespec_from_ms(std::min(static_cast<uint32_t> (timeout_remaining_ms),
                                                timeout_.inter_byte_timeout)));
 
     FD_ZERO (&readfds);
     FD_SET (fd_, &readfds);
-    
+
     // Call select to block for serial data or a timeout
     int r = pselect (fd_ + 1, &readfds, NULL, NULL, &timeout, NULL);
-    
+
     // Figure out what happened by looking at select's response 'r'
     /** Error **/
     if (r < 0) {
@@ -552,10 +552,10 @@ Serial::SerialImpl::write (const uint8_t *data, size_t length)
 
   // Calculate total timeout in milliseconds t_c + (t_m * N)
   long total_timeout_ms = timeout_.write_timeout_constant;
-  total_timeout_ms += timeout_.write_timeout_multiplier*static_cast<long>(length);
+  total_timeout_ms += timeout_.write_timeout_multiplier * static_cast<long> (length);
   MillisecondTimer total_timeout(total_timeout_ms);
 
-  while (bytes_written < length) { 
+  while (bytes_written < length) {
     int64_t timeout_remaining_ms = total_timeout.remaining();
     if (timeout_remaining_ms <= 0) {
       // Timed out

From 2e5e8f940b0f3c44de7f61ed73bc9276702aa101 Mon Sep 17 00:00:00 2001
From: Mike Purvis <mike@uwmike.com>
Date: Thu, 31 Oct 2013 21:48:44 -0400
Subject: [PATCH 08/13] Divide by 1e9 to get seconds from nanoseconds, instead
 of 1e6.

---
 src/impl/unix.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index e4ddc295..65e1b4c3 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -60,8 +60,8 @@ MillisecondTimer::MillisecondTimer (const uint32_t millis)
 {
   int64_t tv_nsec = expiry.tv_nsec + (millis * 1e6);
   if (tv_nsec > 1e9) {
-    int64_t sec_diff = tv_nsec / static_cast<int> (1e6);
-    expiry.tv_nsec = tv_nsec - static_cast<int> (1e6 * sec_diff);
+    int64_t sec_diff = tv_nsec / static_cast<int> (1e9);
+    expiry.tv_nsec = tv_nsec - static_cast<int> (1e9 * sec_diff);
     expiry.tv_sec += sec_diff;
   }
 }

From fbffc18dd77401759fef8c29c2c0b5cfed1ec8df Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Fri, 8 Nov 2013 10:02:54 -0500
Subject: [PATCH 09/13] Fix for computing an expiry without rollover.

---
 src/impl/unix.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index 65e1b4c3..10095308 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -63,6 +63,8 @@ MillisecondTimer::MillisecondTimer (const uint32_t millis)
     int64_t sec_diff = tv_nsec / static_cast<int> (1e9);
     expiry.tv_nsec = tv_nsec - static_cast<int> (1e9 * sec_diff);
     expiry.tv_sec += sec_diff;
+  } else {
+    expiry.tv_nsec = tv_nsec;
   }
 }
 

From c3855adbb0fef6df47510e7ca7318c6f542633e4 Mon Sep 17 00:00:00 2001
From: Mike Purvis <mike@uwmike.com>
Date: Thu, 14 Nov 2013 22:02:07 -0500
Subject: [PATCH 10/13] Wrap nanoseconds when >= 1e9, rather than > 1e9.

---
 src/impl/unix.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index 10095308..58bc52be 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -59,7 +59,7 @@ MillisecondTimer::MillisecondTimer (const uint32_t millis)
   : expiry(timespec_now())
 {
   int64_t tv_nsec = expiry.tv_nsec + (millis * 1e6);
-  if (tv_nsec > 1e9) {
+  if (tv_nsec >= 1e9) {
     int64_t sec_diff = tv_nsec / static_cast<int> (1e9);
     expiry.tv_nsec = tv_nsec - static_cast<int> (1e9 * sec_diff);
     expiry.tv_sec += sec_diff;

From a9bf8d804da3935ad7187b855fe36934bb2c7e05 Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Mon, 18 Nov 2013 11:25:25 -0500
Subject: [PATCH 11/13] Pre-fill buffer at start of read, to avoid the select
 if unnecessary.

---
 src/impl/unix.cc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/impl/unix.cc b/src/impl/unix.cc
index 58bc52be..561db21c 100755
--- a/src/impl/unix.cc
+++ b/src/impl/unix.cc
@@ -469,6 +469,14 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size)
   total_timeout_ms += timeout_.read_timeout_multiplier * static_cast<long> (size);
   MillisecondTimer total_timeout(total_timeout_ms);
 
+  // Pre-fill buffer with available bytes 
+  {
+    ssize_t bytes_read_now = ::read (fd_, buf, size);
+    if (bytes_read_now > 0) {
+      bytes_read = bytes_read_now;
+    }
+  }
+
   while (bytes_read < size) {
     int64_t timeout_remaining_ms = total_timeout.remaining();
     if (timeout_remaining_ms <= 0) {

From 9c432f9bb142693bc83b85003583c9934b9adb5a Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Mon, 18 Nov 2013 20:50:21 -0500
Subject: [PATCH 12/13] Add unix timer tests, clarify tests are only on unix at
 present, move test builds into separate makefile.

---
 CMakeLists.txt                                |  6 +-
 tests/CMakeLists.txt                          | 10 +++
 tests/unit/unix_timer_tests.cc                | 63 +++++++++++++++++++
 .../{serial_tests.cc => unix_serial_tests.cc} |  0
 4 files changed, 74 insertions(+), 5 deletions(-)
 create mode 100644 tests/CMakeLists.txt
 create mode 100644 tests/unit/unix_timer_tests.cc
 rename tests/{serial_tests.cc => unix_serial_tests.cc} (100%)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 627e5ad7..f7f6a7ca 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -59,9 +59,5 @@ install(FILES include/serial/serial.h include/serial/v8stdint.h
 
 ## Tests
 if(CATKIN_ENABLE_TESTING)
-    catkin_add_gtest(${PROJECT_NAME}-test tests/serial_tests.cc)
-    target_link_libraries(${PROJECT_NAME}-test ${PROJECT_NAME} ${Boost_LIBRARIES})
-    if(UNIX AND NOT APPLE)
-        target_link_libraries(${PROJECT_NAME}-test util)
-    endif()
+    add_subdirectory(tests)
 endif()
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
new file mode 100644
index 00000000..38718e6c
--- /dev/null
+++ b/tests/CMakeLists.txt
@@ -0,0 +1,10 @@
+if(UNIX)
+    catkin_add_gtest(${PROJECT_NAME}-test unix_serial_tests.cc)
+    target_link_libraries(${PROJECT_NAME}-test ${PROJECT_NAME} ${Boost_LIBRARIES})
+    if(NOT APPLE)
+        target_link_libraries(${PROJECT_NAME}-test util)
+    endif()
+
+    catkin_add_gtest(${PROJECT_NAME}-test-timer unit/unix_timer_tests.cc)
+    target_link_libraries(${PROJECT_NAME}-test-timer ${PROJECT_NAME})
+endif()
diff --git a/tests/unit/unix_timer_tests.cc b/tests/unit/unix_timer_tests.cc
new file mode 100644
index 00000000..5bbd1edc
--- /dev/null
+++ b/tests/unit/unix_timer_tests.cc
@@ -0,0 +1,63 @@
+#include "gtest/gtest.h"
+#include "serial/impl/unix.h"
+
+#include <unistd.h>
+#include <stdlib.h>
+
+using serial::MillisecondTimer;
+
+namespace {
+
+/**
+ * Do 100 trials of timing gaps between 0 and 19 milliseconds.
+ * Expect accuracy within one millisecond.
+ */
+TEST(timer_tests, short_intervals) {
+  for (int trial = 0; trial < 100; trial++)
+  {
+    uint32_t ms = rand() % 20;
+    MillisecondTimer mt(ms);
+    usleep(1000 * ms);
+    int32_t r = mt.remaining(); 
+
+    // 1ms slush, for the cost of calling usleep.
+    EXPECT_NEAR(r+1, 0, 1);
+  }
+}
+
+TEST(timer_tests, overlapping_long_intervals) {
+  MillisecondTimer* timers[10];
+
+  // Experimentally determined. Corresponds to the extra time taken by the loops,
+  // the big usleep, and the test infrastructure itself.
+  const int slush_factor = 14;
+
+  // Set up the timers to each time one second, 1ms apart.
+  for (int t = 0; t < 10; t++)
+  {
+    timers[t] = new MillisecondTimer(1000);
+    usleep(1000);
+  }
+
+  // Check in on them after 500ms.
+  usleep(500000);
+  for (int t = 0; t < 10; t++)
+  {
+    EXPECT_NEAR(timers[t]->remaining(), 500 - slush_factor + t, 5);
+  }
+
+  // Check in on them again after another 500ms and free them.
+  usleep(500000);
+  for (int t = 0; t < 10; t++)
+  {
+    EXPECT_NEAR(timers[t]->remaining(), -slush_factor + t, 5);
+    delete timers[t];
+  }
+}
+
+}  // namespace
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
diff --git a/tests/serial_tests.cc b/tests/unix_serial_tests.cc
similarity index 100%
rename from tests/serial_tests.cc
rename to tests/unix_serial_tests.cc

From a633418771a9daee89f733053df3004e2bd9d29d Mon Sep 17 00:00:00 2001
From: Mike Purvis <mpurvis@clearpathrobotics.com>
Date: Mon, 18 Nov 2013 20:53:20 -0500
Subject: [PATCH 13/13] Build tests on Travis.

---
 .travis.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 628c7e04..60d45e11 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,7 +6,7 @@ install:
   - sudo sh -c 'echo "deb http://packages.ros.org/ros/ubuntu precise main" > /etc/apt/sources.list.d/ros-latest.list'
   - wget http://packages.ros.org/ros.key -O - | sudo apt-key add -
   - sudo apt-get update
-  - sudo apt-get install ros-groovy-catkin
+  - sudo apt-get install ros-groovy-catkin libboost-dev
   - source /opt/ros/groovy/setup.bash
 script:
-  - make
+  - make && make test