From 10f2fc60fb47795879656d70b693aedd71c59a28 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Sun, 6 Oct 2013 08:54:48 -0400 Subject: [PATCH 01/17] Beginnings of some operator functions for timespec structs --- src/impl/unix.cc | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index 6932ff7..1b5143c 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -425,6 +425,45 @@ diff_timespec (timespec &start, timespec &end, timespec &result) { } } +/*! Simple function to normalize the tv_nsec field to [0..1e9), carrying + * the remainder into the tv_sec field. */ +void normalize(timespec* ts) { + while (ts->tv_nsec < 0) { + ts->tv_nsec += 1e9; + ts->tv_sec -= 1; + } + while (ts->tv_nsec >= 1e9) { + ts->tv_nsec -= 1e9; + ts->tv_sec += 1; + } +} + +inline struct timespec +operator+ (const struct timespec &a, const struct timespec &b) { + struct timespec result = { a.tv_sec + b.tv_sec, + a.tv_nsec + b.tv_nsec }; + normalize(&result); + return result; +} + +inline struct timespec +operator- (const struct timespec &a, const struct timespec &b) { + struct timespec result = { a.tv_sec - b.tv_sec, + a.tv_nsec - b.tv_nsec }; + normalize(&result); + return result; +} + +inline struct timespec +min (const struct timespec &a, const struct timespec &b) { + if (a.tv_sec < b.tv_sec + || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) { + return a; + } else { + return b; + } +} + size_t Serial::SerialImpl::read (uint8_t *buf, size_t size) { @@ -473,8 +512,8 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) // Calculate difference and update the structure get_time_now (end); // Calculate the time select took - struct timespec diff; - diff_timespec (start, end, diff); + struct timespec diff(end - start); + // Update the timeout if (total_timeout.tv_sec <= diff.tv_sec) { total_timeout.tv_sec = 0; From 7a61d4545b6c5d5c622e7ee1d413133b97f889e5 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Sun, 6 Oct 2013 10:35:00 -0400 Subject: [PATCH 02/17] Create key operator/manipulation functions for timespecs, add them to new unix-timespec.h helper header. --- include/serial/impl/unix-timespec.h | 122 ++++++++++++++++++++++++++++ include/serial/impl/unix.h | 10 ++- src/impl/unix.cc | 72 +--------------- 3 files changed, 132 insertions(+), 72 deletions(-) create mode 100644 include/serial/impl/unix-timespec.h diff --git a/include/serial/impl/unix-timespec.h b/include/serial/impl/unix-timespec.h new file mode 100644 index 0000000..79c1d8c --- /dev/null +++ b/include/serial/impl/unix-timespec.h @@ -0,0 +1,122 @@ +/*! + * \file serial/impl/unit-timespec.h + * \author Mike Purvis + * \version 0.1 + * + * \section LICENSE + * + * The MIT License + * + * Copyright (c) 2013 William Woodall + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * \section DESCRIPTION + * + * Provides helper and operator functions for concisely and reliably handling + * timespec instances on unix platforms. + */ + +#ifndef SERIAL_IMPL_UNIX_TIMESPEC_H +#define SERIAL_IMPL_UNIX_TIMESPEC_H + +/*! Smooth over platform variances in getting an accurate timespec + * representing the present moment. */ +static 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 +} + +/*! Simple function to normalize the tv_nsec field to [0..1e9), carrying + * the remainder into the tv_sec field. This will not protect against the + * possibility of an overflow in the nsec field--proceed with caution. */ +static inline void +normalize(struct timespec* ts) { + while (ts->tv_nsec < 0) { + ts->tv_nsec += 1e9; + ts->tv_sec -= 1; + } + while (ts->tv_nsec >= 1e9) { + ts->tv_nsec -= 1e9; + ts->tv_sec += 1; + } +} + +/*! Return a timespec which is the sum of two other timespecs. */ +static inline struct timespec +operator+ (const struct timespec &a, const struct timespec &b) { + struct timespec result = { a.tv_sec + b.tv_sec, + a.tv_nsec + b.tv_nsec }; + normalize(&result); + return result; +} + +/*! Return a timespec which is the difference of two other timespecs. */ +static inline struct timespec +operator- (const struct timespec &a, const struct timespec &b) { + struct timespec result = { a.tv_sec - b.tv_sec, + a.tv_nsec - b.tv_nsec }; + normalize(&result); + return result; +} + +/*! Return a timespec which is a multiplication of a timespec and a positive + * integer. No overflow protection-- not suitable for multiplications with + * large carries, eg a <1s timespec multiplied by a large enough integer + * that the result is muliple seconds. */ +static inline struct timespec +operator* (const struct timespec &ts, const size_t mul) { + struct timespec result = { ts.tv_sec * mul, + ts.tv_nsec * mul }; + normalize(&result); + return result; +} + +/*! Return whichever of two timespecs represents the shortest or most + * negative duration. */ +static inline struct timespec +min (const struct timespec &a, const struct timespec &b) { + if (a.tv_sec < b.tv_sec + || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) { + return a; + } else { + return b; + } +} + +/*! Return a timespec set from a provided number of milliseconds. */ +static struct timespec +timespec_from_millis(const size_t millis) { + struct timespec result = { 0, millis * 1000000 }; + normalize(&result); + return result; +} + +#endif diff --git a/include/serial/impl/unix.h b/include/serial/impl/unix.h index 3e45ec4..c3add0b 100644 --- a/include/serial/impl/unix.h +++ b/include/serial/impl/unix.h @@ -122,7 +122,7 @@ public: getPort () const; void - setTimeout (Timeout &timeout); + setTimeout (const Timeout &timeout); Timeout getTimeout () const; @@ -180,7 +180,13 @@ private: bool xonxoff_; bool rtscts_; - Timeout timeout_; // Timeout for read operations + Timeout timeout_; // Timeouts for read/write operations + struct timespec inter_byte_timeout_; + struct timespec read_timeout_constant_; + struct timespec read_timeout_multiplier_; + struct timespec write_timeout_constant_; + struct timespec write_timeout_multiplier_; + unsigned long baudrate_; // Baudrate parity_t parity_; // Parity diff --git a/src/impl/unix.cc b/src/impl/unix.cc index 1b5143c..115f94f 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -27,6 +27,7 @@ #endif #include "serial/impl/unix.h" +#include "serial/impl/unix-timespec.h" #ifndef TIOCINQ #ifdef FIONREAD @@ -396,74 +397,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; - } -} - -/*! Simple function to normalize the tv_nsec field to [0..1e9), carrying - * the remainder into the tv_sec field. */ -void normalize(timespec* ts) { - while (ts->tv_nsec < 0) { - ts->tv_nsec += 1e9; - ts->tv_sec -= 1; - } - while (ts->tv_nsec >= 1e9) { - ts->tv_nsec -= 1e9; - ts->tv_sec += 1; - } -} - -inline struct timespec -operator+ (const struct timespec &a, const struct timespec &b) { - struct timespec result = { a.tv_sec + b.tv_sec, - a.tv_nsec + b.tv_nsec }; - normalize(&result); - return result; -} - -inline struct timespec -operator- (const struct timespec &a, const struct timespec &b) { - struct timespec result = { a.tv_sec - b.tv_sec, - a.tv_nsec - b.tv_nsec }; - normalize(&result); - return result; -} - -inline struct timespec -min (const struct timespec &a, const struct timespec &b) { - if (a.tv_sec < b.tv_sec - || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) { - return a; - } else { - return b; - } -} - size_t Serial::SerialImpl::read (uint8_t *buf, size_t size) { @@ -611,8 +544,7 @@ Serial::SerialImpl::write (const uint8_t *data, size_t length) // Calculate difference and update the structure get_time_now(end); // Calculate the time select took - struct timespec diff; - diff_timespec(start, end, diff); + struct timespec diff(end - start); // Update the timeout if (timeout.tv_sec <= diff.tv_sec) { timeout.tv_sec = 0; From 4b23d520783f3e96f5bef4ea6820878b01b36024 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Sun, 6 Oct 2013 10:49:25 -0400 Subject: [PATCH 03/17] Switch get_time_now to return timespec, rather than operate on reference. --- include/serial/impl/unix-timespec.h | 12 +++++++----- src/impl/unix.cc | 10 ++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/serial/impl/unix-timespec.h b/include/serial/impl/unix-timespec.h index 79c1d8c..dc642f6 100644 --- a/include/serial/impl/unix-timespec.h +++ b/include/serial/impl/unix-timespec.h @@ -38,20 +38,22 @@ /*! Smooth over platform variances in getting an accurate timespec * representing the present moment. */ -static inline void -get_time_now (struct timespec &time) +static inline struct timespec +get_time_now () { + struct timespec ts; # 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; + ts.tv_sec = mts.tv_sec; + ts.tv_nsec = mts.tv_nsec; # else - clock_gettime(CLOCK_REALTIME, &time); + clock_gettime(CLOCK_REALTIME, &ts); # endif + return ts; } /*! Simple function to normalize the tv_nsec field to [0..1e9), carrying diff --git a/src/impl/unix.cc b/src/impl/unix.cc index 115f94f..9a502fb 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -438,12 +438,11 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) FD_ZERO (&readfds); FD_SET (fd_, &readfds); // Begin timing select - struct timespec start, end; - get_time_now (start); + struct timespec start(get_time_now()); // 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); + struct timespec end(get_time_now()); // Calculate the time select took struct timespec diff(end - start); @@ -535,14 +534,13 @@ Serial::SerialImpl::write (const uint8_t *data, size_t length) // does not occur. #if !defined(__linux__) // Begin timing select - struct timespec start, end; - get_time_now(start); + struct timespec start(get_time_now()); #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); + struct timespec end(get_time_now()); // Calculate the time select took struct timespec diff(end - start); // Update the timeout From b01b47b30186ab33881bd661eab98cd76b990368 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Sun, 6 Oct 2013 11:05:06 -0400 Subject: [PATCH 04/17] Add a bunch of clarifying comments to those timespec functions which are intended for duration timespecs. --- include/serial/impl/unix-timespec.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/serial/impl/unix-timespec.h b/include/serial/impl/unix-timespec.h index dc642f6..c9b1295 100644 --- a/include/serial/impl/unix-timespec.h +++ b/include/serial/impl/unix-timespec.h @@ -71,7 +71,9 @@ normalize(struct timespec* ts) { } } -/*! Return a timespec which is the sum of two other timespecs. */ +/*! Return a timespec which is the sum of two other timespecs. This + * operator only makes logical sense when one or both of the arguments + * represents a duration. */ static inline struct timespec operator+ (const struct timespec &a, const struct timespec &b) { struct timespec result = { a.tv_sec + b.tv_sec, @@ -80,7 +82,9 @@ operator+ (const struct timespec &a, const struct timespec &b) { return result; } -/*! Return a timespec which is the difference of two other timespecs. */ +/*! Return a timespec which is the difference of two other timespecs. + * This operator only makes logical sense when one or both of the arguments + * represents a duration. */ static inline struct timespec operator- (const struct timespec &a, const struct timespec &b) { struct timespec result = { a.tv_sec - b.tv_sec, @@ -92,7 +96,8 @@ operator- (const struct timespec &a, const struct timespec &b) { /*! Return a timespec which is a multiplication of a timespec and a positive * integer. No overflow protection-- not suitable for multiplications with * large carries, eg a <1s timespec multiplied by a large enough integer - * that the result is muliple seconds. */ + * that the result is muliple seconds. Only makes sense when the timespec + * argument is a duration. */ static inline struct timespec operator* (const struct timespec &ts, const size_t mul) { struct timespec result = { ts.tv_sec * mul, @@ -101,8 +106,8 @@ operator* (const struct timespec &ts, const size_t mul) { return result; } -/*! Return whichever of two timespecs represents the shortest or most - * negative duration. */ +/*! Return whichever of two timespec durations represents the shortest or most + * negative period. */ static inline struct timespec min (const struct timespec &a, const struct timespec &b) { if (a.tv_sec < b.tv_sec @@ -113,7 +118,7 @@ min (const struct timespec &a, const struct timespec &b) { } } -/*! Return a timespec set from a provided number of milliseconds. */ +/*! Return a timespec duration set from a provided number of milliseconds. */ static struct timespec timespec_from_millis(const size_t millis) { struct timespec result = { 0, millis * 1000000 }; From 452dd3e67867ab1f97ab8daf5f17bb1b94ea0e06 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Sun, 6 Oct 2013 11:16:55 -0400 Subject: [PATCH 05/17] Add a bunch of clarifying comments to those timespec functions which are intended for duration timespecs. --- include/serial/impl/unix-timespec.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/serial/impl/unix-timespec.h b/include/serial/impl/unix-timespec.h index c9b1295..b1b33a1 100644 --- a/include/serial/impl/unix-timespec.h +++ b/include/serial/impl/unix-timespec.h @@ -39,7 +39,7 @@ /*! Smooth over platform variances in getting an accurate timespec * representing the present moment. */ static inline struct timespec -get_time_now () +timespec_now () { struct timespec ts; # ifdef __MACH__ // OS X does not have clock_gettime, use clock_get_time From 88b628ba9855df3dbf5afb2460fc2c59700cbd63 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Sun, 6 Oct 2013 11:19:47 -0400 Subject: [PATCH 06/17] Switch read and write functions to use cached timespec timeouts, new operator functions, and pselect in place of select. --- include/serial/impl/unix.h | 2 +- src/impl/unix.cc | 129 +++++++++++++------------------------ 2 files changed, 44 insertions(+), 87 deletions(-) diff --git a/include/serial/impl/unix.h b/include/serial/impl/unix.h index c3add0b..45be374 100644 --- a/include/serial/impl/unix.h +++ b/include/serial/impl/unix.h @@ -31,7 +31,7 @@ * \section DESCRIPTION * * This provides a unix based pimpl for the Serial class. This implementation is - * based off termios.h and uses select for multiplexing the IO ports. + * based off termios.h and uses pselect for multiplexing the IO ports. * */ diff --git a/src/impl/unix.cc b/src/impl/unix.cc index 9a502fb..b4c04c8 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -60,6 +60,8 @@ Serial::SerialImpl::SerialImpl (const string &port, unsigned long baudrate, { pthread_mutex_init(&this->read_mutex, NULL); pthread_mutex_init(&this->write_mutex, NULL); + serial::Timeout zero_timeout; + setTimeout(zero_timeout); if (port_.empty () == false) open (); } @@ -404,61 +406,27 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) if (!is_open_) { throw PortNotOpenedException ("Serial::read"); } - 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(size); - total_timeout.tv_sec = total_timeout_ms / 1000; - total_timeout.tv_usec = static_cast(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 (timeout_.inter_byte_timeout % 1000); - inter_byte_timeout.tv_usec *= 1000; // To convert to micro seconds + + // Add the total timeout time to the current time, and mark that as the + // overall expiry point for the function. + struct timespec timeout_endtime(timespec_now() + + read_timeout_constant_ + (read_timeout_multiplier_ * size)); + + size_t bytes_read = 0; 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; - } + // Must determine whether the time remaining before endtime (total read + // timeout) or the inter-byte timeout is sooner, and use that one as the + // timeout for the pselect call. + struct timespec timeout_remaining(timeout_endtime - timespec_now()); + struct timespec timeout(min(timeout_remaining, inter_byte_timeout_)); + + // Call pselect to block for serial data or a timeout + fd_set readfds; FD_ZERO (&readfds); FD_SET (fd_, &readfds); - // Begin timing select - struct timespec start(get_time_now()); - // 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 - struct timespec end(get_time_now()); - // Calculate the time select took - struct timespec diff(end - start); + int r = pselect (fd_ + 1, &readfds, NULL, NULL, &timeout, NULL); - // 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); - } - - // Figure out what happened by looking at select's response 'r' + // Figure out what happened by looking at pselect's response 'r' /** Error **/ if (r < 0) { // Select was interrupted, try again @@ -477,10 +445,10 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) // 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 available now - // Then returning so that select can block again. + // Then returning so that pselect can block again. ssize_t bytes_read_now = ::read (fd_, buf + bytes_read, size - bytes_read); - // read should always return some data as select reported it was + // read should always return some data as pselect reported it was // ready to read when we get to this point. if (bytes_read_now < 1) { // Disconnected devices, at least on Linux, show the @@ -507,7 +475,7 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) } } // 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" + THROW (IOException, "pselect reports ready to read, but our fd isn't" " in the list, this shouldn't happen!"); } } @@ -520,41 +488,22 @@ Serial::SerialImpl::write (const uint8_t *data, size_t length) if (is_open_ == false) { throw PortNotOpenedException ("Serial::write"); } - fd_set writefds; + + // Add the total timeout time to the current time, and mark that as the + // overall expiry point for the function. + struct timespec timeout_endtime(timespec_now() + + write_timeout_constant_ + (write_timeout_multiplier_ * length)); + size_t bytes_written = 0; - struct timeval timeout; - timeout.tv_sec = timeout_.write_timeout_constant / 1000; - timeout.tv_usec = static_cast (timeout_.write_timeout_multiplier % 1000); - timeout.tv_usec *= 1000; // To convert to micro seconds while (bytes_written < length) { + // Determine time remaining before the predetermined endpoint. + struct timespec timeout_remaining(timeout_endtime - timespec_now()); + + // Call pselect to wait on availability of port for writing. + fd_set writefds; 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(get_time_now()); -#endif - // Do the select - int r = select (fd_ + 1, NULL, &writefds, NULL, &timeout); -#if !defined(__linux__) - // Calculate difference and update the structure - struct timespec end(get_time_now()); - // Calculate the time select took - struct timespec diff(end - start); - // 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_remaining, NULL); // Figure out what happened by looking at select's response 'r' /** Error **/ @@ -624,9 +573,17 @@ Serial::SerialImpl::getPort () const } void -Serial::SerialImpl::setTimeout (serial::Timeout &timeout) +Serial::SerialImpl::setTimeout (const serial::Timeout &timeout) { timeout_ = timeout; + + // Cache the timespec conversions, as that's what the rest of the inner + // class operates on. + inter_byte_timeout_ = timespec_from_millis(timeout.inter_byte_timeout); + read_timeout_constant_ = timespec_from_millis(timeout.read_timeout_constant); + read_timeout_multiplier_ = timespec_from_millis(timeout.read_timeout_constant); + write_timeout_constant_ = timespec_from_millis(timeout.write_timeout_constant); + write_timeout_multiplier_ = timespec_from_millis(timeout.write_timeout_multiplier); } serial::Timeout From b992bce88f3c72e5b70efc52fed2ee9eb3ec2069 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Thu, 10 Oct 2013 15:26:33 -0400 Subject: [PATCH 07/17] Add early-return possibility for the base read method. --- src/impl/unix.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index b4c04c8..d4f1b83 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -412,7 +412,20 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) struct timespec timeout_endtime(timespec_now() + read_timeout_constant_ + (read_timeout_multiplier_ * size)); + // If there are already some bytes waiting to read, put those in the return + // buffer before setting up the first select call. This is important for + // performance reasons, as select/pselect can relinquish the thread even + // with data waiting. size_t bytes_read = 0; + if (available() > 0) { + ssize_t bytes_read_now = ::read (fd_, buf, size); + if (bytes_read_now < 1) { + throw SerialException ("device reports readiness to read but " + "returned no data (device disconnected?)"); + } + bytes_read += static_cast (bytes_read_now); + } + while (bytes_read < size) { // Must determine whether the time remaining before endtime (total read // timeout) or the inter-byte timeout is sooner, and use that one as the From e0967cd3fc684d624ee25e75c68d044a92ed63f8 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Thu, 10 Oct 2013 16:32:04 -0400 Subject: [PATCH 08/17] Add time.h include to unix-timespec.h --- include/serial/impl/unix-timespec.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/serial/impl/unix-timespec.h b/include/serial/impl/unix-timespec.h index b1b33a1..8053a32 100644 --- a/include/serial/impl/unix-timespec.h +++ b/include/serial/impl/unix-timespec.h @@ -36,6 +36,8 @@ #ifndef SERIAL_IMPL_UNIX_TIMESPEC_H #define SERIAL_IMPL_UNIX_TIMESPEC_H +#include + /*! Smooth over platform variances in getting an accurate timespec * representing the present moment. */ static inline struct timespec From bd616be97110d20df0e1ec6e8720e5fd7de37e38 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Thu, 10 Oct 2013 17:17:40 -0400 Subject: [PATCH 09/17] Remove the struct keywords from operator returns, as this is a problem for clang. --- include/serial/impl/unix-timespec.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/serial/impl/unix-timespec.h b/include/serial/impl/unix-timespec.h index 8053a32..7fcf08b 100644 --- a/include/serial/impl/unix-timespec.h +++ b/include/serial/impl/unix-timespec.h @@ -76,7 +76,7 @@ normalize(struct timespec* ts) { /*! Return a timespec which is the sum of two other timespecs. This * operator only makes logical sense when one or both of the arguments * represents a duration. */ -static inline struct timespec +static inline timespec operator+ (const struct timespec &a, const struct timespec &b) { struct timespec result = { a.tv_sec + b.tv_sec, a.tv_nsec + b.tv_nsec }; @@ -87,7 +87,7 @@ operator+ (const struct timespec &a, const struct timespec &b) { /*! Return a timespec which is the difference of two other timespecs. * This operator only makes logical sense when one or both of the arguments * represents a duration. */ -static inline struct timespec +static inline timespec operator- (const struct timespec &a, const struct timespec &b) { struct timespec result = { a.tv_sec - b.tv_sec, a.tv_nsec - b.tv_nsec }; @@ -100,7 +100,7 @@ operator- (const struct timespec &a, const struct timespec &b) { * large carries, eg a <1s timespec multiplied by a large enough integer * that the result is muliple seconds. Only makes sense when the timespec * argument is a duration. */ -static inline struct timespec +static inline timespec operator* (const struct timespec &ts, const size_t mul) { struct timespec result = { ts.tv_sec * mul, ts.tv_nsec * mul }; From 9f63f4d2b57d517f7b731ab22d2be5fe52599c5e Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 15 Oct 2013 11:43:00 -0700 Subject: [PATCH 10/17] move timespec funcs into unix.cc --- include/serial/impl/unix-timespec.h | 131 ---------------------------- src/impl/unix.cc | 101 ++++++++++++++++++++- 2 files changed, 99 insertions(+), 133 deletions(-) delete mode 100644 include/serial/impl/unix-timespec.h diff --git a/include/serial/impl/unix-timespec.h b/include/serial/impl/unix-timespec.h deleted file mode 100644 index 7fcf08b..0000000 --- a/include/serial/impl/unix-timespec.h +++ /dev/null @@ -1,131 +0,0 @@ -/*! - * \file serial/impl/unit-timespec.h - * \author Mike Purvis - * \version 0.1 - * - * \section LICENSE - * - * The MIT License - * - * Copyright (c) 2013 William Woodall - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - * - * \section DESCRIPTION - * - * Provides helper and operator functions for concisely and reliably handling - * timespec instances on unix platforms. - */ - -#ifndef SERIAL_IMPL_UNIX_TIMESPEC_H -#define SERIAL_IMPL_UNIX_TIMESPEC_H - -#include - -/*! Smooth over platform variances in getting an accurate timespec - * representing the present moment. */ -static inline struct timespec -timespec_now () -{ - struct timespec ts; -# 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); - ts.tv_sec = mts.tv_sec; - ts.tv_nsec = mts.tv_nsec; -# else - clock_gettime(CLOCK_REALTIME, &ts); -# endif - return ts; -} - -/*! Simple function to normalize the tv_nsec field to [0..1e9), carrying - * the remainder into the tv_sec field. This will not protect against the - * possibility of an overflow in the nsec field--proceed with caution. */ -static inline void -normalize(struct timespec* ts) { - while (ts->tv_nsec < 0) { - ts->tv_nsec += 1e9; - ts->tv_sec -= 1; - } - while (ts->tv_nsec >= 1e9) { - ts->tv_nsec -= 1e9; - ts->tv_sec += 1; - } -} - -/*! Return a timespec which is the sum of two other timespecs. This - * operator only makes logical sense when one or both of the arguments - * represents a duration. */ -static inline timespec -operator+ (const struct timespec &a, const struct timespec &b) { - struct timespec result = { a.tv_sec + b.tv_sec, - a.tv_nsec + b.tv_nsec }; - normalize(&result); - return result; -} - -/*! Return a timespec which is the difference of two other timespecs. - * This operator only makes logical sense when one or both of the arguments - * represents a duration. */ -static inline timespec -operator- (const struct timespec &a, const struct timespec &b) { - struct timespec result = { a.tv_sec - b.tv_sec, - a.tv_nsec - b.tv_nsec }; - normalize(&result); - return result; -} - -/*! Return a timespec which is a multiplication of a timespec and a positive - * integer. No overflow protection-- not suitable for multiplications with - * large carries, eg a <1s timespec multiplied by a large enough integer - * that the result is muliple seconds. Only makes sense when the timespec - * argument is a duration. */ -static inline timespec -operator* (const struct timespec &ts, const size_t mul) { - struct timespec result = { ts.tv_sec * mul, - ts.tv_nsec * mul }; - normalize(&result); - return result; -} - -/*! Return whichever of two timespec durations represents the shortest or most - * negative period. */ -static inline struct timespec -min (const struct timespec &a, const struct timespec &b) { - if (a.tv_sec < b.tv_sec - || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) { - return a; - } else { - return b; - } -} - -/*! Return a timespec duration set from a provided number of milliseconds. */ -static struct timespec -timespec_from_millis(const size_t millis) { - struct timespec result = { 0, millis * 1000000 }; - normalize(&result); - return result; -} - -#endif diff --git a/src/impl/unix.cc b/src/impl/unix.cc index d4f1b83..0ab52f5 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -1,4 +1,8 @@ -/* Copyright 2012 William Woodall and John Harrison */ +/* Copyright 2012 William Woodall and John Harrison + * + * Additional authors: + * - Mike Purvis, Clearpath Robotics, @mikepurvis + */ #include #include @@ -27,7 +31,6 @@ #endif #include "serial/impl/unix.h" -#include "serial/impl/unix-timespec.h" #ifndef TIOCINQ #ifdef FIONREAD @@ -49,6 +52,100 @@ using serial::SerialException; using serial::PortNotOpenedException; using serial::IOException; +/* Timespec related functions provided by @mikepurvis of Clearpath Robotics */ + +/*! Smooth over platform variances in getting an accurate timespec + * representing the present moment. */ +static inline struct timespec +timespec_now () +{ + struct timespec ts; +# 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); + ts.tv_sec = mts.tv_sec; + ts.tv_nsec = mts.tv_nsec; +# else + clock_gettime(CLOCK_REALTIME, &ts); +# endif + return ts; +} + +/*! Simple function to normalize the tv_nsec field to [0..1e9), carrying + * the remainder into the tv_sec field. This will not protect against the + * possibility of an overflow in the nsec field--proceed with caution. */ +static inline void +normalize(struct timespec* ts) { + while (ts->tv_nsec < 0) { + ts->tv_nsec += 1e9; + ts->tv_sec -= 1; + } + while (ts->tv_nsec >= 1e9) { + ts->tv_nsec -= 1e9; + ts->tv_sec += 1; + } +} + +/*! Return a timespec which is the sum of two other timespecs. This + * operator only makes logical sense when one or both of the arguments + * represents a duration. */ +static inline timespec +operator+ (const struct timespec &a, const struct timespec &b) { + struct timespec result = { a.tv_sec + b.tv_sec, + a.tv_nsec + b.tv_nsec }; + normalize(&result); + return result; +} + +/*! Return a timespec which is the difference of two other timespecs. + * This operator only makes logical sense when one or both of the arguments + * represents a duration. */ +static inline timespec +operator- (const struct timespec &a, const struct timespec &b) { + struct timespec result = { a.tv_sec - b.tv_sec, + a.tv_nsec - b.tv_nsec }; + normalize(&result); + return result; +} + +/*! Return a timespec which is a multiplication of a timespec and a positive + * integer. No overflow protection-- not suitable for multiplications with + * large carries, eg a <1s timespec multiplied by a large enough integer + * that the result is muliple seconds. Only makes sense when the timespec + * argument is a duration. */ +static inline timespec +operator* (const struct timespec &ts, const size_t mul) { + struct timespec result = { ts.tv_sec * mul, + ts.tv_nsec * mul }; + normalize(&result); + return result; +} + +/*! Return whichever of two timespec durations represents the shortest or most + * negative period. */ +static inline struct timespec +min (const struct timespec &a, const struct timespec &b) { + if (a.tv_sec < b.tv_sec + || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) { + return a; + } else { + return b; + } +} + +/*! Return a timespec duration set from a provided number of milliseconds. */ +static struct timespec +timespec_from_millis(const size_t millis) { + struct timespec result = { 0, millis * 1000000 }; + normalize(&result); + return result; +} + +/* End timespec related functions */ + Serial::SerialImpl::SerialImpl (const string &port, unsigned long baudrate, bytesize_t bytesize, From 282b79efc65edf1d37eb6362d9236365214a12cb Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 15 Oct 2013 11:45:53 -0700 Subject: [PATCH 11/17] style changes to new/old timespec functions --- src/impl/unix.cc | 75 ++++++++++++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index 0ab52f5..da2bfb7 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -54,13 +54,13 @@ using serial::IOException; /* Timespec related functions provided by @mikepurvis of Clearpath Robotics */ -/*! Smooth over platform variances in getting an accurate timespec +/* Smooth over platform variances in getting an accurate timespec * representing the present moment. */ static inline struct timespec timespec_now () { struct timespec ts; -# ifdef __MACH__ // OS X does not have clock_gettime, use clock_get_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); @@ -68,78 +68,91 @@ timespec_now () mach_port_deallocate(mach_task_self(), cclock); ts.tv_sec = mts.tv_sec; ts.tv_nsec = mts.tv_nsec; -# else +#else clock_gettime(CLOCK_REALTIME, &ts); -# endif +#endif return ts; } -/*! Simple function to normalize the tv_nsec field to [0..1e9), carrying +/* Simple function to normalize the tv_nsec field to [0..1e9), carrying * the remainder into the tv_sec field. This will not protect against the * possibility of an overflow in the nsec field--proceed with caution. */ static inline void -normalize(struct timespec* ts) { - while (ts->tv_nsec < 0) { +normalize (struct timespec* ts) +{ + while (ts->tv_nsec < 0) + { ts->tv_nsec += 1e9; ts->tv_sec -= 1; } - while (ts->tv_nsec >= 1e9) { + while (ts->tv_nsec >= 1e9) + { ts->tv_nsec -= 1e9; ts->tv_sec += 1; } } -/*! Return a timespec which is the sum of two other timespecs. This +/* Return a timespec which is the sum of two other timespecs. This * operator only makes logical sense when one or both of the arguments * represents a duration. */ static inline timespec -operator+ (const struct timespec &a, const struct timespec &b) { - struct timespec result = { a.tv_sec + b.tv_sec, - a.tv_nsec + b.tv_nsec }; +operator+ (const struct timespec &a, const struct timespec &b) +{ + struct timespec result = { + a.tv_sec + b.tv_sec, + a.tv_nsec + b.tv_nsec + }; normalize(&result); return result; } -/*! Return a timespec which is the difference of two other timespecs. +/* Return a timespec which is the difference of two other timespecs. * This operator only makes logical sense when one or both of the arguments * represents a duration. */ static inline timespec -operator- (const struct timespec &a, const struct timespec &b) { - struct timespec result = { a.tv_sec - b.tv_sec, - a.tv_nsec - b.tv_nsec }; +operator- (const struct timespec &a, const struct timespec &b) +{ + struct timespec result = { + a.tv_sec - b.tv_sec, + a.tv_nsec - b.tv_nsec + }; normalize(&result); return result; } -/*! Return a timespec which is a multiplication of a timespec and a positive - * integer. No overflow protection-- not suitable for multiplications with - * large carries, eg a <1s timespec multiplied by a large enough integer +/* Return a timespec which is a multiplication of a timespec and a positive + * integer. No overflow protection-- not suitable for multiplications with + * large carries, eg a <1s timespec multiplied by a large enough integer * that the result is muliple seconds. Only makes sense when the timespec * argument is a duration. */ static inline timespec -operator* (const struct timespec &ts, const size_t mul) { - struct timespec result = { ts.tv_sec * mul, - ts.tv_nsec * mul }; +operator* (const struct timespec &ts, const size_t mul) +{ + struct timespec result = { + ts.tv_sec * mul, + ts.tv_nsec * mul + }; normalize(&result); return result; } -/*! Return whichever of two timespec durations represents the shortest or most +/* Return whichever of two timespec durations represents the shortest or most * negative period. */ static inline struct timespec -min (const struct timespec &a, const struct timespec &b) { - if (a.tv_sec < b.tv_sec - || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) { +min (const struct timespec &a, const struct timespec &b) +{ + if (a.tv_sec < b.tv_sec || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) + { return a; - } else { - return b; } + return b; } -/*! Return a timespec duration set from a provided number of milliseconds. */ +/* Return a timespec duration set from a provided number of milliseconds. */ static struct timespec -timespec_from_millis(const size_t millis) { - struct timespec result = { 0, millis * 1000000 }; +timespec_from_millis (const size_t millis) +{ + struct timespec result = {0, millis * 1000000}; normalize(&result); return result; } From 17e698ce94e561bb6dfb57fde9cdb4abeb9ed1b2 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 15 Oct 2013 11:47:45 -0700 Subject: [PATCH 12/17] use pass by reference in normalize function --- src/impl/unix.cc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index da2bfb7..d2ff963 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -78,17 +78,17 @@ timespec_now () * the remainder into the tv_sec field. This will not protect against the * possibility of an overflow in the nsec field--proceed with caution. */ static inline void -normalize (struct timespec* ts) +normalize (struct timespec &ts) { - while (ts->tv_nsec < 0) + while (ts.tv_nsec < 0) { - ts->tv_nsec += 1e9; - ts->tv_sec -= 1; + ts.tv_nsec += 1e9; + ts.tv_sec -= 1; } - while (ts->tv_nsec >= 1e9) + while (ts.tv_nsec >= 1e9) { - ts->tv_nsec -= 1e9; - ts->tv_sec += 1; + ts.tv_nsec -= 1e9; + ts.tv_sec += 1; } } @@ -102,7 +102,7 @@ operator+ (const struct timespec &a, const struct timespec &b) a.tv_sec + b.tv_sec, a.tv_nsec + b.tv_nsec }; - normalize(&result); + normalize(result); return result; } @@ -116,7 +116,7 @@ operator- (const struct timespec &a, const struct timespec &b) a.tv_sec - b.tv_sec, a.tv_nsec - b.tv_nsec }; - normalize(&result); + normalize(result); return result; } @@ -132,7 +132,7 @@ operator* (const struct timespec &ts, const size_t mul) ts.tv_sec * mul, ts.tv_nsec * mul }; - normalize(&result); + normalize(result); return result; } @@ -153,7 +153,7 @@ static struct timespec timespec_from_millis (const size_t millis) { struct timespec result = {0, millis * 1000000}; - normalize(&result); + normalize(result); return result; } From 80a087e606d89b34e755edcfc9412c09e134ac2e Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 15 Oct 2013 11:48:00 -0700 Subject: [PATCH 13/17] use pass by reference in operator* for timespec --- 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 d2ff963..e6e76aa 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -126,7 +126,7 @@ operator- (const struct timespec &a, const struct timespec &b) * that the result is muliple seconds. Only makes sense when the timespec * argument is a duration. */ static inline timespec -operator* (const struct timespec &ts, const size_t mul) +operator* (const struct timespec &ts, const size_t &mul) { struct timespec result = { ts.tv_sec * mul, From cb4015705baf2d69d996219401bc3e7b375ab296 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 15 Oct 2013 11:55:29 -0700 Subject: [PATCH 14/17] just use inline, not static or static inline --- src/impl/unix.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index e6e76aa..8f84fb6 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -77,7 +77,7 @@ timespec_now () /* Simple function to normalize the tv_nsec field to [0..1e9), carrying * the remainder into the tv_sec field. This will not protect against the * possibility of an overflow in the nsec field--proceed with caution. */ -static inline void +inline void normalize (struct timespec &ts) { while (ts.tv_nsec < 0) @@ -95,7 +95,7 @@ normalize (struct timespec &ts) /* Return a timespec which is the sum of two other timespecs. This * operator only makes logical sense when one or both of the arguments * represents a duration. */ -static inline timespec +inline timespec operator+ (const struct timespec &a, const struct timespec &b) { struct timespec result = { @@ -109,7 +109,7 @@ operator+ (const struct timespec &a, const struct timespec &b) /* Return a timespec which is the difference of two other timespecs. * This operator only makes logical sense when one or both of the arguments * represents a duration. */ -static inline timespec +inline timespec operator- (const struct timespec &a, const struct timespec &b) { struct timespec result = { @@ -125,7 +125,7 @@ operator- (const struct timespec &a, const struct timespec &b) * large carries, eg a <1s timespec multiplied by a large enough integer * that the result is muliple seconds. Only makes sense when the timespec * argument is a duration. */ -static inline timespec +inline timespec operator* (const struct timespec &ts, const size_t &mul) { struct timespec result = { @@ -138,7 +138,7 @@ operator* (const struct timespec &ts, const size_t &mul) /* Return whichever of two timespec durations represents the shortest or most * negative period. */ -static inline struct timespec +inline struct timespec min (const struct timespec &a, const struct timespec &b) { if (a.tv_sec < b.tv_sec || (a.tv_sec == b.tv_sec && a.tv_nsec < b.tv_nsec)) @@ -149,7 +149,7 @@ min (const struct timespec &a, const struct timespec &b) } /* Return a timespec duration set from a provided number of milliseconds. */ -static struct timespec +inline struct timespec timespec_from_millis (const size_t millis) { struct timespec result = {0, millis * 1000000}; From 79993c09283592bfba3543fb2ebb3b6a6b655979 Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Tue, 15 Oct 2013 15:12:13 -0400 Subject: [PATCH 15/17] Remove secondary credit from unix.cc; just the header is fine --- src/impl/unix.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index 8f84fb6..2ad6bee 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -52,8 +52,6 @@ using serial::SerialException; using serial::PortNotOpenedException; using serial::IOException; -/* Timespec related functions provided by @mikepurvis of Clearpath Robotics */ - /* Smooth over platform variances in getting an accurate timespec * representing the present moment. */ static inline struct timespec From d09e18774d60c5fc01e138f0c616b9848e7a945c Mon Sep 17 00:00:00 2001 From: Mike Purvis Date: Tue, 15 Oct 2013 17:15:08 -0400 Subject: [PATCH 16/17] Restore timespec comment. --- src/impl/unix.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index 2ad6bee..aff4772 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -52,6 +52,8 @@ using serial::SerialException; using serial::PortNotOpenedException; using serial::IOException; +/* Timespec related functions */ + /* Smooth over platform variances in getting an accurate timespec * representing the present moment. */ static inline struct timespec From bd0d93832581dddf3199636d9478b0d3627d7403 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 15 Oct 2013 14:41:54 -0700 Subject: [PATCH 17/17] whitespace clean up --- src/impl/unix.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/impl/unix.cc b/src/impl/unix.cc index d4f1b83..d58a53c 100755 --- a/src/impl/unix.cc +++ b/src/impl/unix.cc @@ -413,12 +413,12 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) read_timeout_constant_ + (read_timeout_multiplier_ * size)); // If there are already some bytes waiting to read, put those in the return - // buffer before setting up the first select call. This is important for - // performance reasons, as select/pselect can relinquish the thread even + // buffer before setting up the first select call. This is important for + // performance reasons, as select/pselect can relinquish the thread even // with data waiting. - size_t bytes_read = 0; + size_t bytes_read = 0; if (available() > 0) { - ssize_t bytes_read_now = ::read (fd_, buf, size); + ssize_t bytes_read_now = ::read (fd_, buf, size); if (bytes_read_now < 1) { throw SerialException ("device reports readiness to read but " "returned no data (device disconnected?)"); @@ -489,7 +489,7 @@ Serial::SerialImpl::read (uint8_t *buf, size_t size) } // This shouldn't happen, if r > 0 our fd has to be in the list! THROW (IOException, "pselect reports ready to read, but our fd isn't" - " in the list, this shouldn't happen!"); + " in the list, this shouldn't happen!"); } } return bytes_read; @@ -589,7 +589,7 @@ void Serial::SerialImpl::setTimeout (const serial::Timeout &timeout) { timeout_ = timeout; - + // Cache the timespec conversions, as that's what the rest of the inner // class operates on. inter_byte_timeout_ = timespec_from_millis(timeout.inter_byte_timeout);