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

Parse new param for enabling / disabling IMU orientation output #899

Merged
merged 10 commits into from
Sep 30, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jul 2, 2021

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

🎉 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_orientation SDF 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:

./build/ignition-gazebo4/bin/INTEGRATION_imu_system

You can also edit examples/worlds/sensors.sdf world file and update the imu sensor to use the new param:

        <sensor name="imu" type="imu">
          <always_on>1</always_on>
          <update_rate>100</update_rate>
          <visualize>true</visualize>
          <topic>imu</topic>
          <imu>
            <enable_orientation>false</enable_orientation>
          </imu>
        </sensor>

run ign-gazebo with the updated sensors.sdf file:

ign gazebo -r -v 4 <path_to_ign-gazebo>/examples/worlds/sensors.sdf

echo the imu topic:

ign topic -e -t /imu

And you should see that the msg now does not have the orientation field

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

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033 iche033 added the needs upstream release Blocked by a release of an upstream library label Jul 2, 2021
gazebo::EntityComponentManager & /*_ecm*/,
gazebo::EventManager & /*_eventMgr*/)
{
if (_sdf->HasElement("output_orientation"))
Copy link
Contributor

Choose a reason for hiding this comment

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

This would apply to all IMUs in simulation. Have you considered providing more granular configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
ahcorde previously requested changes Jul 2, 2021
test/integration/imu_system.cc Outdated Show resolved Hide resolved
const std::string sensorName = "imu_sensor";

auto topic =
"world/imu_sensor/model/imu_model/link/link/sensor/imu_sensor/imu";
Copy link
Contributor

Choose a reason for hiding this comment

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

common::joinPaths

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 chapulina mentioned this pull request Jul 13, 2021
7 tasks
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Aug 2, 2021
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033 iche033 added the needs upstream release Blocked by a release of an upstream library label Aug 3, 2021
@nkoenig nkoenig self-requested a review August 9, 2021 20:26
@codecov
Copy link

codecov bot commented Sep 15, 2021

Codecov Report

Merging #899 (9c745a4) into ign-gazebo4 (b278ccc) will increase coverage by 0.03%.
The diff coverage is 93.18%.

Impacted file tree graph

@@               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     
Impacted Files Coverage Δ
include/ignition/gazebo/EntityComponentManager.hh 100.00% <ø> (ø)
src/ServerPrivate.cc 86.93% <ø> (ø)
.../plugins/component_inspector/ComponentInspector.cc 7.14% <ø> (+0.04%) ⬆️
src/gui/plugins/scene3d/Scene3D.cc 9.91% <ø> (ø)
src/systems/physics/Physics.cc 71.85% <ø> (ø)
src/EntityComponentManager.cc 87.24% <92.68%> (+0.30%) ⬆️
src/systems/imu/Imu.cc 74.68% <100.00%> (+0.99%) ⬆️
src/gui/plugins/plot_3d/Plot3D.cc 43.47% <0.00%> (-4.35%) ⬇️

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 3fbddd1...9c745a4. Read the comment docs.

@iche033 iche033 dismissed ahcorde’s stale review September 30, 2021 22:35

comments addressed

@iche033 iche033 merged commit 4f3dd67 into ign-gazebo4 Sep 30, 2021
@iche033 iche033 deleted the imu_orientation branch September 30, 2021 22:35
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 <ichen@osrfoundation.org>

* use enable_orientation sdf elem

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

* use joinPaths

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

* add test world file

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

Co-authored-by: Louise Poubel <louise@openrobotics.org>
Co-authored-by: Nate Koenig <nate@openrobotics.org>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome needs upstream release Blocked by a release of an upstream library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants