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

Improve JogArm thread handling #1816

Merged

Conversation

henningkayser
Copy link
Member

@henningkayser henningkayser commented Dec 23, 2019

This moves thread handling to the JogBaseInterface class and adds atomic bool flags for stopping all main loops from outside their threads.

Also, this fixes an issue where the CollisionCheckThread would indefinitely wait for command messages and won't get to run any collision checks using the C++ API.

@henningkayser henningkayser force-pushed the pr-improve_thread_handling branch 3 times, most recently from 56d0568 to 819b726 Compare December 23, 2019 22:32
@codecov-io
Copy link

codecov-io commented Dec 23, 2019

Codecov Report

Merging #1816 into master will increase coverage by 0.07%.
The diff coverage is 93.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1816      +/-   ##
==========================================
+ Coverage   48.44%   48.51%   +0.07%     
==========================================
  Files         297      298       +1     
  Lines       23278    23301      +23     
==========================================
+ Hits        11276    11304      +28     
+ Misses      12002    11997       -5
Impacted Files Coverage Δ
.../moveit_jog_arm/include/moveit_jog_arm/jog_calcs.h 100% <ø> (ø) ⬆️
...og_arm/include/moveit_jog_arm/jog_interface_base.h 100% <ø> (ø) ⬆️
...rm/include/moveit_jog_arm/collision_check_thread.h 100% <ø> (ø)
...rimental/moveit_jog_arm/src/jog_interface_base.cpp 83.51% <100%> (+4.64%) ⬆️
...erimental/moveit_jog_arm/src/jog_ros_interface.cpp 73.61% <100%> (+0.37%) ⬆️
...ntal/moveit_jog_arm/src/collision_check_thread.cpp 90% <89.18%> (+3.95%) ⬆️
...veit_experimental/moveit_jog_arm/src/jog_calcs.cpp 69.59% <91.66%> (+0.41%) ⬆️
...ma_kinematics_plugin/src/lma_kinematics_plugin.cpp 81.2% <0%> (+2.25%) ⬆️

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 e0b4f85...85a46bc. Read the comment docs.

@AndyZe
Copy link
Member

AndyZe commented Dec 27, 2019

The integration tests still pass, the tutorial still works, the CPU load looks like it hasn't changed much. Looking good so far. Now I'll review the code.

@@ -49,7 +49,7 @@ int main(int argc, char** argv)

// Run the jogging C++ interface in a new thread to ensure a constant outgoing message rate.
moveit_jog_arm::JogCppApi jog_interface;
std::thread jogging_thread(&moveit_jog_arm::JogCppApi::mainLoop, &jog_interface);
std::thread jogging_thread([&]() { jog_interface.startMainLoop(); });
Copy link
Member

Choose a reason for hiding this comment

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

This syntax is weird to me. What's going on here?

Is this similar to this?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a lambda.

Copy link
Member

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

I don't see any issues with the code. 👍

@henningkayser henningkayser merged commit bc752b7 into moveit:master Dec 30, 2019
@henningkayser henningkayser deleted the pr-improve_thread_handling branch December 30, 2019 21:57
@tylerjw tylerjw mentioned this pull request May 8, 2020
20 tasks
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 12, 2020
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 12, 2020
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 20, 2020
sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024
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

5 participants