Deprecate ClientGoalHandle::async_result() (#1120)

Fixes https://github.com/ros2/rclcpp/issues/955

There are currently two public APIs for users to get the result of a goal.
This change deprecates one of the APIs, which was considered to be unsafe as
it may result in a race with user-code and raise an exception.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

rclcpp_action/include/rclcpp_action/client.hpp

@@ -411,7 +411,7 @@ public:
       goal_handle->set_result_callback(result_callback);
     }
     this->make_result_aware(goal_handle);
-    return goal_handle->async_result();
+    return goal_handle->async_get_result();
   }
 
   /// Asynchronously request a goal be canceled.

rclcpp_action/include/rclcpp_action/client_goal_handle.hpp

@@ -91,6 +91,8 @@ public:
 
   /// Get a future to the goal result.
   /**
+   * \deprecated Use rclcpp_action::Client::async_get_result() instead.
+   *
    * This method should not be called if the `ignore_result` flag was set when
    * sending the original goal request (see Client::async_send_goal).
    *
@@ -99,6 +101,7 @@ public:
    * \throws exceptions::UnawareGoalHandleError If the the goal handle is unaware of the result.
    * \return A future to the result.
    */
+  [[deprecated("use rclcpp_action::Client::async_get_result() instead")]]
   std::shared_future<WrappedResult>
   async_result();
 
@@ -134,6 +137,19 @@ private:
     typename ClientGoalHandle<ActionT>::SharedPtr shared_this,
     typename std::shared_ptr<const Feedback> feedback_message);
 
+  /// Get a future to the goal result.
+  /**
+   * This method should not be called if the `ignore_result` flag was set when
+   * sending the original goal request (see Client::async_send_goal).
+   *
+   * `is_result_aware()` can be used to check if it is safe to call this method.
+   *
+   * \throws exceptions::UnawareGoalHandleError If the the goal handle is unaware of the result.
+   * \return A future to the result.
+   */
+  std::shared_future<WrappedResult>
+  async_get_result();
+
   /// Returns the previous value of awareness
   bool
   set_result_awareness(bool awareness);

rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp

@@ -58,6 +58,13 @@ ClientGoalHandle<ActionT>::get_goal_stamp() const
 template<typename ActionT>
 std::shared_future<typename ClientGoalHandle<ActionT>::WrappedResult>
 ClientGoalHandle<ActionT>::async_result()
+{
+  return this->async_get_result();
+}
+
+template<typename ActionT>
+std::shared_future<typename ClientGoalHandle<ActionT>::WrappedResult>
+ClientGoalHandle<ActionT>::async_get_result()
 {
   std::lock_guard<std::mutex> guard(handle_mutex_);
   if (!is_result_aware_) {

rclcpp_action/test/test_client.cpp

@@ -304,7 +304,6 @@ TEST_F(TestClient, async_send_goal_no_callbacks)
   EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status());
   EXPECT_FALSE(goal_handle->is_feedback_aware());
   EXPECT_FALSE(goal_handle->is_result_aware());
-  EXPECT_THROW(goal_handle->async_result(), rclcpp_action::exceptions::UnawareGoalHandleError);
 }
 
 TEST_F(TestClient, async_send_goal_no_callbacks_wait_for_result)