-
Notifications
You must be signed in to change notification settings - Fork 7
Add example for dynamic messages #7
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
base: main
Are you sure you want to change the base?
Add example for dynamic messages #7
Conversation
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
esteve
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.
@luca-della-vedova thanks! Does this work for you without a package.xml file? Perhaps we don't need it for the examples, anyway.
rclrs/dynamic_pub_sub/Cargo.toml
Outdated
|
|
||
| [dependencies] | ||
| # TODO(luca) change this to the correct version once dynamic message support is released on crates.io | ||
| rclrs = { version = "0.4", features = ["dyn_msg"] } |
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.
Now that dyn_msg is no longer a flag in rclrs, could you remove it here?
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 removed it in 7f8ac88 but note that as the TODO says this will only be merge-able after the upstream PR is merged and we have a re-release.
To make it compile-able I temporarily set it as a git dependency on my branch, otherwise this can't really be tested.
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
9826150 to
7f8ac88
Compare
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
|
I haven't really put a lot of work into this PR and was leaving it as a "cleanup after the upstream is merged" but after your comment I realized that this could really be a good test platform to make sure the upstream PR is working as intended, so I cleaned this up and added functionality.
It's an interesting question, "it builds" but upon further inspection it's more complicated than that. I also noticed that there was no dynamic_publisher examples, so I added that in 5816dbd. I also changed the message type into the standard This means that, after all these changes, you can finally use this PR to test the upstream PR and even mix and match normal and dynamic pub sub! For example: # You can of course try this
ros2 run examples_rclrs_dynamic_pub_sub dynamic_publisher
ros2 run examples_rclrs_dynamic_pub_sub dynamic_subscriber
# But also
ros2 run examples_rclrs_minimal_pub_sub minimal_publisher
ros2 run examples_rclrs_dynamic_pub_sub dynamic_subscriber
# And also!
ros2 run examples_rclrs_dynamic_pub_sub dynamic_publisher
ros2 run examples_rclrs_minimal_pub_sub minimal_subscriber |
Split off ros2-rust/ros2_rust#492 now that examples are in a different repo.
Will need the upstream merged and re-released with a new version for it to actually work (otherwise I can hardcode a branch / path if we want CI to be tested)