Skip to content

Patch and rebuild #4

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Patch and rebuild #4

wants to merge 23 commits into from

Conversation

mxgrey
Copy link
Owner

@mxgrey mxgrey commented Dec 19, 2024

No description provided.

mxgrey and others added 23 commits October 3, 2024 08:50
* Wrap slice::from_raw_parts to be compatible with rcl

Signed-off-by: Michael X. Grey <[email protected]>

* Fix style

Signed-off-by: Michael X. Grey <[email protected]>

---------

Signed-off-by: Michael X. Grey <[email protected]>
* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: Esteve Fernandez <[email protected]>
Co-authored-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
* Use latest stable Rust CI

Signed-off-by: Michael X. Grey <[email protected]>

* Fix quotation in doc

Signed-off-by: Michael X. Grey <[email protected]>

* Fix serde feature for vendored messages

Signed-off-by: Michael X. Grey <[email protected]>

* Fix formatting of lists in docs

Signed-off-by: Michael X. Grey <[email protected]>

* Update minimum supported Rust version based on the currently used language features

Signed-off-by: Michael X. Grey <[email protected]>

* Conform to clippy style guide

Signed-off-by: Michael X. Grey <[email protected]>

* Add rust toolchain as a matrix dimension

Signed-off-by: Michael X. Grey <[email protected]>

* Bump declaration of minimum supported rust version

Signed-off-by: Michael X. Grey <[email protected]>

* Limit the scheduled runs to once a week

Signed-off-by: Michael X. Grey <[email protected]>

* Define separate stable and minimal workflows because matrix does not work with reusable actions

Signed-off-by: Michael X. Grey <[email protected]>

* Reduce minimum version to 1.75 to target Noble

Signed-off-by: Michael X. Grey <[email protected]>

---------

Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
* Update rclrs vendor_interfaces.py script

This updates the vendor_interfaces.py script to also vendor in the
action_msgs and unique_identifier_msgs packages. The script is modified
to always use a list of package names rather than hard-coding the
package names everywhere.

* Silence certain clippy lints on generated code

In case a user enforces clippy linting on these generated packages,
silence expected warnings. This is already the case in rclrs, but should
be applied directly to the generated packages for the sake of downstream
users.

The `clippy::derive_partial_eq_without_eq` lint was already being
disabled for the packages vendored by rclrs, but is now moved to the
rosidl_generator_rs template instead. This is necessary since we always
derive the PartialEq trait, but can't necessary derive Eq, and so don't.

The `clippy::upper_case_acronyms` is new and was added to account for
message type names being upper-case acrynyms, like
unique_identifier_msgs::msg::UUID.

* Update vendored message packages

This updates the message packages vendored under the rclrs `vendor`
module. The vendor_interfaces.py script was used for reproducibility.

The existing packages – rcl_interfaces, rosgraph_msgs, and
builtin_interfaces – are only modified in that they now use the
std::ffi::c_void naming for c_void instead of the std::os::raw::c_void
alias and disable certain clippy lints.

The action_msgs and unique_identifier_msgs packages are newly added to
enable adding action support to rclrs. action_msgs is needed for
interaction with action goal info and statuses, and has a dependency on
unique_identifier_msgs.
* Initial logging implementation

* * Moved logging_fini to follow correct call sequence based on rclcpp

* Primary change to fix builder's node variable going out of scope

* Fix linting issue with use statement

* Remove the need to Box rcl_node_t (ros2-rust#5)

Signed-off-by: Michael X. Grey <[email protected]>

* Fix linting issues

* Fix logging lifecycles and clean up tests

Signed-off-by: Michael X. Grey <[email protected]>

* Linting fixes after logging lifecycle improvements

* Introduce Logger and LogParams

Signed-off-by: Michael X. Grey <[email protected]>

* Cleaner logging tests

Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Co-authored-by: Nathan Wiebe Neufeldt <[email protected]>

* Address feedback for logging PR

Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Co-authored-by: Nathan Wiebe Neufeldt <[email protected]>

---------

Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Co-authored-by: Grey <[email protected]>
Co-authored-by: Nathan Wiebe Neufeldt <[email protected]>
* Add Publisher::can_loan_msgs

* Fix formatting
* Update for clippy 1.83

Signed-off-by: Michael X. Grey <[email protected]>

* Update clippy for cfg(test) as well

Signed-off-by: Michael X. Grey <[email protected]>

* Exclude dependencies from clippy test

Signed-off-by: Michael X. Grey <[email protected]>

* Fix clippy for rosidl_runtime_rs

Signed-off-by: Michael X. Grey <[email protected]>

---------

Signed-off-by: Michael X. Grey <[email protected]>
* Fix buffer overflow for bounded sequence example

Signed-off-by: Michael X. Grey <[email protected]>

* Allow longer characters for wstring

Signed-off-by: Michael X. Grey <[email protected]>

---------

Signed-off-by: Michael X. Grey <[email protected]>
Also, add a link to the installation instruction from the main README.md

Co-authored-by: Lucas Chiesa <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants