[WIP] Action support #410

Update uses of a `Mutex<rcl_node_t>` with a `NodeHandle`, as was done elsewhere in the crate. Also, correct the documented UUID size to 16 rather than 24. The minimal_action_client compiles, but the minimal_action_server does not yet, complaining about expecting the `rmw` versions of messages rather than the idiomatic ones.

This follows generally the same pattern as the service server. It required adding a typesupport function to the Action trait and pulling in some more rcl_action bindings.

This is needed to initialize the rcl_action_server_t. In rclcpp, the clock ends up stored in the Server itself (by way of ServerBase and ServerBaseImpl); we'll see if that ends up being necessary here.

So far, none of the implemented functionality actually depends on the action type. It could be separated out into a concrete type like is done in rclcpp, in case that reduces the cost of monomorphization.

I'm not sure that this is actually the signature that we'll want, but we'll start from there.

They still aren't handled in the .wait() method, though.

Currently, action servers and clients that are ready in multiple ways are returned to the executor as a list of pairs, with one readiness mode per entry. This could alternatively be encoded as a bitfield or similar struct, with any given client/server only occurring once in the list. However, to ensure that the executor has control over execution order, we would need to expose individual `execute_readiness_mode()` methods from the client and server, rather than a unified `execute(Mode)` method. That's fine too, but something to keep in mind.

There's one error code, ActionNameInvalid, that conflicts with the EventInvalid code. We should consult with upstream about this, as it causes issues for representing error codes as enum values. These two error codes are probably never returned by the same function, but it would be better to keep them unique.

This is incomplete, since the service types aren't yet being generated.

This is just the basic layout. I'm trying to avoid defining the `take_*` functions that rclcpp has to link taking messages to executing on them. I'm not sure if that's actually worthwhile yet. This area should also be revisited once it's functional to see whether portions can be moved into the non-polymorphic subfunctions. Doing so could reduce compile times by avoiding excessive monomorphization.

This results in the exact same file being produced for services, except for some whitespace changes. However, it enables actions to invoke the respective service template for its generation, similar to how the it works for services and their underlying messages.

Not tested

C++ uses duck typing for this, knowing that for any `Action`, the type `Action::Impl::SendGoalService::Request` will always have a `goal_id` field of type `unique_identifier_msgs::msg::UUID` without having to prove this to the compiler. Rust's generics are more strict, requiring that this be proven using type bounds. The `Request` type is also action-specific as it contains a `goal` field containing the `Goal` message type of the action. We therefore cannot enforce that all `Request`s are a specific type in `rclrs`. This seems most easily represented using associated type bounds on the `SendGoalService` associated type within `ActionImpl`. To avoid introducing to `rosidl_runtime_rs` a circular dependency on message packages like `unique_identifier_msgs`, the `ExtractUuid` trait only operates on a byte array rather than a more nicely typed `UUID` message type. I'll likely revisit this as we introduce more similar bounds on the generated types.

The `rcl_action_accept_new_goal()` function returns a pre-allocated `rcl_action_goal_handle_t` pointer, which is also stored within the action server proper. This means we cannot store a non-pointer version of this in the `ServerGoalHandle`. Instead, we'll keep a mutex-guarded mutable pointer. The `Arc` is unnecessary since this pointer is never shared with anyone. Also, we need to clean up the goal handle by calling `rcl_action_goal_handle_fini()` when the `ServerGoalHandle` is dropped.

The logic in `execute_goal_request()` was starting to get unwieldy, so I split it into three functions. The idea is that the first, `take_goal_request()`, should handle everything up until we call the user's callback. Meanwhile, `send_goal_response()` handles everything after the user callback, sending the response and, if accepted, setting everything up for the action server. `execute_goal_request()` is the overall function coordinating all of this. It passes request data from the `take*` function to the user callback and passes the returned response into the `send*` function. In addition to splitting the logic into digestible pieces, this means we could also easily make the goal-request callback asynchronous by delaying the `send*` function until the user has given their asynchronous response.

This provides a convenient RAII-style way to ensure that a function is always called when a specific value gets dropped.

This skips some of the steps that rclcpp performs, as they appear to be unnecessary. Testing should reveal whether that's true or not.

This trims the send_goal_response() function down to only be a wrapper around the rcl_action equivalent. In addition to improving logical separation, this also simplifies control flow when handling rejection.

Rather than having a bunch of standalone traits implementing various message functions like `ExtractUuid` and `SetAccepted`, with the trait bounds on each associated type in `ActionImpl`, we'll instead add these functions directly to the `ActionImpl` trait. This is simpler on both the rosidl_runtime_rs and the rclrs side.

This requires a mutex to enable simultaneous goal acceptance in a multithreaded executor. We also make ServerGoalHandles implement Send and Sync, since they will be accessed by multiple threads during callbacks. Due to containing a raw pointer field, the type doesn't automatically implement Send/Sync; however, we guarantee that the uses of this pointer is safe and synchronized, allowing us to safely implement the traits.

Adds a trait method to create a feedback message given the goal ID and the user-facing feedback message type. Depending on how many times we do this, it may end up valuable to define a GoalUuid type in rosidl_runtime_rs itself. We wouldn't be able to utilize the `RCL_ACTION_UUID_SIZE` constant imported from `rcl_action`, but this is pretty much guaranteed to be 16 forever. Defining this method signature also required inverting the super-trait relationship between Action and ActionImpl. Now ActionImpl is the sub-trait as this gives it access to all of Action's associated types. Action doesn't need to care about anything from ActionImpl (hopefully).

This still needs to be hooked up to the ActionServerGoalHandle, though.

These still don't build without errors, but it's close.

These aren't actually implemented as callbacks like in rclcpp, but rather just regular method calls. This required making some of the ActionServer and ActionServerBase methods use an Arc receiver, but the ActionServer is only ever created within an Arc, so this is fine.

rclrs needs to be able to generically construct result responses, including the user-defined result field.

These now call into the action server to send a response to prior (queued) and future goal result requests. There are still some synchronization tweaks needed for this to be correct.

This adds methods to ActionImpl for creating and accessing action-specific message types. These are needed by the rclrs ActionClient to generically read and write RMW messages. Due to issues with qualified paths in certain places (rust-lang/rust#86935), the generator now refers directly to its service and message types rather than going through associated types of the various traits. This also makes the generated code a little easier to read, with the trait method signatures from rosidl_runtime_rs still enforcing type-safety.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Action support #410

[WIP] Action support #410

Commits on Sep 28, 2024

[WIP] Action support #410

Are you sure you want to change the base?

[WIP] Action support #410

Commits on Sep 28, 2024