Closed torydebra closed 4 years ago
Found out how moveit parse the mimics. If I understood well, first loop all joints to see if they are mimic (i.e. they have mimic tag). If so, mimic params (multiplier, offset and father joint) are stored along with the joint name. After, it loops all the mimics and, for each one, it goes in the "father" joint. From "father" (if this is mimic) it goes to its father... and so on until the actuated "ancestor" joint. Then Multipliers, offset and ancestor name are updated considering the whole "family chain" (such that each mimic joint point to the real actuated joint with right params, in general a mimic joint can mimic another mimic joint).
void moveit::core::RobotModel::buildMimic(const urdf::ModelInterface& urdf_model)
{
// compute mimic joints
for (std::size_t i = 0; i < joint_model_vector_.size(); ++i)
{
const urdf::Joint* jm = urdf_model.getJoint(joint_model_vector_[i]->getName()).get();
if (jm)
if (jm->mimic)
{
JointModelMap::const_iterator jit = joint_model_map_.find(jm->mimic->joint_name);
if (jit != joint_model_map_.end())
{
if (joint_model_vector_[i]->getVariableCount() == jit->second->getVariableCount())
joint_model_vector_[i]->setMimic(jit->second, jm->mimic->multiplier, jm->mimic->offset);
else
logError("Join '%s' cannot mimic joint '%s' because they have different number of DOF",
joint_model_vector_[i]->getName().c_str(), jm->mimic->joint_name.c_str());
}
else
logError("Joint '%s' cannot mimic unknown joint '%s'", joint_model_vector_[i]->getName().c_str(),
jm->mimic->joint_name.c_str());
}
}
// in case we have a joint that mimics a joint that already mimics another joint, we can simplify things:
bool change = true;
while (change)
{
change = false;
for (std::size_t i = 0; i < joint_model_vector_.size(); ++i)
if (joint_model_vector_[i]->getMimic())
{
if (joint_model_vector_[i]->getMimic()->getMimic())
{
joint_model_vector_[i]->setMimic(
joint_model_vector_[i]->getMimic()->getMimic(),
joint_model_vector_[i]->getMimicFactor() * joint_model_vector_[i]->getMimic()->getMimicFactor(),
joint_model_vector_[i]->getMimicOffset() +
joint_model_vector_[i]->getMimicFactor() * joint_model_vector_[i]->getMimic()->getMimicOffset());
change = true;
}
if (joint_model_vector_[i] == joint_model_vector_[i]->getMimic())
{
logError("Cycle found in joint that mimic each other. Ignoring all mimic joints.");
for (std::size_t i = 0; i < joint_model_vector_.size(); ++i)
joint_model_vector_[i]->setMimic(NULL, 0.0, 0.0);
change = false;
break;
}
}
}
// build mimic requests
for (std::size_t i = 0; i < joint_model_vector_.size(); ++i)
if (joint_model_vector_[i]->getMimic())
{
const_cast<JointModel*>(joint_model_vector_[i]->getMimic())->addMimicRequest(joint_model_vector_[i]);
mimic_joints_.push_back(joint_model_vector_[i]);
}
}
Moveit seems to parse the urdf with ros/urdf. The urdf::Joint have a member mimic which is a pointer to the "father" joint, so it is easy to explore them. (KDL::Joint seems not have this information).
The problem is that we should need to refactor the code to use urdf instead of kdl right?
I see now that Parser.cpp use also urdf, so I should manage to use it easily, using __urdfmodel member. I try to solve the issue with this in mind
I modified the parse function to explore the fathers of mimic: https://github.com/ADVRHumanoids/ROSEndEffector/blob/5a520ee39be4e22b209c396b387ffb2136ac1d4e/src/Parser.cpp#L59-L122
This does not solve completely the issue. The problem is that actuated joint in ee interface are taken from finger_jointmap and not __urdf_jointmap . An actuated joint put in a virtual finger (qbhand) can not be present in finger_jointmap (which would be its "finger"?) and so the controller gives the error [ WARN] [1585995340.336778128]: Trying to move Joint: qbhand_synergy_joint with ID: -1
@liesrock do you have any idea on how to solve without upsetting how ee interface take joints from parser?
I just read the whole discussion: it's interesting to understand how moveit parses the mimic joints.
I got the issue related with the __finger_jointmap and the virtual fingers: maybe a solution can be to have the virtual fingers handled in a special way by the ee interface as we do in the XBotInterface (ModelInterface) part for the virtual chain in floating base robots.
I will try to check this in a more careful way in the code: can you push your commits in a branch with called issue_30 ?
Yes, maybe we make the interface to "create" virtual finger? Or we may say the user to add a finger as virtual in someway... (maybe adding group in the srdf but then not adding this group in the end_effectors group of groups
Here is the branch: issue_30
I think we do not need additional structure to store somewhere the mimic argument (offset, multiplier...) because we do not need to send commands to all mimicking joint when the actuated ancestor is commanded. Why not? Because this is done by joint_state_publisher.
So the problem is limited to this:
This does not solve completely the issue. The problem is that actuated joint in ee interface are taken from _finger_joint_map and not _urdf_joint_map . An actuated joint put in a virtual finger (qbhand) can not be present in _finger_joint_map (which would be its "finger"?) and so the controller gives the error [ WARN] [1585995340.336778128]: Trying to move Joint: qbhand_synergy_joint with ID: -1
I thought about 3 possible solution:
We modify the way the core rosee controller get the joint: Now it takes them from _finger_jointmap, where obviously not-in-a-finger joints are not present. We can takes them from _urdf_jointmap instead where not-in-a-finger joints are present. Problem of this is that we will have these 2 map (and the new one joint fingermap) not consistent: the not-in-a-finger joints will be only in the _urdf_jointmap
When writing the srdf file, if a hand urdf has one or more actuated joints in virtual fingers, the user must add an additional special finger with specific name "virtual_finger". Here, all these not-in-finger joints must be present. In the code, we will recognize this as virtual finger, and it will not be included in the count of "real" finger, but we will be aware of actuated joint inside. This permit to have consisent maps (unlike 1 option). The virtual_finger group may be not a chain sometimes, but I think this is not a problem.
We put in srdf file a group "actuated_joint" where ALL actuated joint must be present, so we are aware of them always. But, with this info, some other info that we already use will become useless, like the passive joint tag. Also, some other parsing thing could be better with this new info, so a bit of refactoring of code I think is needed.
Now no simplier solutions come in my mind
Regarding the above issue, it is not easy to find a works-for-all solution: I believe we should keep the following rules:
"a joint should always have a unique associated finger"
This will probably mean that the option number 2, i.e. the virtual_finger group in the SRDF would be the best to solve the issue.
What do you think?
I would go also for the option 2, I can start working on it
I thought a possible option 2 variant. The user does not put anything about this virtual finger (so srdf remain as it is now)
Instead in code, we "create" the virtual finger: if we found a actuated joint that is not in any finger, we insert it in the group/finger "virtual_finger". To do this, I am thinking about exploring the _urdf_joint_map (where previously all actuated are put), check if any joint here IS NOT present in __joint_fingermap . If present, we already good, if not present, we add it in the finger "virtual_finger".
Even if we need an additional loop over the _urdf_joint_map, we can leave the srdf as it is, so no need to check "if a group is called virtual_finger... do not count as real finger" and things like that.
What do you think @liesrock ?
This could be a good option: I believe it is worth to try the implementation, given the current use-cases we have.
Ok, I solved with the option 2 variant
I (re)opened the pull request. To test you can try the qbhand, where there is the only actuated joint in a sixth "virtual_finger"
Please note this: https://github.com/ADVRHumanoids/ROSEndEffector/blob/3bca4efdaa0d26d17e7ad44bf998a4917234113f/src/Parser.cpp#L43-L46
So now we can have fingers with no joints inside. Before this issue, they contained the mimic joints, but now we do not want them because they are not actuated.
P.S. with this addition and with issue #16 solutions, I found out that probably we can parse the model more efficently. It is not fundamental (I think) but we can improve it.
@torydebra can we close this?
We need to parse mimic joint to be aware of them. We do not only need to know that a joint is mimic, but also find the joint that it is mimicking, and store this info in someway. Related to #16
Related comments from another issue:
I think that orocos kdl parser can not deal with mimic joints, thus we can not parse them with it. I found no info about this on Internet and neither on kdl api doc.
MoveIt (which uses kdl) probably developed some additional code to parse them, as I suppose from https://github.com/ros-planning/moveit/pull/1517
@liesrock, are you aware of someone who uses orocos kdl for parsing urdf with mimic tags?
Originally posted by @torydebra in https://github.com/ADVRHumanoids/ROSEndEffector/issues/23#issuecomment-605962607
no unfortunately I am not aware of anyone using orocos_kdl for this: the feature is currently missing as you mentioned.
Can we extend it or use another parser only for mimic joints ?
Originally posted by @liesrock in https://github.com/ADVRHumanoids/ROSEndEffector/issues/23#issuecomment-606014301
As you know I used moveit functions for parsing. In Parser, we could parse urdf also with moveit, then, for each mimic joint, delete it from the finger_jointmap and __urdf_jointmap, and decrease the jointsnum counter.
But at this point is not easier to parse the model directly with moveit structures? So we do not have to "loop" twice.
However, these problems are related to the issue about passive joints ( #16 )
Originally posted by @torydebra in https://github.com/ADVRHumanoids/ROSEndEffector/issues/23#issuecomment-606024010