Planning Scene Monitor Node Executor

[jrgnicho]

Description

Executes internal timer callback using a Single Threaded Executor, this is an attempt to mimic the ROS1 behavior that used an Asynch Spinner and an internal Callback Queue to manage the timer callback invocation. Need some guidance on how best to test this.

Checklist

• Required by CI: Code is auto formatted using clang-format
• Document API changes relevant to the user in the MIGRATION.md notes
• Create tests, which fail without this PR reference
• While waiting for someone to review your request, please help review another open pull request to support the maintainers

[codecov]

Codecov Report

Merging #230 into master will decrease coverage by 0.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
- Coverage   46.63%   46.45%   -0.19%     
==========================================
  Files         122      123       +1     
  Lines       12740    12781      +41     
==========================================
- Hits         5941     5937       -4     
- Misses       6799     6844      +45     

Impacted Files	Coverage Δ
...e/src/parameterization/model_based_state_space.cpp	65.97% <0.00%> (-2.78%)	⬇️
moveit_core/planning_scene/src/planning_scene.cpp	33.68% <0.00%> (-0.10%)	⬇️
moveit_core/profiler/src/profiler.cpp	64.70% <0.00%> (ø)
moveit_core/robot_model/src/robot_model.cpp	76.38% <0.00%> (ø)
...veit_core/robot_model/src/revolute_joint_model.cpp	79.52% <0.00%> (ø)
...lanners/ompl/ompl_interface/src/ompl_interface.cpp	0.00% <0.00%> (ø)
.../transforms/include/moveit/transforms/transforms.h	100.00% <0.00%> (ø)
...lision_distance_field/src/collision_env_hybrid.cpp	0.00% <0.00%> (ø)
...t_state/include/moveit/robot_state/attached_body.h	87.50% <0.00%> (ø)
...raint_samplers/src/default_constraint_samplers.cpp	73.42% <0.00%> (ø)
... and 18 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 247b667...ed89d08. Read the comment docs.

[jrgnicho]

I tested this change by running the run_moveit_cpp demo
This is ready for review

[jrgnicho]

@henningkayser the CI just passed

[henningkayser]

@jrgnicho looks good! But shouldn't we then use the private node for running the other callbacks as well? The original implementation had an AsyncSpinner spinning a callback queue assigned to the node handle (which effectively runs the callbacks in a separate thread). Here we initialize the executor in a separate thread and use this for spinning a private node. So for actually running the callbacks in that thread I think we have to initialize publishers/subscribers/services with the private node instead of node_. Related to this, there is currently being worked on supporting callback groups (ros2/rclcpp#519) which might at some point allow a more direct replacement of the old behavior.

[jrgnicho]

@henningkayser I've made the requested changes

[henningkayser]

I fixed name and namespace of the private node (get_fully_qualified_name() was giving the full namespace which would result in an invalid node name). Now everything seems to work just fine.