-
Notifications
You must be signed in to change notification settings - Fork 163
Add ROS parameters for configuring GZ to ROS optical frame conversion #703
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: ros2
Are you sure you want to change the base?
Conversation
Signed-off-by: Ian Chen <ichen@openrobotics.org>
Signed-off-by: Ian Chen <ichen@openrobotics.org>
Signed-off-by: Ian Chen <ichen@openrobotics.org>
Signed-off-by: Ian Chen <ichen@openrobotics.org>
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.
Code is not compiling:
tf2::tf2
but the target was not found. Possible reasons include:
* There is a typo in the target name.
* A find_package call is missing for an IMPORTED target.
* An ALIAS target is missing.
Signed-off-by: Ian Chen <ichen@openrobotics.org>
Signed-off-by: Ian Chen <ichen@openrobotics.org>
fixed. |
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.
@iche033 is this ready for review?
Not yet, I'm planning to add more tests. But I am looking for feedback on the proposed ROS parameters for converting to optical frame, mainly If the general direction is ok. |
I think this is going in good direction!
|
Thanks for the feedback!
Currently the static tf broadcaster sends a transform and goes out of scope so I think this caused a problem I ran into, which is that the static TF is not latched, and new subscriber couldn't get the TF. I'll look into getting rid of throttling.
Maybe
We can remove the need to publish the frame twice if
The problem is that if the user does not set the extra optional
yeah this is a ROS-specific solution to the problem. I think having a way to deal with this in SDF would be nice but it was decided to scope it down and go with this for now. |
This all seems super complicated. I can imagine lots of frustrated users. Even if they just copy code from tutorials (or ChatGPT does it for them), they forget one thing and the whole camera machinery doesn't work in ROS. I have an alternative proposition: what if this package provides a convenience launch file that would just get the two Gz and two ROS topic names as arguments and an optional argument with the new frame ID (defaulting to
What do you think? |
I think this does not need to be that complicated for most users. To migrate their existing bridge to use the new optical frame conversion feature, I expect most users to only set the We can also provide a convenience launch file like the one you proposed. It could be implemented on top of this PR and uses the ROS parameters introduced here. How does this sound? |
Agree on this, most users will care about setting up |
Marked as draft to get feedback
🎉 New feature
Summary
Adds a number of ROS parameters for configuring GZ to ROS frame conversion.
The main feature is the
publish_optical_frame
ROS parameter, which applies to the GZ to ROS direction. When set to true, the bridge automatically converts the messages to z-forward optical frame. The outgoing messages will have a new frame_id with "_optical" suffix string, and a TF with x-forward to z-forward transform is published using a static transform broadcaster.Example usage:
The bridge sets the
publish_optical_frame
ROS param totrue
and also remaps the original topic to a new name with anoptical
sub-namespace.If the user wants to set a different frame id or transform, they can override them using a couple of other ROS parameters:
override_frame_id_string
(string) - Overrides frame_id of the outgoing messagesoverride_frame_transform
(double array) - Custom transform [x, y, z, roll, pitch, yaw]See the
API
section of README.md for more info.Note that this feature adds 2 new ROS package dependencies:
Test it
See the
GZ to ROS Optical frame conversion
section of README.md for more instructions.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.