Fix thread-safety issues in Franka control threads#1985
Open
fspindle wants to merge 3 commits into
Open
Conversation
Several control_thread implementations read variables shared with the calling thread (setVelocity()/setForceTorque()) without holding the mutex that protects them, while the corresponding writes were already locked. This was a data race between the 1kHz libfranka callback thread and the user thread. vpJointVelTrajGenerator_impl.cpp: - joint_velocity_callback: take a local copy of dq_des under mutex lock before use, instead of reading the shared array directly (mirrors the existing v_cart_des pattern). - Move the vpJointVelTrajGenerator instance out of the lambdas: it was declared `static` inside joint_velocity_callback and cartesian_velocity_callback, so two concurrent control_thread executions (e.g. two robots) would share and corrupt the same trajectory generator state. It is now a local variable of control_thread, captured by reference. - cartesian_velocity_callback: fix applyVel() being called with dq_des_local (a copy of the JOINT_STATE-only dq_des parameter, irrelevant in cartesian mode) instead of dq_des_ (the velocity actually computed from v_cart_des via the Jacobian pseudo-inverse). This bug made cartesian velocity control ineffective. Removed the now-unused dq_des_local/lock block and log the correct dq_des_ in dq-des.log instead. vpForceTorqueGenerator_impl.cpp: - force_joint_control_callback: take a local copy of tau_J_des under mutex lock before use. - force_cart_control_callback: take a single local snapshot of ft_cart_des under mutex lock at the start of the cycle, reused for both control and logging (previously read without protection). - Replace the BEGIN_VISP_NAMESPACE-scope global ft_cart_des_prev, which would be shared/raced across concurrent control_thread instances, with a variable local to control_thread. Restore the tau_diff_prev.log semantics (diff against the previous cycle's desired force/torque) that were incorrectly reduced to always logging zero when the global variable was first removed.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1985 +/- ##
==========================================
+ Coverage 48.71% 48.72% +0.01%
==========================================
Files 532 532
Lines 69402 69398 -4
Branches 32436 32431 -5
==========================================
+ Hits 33810 33816 +6
+ Misses 31445 31438 -7
+ Partials 4147 4144 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.