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

Pose, twist and odometry with covariance msgs #224

Merged
merged 7 commits into from
Mar 9, 2022

Conversation

adityapande-1995
Copy link
Contributor

@adityapande-1995 adityapande-1995 commented Feb 23, 2022

Signed-off-by: Aditya [email protected]

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

🎉 New feature

Summary

This PR adds 2 new message types : TwistWithCovariance and PoseWithCovariance which have covariance matrices along with the original twist / pose message. This type was available in geometry_msgs/*WithCovariance and was used in gazebo_ros_pkgs in cases where the poses have noise associated with them.

Also fixes a minor typo in twist.proto

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@adityapande-1995 adityapande-1995 changed the title Added pose and twist with covariance msgs Pose and twist with covariance msgs Feb 23, 2022
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Feb 23, 2022
@codecov
Copy link

codecov bot commented Feb 23, 2022

Codecov Report

Merging #224 (91507a8) into ign-msgs8 (e7c0e3c) will not change coverage.
The diff coverage is n/a.

❗ Current head 91507a8 differs from pull request most recent head 78ffa6a. Consider uploading reports for the commit 78ffa6a to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           ign-msgs8     #224   +/-   ##
==========================================
  Coverage      85.38%   85.38%           
==========================================
  Files              9        9           
  Lines            951      951           
==========================================
  Hits             812      812           
  Misses           139      139           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7c0e3c...78ffa6a. Read the comment docs.

@scpeters
Copy link
Member

actually, have you tested whether the ros_ign bridge properly converts this to the corresponding ROS message?

@scpeters
Copy link
Member

I'd rather not merge this to a stable branch if it hasn't been tested with the bridge

@adityapande-1995
Copy link
Contributor Author

I haven't tested it with the ros_ign bridge yet. On it 👍

@adityapande-1995 adityapande-1995 changed the title Pose and twist with covariance msgs Pose, twist and odometry with covariance msgs Mar 2, 2022
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

One tiny nit

proto/ignition/msgs/odometry_with_covariance.proto Outdated Show resolved Hide resolved
Copy link

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM, one question about style below.

proto/ignition/msgs/pose_with_covariance.proto Outdated Show resolved Hide resolved
proto/ignition/msgs/twist_with_covariance.proto Outdated Show resolved Hide resolved
Signed-off-by: Aditya <[email protected]>
Signed-off-by: Aditya <[email protected]>
@adityapande-1995
Copy link
Contributor Author

PR to bridge these msgs to ROS : gazebosim/ros_gz#222

@scpeters scpeters merged commit b41812f into ign-msgs8 Mar 9, 2022
@scpeters scpeters deleted the aditya/covariance_msgs branch March 9, 2022 21:17
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants