Addressed PR comments

Author: remi-siffert-ocado

include/ur_client_library/rtde/rtde_client.h

@@ -103,13 +103,13 @@ class RTDEClient
    * \param notifier The notifier to use in the pipeline
    * \param output_recipe_file Path to the file containing the output recipe
    * \param input_recipe_file Path to the file containing the input recipe
+   * \param target_frequency Frequency to run at. Defaults to 0.0 which means maximum frequency.
    * \param ignore_unavailable_outputs Configure the behaviour when a variable of the output recipe is not available
    * from the robot: output is silently ignored if true, a UrException is raised otherwise.
-   * \param target_frequency Frequency to run at. Defaults to 0.0 which means maximum frequency.
    */
   RTDEClient(std::string robot_ip, comm::INotifier& notifier, const std::string& output_recipe_file,
-             const std::string& input_recipe_file, bool ignore_unavailable_outputs = false,
-             double target_frequency = 0.0);
+             const std::string& input_recipe_file, double target_frequency = 0.0,
+             bool ignore_unavailable_outputs = false);
 
   /*!
    * \brief Creates a new RTDEClient object, including a used URStream and Pipeline to handle the
@@ -119,13 +119,13 @@ class RTDEClient
    * \param notifier The notifier to use in the pipeline
    * \param output_recipe Vector containing the output recipe
    * \param input_recipe Vector containing the input recipe
+   * \param target_frequency Frequency to run at. Defaults to 0.0 which means maximum frequency.
    * \param ignore_unavailable_outputs Configure the behaviour when a variable of the output recipe is not available
    * from the robot: output is silently ignored if true, a UrException is raised otherwise.
-   * \param target_frequency Frequency to run at. Defaults to 0.0 which means maximum frequency.
    */
   RTDEClient(std::string robot_ip, comm::INotifier& notifier, const std::vector<std::string>& output_recipe,
-             const std::vector<std::string>& input_recipe, bool ignore_unavailable_outputs = false,
-             double target_frequency = 0.0);
+             const std::vector<std::string>& input_recipe, double target_frequency = 0.0,
+             bool ignore_unavailable_outputs = false);
   ~RTDEClient();
   /*!
    * \brief Sets up RTDE communication with the robot. The handshake includes negotiation of the

include/ur_client_library/ur/ur_driver.h

@@ -605,13 +605,13 @@ class UrDriver
    *
    * \param output_recipe Vector containing the output recipe
    * \param input_recipe Vector containing the input recipe
-   * \param ignore_unavailable_outputs Configure the behaviour when a variable of the output recipe is not available
-   * from the robot: output is silently ignored if true, a UrException is raised otherwise.
    * \param target_frequency
    * Frequency to run the RTDE client at. Defaults to 0.0 which means maximum frequency.
+   * \param ignore_unavailable_outputs Configure the behaviour when a variable of the output recipe is not available
+   * from the robot: output is silently ignored if true, a UrException is raised otherwise.
    */
   void resetRTDEClient(const std::vector<std::string>& output_recipe, const std::vector<std::string>& input_recipe,
-                       bool ignore_unavailable_outputs = false, double target_frequency = 0.0);
+                       double target_frequency = 0.0, bool ignore_unavailable_outputs = false);
 
   /**
    * \brief Reset the RTDE client. As during initialization the driver will start RTDE communication
@@ -623,12 +623,12 @@ class UrDriver
    *
    * \param output_recipe_filename Filename where the output recipe is stored in.
    * \param input_recipe_filename Filename where the input recipe is stored in.
+   * \param target_frequency Frequency to run the RTDE client at. Defaults to 0.0 which means maximum frequency.
    * \param ignore_unavailable_outputs Configure the behaviour when a variable of the output recipe is not available
    * from the robot: output is silently ignored if true, a UrException is raised otherwise.
-   * \param target_frequency Frequency to run the RTDE client at. Defaults to 0.0 which means maximum frequency.
    */
   void resetRTDEClient(const std::string& output_recipe_filename, const std::string& input_recipe_filename,
-                       bool ignore_unavailable_outputs = false, double target_frequency = 0.0);
+                       double target_frequency = 0.0, bool ignore_unavailable_outputs = false);
 
 private:
   static std::string readScriptFile(const std::string& filename);

src/rtde/rtde_client.cpp

@@ -35,7 +35,7 @@ namespace urcl
 namespace rtde_interface
 {
 RTDEClient::RTDEClient(std::string robot_ip, comm::INotifier& notifier, const std::string& output_recipe_file,
-                       const std::string& input_recipe_file, bool ignore_unavailable_outputs, double target_frequency)
+                       const std::string& input_recipe_file, double target_frequency, bool ignore_unavailable_outputs)
   : stream_(robot_ip, UR_RTDE_PORT)
   , output_recipe_(ensureTimestampIsPresent(readRecipe(output_recipe_file)))
   , ignore_unavailable_outputs_(ignore_unavailable_outputs)
@@ -52,8 +52,8 @@ RTDEClient::RTDEClient(std::string robot_ip, comm::INotifier& notifier, const st
 }
 
 RTDEClient::RTDEClient(std::string robot_ip, comm::INotifier& notifier, const std::vector<std::string>& output_recipe,
-                       const std::vector<std::string>& input_recipe, bool ignore_unavailable_outputs,
-                       double target_frequency)
+                       const std::vector<std::string>& input_recipe, double target_frequency,
+                       bool ignore_unavailable_outputs)
   : stream_(robot_ip, UR_RTDE_PORT)
   , output_recipe_(ensureTimestampIsPresent(output_recipe))
   , ignore_unavailable_outputs_(ignore_unavailable_outputs)
@@ -88,8 +88,8 @@ bool RTDEClient::init(const size_t max_num_tries, const std::chrono::millisecond
     if (client_state_ == ClientState::INITIALIZED)
       return true;
 
-    URCL_LOG_ERROR("Failed to initialize RTDE client, retrying in 1 seconds");
-    std::this_thread::sleep_for(std::chrono::seconds(1));
+    URCL_LOG_ERROR("Failed to initialize RTDE client, retrying in 10 seconds");
+    std::this_thread::sleep_for(std::chrono::seconds(10));
     attempts++;
   }
   std::stringstream ss;
@@ -275,7 +275,7 @@ void RTDEClient::setupOutputs(const uint16_t protocol_version)
 
   while (num_retries < MAX_REQUEST_RETRIES)
   {
-    URCL_LOG_INFO("Sending output recipe");
+    URCL_LOG_DEBUG("Sending output recipe");
     if (protocol_version == 2)
     {
       size = ControlPackageSetupOutputsRequest::generateSerializedRequest(buffer, target_frequency_, output_recipe_);

src/ur/ur_driver.cpp

@@ -691,19 +691,19 @@ void UrDriver::setKeepaliveCount(const uint32_t count)
 }
 
 void UrDriver::resetRTDEClient(const std::vector<std::string>& output_recipe,
-                               const std::vector<std::string>& input_recipe, bool ignore_unavailable_outputs,
-                               double target_frequency)
+                               const std::vector<std::string>& input_recipe, double target_frequency,
+                               bool ignore_unavailable_outputs)
 {
-  rtde_client_.reset(
-      new rtde_interface::RTDEClient(robot_ip_, notifier_, output_recipe, input_recipe, target_frequency));
+  rtde_client_.reset(new rtde_interface::RTDEClient(robot_ip_, notifier_, output_recipe, input_recipe, target_frequency,
+                                                    ignore_unavailable_outputs));
   initRTDE();
 }
 
 void UrDriver::resetRTDEClient(const std::string& output_recipe_filename, const std::string& input_recipe_filename,
-                               bool ignore_unavailable_outputs, double target_frequency)
+                               double target_frequency, bool ignore_unavailable_outputs)
 {
   rtde_client_.reset(new rtde_interface::RTDEClient(robot_ip_, notifier_, output_recipe_filename, input_recipe_filename,
-                                                    ignore_unavailable_outputs, target_frequency));
+                                                    target_frequency, ignore_unavailable_outputs));
   initRTDE();
 }
 

tests/test_rtde_client.cpp

@@ -143,12 +143,12 @@ TEST_F(RTDEClientTest, invalid_target_frequency)
 {
   // Setting target frequency below 0 or above 500, should throw an exception
   client_.reset(
-      new rtde_interface::RTDEClient(ROBOT_IP, notifier_, output_recipe_file_, input_recipe_file_, false, -1.0));
+      new rtde_interface::RTDEClient(ROBOT_IP, notifier_, output_recipe_file_, input_recipe_file_, -1.0, false));
 
   EXPECT_THROW(client_->init(), UrException);
 
   client_.reset(
-      new rtde_interface::RTDEClient(ROBOT_IP, notifier_, output_recipe_file_, input_recipe_file_, false, 1000));
+      new rtde_interface::RTDEClient(ROBOT_IP, notifier_, output_recipe_file_, input_recipe_file_, 1000, false));
 
   EXPECT_THROW(client_->init(), UrException);
 }
@@ -167,7 +167,7 @@ TEST_F(RTDEClientTest, unconfigured_target_frequency)
 
 TEST_F(RTDEClientTest, set_target_frequency)
 {
-  client_.reset(new rtde_interface::RTDEClient(ROBOT_IP, notifier_, output_recipe_file_, input_recipe_file_, false, 1));
+  client_.reset(new rtde_interface::RTDEClient(ROBOT_IP, notifier_, output_recipe_file_, input_recipe_file_, 1, false));
   client_->init();
 
   // Maximum frequency should still be equal to the robot's maximum frequency
@@ -376,8 +376,8 @@ TEST_F(RTDEClientTest, check_all_rtde_output_variables_exist)
   client_->init();
 
   // Ignore unknown output variables to account for variables not available in old urcontrol versions.
-  client_.reset(
-      new rtde_interface::RTDEClient(ROBOT_IP, notifier_, exhaustive_output_recipe_file_, input_recipe_file_, true));
+  client_.reset(new rtde_interface::RTDEClient(ROBOT_IP, notifier_, exhaustive_output_recipe_file_, input_recipe_file_,
+                                               0.0, true));
 
   EXPECT_NO_THROW(client_->init());
   client_->start();
@@ -405,8 +405,8 @@ TEST_F(RTDEClientTest, check_unknown_rtde_output_variable)
   std::vector<std::string> incorrect_output_recipe = client_->getOutputRecipe();
   incorrect_output_recipe.push_back("unknown_rtde_variable");
 
-  client_.reset(
-      new rtde_interface::RTDEClient(ROBOT_IP, notifier_, incorrect_output_recipe, resources_input_recipe_, false));
+  client_.reset(new rtde_interface::RTDEClient(ROBOT_IP, notifier_, incorrect_output_recipe, resources_input_recipe_,
+                                               0.0, false));
 
   EXPECT_THROW(client_->init(), UrException);
 }

tests/test_ur_driver.cpp

@@ -375,7 +375,7 @@ TEST_F(UrDriverTest, send_robot_program_retry_on_failure)
 TEST_F(UrDriverTest, reset_rtde_client)
 {
   double target_frequency = 50;
-  g_ur_driver_->resetRTDEClient(OUTPUT_RECIPE, INPUT_RECIPE, false, target_frequency);
+  g_ur_driver_->resetRTDEClient(OUTPUT_RECIPE, INPUT_RECIPE, target_frequency);
   ASSERT_EQ(g_ur_driver_->getControlFrequency(), target_frequency);
 }