Skip to content
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

Transform poses in Path message correctly into fixed frame. #1246

Merged
merged 3 commits into from
Mar 9, 2019

Conversation

wngreene
Copy link
Contributor

Orientations were not being transformed into fixed frame in the Path plugin.

Orientations were not being transformed into fixed frame.
@wjwwood wjwwood added the review label May 31, 2018
Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just recently took over maintenance of rviz and I'm trying to catch up with open PRs.
While I think your PR is reasonable to consider the orientation of individual poses, I'm wondering why the frame_id of all those individual stamped poses was and is still ignored. Currently, only the global frame_id of the Path msg is considered.
Is there a silent agreement that those stamps should be ignored? @wjwwood, can you shed light on this?

msg->poses[ i ].pose.orientation.x,
msg->poses[ i ].pose.orientation.y,
msg->poses[ i ].pose.orientation.z);
axes_vect[i]->setOrientation(orientation);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you simply use
axes_vect[i]->setOrientation(transform * orientation);
here?

@@ -499,11 +499,11 @@ void PathDisplay::processMessage( const nav_msgs::Path::ConstPtr& msg )
const geometry_msgs::Point& pos = msg->poses[ i ].pose.position;
Ogre::Vector3 xpos = transform * Ogre::Vector3( pos.x, pos.y, pos.z );
axes_vect[i]->setPosition(xpos);
Ogre::Quaternion orientation(msg->poses[ i ].pose.orientation.w,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use const geometry_msgs::Quaternion& quat = msg->poses[ i ].pose.orientation as a shortcut here.
I know, it wasn't used before, but if we touch this code anyway ...

@wjwwood
Copy link
Member

wjwwood commented Mar 8, 2019

Is there a silent agreement that those stamps should be ignored? @wjwwood, can you shed light on this?

Not that I'm aware. I don't have any insight based on my experience.

@ghost ghost assigned rhaschke Mar 8, 2019
@rhaschke rhaschke merged commit 0b19d88 into ros-visualization:kinetic-devel Mar 9, 2019
@ghost ghost removed the in progress label Mar 9, 2019
rhaschke pushed a commit to rhaschke/rviz that referenced this pull request Mar 9, 2019
…alization#1246)

Orientations were not being transformed into fixed frame.
Extend test/send_path.py to also send oriented poses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants