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

Sanitize CHOMP initialization method parameter #2540

Merged
merged 2 commits into from
Mar 10, 2021

Conversation

onionsflying
Copy link
Contributor

@onionsflying onionsflying commented Mar 8, 2021

Return false when interpolation method is invalid.

Edit by v4hn: Additionally verify parameter during planner initialization.

@welcome
Copy link

welcome bot commented Mar 8, 2021

Thanks for helping in improving MoveIt and open source robotics!

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #2540 (dca390a) into master (30a170f) will decrease coverage by 0.01%.
The diff coverage is 22.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2540      +/-   ##
==========================================
- Coverage   60.20%   60.19%   -0.00%     
==========================================
  Files         351      351              
  Lines       26485    26492       +7     
==========================================
+ Hits        15942    15944       +2     
- Misses      10543    10548       +5     
Impacted Files Coverage Δ
...er/include/chomp_motion_planner/chomp_parameters.h 100.00% <ø> (ø)
...homp/chomp_motion_planner/src/chomp_parameters.cpp 83.88% <0.00%> (-16.12%) ⬇️
...s/chomp/chomp_motion_planner/src/chomp_planner.cpp 70.00% <0.00%> (-0.70%) ⬇️
...ners/chomp/chomp_interface/src/chomp_interface.cpp 95.84% <66.67%> (-4.16%) ⬇️
...raint_samplers/src/default_constraint_samplers.cpp 81.53% <0.00%> (-0.36%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 67.62% <0.00%> (+0.16%) ⬆️
...meterization/work_space/pose_model_state_space.cpp 84.56% <0.00%> (+1.48%) ⬆️

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 30a170f...be3448b. Read the comment docs.

@v4hn
Copy link
Contributor

v4hn commented Mar 8, 2021

Thanks for your contribution @onionsflying .
I extended your one-line patch to validate parameters during setup already.

Please verify this works for you too.

@onionsflying
Copy link
Contributor Author

Thanks, it looks good to me. @v4hn

Also return false when interpolation method is somehow invalid.

Co-Author: Cong Liu <liucong.cdhaw@gmail.com>
Copy link
Contributor

@simonschmeisser simonschmeisser left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! Don't feel annoyed by my nitpicking please

@simonschmeisser
Copy link
Contributor

Oh and one more thing: there's an edit link somewhere on this page and you could use it to set a more descriptive PR title (you could copy the commit message)

@v4hn v4hn changed the title Update chomp_planner.cpp Sanitize CHOMP initialization method parameter Mar 10, 2021
@henningkayser henningkayser merged commit ae7d1ad into moveit:master Mar 10, 2021
@welcome
Copy link

welcome bot commented Mar 10, 2021

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

@tylerjw tylerjw mentioned this pull request Apr 9, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request Apr 29, 2021
@tylerjw tylerjw mentioned this pull request Apr 29, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request May 3, 2021
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.

None yet

4 participants