-
Notifications
You must be signed in to change notification settings - Fork 6
Add rosidl_runtime_rs from the ros2-rust repository #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Previously, only messages consisting of basic types and strings were supported. Now, all message types will work, including those that have fields of nested types, bounded types, or arrays. Changes: - The "rsext" library is deleted - Unused messages in "rosidl_generator_rs" are deleted - There is a new package, "rosidl_runtime_rs", see below - The RMW-compatible messages from C, which do not require an extra conversion step, are exposed in addition to the "idiomatic" messages - Publisher and subscription are changed to work with both idiomatic and rmw types, through the unifying `Message` trait On `rosidl_runtime_rs`: This package is the successor of `rclrs_msg_utilities` package, but doesn't have much in common with it anymore. It provides common types and functionality for messages. The `String` and `Sequence` types and their variants in that package essentially wrap C types from the `rosidl_runtime_c` package and C messages generated by the "rosidl_generator_c" package. A number of functions and traits are implemented on these types, so that they feel as ergonomic as possible, for instance, a `seq!` macro for creating a sequence. There is also some documentation and doctests. The memory for the (non-pretty) message types is managed by the C allocator. Not yet implemented: - long double - constants - Services/clients - @verbatim comments - ndarray for sequences/arrays of numeric types - implementing `Eq`, `Ord` and `Hash` when a message contains no floats
Also remove unused dependency from rosidl_runtime_rs
* Removed support for no_std * Removed more no_std dependencies * Removed more no_std dependencies * Removed more no_std dependencies * Removed downcast * Removed TryFrom, not needed with Rust 2021 * Fix visibility of modules
This is required to make multithreading work. For instance, calling publish() on a separate thread doesn't work without these impls.
* Copied tests from jhdcs' fork * Ran cargo fmt * Run clippy on tests as well * Make Clippy happy * Fix rustfmt warning * Added std_msgs to package.xml * Disable deref_nullptr warning for generated code * Include Soya-Onishi's changes * Fix Node::new call * Fix spelling mistakes Co-authored-by: jhdcs <48914066+jhdcs@users.noreply.github.com> * Fix spelling mistakes Co-authored-by: jhdcs <48914066+jhdcs@users.noreply.github.com> * Fix spelling mistakes Co-authored-by: jhdcs <48914066+jhdcs@users.noreply.github.com> * Fix formatting Co-authored-by: jhdcs <48914066+jhdcs@users.noreply.github.com>
Previously, it was assumed by the $string_conversion_func, for instance, that the output of deref() would be an unsigned integer.
* Added support for clients and services
* Fixups for releasing to crates.io * Removed std_msgs as test dependency. Fix rosidl_runtime_rs version * Removed test * Removed test
* Generalize callbacks to subscriptions By implementing a trait (SubscriptionCallback) on valid function signatures, and making subscriptions accept callbacks that implement this trait, we can now * Receive both plain and boxed messages * Optionally receive a MessageInfo along with the message, as the second argument * Soon, receive a loaned message instead of an owned one This corresponds to the functionality in any_subscription_callback.hpp in rclcpp.
These files will be shown by crates.io.
- Upgraded libloading to 0.8 - Added instructions to rerun build scripts if dependent files have changed - Enabled bindgen to invalidate the built crate whenever any transitively included header files from the wrapper change - Removed some default true options from our bindgen builder Co-authored-by: Sam Privett <sam@privett.dev>
Signed-off-by: Esteve Fernandez <esteve@apache.org>
This reverts commit 8948409e0c6d9ced291b5cffda82036d067bd36d.
* Allow rustdoc to run without a ROS distribution * Extensions to get working * Fail if generate_docs feature is not enabled and ROS_DISTRO is not set * Fix formatting * Fix formatting * Make clippy slightly less unhappy * Do not run clippy for all features if checking rclrs * Clean up rcl_bindings.rs * Fix comparison * Avoid warnings for rosidl_runtime_rs * Avoid running cargo test with all features for rclrs * Ignore rosidl_runtime_rs for cargo test * rosidl_runtime_rs does not have a dyn_msg feature * Add comment about the generate_docs feature --------- Co-authored-by: carter <carterjschultz@gmail.com>
* Update GitHub actions * Downgrade rust-toolchain to the latest stable version (1.74.0) from 1.80.0 * Fix issues with latest clippy * Fix rustdoc check issue * Update Rust toolchain in Dockerfile
According to https://design.ros2.org/articles/wide_strings.html, the `string` and `wstring` types must use the UTF-8 and UTF-16 encodings (not necessarily enforced), cannot contain null bytes or words (enforced), and, when bounded, are measured in terms of bytes or words. Moreover, though the rosidl_runtime_c `U16String` type uses `uint_least16_t`, Rust guarantees the existence of a precise `u16` type, so we should use that instead of `ushort`, which isn't guaranteed to be the same as `uint_least16_t` either. (Rust doesn't support platforms where `uint_least16_t != uint16_t`.)
* Use nightly for style check Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai> * Install nightly for cargo +nightly fmt Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai> * Fix style in examples Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai> * Update style for rosidl_runtime_rs Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai> * Add a comment indicating that nightly release is needed for formatting Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai> --------- Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
* WIP Adding describe paramater service * Implement parameter setting services Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Restructure and cleanup Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Implement list_parameters with prefixes Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Minor cleanups Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Fix tests, cleanups Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Fix order of drop calls Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Add first bunch of unit tests for list and get / set parameters Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Clear warnings in rclrs Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Fix clippy, add set atomically tests Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Add describe parameter and get parameter types tests Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Minor cleanups, remove several unwraps Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Remove commented code Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Address first round of feedback Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Allow undeclared parameters in parameter getting services Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Clippy Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Run rustfmt Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai> * Update rclrs/src/parameter/service.rs Co-authored-by: jhdcs <48914066+jhdcs@users.noreply.github.com> * Change behavior to return NOT_SET for non existing parameters Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Make use_sim_time parameter read only Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Format Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Add a comment to denote why unwrap is safe Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Use main fmt Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> * Add a builder parameter to start parameter services Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> --------- Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai> Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai> Co-authored-by: Michael X. Grey <mxgrey@intrinsic.ai> Co-authored-by: jhdcs <48914066+jhdcs@users.noreply.github.com>
…` (#416) * Add in missing nullptr check when calling `std::slice::from_raw_parts` * Added missing testcase
* Added action template * Added action generation * Added basic create_action_client function * dded action generation * checkin * Fix missing exported pre_field_serde field * Removed extra code * Sketch out action server construction and destruction 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. * Fix action typesupport function * Add ActionImpl trait with internal messages and services This is incomplete, since the service types aren't yet being generated. * Split srv.rs.em into idiomatic and rmw template files 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. * Generate underlying service definitions for actions Not tested * Add runtime trait to get the UUID from a goal request 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. * Integrate RMW message methods into ActionImpl 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. * Add rosidl_runtime_rs::ActionImpl::create_feedback_message() 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). * Add GetResultService methods to ActionImpl * Implement ActionImpl trait methods in generator These still don't build without errors, but it's close. * Replace set_result_response_status with create_result_response rclrs needs to be able to generically construct result responses, including the user-defined result field. * Implement client-side trait methods for action messages 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. * Format the rosidl_runtime_rs::ActionImpl trait * Wrap longs lines in rosidl_generator_rs action.rs This at least makes the template easier to read, but also helps with the generated code. In general, the generated code could actually fit on one line for the function signatures, but it's not a big deal to split it across multiple. * Use idiomatic message types in Action trait This is user-facing and so should use the friendly message types. * Cleanup ActionImpl using type aliases Signed-off-by: Michael X. Grey <grey@openrobotics.org> * Formatting * Switch from std::os::raw::c_void to std::ffi::c_void While these are aliases of each other, we might as well use the more appropriate std::ffi version, as requested by reviewers. * Clean up rosidl_generator_rs's cmake files Some of the variables are present but no longer used. Others were not updated with the action changes. * Add a short doc page on the message generation pipeline This should help newcomers orient themselves around the rosidl_*_rs packages. --------- Signed-off-by: Michael X. Grey <grey@openrobotics.org> Co-authored-by: Esteve Fernandez <esteve@apache.org> Co-authored-by: Michael X. Grey <grey@openrobotics.org>
* Update for clippy 1.83 Signed-off-by: Michael X. Grey <greyxmike@gmail.com> * Update clippy for cfg(test) as well Signed-off-by: Michael X. Grey <greyxmike@gmail.com> * Exclude dependencies from clippy test Signed-off-by: Michael X. Grey <greyxmike@gmail.com> * Fix clippy for rosidl_runtime_rs Signed-off-by: Michael X. Grey <greyxmike@gmail.com> --------- Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
nwn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this was discussed in the working group meeting, but it might be worth recording here the reasoning for why we're splitting rosidl_runtime_rs into a separate repo from the generator in rosidl_rust. I believe we chose to model this after how it's done for Python with the rosidl_python/rosidl_runtime_py split, rather than how it's done for C++, where everything is in rosidl. However, I don't recall the reasoning behind that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but yeah good question @nwn. Do we really gain anything by having the generator and the runtime being separate repos? Do we ever actually want to version these separately?
Yes, the first thing to be aware of is that the ROS buildfarm currently requires all packages within the same repo to have the same version number. I can't speak to the reason for that requirement, but I've been told it's baked in right now and inviolable. The next thing to be aware of is that any time a version gets bumped for any package on the buildfarm, it will have to rebuild all packages that (transitively) depend on the one whose version was bumped. Our goal is for In contrast, By keeping them in separate repos, we can make changes to |
mxgrey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retaining all the commits is really great. The loss of timestamps is unfortunate, but at least we can see the order of all the commits, so we can still retrace how the implementation evolved over time.
Very clean migration 👍
2018cf2 to
4993476
Compare
|
@maspe36 @nwn @mxgrey thanks for your reviews. I gave it another try and I've managed to keep the timestamp thanks to this answer on Stackoverflow: https://stackoverflow.com/a/5520650 For future reference the command I used was: $ git filter-branch --commit-filter 'export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME"; export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"; export GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE"; git commit-tree "$@"' -- origin/main..HEADI was also able to keep the original author for each commit without adding myself as a committer. |
|
Ugh, turns out that when merging this onto main, GitHub decided to do a rebase and undo the timestamp trick, now all the commits have the current timestamp. I'll do a force push onto main to correct that. |
This PR moves the
rosidl_runtime_rsfolder from ros2-rust @ 9a845c17873cbdf49e8017d5f0af6d8f795589cc