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

[jog_arm] Fix crash on empty jog msgs #2094

Merged
merged 2 commits into from
May 20, 2020
Merged

[jog_arm] Fix crash on empty jog msgs #2094

merged 2 commits into from
May 20, 2020

Conversation

rfeistenauer
Copy link
Contributor

Description

Added a Point message to an empty joint_trajectory to prevent the jog server to crash on icall of invalid index.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@welcome
Copy link

welcome bot commented May 19, 2020

Thanks for helping in improving MoveIt and open source robotics!

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2020

Codecov Report

Merging #2094 into master will increase coverage by 3.72%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2094      +/-   ##
==========================================
+ Coverage   54.07%   57.80%   +3.72%     
==========================================
  Files         319      328       +9     
  Lines       25019    25663     +644     
==========================================
+ Hits        13530    14834    +1304     
+ Misses      11489    10829     -660     
Impacted Files Coverage Δ
...veit_experimental/moveit_jog_arm/src/jog_calcs.cpp 72.97% <66.66%> (-2.03%) ⬇️
.../ompl_interface/src/detail/constrained_sampler.cpp 46.15% <0.00%> (-17.95%) ⬇️
...rimental/moveit_jog_arm/src/jog_interface_base.cpp 73.01% <0.00%> (-5.62%) ⬇️
...cessing/src/time_optimal_trajectory_generation.cpp 75.83% <0.00%> (-0.37%) ⬇️
...raint_samplers/src/default_constraint_samplers.cpp 82.54% <0.00%> (-0.37%) ⬇️
...pl/ompl_interface/src/planning_context_manager.cpp 70.94% <0.00%> (-0.17%) ⬇️
...veit_core/robot_model/src/floating_joint_model.cpp 48.35% <0.00%> (ø)
...bot_state/include/moveit/robot_state/robot_state.h 91.02% <0.00%> (ø)
...veit_jog_arm/include/moveit_jog_arm/jog_arm_data.h 100.00% <0.00%> (ø)
... and 52 more

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 ad88e18...8040e08. Read the comment docs.

Copy link
Member

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

Just noticed that the tutorial was crashing because of this issue. Great catch!

To be consistent, can you please delete L81 of jog_calcs.cpp? Because the "effort" field isn't used. Delete this line:

internal_joint_state_.effort.resize(num_joints_);

it's not used internally and should be consistent.
@v4hn
Copy link
Contributor

v4hn commented May 20, 2020

I removed the line as requested by @AndyZe .

I take it this can be merged once CI passes.

@rfeistenauer
Copy link
Contributor Author

Thanks both @AndyZe for the review and @v4hn for adding the requested change.
I gues this can be merged now.

@AndyZe
Copy link
Member

AndyZe commented May 20, 2020

Thanks v4hn. Looks ready to merge to me, too

Copy link
Member

@tylerjw tylerjw left a comment

Choose a reason for hiding this comment

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

lgtm... I tested this and it fixes the bug. Please merge as we need this to have a stable melodic backport of jog_arm.

@v4hn v4hn merged commit 11cbc82 into moveit:master May 20, 2020
@welcome
Copy link

welcome bot commented May 20, 2020

Congrats on getting your first MoveIt pull request merged and improving open source robotics!

tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 20, 2020
* adds empty point msg to prevent invalid indexing

* do not add effort

it's not used internally and should be consistent.

Co-authored-by: Michael Görner <me@v4hn.de>
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 20, 2020
* adds empty point msg to prevent invalid indexing

* do not add effort

it's not used internally and should be consistent.

Co-authored-by: Michael Görner <me@v4hn.de>
pradeepr-roboticist pushed a commit to pradeepr-roboticist/moveit that referenced this pull request Jun 3, 2020
* adds empty point msg to prevent invalid indexing

* do not add effort

it's not used internally and should be consistent.

Co-authored-by: Michael Görner <me@v4hn.de>
sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jog_server dies if first msg is a joint_delta_jog_cmd
5 participants