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

Fix errors while orientation covariance visualization in CovarianceVi… #1540

Open
wants to merge 10 commits into
base: noetic-devel
Choose a base branch
from

Conversation

UniBwTAS
Copy link
Contributor

@UniBwTAS UniBwTAS commented Aug 24, 2020

Hello,
this PR fixes some problems when visualizing the orientation uncertainty/covariance.
Steps to reproduce:

Execute in bash/zsh: rostopic pub "/test" geometry_msgs/PoseWithCovarianceStamped '{header: {stamp: 0, frame_id: "map"}, pose: {pose: {position: {x: 0, y: 0, z: 0}, orientation: {x: 0, y: 0, z: 0, w: 1}}, covariance: [1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.01, 0, 0, 0, 0, 0, 0, 0.2, 0, 0, 0, 0, 0, 0, 0.01]}}'

This publishes a pose with following orientation covarariance (roll, pitch, yaw):

0.01 0 0
0 0.2 0
0 0 0.01

This leads to the following visualization in RVIZ:
Screenshot from 2020-08-24 17-08-48

The problem is that the covariance ellipse of the z-axis (blue) has the wrong orientation. I think it should be rotated by 90 degrees. The variance is large only for the y-axis (green) while the other two are very small. When one imagines it rotates around the green y-axis then the blue z-axis goes back and forth and not left and right.

With my PR it looks like this:
Screenshot from 2020-08-24 17-27-14

Another example that it works as intended now:

Execute in bash/zsh: rostopic pub "/test" geometry_msgs/PoseWithCovarianceStamped '{header: {stamp: 0, frame_id: "map"}, pose: {pose: {position: {x: 0, y: 0, z: 0}, orientation: {x: 0, y: 0, z: 0, w: 1}}, covariance: [1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.01, 0, 0, 0, 0, 0, 0, 0.2, 0.1, 0, 0, 0, 0, 0.1, 0.2]}}'

This publishes a pose with following orientation covarariance (roll, pitch, yaw):

0.01 0 0
0 0.2 0.1
0 0.1 0.2

This leads to the following visualization in RVIZ:
Screenshot from 2020-08-24 17-20-57

Again the the covariance ellipse of the blue z-axis should be rotated by 90 degrees. Additionally we now have high variances of y- and z-axis as well as non-diagonal entries in the covariance matrix, which means that y and z rotations are highly correlated.

With my PR it looks like this:
Screenshot from 2020-08-24 18-54-51

@UniBwTAS UniBwTAS force-pushed the noetic-devel branch 2 times, most recently from 4affaad to e30f993 Compare August 24, 2020 17:44
@UniBwTAS
Copy link
Contributor Author

UniBwTAS commented Aug 24, 2020

Further examples:
rostopic pub "/test" geometry_msgs/PoseWithCovarianceStamped '{header: {stamp: 0, frame_id: "map"}, pose: {pose: {position: {x: 0, y: 0, z: 0}, orientation: {x: 0, y: 0, z: 0, w: 1}}, covariance: [1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.2, 0, 0.1, 0, 0, 0, 0, 0.01, 0, 0, 0, 0, 0.1, 0, 0.2]}}'

This publishes a pose with following orientation covarariance (roll, pitch, yaw):

0.2 0 0.1
0 0.01 0
0.1 0 0.2

This leads to the following visualization in RVIZ:
Screenshot from 2020-10-14 11-41-32

After PR looks like:
Screenshot from 2020-08-24 19-49-27

rostopic pub "/test" geometry_msgs/PoseWithCovarianceStamped '{header: {stamp: 0, frame_id: "map"}, pose: {pose: {position: {x: 0, y: 0, z: 0}, orientation: {x: 0, y: 0, z: 0, w: 1}}, covariance: [1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.1, 0, 0, 0, 0, 0, 0, 0.2, 0.1, 0, 0, 0, 0, 0.1, 0.2, 0, 0, 0, 0, 0, 0, 0.01]}}'

This publishes a pose with following orientation covarariance (roll, pitch, yaw):

0.2 0.1 0
0.1 0.2 0
0 0 0.01

This leads to the following visualization in RVIZ:
Screenshot from 2020-10-14 11-43-43

After PR looks the same:
Screenshot from 2020-08-24 19-53-08

I think everything looks good now.

@UniBwTAS
Copy link
Contributor Author

Maybe related to: #1191

@rhaschke
Copy link
Contributor

@Ellon, could you please review this, as you initially implemented covariance visualization?
I'm not sure what's the intended visualization. But I agree with @UniBwTAS that it seems to be inconsistent with the present orientation.

@Ellon
Copy link
Contributor

Ellon commented Aug 27, 2020 via email

@rhaschke
Copy link
Contributor

rhaschke commented Oct 6, 2020

@Ellon: Ping.

@Ellon
Copy link
Contributor

Ellon commented Oct 8, 2020

Hi @UniBwTAS and @rhaschke, sorry for the late reply.

@UniBwTAS I debugged all your examples "by hand" and it seems there's indeed a bug in the current code, at least for the cylinder on the z-axis. Could you show images "before PR" for your further examples? I'm having problems to compile and test noetic-devel on my current system and I cannot see how they were behaving before the PR. I imagine this would explain why you are proposing a change to orientation_offset_node_[kRoll] as well ?

While I'm at it, I'll investigate the relationship with #1191 .

@UniBwTAS
Copy link
Contributor Author

UniBwTAS commented Oct 14, 2020

Hello @Ellon

I updated the above "further examples" post. As far as I remember for actual visualization the change at the X Axis makes no difference. I just changed it to make it consistent such that the surface normal of the top of the cylinder points away from the center, I think. However this is not visible as the top and bottom look the same. I can remove the change for the X axis, if you prefer this.

@rhaschke
Copy link
Contributor

@Ellon, @UniBwTAS: What's the status of this PR?

@rhaschke
Copy link
Contributor

rhaschke commented Feb 4, 2021

@Ellon, @UniBwTAS, and @ViktorWalter: Could please review this PR (#1540) and #1576? If you all agree with the proposed solution(s), I'will be happy to merge them. Thanks a lot in advance.

@ViktorWalter
Copy link

ViktorWalter commented Feb 6, 2021

I have looked at #1540 - I can replicate the issue described here - the visualization of the "shape" attached to "Z" axis is statically rotated by 90 degrees and this commit fixes it. However, the change in line 276 as far as I can tell does nothing, as the "Covariance ellipses" are symmetrical. Still, I suggest merging #1540. This should be fully compatible with also merging #1576.
Edit: I would also suggest renaming kYaw kRoll and kPitch to better reflect their function - each of these indices refers to an object that in fact represents the pair of the other two values than its name suggests.

@rhaschke
Copy link
Contributor

rhaschke commented Feb 6, 2021

Thanks, @ViktorWalter for your feedback. I will await the feedback.

@Ellon
Copy link
Contributor

Ellon commented Feb 8, 2021

@Ellon, @UniBwTAS: What's the status of this PR?

Hi @rhaschke ! My apologies for being silent about this PR.

By looking at the commit and the examples @UniBwTAS gave it seems to be correcting the problem I didn't see at the time, but today it's not straight forward for me to test this PR in my current system (I'm using an hybrid of old ROS indigo with some other in-house things) so I couldn't run some tests with this PR by myself. I'll give myself this week evenings to have noetic running in a docker and test this PR.

@rhaschke
Copy link
Contributor

rhaschke commented Feb 9, 2021

I'll give myself this week evenings to have noetic running in a docker and test this PR.

Thanks @Ellon for helping to review this. Please also consider #1576.

@Ellon
Copy link
Contributor

Ellon commented Feb 9, 2021

Please also consider #1576.

OK.

Small update: I managed to have rviz from noetic-devel compiled inside a docker from ubuntu focal, but I'm unable to run rviz due to something related with GLX/nvidia/X11/cuda. I'll continue trying tomorrow, but if anyone knows what could be the problem please let me know.

@rhaschke
Copy link
Contributor

rhaschke commented Feb 9, 2021

Running rviz from a docker container requires some extra work as described in section "Running MoveIt container" of https://moveit.ros.org/install/docker. We do have a script gui-docker to facilitate the task.

@Ellon
Copy link
Contributor

Ellon commented Feb 10, 2021

We do have a script gui-docker to facilitate the task.

Thanks for the info. I'll take a look at it tonight.

@rhaschke
Copy link
Contributor

@Ellon, did you manage to review this PR and #1576?

…r.t. fixed frame

This option is particularly useful for messages, whose frame is moving w.r.t. fixed frame.
…values in odometry message without getting numerical issues

These large positional values can occur when the odometry message comes from a GNSS receiver where GPS positions are converted into UTM coordinates.
…w.r.t. fixed frame

This option is particularly useful for messages, whose frame is moving w.r.t. fixed frame.
…ce conditions during update cycle of all displays and cameras

During update cycle of displays and views (cameras) the tf buffer should not be updated. Otherwise it is possible that displays or cameras, which are updated at a later point in time within the cycle, use a more recent transform. This leads to undesired jittering of visuals. This affects displays, which use the latest available transform with ros::Time(). And all views/cameras as they always use the latest available transform.
…cture in order to avoid jittering of movable links due to tf latency.
Previously all materials are set opaque when alpha option is set to 1. With this commit the materials are set to their original state when alpha is set to 1.
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.

5 participants