Set reconnection_time from setup method

Also deprecated the explicit method for setting up reconnection time.

Author: Felix Exner

include/ur_client_library/comm/producer.h

@@ -67,12 +67,11 @@ class URProducer : public IProducer<T>
   void setupProducer(const size_t max_num_tries = 0,
                      const std::chrono::milliseconds reconnection_time = std::chrono::seconds(10)) override
   {
-    stream_.setReconnectionTime(reconnection_time);
     timeval tv;
     tv.tv_sec = 1;
     tv.tv_usec = 0;
     stream_.setReceiveTimeout(tv);
-    if (!stream_.connect(max_num_tries))
+    if (!stream_.connect(max_num_tries, reconnection_time))
     {
       throw UrException("Failed to connect to robot. Please check if the robot is booted and connected.");
     }

include/ur_client_library/comm/stream.h

@@ -23,6 +23,7 @@
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <atomic>
+#include <chrono>
 #include <mutex>
 #include <string>
 #include "ur_client_library/log.h"
@@ -58,12 +59,14 @@ class URStream : public TCPSocket
    *
    * \param max_num_tries Maximum number of connection attempts before counting the connection as
    * failed. Unlimited number of attempts when set to 0.
+   * \param reconnection_time time in between connection attempts to the server
    *
    * \returns True on success, false if connection could not be established
    */
-  bool connect(const size_t max_num_tries = 0)
+  bool connect(const size_t max_num_tries = 0,
+               const std::chrono::milliseconds reconnection_time = std::chrono::seconds(10))
   {
-    return TCPSocket::setup(host_, port_, max_num_tries);
+    return TCPSocket::setup(host_, port_, max_num_tries, reconnection_time);
   }
 
   /*!

include/ur_client_library/comm/tcp_socket.h

@@ -23,6 +23,7 @@
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <atomic>
+#include <chrono>
 #include <mutex>
 #include <string>
 #include <memory>
@@ -51,6 +52,7 @@ class TCPSocket
   std::atomic<int> socket_fd_;
   std::atomic<SocketState> state_;
   std::chrono::milliseconds reconnection_time_;
+  bool reconnection_time_modified_deprecated_ = false;
 
 protected:
   virtual bool open(int socket_fd, struct sockaddr* address, size_t address_len)
@@ -59,11 +61,13 @@ class TCPSocket
   }
   virtual void setOptions(int socket_fd);
 
-  bool setup(const std::string& host, const int port, const size_t max_num_tries = 0);
+  bool setup(const std::string& host, const int port, const size_t max_num_tries = 0,
+             const std::chrono::milliseconds reconnection_time = DEFAULT_RECONNECTION_TIME);
 
   std::unique_ptr<timeval> recv_timeout_;
 
 public:
+  static constexpr std::chrono::milliseconds DEFAULT_RECONNECTION_TIME{ 10000 };
   /*!
    * \brief Creates a TCPSocket object
    */
@@ -146,10 +150,8 @@ class TCPSocket
    *
    * \param reconnection_time time in between connection attempts to the server
    */
-  void setReconnectionTime(const std::chrono::milliseconds reconnection_time)
-  {
-    reconnection_time_ = reconnection_time;
-  }
+  [[deprecated("Reconnection time is passed to setup directly now.")]] void
+  setReconnectionTime(const std::chrono::milliseconds reconnection_time);
 };
 }  // namespace comm
 }  // namespace urcl

src/comm/tcp_socket.cpp

@@ -24,6 +24,7 @@
 #include <endian.h>
 #include <netinet/tcp.h>
 #include <unistd.h>
+#include <chrono>
 #include <cstring>
 #include <sstream>
 #include <thread>
@@ -55,8 +56,19 @@ void TCPSocket::setOptions(int socket_fd)
   }
 }
 
-bool TCPSocket::setup(std::string& host, int port, size_t max_num_tries)
+bool TCPSocket::setup(const std::string& host, const int port, const size_t max_num_tries,
+                      const std::chrono::milliseconds reconnection_time)
 {
+  // This can be removed once we remove the setReconnectionTime() method
+  auto reconnection_time_resolved = reconnection_time;
+  if (reconnection_time_modified_deprecated_)
+  {
+    URCL_LOG_WARN("TCPSocket::setup(): Reconnection time was modified using `setReconnectionTime()` which is "
+                  "deprecated. Please change your code to set reconnection_time through the `setup()` method "
+                  "directly. The value passed to this function will be ignored.");
+    reconnection_time_resolved = reconnection_time_;
+  }
+
   if (state_ == SocketState::Connected)
     return false;
 
@@ -113,9 +125,9 @@ bool TCPSocket::setup(std::string& host, int port, size_t max_num_tries)
       std::stringstream ss;
       ss << "Failed to connect to robot on IP " << host_name
          << ". Please check that the robot is booted and reachable on " << host_name << ". Retrying in "
-         << std::chrono::duration_cast<std::chrono::duration<float>>(reconnection_time_).count() << " seconds";
+         << std::chrono::duration_cast<std::chrono::duration<float>>(reconnection_time_resolved).count() << " seconds";
       URCL_LOG_ERROR("%s", ss.str().c_str());
-      std::this_thread::sleep_for(reconnection_time_);
+      std::this_thread::sleep_for(reconnection_time_resolved);
     }
   }
   setOptions(socket_fd_);
@@ -221,5 +233,13 @@ void TCPSocket::setReceiveTimeout(const timeval& timeout)
   }
 }
 
+void TCPSocket::setReconnectionTime(const std::chrono::milliseconds reconnection_time)
+{
+  URCL_LOG_ERROR("Calling setReconnectionTime is deprecated. Reconnection timeout is passed to the setup method "
+                 "directly.");
+  reconnection_time_ = reconnection_time;
+  reconnection_time_modified_deprecated_ = true;
+}
+
 }  // namespace comm
 }  // namespace urcl

src/ur/dashboard_client.cpp

@@ -61,14 +61,13 @@ bool DashboardClient::connect(const size_t max_num_tries, const std::chrono::mil
 
   while (not ret_val)
   {
-    TCPSocket::setReconnectionTime(reconnection_time);
     // The first read after connection can take more time.
     tv.tv_sec = 10;
     tv.tv_usec = 0;
     TCPSocket::setReceiveTimeout(tv);
     try
     {
-      if (TCPSocket::setup(host_, port_, max_num_tries))
+      if (TCPSocket::setup(host_, port_, max_num_tries, reconnection_time))
       {
         URCL_LOG_INFO("%s", read().c_str());
         ret_val = true;

tests/test_stream.cpp

@@ -29,6 +29,7 @@
 // -- END LICENSE BLOCK ------------------------------------------------
 
 #include <gtest/gtest.h>
+#include <chrono>
 #include <condition_variable>
 
 #include <ur_client_library/comm/stream.h>
@@ -323,8 +324,7 @@ TEST_F(StreamTest, write_data_package)
 TEST_F(StreamTest, connect_non_connected_robot)
 {
   comm::URStream<rtde_interface::RTDEPackage> stream("127.0.0.1", 12321);
-  stream.setReconnectionTime(std::chrono::milliseconds(500));
-  EXPECT_FALSE(stream.connect(2));
+  EXPECT_FALSE(stream.connect(2, std::chrono::milliseconds(500)));
 }
 
 int main(int argc, char* argv[])

tests/test_tcp_socket.cpp

@@ -103,22 +103,23 @@ class TCPSocketTest : public ::testing::Test
   class Client : public comm::TCPSocket
   {
   public:
-    Client(int port, const std::string& ip = "127.0.0.1", const size_t max_num_tries = 0)
+    Client(int port, const std::string& ip = "127.0.0.1")
     {
       port_ = port;
       ip_ = ip;
-      max_num_tries_ = max_num_tries;
     }
 
-    bool setup()
+    bool setup(const size_t max_num_tries = 0,
+               const std::chrono::milliseconds reconnection_time = std::chrono::seconds(10))
     {
-      return TCPSocket::setup(ip_, port_, max_num_tries_);
+      return TCPSocket::setup(ip_, port_, max_num_tries, reconnection_time);
     }
 
-    void setupClientBeforeServer()
+    void setupClientBeforeServer(const size_t max_num_tries = 0,
+                                 const std::chrono::milliseconds reconnection_time = std::chrono::seconds(10))
     {
       done_setting_up_client_ = false;
-      client_setup_thread_ = std::thread(&Client::setupClient, this, port_);
+      client_setup_thread_ = std::thread(&Client::setupClient, this, port_, max_num_tries, reconnection_time);
     }
 
     bool waitForClientSetupThread()
@@ -149,10 +150,10 @@ class TCPSocketTest : public ::testing::Test
     std::thread client_setup_thread_;
     int port_;
     std::string ip_;
-    size_t max_num_tries_;
     bool done_setting_up_client_;
 
-    void setupClient(int port)
+    void setupClient(int port, const size_t max_num_tries = 0,
+                     std::chrono::milliseconds reconnection_time = std::chrono::seconds(10))
     {
       std::string ip = "127.0.0.1";
       TCPSocket::setup(ip, port);
@@ -205,8 +206,7 @@ TEST_F(TCPSocketTest, setup_client_before_server)
   // Make server unavailable
   server_.reset();
 
-  client_->setReconnectionTime(std::chrono::seconds(1));
-  client_->setupClientBeforeServer();
+  client_->setupClientBeforeServer(0, std::chrono::seconds(1));
 
   // Make sure that the client has tried to connect to the server, before creating the server
   std::this_thread::sleep_for(std::chrono::seconds(1));
@@ -348,9 +348,8 @@ TEST_F(TCPSocketTest, setup_while_client_is_connected)
 
 TEST_F(TCPSocketTest, connect_non_running_robot)
 {
-  Client client(12321, "127.0.0.1", 2);
-  client.setReconnectionTime(std::chrono::milliseconds(500));
-  EXPECT_FALSE(client.setup());
+  Client client(12321, "127.0.0.1");
+  EXPECT_FALSE(client.setup(2, std::chrono::milliseconds(500)));
 }
 
 int main(int argc, char* argv[])