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

Add test case for #2824 #2878

Merged
merged 3 commits into from Oct 19, 2021
Merged

Add test case for #2824 #2878

merged 3 commits into from Oct 19, 2021

Conversation

cambel
Copy link
Contributor

@cambel cambel commented Sep 19, 2021

Test case for planning with collision objects that collide with the environment with Pilz planners (#2824). Related to #2843

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

@codecov
Copy link

codecov bot commented Sep 19, 2021

Codecov Report

Merging #2878 (21f6848) into master (d1e1c3d) will increase coverage by 0.32%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2878      +/-   ##
==========================================
+ Coverage   61.04%   61.36%   +0.32%     
==========================================
  Files         373      373              
  Lines       31727    31727              
==========================================
+ Hits        19366    19466     +100     
+ Misses      12361    12261     -100     
Impacted Files Coverage Δ
...eit_ros/manipulation/pick_place/src/pick_place.cpp 89.48% <0.00%> (-3.15%) ⬇️
...e/src/parameterization/model_based_state_space.cpp 68.54% <0.00%> (-2.79%) ⬇️
moveit_core/robot_model/src/joint_model_group.cpp 61.75% <0.00%> (-2.11%) ⬇️
...ipulation/pick_place/src/manipulation_pipeline.cpp 71.30% <0.00%> (-0.92%) ⬇️
...on/pick_place/src/approach_and_translate_stage.cpp 73.38% <0.00%> (-0.64%) ⬇️
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 77.87% <0.00%> (-0.38%) ⬇️
...kinematic_constraints/src/kinematic_constraint.cpp 75.04% <0.00%> (-0.14%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 68.13% <0.00%> (-0.13%) ⬇️
moveit_core/planning_scene/src/planning_scene.cpp 64.11% <0.00%> (+0.26%) ⬆️
.../move_group_interface/src/move_group_interface.cpp 47.87% <0.00%> (+0.93%) ⬆️
... and 6 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 d1e1c3d...21f6848. Read the comment docs.

Copy link
Member

@v4hn v4hn 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 the effort! Please have a look at the inline feedback.

@cambel
Copy link
Contributor Author

cambel commented Sep 21, 2021

Thanks for the comments. I have updated the PR.

@v4hn
Copy link
Member

v4hn commented Sep 21, 2021

CI failure:

Test node [moveit_ros_planning_interface/python_move_group_planning.py] does not exist or is not executable

Format code


Update test
Copy link
Contributor

@jschleicher jschleicher left a comment

Choose a reason for hiding this comment

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

Thank you very much for the follow-up! See another inline nitpick.

Merging master to (hopefully) fix cross-platform CI.

jschleic and others added 2 commits September 24, 2021 12:03
Co-authored-by: jschleicher <j.schleicher@pilz.de>
@jschleicher jschleicher added the awaits 2nd review one maintainer approved this request label Sep 29, 2021
@tylerjw tylerjw mentioned this pull request Oct 14, 2021
6 tasks
@v4hn
Copy link
Member

v4hn commented Oct 19, 2021

Sorry to leave this dangling for so long after review. It fell of the radar (as many others at the moment...)

Thanks again!

@v4hn v4hn merged commit 38b3010 into ros-planning:master Oct 19, 2021
@rhaschke rhaschke mentioned this pull request Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits 2nd review one maintainer approved this request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants