Improves documentation for setJointPositions() #1921
Conversation
|
Thanks for helping in improving MoveIt and open source robotics! |
Codecov Report
@@ Coverage Diff @@
## master #1921 +/- ##
==========================================
- Coverage 50.28% 50.28% -0.01%
==========================================
Files 313 313
Lines 24647 24647
==========================================
- Hits 12394 12393 -1
- Misses 12253 12254 +1
Continue to review full report at Codecov.
|
| @@ -490,33 +490,44 @@ class RobotState | |||
| /** \name Getting and setting joint positions, velocities, accelerations and effort | |||
| * @{ | |||
| */ | |||
|
|
|||
| /** \brief Sets position for 1 joint only. Check setVariablePositions() to set position values to multiple joints. */ | |||
There was a problem hiding this comment.
Please change all comments to this:
| /** \brief Sets position for 1 joint only. Check setVariablePositions() to set position values to multiple joints. */ | |
| ///@{ | |
| /** \brief Set position(s) for a single, potentially multi-dof joint. See setVariablePositions() to handle multiple joints. */ |
Using doxygen grouping, we can also avoid duplication of the doc.
There was a problem hiding this comment.
Sure. Thank you for your feedback.
Also, the setJointPositions/Velocities/Efforts are already grouped in one. Do you want me further break them?
There was a problem hiding this comment.
Actually, I didn't notice that there is a general comment for the group already.
In this case, my suggestion is to slightly extend this general comment only.
There was a problem hiding this comment.
Made the changes.
There was a problem hiding this comment.
Please only modify the single grouping comment at the top. There is no need to repeat the same comment over and over.
There was a problem hiding this comment.
Is this better?
There was a problem hiding this comment.
The screenshot looks good. However, this isn't yet committed, is it?
There was a problem hiding this comment.
Just committed this change now.
| @@ -487,49 +487,38 @@ class RobotState | |||
|
|
|||
| /** @} */ | |||
|
|
|||
| /** \name Getting and setting joint positions, velocities, accelerations and effort | |||
| /** \name Getting and setting joint positions, velocities, accelerations and effort for a single, potentially multiple-dof, joint | |||
There was a problem hiding this comment.
| /** \name Getting and setting joint positions, velocities, accelerations and effort for a single, potentially multiple-dof, joint | |
| /** \name Getting and setting joint positions, velocities, accelerations and effort for a single joint (potentially with more than 1 DOF) |
moveit_core/robot_state/include/moveit/robot_state/robot_state.h
Outdated
Show resolved
Hide resolved
Co-Authored-By: Felix von Drigalski <FvDrigalski@gmail.com>
Description
Fixes issue #1908.
setJointPositionsmethod is misleading for some users to believe that they can set positions for multiple joints.setVariablePositionsis for that purpose. I have added documentation explaining the same.Checklist