Feature/action

[iv461]

This PR implements an async await API for both action server and action clients.
It does so by adding a copy of the rclcpp_actions source code and patching some limitations as well as critical bugs that are absolutely necessary for implementing an async await API.

Feature set

Action server

• Allowing to use asynchronous functions, i.e. coroutines besides synchronous functions for handling the goal request, cancellation and the execution of the goal.

The main limitation in rclcpp was a missing support for deferred response. So I've split the internal methods handling the service calls in half and moved the second half into separate send_respose methods (similar to the ones present in the services API).
Patch, see file icey/include/icey/action/server.hpp/cpp

Action client

• awaiting the result of setting a goal
• receiving feedback
• obtaining the final result
• receiving feedback

There were a couple of bugs in rclcpp that prevented a correct implementation:

• Deadlock occurs when chaining send_goal requests. This is caused by non-reentrant mutexes being held locked unnecessarily during user callbacks. This was fixed in the rolling version (async_send_goal deadlocks ros2/rclcpp#2796), but Tomoja Fujita says it won't be backported (async_send_goal deadlocks ros2/rclcpp#2796 (comment)). Unfortunately, this conceptual issue is present in many places in rclcpp. I fixed it by not locking at all during user callbacks (Patch](ef48fc1...ef1258f), file /icey/src/actions/client.cpp, tested by test case ActionTimeoutAndMultipleGoalsTest.)

• No cancellation API for cancelling send_goal and cancel requests (similar to regular service calls) is present. Currently, it is not possible to cancel callbacks or delete a pending goal request. If the server dies, the client leaks memory. This issue has not yet been resolved in the rolling version, as there is currently no way to delete items from the pimpl_->pending_goal_responses map added here. I've fixed this by adding returning the request ID of the underlying service calls and then adding cancellation function to cancel these requests (See patch, file icey/include/icey/action/client.hpp
  )

The action client API problem

One problem that remains unsolved in this PR is the action client API.
The problem is that the action protocol specification as given by the sequence diagrams in the action design document is not implemented correctly by ROS.
Whether or not it is implemented correctly however critically influences the client API.
If we would implement an API that follows the specification, we would get lost feedback messages.

Details

The issue is that per specification, the client should be able to receive feedback messages only after requesting the result request. However, the current ROS implementation does not adhere to this specification. This standpoint is also shared by Janosch Machowinski (ros2/rclcpp#2782 (comment)).
It instead allows the client to receive feedback messages before the result request has been sent.

This means a sensible async/await API such as:

auto maybe_goal_handle = co_await action_client.send_goal(goal, 2s);
if(maybe_goal_handle.has_value()) {
   auto goal_handle = maybe_goal_handle.value();
   for(auto fb: co_await goal_handle.get_result(20s)) {
       if(fb.is_feedback()) {
          
       } else {
            /// We got the final result
            auto result = fb.result();
            break;
       }
   }
} else {
   /// Goal request rejected
}

would have a race condition, although it adheres to the specification. Feedback messages may be received before the get_result function is called, meaning they may be lost. This issue has been discussed here and a fix proposed. The proposed fix lead to the implementation following the specification and therefore enabling the above async/await API.

I've decided to provide an API that uses a feedback callback to avoid loosing feedback messages. Once the bug in the underlying actions implementation is fixed, we could switch to the above API.

Other minor changes:

• fixes Result-type to work with non-default-constructible values and errors
• Fixes missing cancellations of the timeout timer in the promise destructor
• Refactors the deferred cleanup of timeout timers to the NodeBase class (but generally the timeout handling can be refactored further, i.e. to a with_timeout() function)
• fixes flaky count_subscribers and similar tests