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

PS: do not list object as destroyed when they get attached #2864

Merged
merged 2 commits into from Sep 3, 2021

Conversation

v4hn
Copy link
Contributor

@v4hn v4hn commented Sep 2, 2021

The information in the diff is redundant because attaching an object implies its removal from the PlanningScene.
In the unlikely case you relied on the REMOVE entry in the diff message, use the newly attached collision object to indicate the same instead.

Since fd88dd6 applying the message with both entries via PlanningScene::usePlanningSceneMsg fails, but messages generated by PlanningScene::getPlanningSceneDiffMsg should always be accepted there!

This currently breaks task execution through the MTC capability here whenever an object becomes attached (e.g., in the pick-place demo - aside from the missing frame issue).

Edit: also warn when anything tries to remove a world object that does not exist.

The information in the diff is redundant because attaching implies the removal from the PlanningScene.
In the unlikely case you relied on the REMOVE entry in the diff message,
use the newly attached collision object to indicate the same instead.
@codecov
Copy link

codecov bot commented Sep 2, 2021

Codecov Report

Merging #2864 (98cf3ed) into master (e1f6c6a) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2864      +/-   ##
==========================================
+ Coverage   60.88%   60.99%   +0.12%     
==========================================
  Files         368      373       +5     
  Lines       31715    31731      +16     
==========================================
+ Hits        19306    19351      +45     
+ Misses      12409    12380      -29     
Impacted Files Coverage Δ
moveit_core/planning_scene/src/planning_scene.cpp 63.85% <100.00%> (+0.54%) ⬆️
...ipulation/pick_place/src/manipulation_pipeline.cpp 69.45% <0.00%> (-1.85%) ⬇️
...n_fcl/src/collision_detector_fcl_plugin_loader.cpp 100.00% <0.00%> (ø)
...tection_fcl/collision_detector_fcl_plugin_loader.h 100.00% <0.00%> (ø)
...n_bullet/collision_detector_bullet_plugin_loader.h 100.00% <0.00%> (ø)
...et/src/collision_detector_bullet_plugin_loader.cpp 100.00% <0.00%> (ø)
...lude/moveit/collision_detection/collision_plugin.h 100.00% <0.00%> (ø)
moveit_core/robot_state/src/conversions.cpp 64.95% <0.00%> (+0.80%) ⬆️
...s/ompl/ompl_interface/src/ompl_planner_manager.cpp 56.93% <0.00%> (+3.08%) ⬆️
... and 2 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 e1f6c6a...98cf3ed. Read the comment docs.

@rhaschke rhaschke merged commit 5b27bce into moveit:master Sep 3, 2021
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

2 participants