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 joint controller with empty joint velocity data #937

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jul 27, 2021

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

🦟 Bug fix

Summary

Joint controller system assumes that joint velocity data is always available and tries to access the data by indexing into an std::vector object. However, if the physics system is not loaded, the vector will be empty and the system will throw an 'std::out_of_range' error. This PR checks for empty vector before doing any indexing.

One example of this happening is when trying to run the collada exporter of a model that has a joint controller but in a world without the physics system.

To reproduce:

Here's the world file

tmp_world.sdf
<sdf version="1.6">
  <world name="marble_hd2_sensor_config_1">
    <plugin 
      filename="ignition-gazebo-collada-world-exporter-system" 
      name="ignition::gazebo::systems::ColladaWorldExporter">
    </plugin>
    <include>
      <static>true</static>
      <name>body</name>
      <pose>0 0 0 0 0 0</pose>
      <uri>https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/marble_hd2_sensor_config_1</uri>
    </include>
  </world> 
</sdf>

and run this cmd:

ign gazebo -s -r --iterations 1 tmp_world.sdf

Without the changes in this PR, you should get a crash with backtrace pointing to the joint controller system

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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 requested a review from chapulina as a code owner July 27, 2021 18:56
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Jul 27, 2021
@codecov
Copy link

codecov bot commented Jul 27, 2021

Codecov Report

Merging #937 (385d542) into ign-gazebo4 (488cc87) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 385d542 differs from pull request most recent head f2d8825. Consider uploading reports for the commit f2d8825 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo4     #937      +/-   ##
===============================================
- Coverage        65.98%   65.92%   -0.06%     
===============================================
  Files              239      239              
  Lines            17686    17688       +2     
===============================================
- Hits             11670    11661       -9     
- Misses            6016     6027      +11     
Impacted Files Coverage Δ
src/systems/joint_controller/JointController.cc 77.65% <100.00%> (+0.48%) ⬆️
...int_position_controller/JointPositionController.cc 53.14% <0.00%> (-7.43%) ⬇️
src/gui/GuiRunner.cc 88.00% <0.00%> (+2.66%) ⬆️

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 488cc87...f2d8825. Read the comment docs.

@chapulina chapulina merged commit b3b1bca into ign-gazebo4 Jul 28, 2021
@chapulina chapulina deleted the joint_cont_check branch July 28, 2021 00:09
blakermchale pushed a commit to blakermchale/ign-gazebo that referenced this pull request Aug 15, 2021
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Blake McHale <mchale.blake@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants