getFrameInfo(): avoid double search for link name #1853
Conversation
There was a problem hiding this comment.
Thanks! I originally wrote the offending line like this because I wasn't sure if the pointer in getLinkModel is reliably null and would evaluate to false if the link doesn't exist.
This looks good to me except that the frame_found parameter should not be deleted, as it is used outside of the function.
| BOOST_VERIFY(checkLinkTransforms()); | ||
| frame_found = true; |
There was a problem hiding this comment.
This boolean is used outside of the function and shouldn't be deleted.
There was a problem hiding this comment.
Both, robot_link and frame_found are set in the above call to getLinkModel.
There was a problem hiding this comment.
I overlooked it, my bad. Good call.
| @@ -1019,11 +1019,9 @@ const Eigen::Isometry3d& RobotState::getFrameInfo(const std::string& frame_id, c | |||
| frame_found = true; | |||
| return IDENTITY_TRANSFORM; | |||
| } | |||
| if (robot_model_->hasLinkModel(frame_id)) | |||
| if ((robot_link = robot_model_->getLinkModel(frame_id, &frame_found))) | |||
There was a problem hiding this comment.
nit: Remove the outermost bracket?
There was a problem hiding this comment.
The extra pair of brackets is required to interpret the assignment result as a boolean (and silent compiler warnings, because this might be wrongly typed == comparison as well.)
| BOOST_VERIFY(checkLinkTransforms()); | ||
| frame_found = true; |
There was a problem hiding this comment.
I overlooked it, my bad. Good call.
| @@ -1019,11 +1019,9 @@ const Eigen::Isometry3d& RobotState::getFrameInfo(const std::string& frame_id, c | |||
| frame_found = true; | |||
| return IDENTITY_TRANSFORM; | |||
| } | |||
| if (robot_model_->hasLinkModel(frame_id)) | |||
| if ((robot_link = robot_model_->getLinkModel(frame_id, &frame_found))) | |||
* Revert moveit#1739: moveit_msgs no longer needs to be built from source This reverts commit 6e0fce3. * Remove obsolete include: moveit_msgs/srv/execute_known_trajectory.hpp * Cleanup moveit2.repos
hasLinkModel()andgetLinkModel()both search for the link name...