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 bug in applying planning scene diffs that have attached collision objects (#3124) #1251

Merged
merged 1 commit into from
May 16, 2022

Conversation

JafarAbdi
Copy link
Contributor

Description

Backport of moveit/moveit#3124

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

… objects (#3124)

* Add test that trigger the bug in applying scene that have robot state
diffs

* Fix a bug in planning scene diffs where having applying two diffs would cause them to override the aco

* Fix a bug where removing an attached collision object would keep it in the scene

* Update moveit_core/planning_scene/test/test_planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Update moveit_core/planning_scene/test/test_planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Update moveit_core/planning_scene/src/planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Check parent pointer

Co-authored-by: AndyZe <andyz@utexas.edu>
@JafarAbdi JafarAbdi requested a review from AndyZe May 16, 2022 16:06
@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #1251 (7e73858) into main (28f87cf) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1251      +/-   ##
==========================================
- Coverage   61.28%   61.21%   -0.07%     
==========================================
  Files         274      274              
  Lines       24929    24936       +7     
==========================================
- Hits        15275    15261      -14     
- Misses       9654     9675      +21     
Impacted Files Coverage Δ
moveit_core/planning_scene/src/planning_scene.cpp 51.78% <100.00%> (+2.95%) ⬆️
...nning_scene_monitor/src/planning_scene_monitor.cpp 40.79% <0.00%> (-4.93%) ⬇️
moveit_ros/moveit_servo/src/collision_check.cpp 81.54% <0.00%> (-4.61%) ⬇️
...e/collision_detection_fcl/src/collision_common.cpp 73.08% <0.00%> (-0.67%) ⬇️
moveit_ros/moveit_servo/src/servo_calcs.cpp 65.56% <0.00%> (-0.66%) ⬇️
moveit_core/robot_state/src/robot_state.cpp 47.44% <0.00%> (+0.16%) ⬆️

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 28f87cf...7e73858. Read the comment docs.

@AndyZe AndyZe merged commit d482c1d into moveit:main May 16, 2022
@JafarAbdi JafarAbdi deleted the pr-rs_diff branch May 16, 2022 19:41
@JafarAbdi JafarAbdi added backport-foxy Mergify label that triggers a PR backport to Foxy backport-galactic Mergify label that triggers a PR backport to Galactic labels Jun 15, 2022
mergify bot pushed a commit that referenced this pull request Jun 15, 2022
… objects (#3124) (#1251)

* Add test that trigger the bug in applying scene that have robot state
diffs

* Fix a bug in planning scene diffs where having applying two diffs would cause them to override the aco

* Fix a bug where removing an attached collision object would keep it in the scene

* Update moveit_core/planning_scene/test/test_planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Update moveit_core/planning_scene/test/test_planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Update moveit_core/planning_scene/src/planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Check parent pointer

Co-authored-by: AndyZe <andyz@utexas.edu>

Co-authored-by: AndyZe <andyz@utexas.edu>
(cherry picked from commit d482c1d)

# Conflicts:
#	moveit_core/planning_scene/test/test_planning_scene.cpp
mergify bot pushed a commit that referenced this pull request Jun 15, 2022
… objects (#3124) (#1251)

* Add test that trigger the bug in applying scene that have robot state
diffs

* Fix a bug in planning scene diffs where having applying two diffs would cause them to override the aco

* Fix a bug where removing an attached collision object would keep it in the scene

* Update moveit_core/planning_scene/test/test_planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Update moveit_core/planning_scene/test/test_planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Update moveit_core/planning_scene/src/planning_scene.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* Check parent pointer

Co-authored-by: AndyZe <andyz@utexas.edu>

Co-authored-by: AndyZe <andyz@utexas.edu>
(cherry picked from commit d482c1d)
tylerjw pushed a commit that referenced this pull request Jun 15, 2022
… objects (#3124) (#1251) (#1369)

Co-authored-by: AndyZe <andyz@utexas.edu>
Co-authored-by: Jafar <cafer.abdi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-foxy Mergify label that triggers a PR backport to Foxy backport-galactic Mergify label that triggers a PR backport to Galactic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants