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 Callback execution in MGI #1305

Merged

Conversation

henningkayser
Copy link
Member

@henningkayser henningkayser commented Jun 2, 2022

The initial implementation with the private node allowed for concurrent spinning of the same node, producing runtime exceptions. This change removes the need for a private node by letting MGI manage its own CallbackGroup and Executor thread.

Fixes #1302

@henningkayser henningkayser added WMD22 backport-galactic Mergify label that triggers a PR backport to Galactic backport-humble Mergify label that triggers a PR backport to Humble labels Jun 2, 2022
@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #1305 (e12b334) into main (ed7c6c9) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1305      +/-   ##
==========================================
- Coverage   50.92%   50.90%   -0.02%     
==========================================
  Files         381      381              
  Lines       31786    31786              
==========================================
- Hits        16184    16176       -8     
- Misses      15602    15610       +8     
Impacted Files Coverage Δ
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 75.36% <0.00%> (-1.07%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 45.29% <0.00%> (-0.43%) ⬇️
moveit_core/robot_state/src/robot_state.cpp 47.37% <0.00%> (-0.07%) ⬇️

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 ed7c6c9...e12b334. Read the comment docs.

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.

This looks like a good change, merging.

The initial implementation with the private node allowed for
concurrent spinning of the same node, producing runtime exceptions.
This change removes the need for a private node by letting MGI
manage its own CallbackGroup and Executor thread.
@tylerjw tylerjw force-pushed the pr/fix_mgi_callback_execution branch from 24646d8 to e12b334 Compare July 29, 2022 13:06
@JafarAbdi JafarAbdi self-assigned this Jul 29, 2022
@henningkayser henningkayser merged commit 042186a into moveit:main Jul 29, 2022
mergify bot pushed a commit that referenced this pull request Jul 29, 2022
The initial implementation with the private node allowed for concurrent spinning of the same node, producing runtime exceptions. This change removes the need for a private node by letting MGI manage its own CallbackGroup and Executor thread.

(cherry picked from commit 042186a)
mergify bot pushed a commit that referenced this pull request Jul 29, 2022
The initial implementation with the private node allowed for concurrent spinning of the same node, producing runtime exceptions. This change removes the need for a private node by letting MGI manage its own CallbackGroup and Executor thread.

(cherry picked from commit 042186a)
henningkayser added a commit that referenced this pull request Aug 9, 2022
The initial implementation with the private node allowed for concurrent spinning of the same node, producing runtime exceptions. This change removes the need for a private node by letting MGI manage its own CallbackGroup and Executor thread.

(cherry picked from commit 042186a)
henningkayser added a commit that referenced this pull request Aug 10, 2022
The initial implementation with the private node allowed for concurrent spinning of the same node, producing runtime exceptions. This change removes the need for a private node by letting MGI manage its own CallbackGroup and Executor thread.

(cherry picked from commit 042186a)

Co-authored-by: Henning Kayser <henningkayser@picknik.ai>
henningkayser added a commit that referenced this pull request Aug 10, 2022
The initial implementation with the private node allowed for concurrent spinning of the same node, producing runtime exceptions. This change removes the need for a private node by letting MGI manage its own CallbackGroup and Executor thread.

(cherry picked from commit 042186a)
henningkayser added a commit that referenced this pull request Aug 10, 2022
The initial implementation with the private node allowed for concurrent spinning of the same node, producing runtime exceptions. This change removes the need for a private node by letting MGI manage its own CallbackGroup and Executor thread.

(cherry picked from commit 042186a)

Co-authored-by: Henning Kayser <henningkayser@picknik.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-galactic Mergify label that triggers a PR backport to Galactic backport-humble Mergify label that triggers a PR backport to Humble WMD22
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RViZ dies when switching planner during planning/execution
3 participants