bugfix for chomp fixed base joint bug #870
Conversation
| @@ -204,7 +204,11 @@ void ChompOptimizer::initialize() | |||
| for (size_t i = 0; i < joint_model_group_->getFixedJointModels().size(); i++) | |||
| { | |||
| const moveit::core::JointModel* model = joint_model_group_->getFixedJointModels()[i]; | |||
There was a problem hiding this comment.
Please take the chance and replace the for-loop by for (const moveit::core::JointModel* model : joint_model_group_->getFixedJointModels()).
| @@ -204,7 +204,11 @@ void ChompOptimizer::initialize() | |||
| for (size_t i = 0; i < joint_model_group_->getFixedJointModels().size(); i++) | |||
| { | |||
| const moveit::core::JointModel* model = joint_model_group_->getFixedJointModels()[i]; | |||
| fixed_link_resolution_map[model->getName()] = model->getParentLinkModel()->getParentJointModel()->getName(); | |||
| // In case this is the base joint (eg: a fixed virtual_joint) then we need to check for NULL | |||
| if (model->getParentLinkModel()) | |||
There was a problem hiding this comment.
I would prefer a
if (!model->getParentLinkModel())
continue;
However, that's probably a matter of taste.
There was a problem hiding this comment.
I think it's better to avoid goto-derivatives in general.
There was a problem hiding this comment.
As I said, that's a matter of taste. Leave it like it is.
There was a problem hiding this comment.
Yes, this is a matter of taste here.
Please keep in mind though that multiple maintainers agreed some time ago to prefer continue/early return if the code would otherwise end up with many lines indented for no reason.
We have some cases in the source where after 50+ lines of indented code there is an else branch that prints one ROS_ERROR.
There was a problem hiding this comment.
+1 to using continue or return early
Gracefully handle active root joint, which doesn't have a parent.
Description
This fixes an issue where where the chomp optimizer was requesting the parent joint of a fixed base joint without checking for Null moveit_tutorials #164
Checklist