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.

Sign up for GitHub

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

Jump to bottom

Check imu orientation field #168

Merged
merged 2 commits into from
Jul 2, 2021
Merged

Check imu orientation field #168

merged 2 commits into from
Jul 2, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jul 2, 2021 •
edited
Loading

Signed-off-by: Ian Chen ichen@osrfoundation.org

🎉 New feature

Summary

related PRs

Ign may not always publish imu orientation values (see PRs above). The changes here make sure that the bridge checks to see if the field exists before converting to the orientation field in the ROS imu msg. If the field does not exist, we set the 1st element of the orientation covariance matrix to -1 per ROS imu msg documentation

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

@iche033
check imu orientation field
e958c0d 
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033 iche033 requested a review from chapulina as a code owner July 2, 2021 00:05
@iche033 iche033 changed the title check imu orientation field Check imu orientation field Jul 2, 2021
@iche033 iche033 mentioned this pull request Jul 2, 2021
Merged
@chapulina chapulina added the ROS 1 ROS 1 label Jul 2, 2021
chapulina
chapulina suggested changes Jul 2, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

How about checking for -1 when the message comes from ROS?

https://github.com/ignitionrobotics/ros_ign/blob/6cb9436a1b5bb9ad01c017c2dbdcbfec99787617/ros_ign_bridge/src/convert.cpp#L961

@iche033
check -1 in imu orientation cov field from ros to ign
5978fc2 
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033
Copy link
Contributor Author

iche033 commented Jul 2, 2021

How about checking for -1 when the message comes from ROS?

yep added check in 5978fc2

@chapulina chapulina merged commit 3af3d38 into melodic Jul 2, 2021
@chapulina chapulina deleted the imu_orientation branch July 2, 2021 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

Assignees
No one assigned
Labels
ROS 1 ROS 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iche033 @chapulina