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

Servo: protect paused_ flag, for thread safety #2494

Merged
merged 1 commit into from
Jan 22, 2021

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Jan 21, 2021

In MoveIt Servo I was calling setPaused() from another thread while printing the value of paused_ here in ServoCalcs. And I saw that it often didn't change. This fixes it.

@AndyZe AndyZe requested a review from tylerjw January 21, 2021 15:55
@AndyZe AndyZe changed the title Protect paused_ flag, for thread safety Servo: Protect paused_ flag, for thread safety Jan 21, 2021
@AndyZe AndyZe changed the title Servo: Protect paused_ flag, for thread safety Servo: protect paused_ flag, for thread safety Jan 21, 2021
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.

lgtm

@codecov
Copy link

codecov bot commented Jan 21, 2021

Codecov Report

Merging #2494 (c351a74) into master (664ae01) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2494      +/-   ##
==========================================
- Coverage   60.26%   60.26%   -0.00%     
==========================================
  Files         351      351              
  Lines       26477    26477              
==========================================
- Hits        15955    15954       -1     
- Misses      10522    10523       +1     
Impacted Files Coverage Δ
moveit_ros/moveit_servo/src/servo_calcs.cpp 63.08% <100.00%> (ø)
...raint_samplers/src/default_constraint_samplers.cpp 81.53% <0.00%> (-0.36%) ⬇️

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 664ae01...c351a74. Read the comment docs.

@tylerjw
Copy link
Member

tylerjw commented Jan 21, 2021

This is failing Travis due to docker pull limits: https://travis-ci.com/github/ros-planning/moveit/jobs/473939663

We have this issue to track this problem: #2493

I don't know when I'll have time to work on this. If anyone has the capacity to figure it out I'd appreciate it. @nbbrooks

@AndyZe
Copy link
Member Author

AndyZe commented Jan 21, 2021

^At least it's only 5 bucks a month to get unlimited Docker pulls

@AndyZe AndyZe merged commit 0e59113 into moveit:master Jan 22, 2021
@tylerjw tylerjw mentioned this pull request Apr 9, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request Apr 29, 2021
@tylerjw tylerjw mentioned this pull request Apr 29, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request May 3, 2021
tylerjw pushed a commit that referenced this pull request May 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