-
Notifications
You must be signed in to change notification settings - Fork 109
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
Create new messages with all fields needed to define a velocity and transform it #233
Create new messages with all fields needed to define a velocity and transform it #233
Conversation
…transform it Signed-off-by: Dr. Denis <[email protected]>
geometry_msgs/msg/VelocityStamped
Outdated
# The message is often used in simplified versions where: | ||
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; | ||
# - 'body_frame_id' and 'header.frame_id' are identical, i.e., the velocity is observed and defined in the local coordinates system of the body | ||
# which is the usual use-case in mobile robotics. |
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 think you can remove this particular line
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 am with you on this, my only concern is not to confuse people that are moving from Twist as used currently in Nav2 and ros2_control.
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.
# which is the usual use-case in mobile robotics. | |
# as common in mobile robotics. |
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'm a little confused. if body_frame_id
and header.frame_id
are identical, shouldn't the velocity be always zero? i.e, we're asking the velocity of a body relative to itself. On the other hand, we could have body_frame_id
and observation_pose
(via its frame_id
) be identical and header.frame_id
be something different from the two. This makes sense to me since it's asking the velocity of the body relative to whatever header.frame_id
is but expressed in the body frame.
Signed-off-by: Dr. Denis <[email protected]>
@tfoote any comments on this? |
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.
The changes look good. I'd like to flesh out the documentation more completely so that people in the future clearly understand the design. As well as confirming that we can confirm that the transform velocity operations have enough semantic information to be restored.
Great, feel free to add any comments. It would be great if we can merge this sometime in February, so we can adjust controllers already for Jazzy. |
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 will be working int this feature ros2/geometry2#643 but this PR requires some changes to be operational
- Modify name
geometry_msgs/msg/VelocityStamped
togeometry_msgs/msg/VelocityStamped.msg
- Include this new message in the
CMakeLists.txt
@destogl do you think you can include these changes?
I think that this is very close, but before merging I'd like to make sure that we have both documentation but also the example implementation that validates that we have actually covered all our bases. |
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
I added this PR StoglRobotics-forks#1 to include the required changes |
…tamped-msg Suggestions to VelocityStamped
@tfoote what do you mean about documentation? Should be adapted this in the message “header” or do you have something else in mind? |
The names right now are not referencing a standard of notation and talking to others, they have made different assumptions about what each field means. This is coming to a head as we're trying to implement the usage in geometry2. I'm going to try to make a descriptive doc we can use as a baseline. But this message needs to be documented in the comments enough that someone can use that self documenting feature to interpret the data unambiguously. |
OK, can you make a proposal? |
Ey @destogl, @azeey and @SteveMacenski , @tfoote added this documentation to detail things ros2/ros2_documentation#4235 Feel free to add any comments |
@ahcorde Thanks for putting it into a PR from my draft. Having created that writeup I'd like to propose that we change how we're using/naming the different coordinates. First of all I believe that the best analogy to the So this leaves that we need to be able to express the two entities between which the velocity is being computed. There's object A which we'll take as the moving object I would like to suggest that we consider thus just having two frame_ids inside the message which indicate the entities A and B. With another round of research I think that the standards in academia for these frames are the
And our message will represent a We don't actually need the full PoseStamped to represent any fixed offsets in the position as they will effectively drop out when the derivative in time happens. |
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.
Here's my concrete suggestions for changes to follow those external conventions. I think that following those as closely as possible will hopefully decrease the amount of confusion in the long run.
|
||
std_msgs/Header header | ||
string body_frame_id | ||
PoseStamped observation_pose |
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.
PoseStamped observation_pose | |
string space_frame_id |
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.
PoseStamped observation_pose | |
string reference_frame_id |
Changing this to reference as this is the frame of reference to which the velocity is being measured.
@@ -0,0 +1,11 @@ | |||
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. |
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.
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | |
# This expresses the velocity of a frame 'body_frame_id' in the space frame 'space_frame_id' expressed from arbitrary observation pose 'header.frame_id' at time `header.stamp`. |
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.
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | |
# This expresses the timestamped velocity vector of a frame 'body_frame_id' in the reference frame 'reference_frame_id' transformed to be expressed from arbitrary observation frame 'header.frame_id'. |
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; | ||
# - if the observation pose is an established frame, you may set its frame_id only in the pose (since the relative pose to that frame is Identity); | ||
# - 'body_frame_id' and 'header.frame_id' are identical, i.e., the velocity is observed and defined in the local coordinates system of the body | ||
# which is the usual use-case in mobile robotics. |
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.
# which is the usual use-case in mobile robotics. | |
# which is the usual use-case in mobile robotics and is also known as a body twist. |
# The message is often used in simplified versions where: | ||
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; | ||
# - if the observation pose is an established frame, you may set its frame_id only in the pose (since the relative pose to that frame is Identity); | ||
# - 'body_frame_id' and 'header.frame_id' are identical, i.e., the velocity is observed and defined in the local coordinates system of the body |
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.
# - 'body_frame_id' and 'header.frame_id' are identical, i.e., the velocity is observed and defined in the local coordinates system of the body | |
# - If the 'body_frame_id' and 'header.frame_id' are identical, i.e., the velocity is observed and defined in the local coordinates system of the body |
@@ -0,0 +1,11 @@ | |||
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | |||
# The message is often used in simplified versions where: | |||
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; |
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.
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; | |
# - If the 'space_frame_id' and 'header.frame_id' are identical, i.e., the velocity is observed and defined in the local coordinates system of the spacial frame. |
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | ||
# The message is often used in simplified versions where: | ||
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; | ||
# - if the observation pose is an established frame, you may set its frame_id only in the pose (since the relative pose to that frame is Identity); |
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.
# - if the observation pose is an established frame, you may set its frame_id only in the pose (since the relative pose to that frame is Identity); | |
# This is also known as a spacial twist. |
Thanks to @azeey for following up and flagging that this isn't actually spacial twist. Using the reference https://www.cse.lehigh.edu/~trink/Courses/RoboticsII/reading/murray-li-sastry-94-complete.pdf Pages 54-55 So I will update my suggestions based on that. When the frame in which the twist is represented is the same as the reference frame then what we're representing the body twist offset to that frame and not changing the math to be a spacial twist. And to that end we're simply transforming the body twist to the new coordinate frame. So to pull this all back together we will represent a body twist between |
@@ -0,0 +1,11 @@ | |||
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. |
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.
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | |
# This expresses the timestamped velocity vector of a frame 'body_frame_id' in the reference frame 'reference_frame_id' transformed to be expressed from arbitrary observation frame 'header.frame_id'. |
@@ -0,0 +1,11 @@ | |||
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | |||
# The message is often used in simplified versions where: |
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.
# The message is often used in simplified versions where: |
With the removal of the PoseStamped and only using the frame_ids this optional version is no longer necessary.
@@ -0,0 +1,11 @@ | |||
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | |||
# The message is often used in simplified versions where: | |||
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; |
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.
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; |
Also removed
# This expresses the timestamped velocity of a frame 'body_frame_id' in the reference frame 'header.frame_id' expressed from arbitrary observation pose 'observation_pose'. | ||
# The message is often used in simplified versions where: | ||
# - observation pose is not defined, i.e., it is assumed that the observation pose is the origin of the 'header.frame_id'; | ||
# - if the observation pose is an established frame, you may set its frame_id only in the pose (since the relative pose to that frame is Identity); |
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.
# - if the observation pose is an established frame, you may set its frame_id only in the pose (since the relative pose to that frame is Identity); |
Also removed
I created a new PR #240 to include the suggestions that Tully made |
#240 had been merged closing this one. 5.3.3 is available in rolling testing now with these changes ros/rosdistro#40550 |
This PR proposes a new message type for a full description of velocities, unambiguous definition, and the possibility to transform velocities using TF (this is currently commented out in
geometry2
library).As found in the literature and comments across the
geometry2
library,geometry_msgs/TwistStamped
is insufficient information for the complete representation.The new fields should be sufficiently described in the message. If not clear, please provide feedback for improvement.
@tfoote @SteveMacenski @azeey looking forward to your comments.
References: