Parse new param for enabling / disabling IMU orientation output #899
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ian Chen <[email protected]>
This was referenced Jul 2, 2021
iche033
added
the
needs upstream release
chapulina
reviewed
Jul 2, 2021
src/systems/imu/Imu.cc
Outdated
| gazebo::EntityComponentManager & /*_ecm*/, | ||
| gazebo::EventManager & /*_eventMgr*/) | ||
| { | ||
| if (_sdf->HasElement("output_orientation")) |
There was a problem hiding this comment.
This would apply to all IMUs in simulation. Have you considered providing more granular configuration?
There was a problem hiding this comment.
updated changes in this PR to parse a new enable_orientation SDF element (gazebosim/sdformat#651) that allows each imu sensor to enable/disable orientation output. 48e171f
ahcorde
previously requested changes
Jul 2, 2021
| const std::string sensorName = "imu_sensor"; | ||
|
|
||
| auto topic = | ||
| "world/imu_sensor/model/imu_model/link/link/sensor/imu_sensor/imu"; |
There was a problem hiding this comment.
hmm this string if for topic name so I think it's not necessary to use joinPaths as that's intended for file paths?
chapulina
removed
the
needs upstream release
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
iche033
added
the
needs upstream release
Codecov Report
@@ Coverage Diff @@
## ign-gazebo4 #899 +/- ##
===============================================
+ Coverage 67.13% 67.16% +0.03%
===============================================
Files 243 243
Lines 18215 18253 +38
===============================================
+ Hits 12229 12260 +31
- Misses 5986 5993 +7
Continue to review full report at Codecov.
|
nkoenig
approved these changes
Sep 27, 2021
WilliamLewww
pushed a commit
to WilliamLewww/ign-gazebo
that referenced
this pull request
Dec 7, 2021
…bosim#899) * parse imu orientation param Signed-off-by: Ian Chen <[email protected]> * use enable_orientation sdf elem Signed-off-by: Ian Chen <[email protected]> * use joinPaths Signed-off-by: Ian Chen <[email protected]> * add test world file Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Louise Poubel <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Signed-off-by: William Lew <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Ian Chen [email protected]
🎉 New feature
Summary
Depends on:
The PR updates the IMU system to parse a new param called<output_orientation>to determine whether or not to publish orientation values. By default, the variable is set to true to preserve the existing behavior.Updated IMU system to parse a new
enable_orientationSDF element to determine whether or not to generate and publish orientation values. By default, the variable is set to true to preserve the existing behavior.Test it
run the imu integration test:
You can also edit
examples/worlds/sensors.sdfworld file and update the imu sensor to use the new param:run ign-gazebo with the updated
sensors.sdffile:echo the imu topic:
And you should see that the msg now does not have the
orientationfieldChecklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge