sendScriptBlocking: Restart interface and resend script on readOnlyInterfaceException (#522)

Implements functionality to restart primary interface and retry
execution of script code, if the interface was initially in read-only
mode.

Author: URJala

doc/architecture/primary_client.rst

@@ -26,18 +26,21 @@ Method signature:
 .. code-block:: c++
 
    bool sendScriptBlocking(
-     std::string program,
-     std::string script_name = "",
-     std::chrono::milliseconds timeout = std::chrono::seconds(1),
-     bool fail_on_warnings = true
+     const std::string& program,
+     const std::string& script_name = "",
+     const std::chrono::milliseconds start_timeout = std::chrono::seconds(1),
+     const bool fail_on_warnings = true,
+     const bool retry_on_readonly_interface = true
    );
 
 | The ``sendScriptBlocking`` method will also accept valid URScript code, but blocks until the execution result of the given program is available.
 | Prior to transferring the program it will first check that the robot is in a state where it can execute programs, otherwise an exception is thrown.
 | If the robot is ready, the program is then transferred, and the method will wait for the robot to report that the program has either started, finished or encountered an error.
-| If the program has not started within the given ``timeout``, the method throws an exception.
+| If the program has not started within the given ``start_timeout``, the method throws an exception.
 | If the robot encounters an error or runtime exception during program execution the method also throws an exception.
 | If ``fail_on_warnings`` is true, it will also throw an exception, if the robot reports a warning during program execution. Note: protective stops are reported as warnings by the robot.
+| If ``retry_on_readonly_interface`` is true, the method will restart the primary interface and retry sending the program, if the primary interface is read-only. It will retry once, and if the interface is still read-only, an exception will be thrown. If false, the exception will be thrown, without restarting/retrying.
 | If no exceptions are thrown, the script has been executed successfully.
 | This method also accepts secondary programs, but no feedback is available for those, so it will behave similarly to the ``sendScript`` method in those cases, except for the pre-transfer checks.
 | The exact exceptions that are thrown in various cases can be seen in the `primary client header file <https://github.com/UniversalRobots/Universal_Robots_Client_Library/blob/master/include/ur_client_library/primary/primary_client.h>`_.
+| Note: This method clears all stored error codes in the client during execution.

include/ur_client_library/primary/primary_client.h

@@ -95,7 +95,10 @@ class PrimaryClient
   /*!
    * \brief Sends a custom script program to the robot.
    *
-   * The given code must be valid according the UR Scripting Manual.
+   * The given code must be valid according the UR Scripting Manual. This function doesn't give any
+   * feedback whether the script was executed successfully or not, it only reports whether the
+   * script was uploaded to the robot or not. For feedback on the execution of the script, use
+   * sendScriptBlocking().
    *
    * \param program URScript code that shall be executed by the robot.
    *
@@ -123,6 +126,14 @@ class PrimaryClient
    * \param fail_on_warnings Whether or not the function should report a failure, if the robot reports a warning-level
    * error during execution. Default true
    *
+   * \param retry_on_readonly_interface Whether to retry, if the primary interface is read-only. This will restart the
+   * primary interface connection, and then try sending the script again. If the interface is still read-only a
+   * ReadOnlyInterfaceException will be thrown. Default true
+   * \note The primary interface connection is read-only when the robot is not in remote control
+   * mode. If the robot switches from local control mode to remote control mode, the connection
+   * remains read-only. In this case, reconnecting will result in a read-write primary interface connection, and the
+   * script can be executed successfully.
+   *
    * \throw urcl::ScriptCodeSyntaxException if the given script code has syntax errors, which are checked here.
    * \throw urcl::UrException if the stop command cannot be sent to the robot.
    * \throw urcl::TimeoutException if the robot doesn't stop the program within the given timeout.
@@ -136,9 +147,9 @@ class PrimaryClient
    * transferred. This can happen if the robot was recently switched from manual to remote control mode.
    * \throw urcl::RobotErrorCodeException if the robot encounters an error during script execution.
    */
-  void sendScriptBlocking(const std::string& program, std::string script_name = "",
-                          std::chrono::milliseconds start_timeout = std::chrono::seconds(1),
-                          bool fail_on_warnings = true);
+  void sendScriptBlocking(const std::string& program, const std::string& script_name = "",
+                          const std::chrono::milliseconds start_timeout = std::chrono::seconds(1),
+                          const bool fail_on_warnings = true, const bool retry_on_readonly_interface = true);
 
   bool checkCalibration(const std::string& checksum);
 
@@ -338,7 +349,8 @@ class PrimaryClient
    */
   RobotSeries getRobotSeries();
 
-  /* \brief Check if the current safety mode allows for script execution
+  /*!
+   * \brief Check if the current safety mode allows for script execution
    *
    * Safety modes allowing for execution are: NORMAL, REDUCED, RECOVERY, UNDEFINED_SAFETY_MODE
    */
@@ -359,9 +371,11 @@ class PrimaryClient
   void keyMessageCallback(KeyMessage& msg);
   void runtimeExceptionCallback(RuntimeExceptionMessage& msg);
 
-  ScriptInfo prepare_script(std::string script, std::string script_name);
-  std::vector<std::string> strip_comments_and_whitespace(std::vector<std::string> script_lines);
-  std::string truncate_script_name(std::string candidate_name);
+  ScriptInfo prepareScript(std::string script, std::string script_name);
+  std::vector<std::string> stripCommentsAndWhitespace(std::vector<std::string> script_lines);
+  std::string truncateScriptName(std::string candidate_name);
+  void sendScriptMonitorExecution(const ScriptInfo& script_info, const std::chrono::milliseconds& timeout,
+                                  const bool fail_on_warnings);
 
   PrimaryParser parser_;
   std::shared_ptr<PrimaryConsumer> consumer_;

src/primary/primary_client.cpp

@@ -137,18 +137,19 @@ bool PrimaryClient::safetyModeAllowsExecution()
   }
 }
 
-void PrimaryClient::sendScriptBlocking(const std::string& program, std::string script_name,
-                                       std::chrono::milliseconds timeout, bool fail_on_warnings)
+void PrimaryClient::sendScriptBlocking(const std::string& program, const std::string& script_name,
+                                       const std::chrono::milliseconds start_timeout, const bool fail_on_warnings,
+                                       const bool retry_on_readonly_interface)
 {
-  ScriptInfo script_info = prepare_script(program, script_name);
+  ScriptInfo script_info = prepareScript(program, script_name);
 
   RobotMode robot_mode = getRobotMode();
   std::chrono::milliseconds robot_mode_timeout(1000);
-  auto start = std::chrono::system_clock::now();
+  auto start_time = std::chrono::system_clock::now();
   while (robot_mode == RobotMode::UNKNOWN)
   {
     auto now = std::chrono::system_clock::now();
-    if (std::chrono::duration_cast<std::chrono::milliseconds>(now - start).count() > robot_mode_timeout.count())
+    if (std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time).count() > robot_mode_timeout.count())
     {
       throw TimeoutException("Robot mode not received within timeout. ", robot_mode_timeout);
     }
@@ -172,6 +173,31 @@ void PrimaryClient::sendScriptBlocking(const std::string& program, std::string s
 
     throw SafetyModeException("Script execution via primary interface", allowed_modes, getSafetyMode());
   }
+
+  try
+  {
+    sendScriptMonitorExecution(script_info, start_timeout, fail_on_warnings);
+  }
+  catch ([[maybe_unused]] const ReadOnlyInterfaceException& exc)
+  {
+    if (retry_on_readonly_interface)
+    {
+      URCL_LOG_INFO("Script execution failed due to the primary interface being read-only. Restarting primary "
+                    "interface and retrying once.");
+      stop();
+      start();
+      sendScriptMonitorExecution(script_info, start_timeout, fail_on_warnings);
+    }
+    else
+    {
+      throw;
+    }
+  }
+}
+
+void PrimaryClient::sendScriptMonitorExecution(const ScriptInfo& script_info, const std::chrono::milliseconds& timeout,
+                                               const bool fail_on_warnings)
+{
   // Clear runtime exception
   {
     std::scoped_lock lock(runtime_exception_mutex_);
@@ -191,6 +217,7 @@ void PrimaryClient::sendScriptBlocking(const std::string& program, std::string s
     throw StreamNotConnectedException("Script could not be sent to the robot. Ensure that the primary interface is "
                                       "connected.");
   }
+
   // No feedback from secondary programs, so we assume success
   if (script_info.script_type == ScriptTypes::SEC)
   {
@@ -389,7 +416,7 @@ void PrimaryClient::sendScriptBlocking(const std::string& program, std::string s
   }
 }
 
-std::vector<std::string> PrimaryClient::strip_comments_and_whitespace(std::vector<std::string> split_script)
+std::vector<std::string> PrimaryClient::stripCommentsAndWhitespace(std::vector<std::string> split_script)
 {
   std::vector<std::string> stripped_script;
   for (auto line : split_script)
@@ -413,7 +440,7 @@ std::vector<std::string> PrimaryClient::strip_comments_and_whitespace(std::vecto
   return stripped_script;
 }
 
-std::string PrimaryClient::truncate_script_name(const std::string candidate_name)
+std::string PrimaryClient::truncateScriptName(const std::string candidate_name)
 {
   std::string final_name = candidate_name;
   // Limit script name length to 31, to ensure backwards compatibility
@@ -426,13 +453,13 @@ std::string PrimaryClient::truncate_script_name(const std::string candidate_name
   return final_name;
 }
 
-ScriptInfo PrimaryClient::prepare_script(std::string script, std::string script_name)
+ScriptInfo PrimaryClient::prepareScript(std::string script, std::string script_name)
 {
   // Split the given script in to separate lines
   std::vector<std::string> split_script = splitString(script, "\n");
 
   // Remove all comments and white-space-only lines
-  std::vector<std::string> stripped_script = strip_comments_and_whitespace(split_script);
+  std::vector<std::string> stripped_script = stripCommentsAndWhitespace(split_script);
 
   if (stripped_script.size() == 0)
   {
@@ -452,7 +479,7 @@ ScriptInfo PrimaryClient::prepare_script(std::string script, std::string script_
       stripped_script[0].substr(0, 4).find("sec ") == script.npos)
   {
     // Check that the final name is not too long
-    actual_script_name = truncate_script_name(actual_script_name);
+    actual_script_name = truncateScriptName(actual_script_name);
     std::string definition = "def " + actual_script_name + "():";
     std::string end = "end";
     // Add indentation to the existing script code
@@ -483,7 +510,7 @@ ScriptInfo PrimaryClient::prepare_script(std::string script, std::string script_
       actual_script_type = ScriptTypes::SEC;
     }
     // Check that the script name is not too long, replace it, if it is
-    actual_script_name = truncate_script_name(name_in_script);
+    actual_script_name = truncateScriptName(name_in_script);
     if (actual_script_name.size() != name_in_script.size())
     {
       stripped_script[0].replace(stripped_script[0].find(name_in_script), name_in_script.size(), actual_script_name);

tests/test_primary_client.cpp

@@ -666,12 +666,41 @@ TEST_F(PrimaryClientFakeTest, test_send_script_to_read_only_server)
   // Make the fake server send an error code message with code 210 (read-only primary interface) when it receives a
   // script
   server_->setScriptCallback([this, script_code]([[maybe_unused]] const std::string& payload) {
-    ASSERT_TRUE(
-        server_->sendErrorCodeMessage(210, 0, ReportLevel::VIOLATION, "Simulated read-only primary interface error"));
+    if (payload.find(script_code) != std::string::npos)
+    {
+      ASSERT_TRUE(
+          server_->sendErrorCodeMessage(210, 0, ReportLevel::VIOLATION, "Simulated read-only primary interface error"));
+    }
   });
 
-  EXPECT_THROW(client_->sendScriptBlocking(script_code, "test_fun", std::chrono::milliseconds(1000), false),
+  // Fails even with retry
+  EXPECT_THROW(client_->sendScriptBlocking(script_code, "test_fun", std::chrono::milliseconds(1000), false, true),
                ReadOnlyInterfaceException);
+
+  bool retry = false;
+  // Send C210 on first try, then succeed
+  server_->setScriptCallback([this, script_code, &retry]([[maybe_unused]] const std::string& payload) {
+    if (!retry && payload.find(script_code) != std::string::npos)
+    {
+      ASSERT_TRUE(
+          server_->sendErrorCodeMessage(210, 0, ReportLevel::VIOLATION, "Simulated read-only primary interface error"));
+      retry = true;
+    }
+    else if (payload.find(script_code) != std::string::npos)
+    {
+      server_->sendKeyMessage("PROGRAM_XXX_STARTED", "test_fun");
+      ASSERT_EQ(payload, "def test_fun():\n  " + script_code + "\nend\n\n");
+      server_->sendKeyMessage("PROGRAM_XXX_STOPPED", "test_fun");
+    }
+  });
+
+  // Fails with no retry
+  EXPECT_THROW(client_->sendScriptBlocking(script_code, "test_fun", std::chrono::milliseconds(1000), false, false),
+               ReadOnlyInterfaceException);
+
+  retry = false;
+  // Succeeds on retry
+  EXPECT_NO_THROW(client_->sendScriptBlocking(script_code, "test_fun", std::chrono::milliseconds(1000), false, true));
 }
 
 TEST_F(PrimaryClientFakeTest, test_send_script_blocking_timeout_on_no_response)
@@ -681,7 +710,7 @@ TEST_F(PrimaryClientFakeTest, test_send_script_blocking_timeout_on_no_response)
   const std::string script_code = "textmsg(\"Still running\")";
 
   // We do not set a script callback on the fake server, so it will not respond to the script being sent. This should
-  // cause sendScriptBlocking to time out and return false.
+  // cause sendScriptBlocking to time out and throw a TimeoutException.
   EXPECT_THROW(client_->sendScriptBlocking(script_code, "test_fun", std::chrono::milliseconds(100), false),
                TimeoutException);
 }