-
Notifications
You must be signed in to change notification settings - Fork 355
allow EnvironmentBodyRemover not to restore a grabbed body when the grabbing link does not exist #1359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: production
Are you sure you want to change the base?
allow EnvironmentBodyRemover not to restore a grabbed body when the grabbing link does not exist #1359
Changes from 3 commits
916ce09
75bd3d9
a9b772c
3ba1b8a
dd08d7f
f22dd99
d551f19
cbdb9bc
8397d76
a001c6a
ad73c54
05d1b0f
4dc88d8
460c0f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -447,42 +447,69 @@ void RobotBase::RobotStateSaver::Release() | |
| KinBodyStateSaver::Release(); | ||
| } | ||
|
|
||
| ///\brief removes the robot from the environment temporarily while in scope | ||
| class EnvironmentBodyRemover | ||
| { | ||
| public: | ||
| EnvironmentBodyRemover(OpenRAVE::KinBodyPtr pBody) : _pBody(pBody), _grabbedStateSaver(pBody, OpenRAVE::KinBody::Save_GrabbedBodies) { | ||
| if( _pBody->IsRobot() ) { | ||
| // If the manip comes from a connected body, the information of which manip is active is lost once the robot | ||
| // is removed from env. Need to save the active manip name so that we can set it back later when the robot | ||
| // is re-added to the env. | ||
| _pBodyRobot = OpenRAVE::RaveInterfaceCast<OpenRAVE::RobotBase>(_pBody); | ||
| if( !!_pBodyRobot->GetActiveManipulator() ) { | ||
| _activeManipName = _pBodyRobot->GetActiveManipulator()->GetName(); | ||
| } | ||
| EnvironmentBodyRemover::EnvironmentBodyRemover(KinBodyPtr pBody) : _pBody(pBody) { | ||
| if( _pBody->IsRobot() ) { | ||
| // If the manip comes from a connected body, the information of which manip is active is lost once the robot | ||
| // is removed from env. Need to save the active manip name so that we can set it back later when the robot | ||
| // is re-added to the env. | ||
| _pBodyRobot = RaveInterfaceCast<RobotBase>(_pBody); | ||
| if( !!_pBodyRobot->GetActiveManipulator() ) { | ||
| _activeManipName = _pBodyRobot->GetActiveManipulator()->GetName(); | ||
| } | ||
| _pBody->GetEnv()->Remove(_pBody); | ||
| } | ||
| _pBody->GetGrabbedInfo(_pGrabbedInfos); | ||
| _pBody->GetEnv()->Remove(_pBody); | ||
| } | ||
|
|
||
| ~EnvironmentBodyRemover() { | ||
| _pBody->GetEnv()->Add(_pBody, IAM_StrictNameChecking); | ||
| _grabbedStateSaver.Restore(); | ||
| _grabbedStateSaver.Release(); | ||
| if( !!_pBodyRobot && !_activeManipName.empty() ) { | ||
| OpenRAVE::RobotBase::ManipulatorPtr pmanip = _pBodyRobot->GetManipulator(_activeManipName); | ||
| // it might be ok with manipulator doesn't exist if ConnectedBody acitve state changes. | ||
| if( !!pmanip ) { | ||
| _pBodyRobot->SetActiveManipulator(pmanip); | ||
| EnvironmentBodyRemover::~EnvironmentBodyRemover() { | ||
| _pBody->GetEnv()->Add(_pBody, IAM_StrictNameChecking); | ||
| if( !!_pBodyRobot && !_activeManipName.empty() ) { | ||
| RobotBase::ManipulatorPtr pmanip = _pBodyRobot->GetManipulator(_activeManipName); | ||
| // it might be ok with manipulator doesn't exist if ConnectedBody acitve state changes. | ||
| if( !!pmanip ) { | ||
| _pBodyRobot->SetActiveManipulator(pmanip); | ||
| } | ||
| else { | ||
| pmanip = _pBodyRobot->GetActiveManipulator(); | ||
| RAVELOG_WARN_FORMAT("env=%s, robot=%s, cannot restore previous active manip=%s because it does not exist anymore. current active manip=%s", _pBodyRobot->GetEnv()->GetNameId()%_pBodyRobot->GetName()%_activeManipName%(!!pmanip ? pmanip->GetName() : "")); | ||
| } | ||
| } | ||
| if( !_pGrabbedInfos.empty() ) { | ||
| // it might be ok with grabbing link doesn't exist if ConnectedBody acitve state changes. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eisoku9618 I think you should not change the behavior of
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yoshikikanemoto Eventually we need to take care of grabbed bodies restoring failures either by My only concern is an inconsistency of current
How about adding the input arguments to EnvironmentBodyRemover(KinBodyPtr pBody, bool abortOnActiveManipulatorLost=false, bool abortOnGrabbedBodiesLost=true);If this is still not good and we can leave the inconsistent behavior of void SetConnectedBodyActiveStatesKeepingGrabbedBodiesAsMuchAsPossible(pRobot, requestedConnectedBodyActiveStates) {
std::vector<KinBody::GrabbedInfoPtr> pGrabbedInfos;
pRobot->GetGrabbedInfo(pGrabbedInfos);
// manually restore grabbed state instead of using EnvironmentBodyRemover because a grabbing link might get removed on connected body active state change.
pRobot->ReleaseAllGrabbed();
{
EnvironmentBodyRemover robotremover(pRobot);
pRobot->SetConnectedBodyActiveStates(requestedConnectedBodyActiveStates);
}
std::vector<KinBody::GrabbedInfoPtr>::iterator itRemoveFirst = pGrabbedInfos.begin();
for (std::vector<KinBody::GrabbedInfoPtr>::const_iterator itGrabbedInfo = pGrabbedInfos.begin(); itGrabbedInfo != pGrabbedInfos.end(); ++itGrabbedInfo) {
const KinBody::GrabbedInfoPtr pGrabbedInfo = *itGrabbedInfo;
bool needToDelete = false;
if( !pRobot->GetLink(pGrabbedInfo->_robotlinkname) ) {
RAVELOG_WARN_FORMAT("env=%s, body=%s, cannot re-grab '%s' because grabbing link '%s' does not exist anymore.", pRobot->GetEnv()->GetNameId()%pRobot->GetName()%pGrabbedInfo->_grabbedname%pGrabbedInfo->_robotlinkname);
needToDelete = true;
}
else if( !pRobot->GetEnv()->GetKinBody(pGrabbedInfo->_grabbedname) ) {
RAVELOG_WARN_FORMAT("env=%s, body=%s, cannot re-grab '%s' because it does not exist in the environment.", pRobot->GetEnv()->GetNameId()%pRobot->GetName()%pGrabbedInfo->_grabbedname);
needToDelete = true;
}
else {
for( std::set<std::string>::const_iterator itLinkName = pGrabbedInfo->_setIgnoreRobotLinkNames.begin(); itLinkName != pGrabbedInfo->_setIgnoreRobotLinkNames.end(); ++itLinkName ) {
if( !pRobot->GetLink(*itLinkName) ) {
RAVELOG_WARN_FORMAT("env=%s, body=%s, cannot re-grab '%s' because '%s' in _setIgnoreRobotLinkNames does not exist anymore.", pRobot->GetEnv()->GetNameId()%pRobot->GetName()%pGrabbedInfo->_grabbedname%(*itLinkName));
needToDelete = true;
break;
}
}
}
if( needToDelete ) {
continue;
}
// will regrasp this info
if( itRemoveFirst != itGrabbedInfo ) {
*itRemoveFirst = std::move(*itGrabbedInfo);
}
++itRemoveFirst;
}
const std::vector<KinBody::GrabbedInfoConstPtr> pConstGrabbedInfos(pGrabbedInfos.begin(), itRemoveFirst);
pRobot->ResetGrabbed(pConstGrabbedInfos);
}
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. discussed with @yoshikikanemoto
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yoshikikanemoto As we discussed, I changed this PR to use a bit mask for the options. Could you check this PR and the changes in the related internal repositories? pipelineid=456553 |
||
| std::vector<KinBody::GrabbedInfoPtr>::iterator itRemoveFirst = _pGrabbedInfos.begin(); | ||
| for( std::vector<KinBody::GrabbedInfoPtr>::const_iterator itGrabbedInfo = _pGrabbedInfos.begin(); itGrabbedInfo != _pGrabbedInfos.end(); ++itGrabbedInfo ) { | ||
| const KinBody::GrabbedInfoPtr pGrabbedInfo = *itGrabbedInfo; | ||
| bool needToDelete = false; | ||
| if( !_pBody->GetLink(pGrabbedInfo->_robotlinkname) ) { | ||
| RAVELOG_WARN_FORMAT("env=%s, body=%s, cannot re-grab '%s' because grabbing link '%s' does not exist anymore.", _pBody->GetEnv()->GetNameId()%_pBody->GetName()%pGrabbedInfo->_grabbedname%pGrabbedInfo->_robotlinkname); | ||
| needToDelete = true; | ||
| } | ||
| else if( !_pBody->GetEnv()->GetKinBody(pGrabbedInfo->_grabbedname) ) { | ||
| RAVELOG_WARN_FORMAT("env=%s, body=%s, cannot re-grab '%s' because it does not exist in the environment.", _pBody->GetEnv()->GetNameId()%_pBody->GetName()%pGrabbedInfo->_grabbedname); | ||
| needToDelete = true; | ||
| } | ||
| else { | ||
| for( std::set<std::string>::const_iterator itLinkName = pGrabbedInfo->_setIgnoreRobotLinkNames.begin(); itLinkName != pGrabbedInfo->_setIgnoreRobotLinkNames.end(); ++itLinkName ) { | ||
| if( !_pBody->GetLink(*itLinkName) ) { | ||
| RAVELOG_WARN_FORMAT("env=%s, body=%s, cannot re-grab '%s' because '%s' in _setIgnoreRobotLinkNames does not exist anymore.", _pBody->GetEnv()->GetNameId()%_pBody->GetName()%pGrabbedInfo->_grabbedname%(*itLinkName)); | ||
| needToDelete = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| if( needToDelete ) { | ||
| continue; | ||
| } | ||
| // will regrasp this info | ||
| if( itRemoveFirst != itGrabbedInfo ) { | ||
| *itRemoveFirst = std::move(*itGrabbedInfo); | ||
| } | ||
| ++itRemoveFirst; | ||
| } | ||
| const std::vector<KinBody::GrabbedInfoConstPtr> pConstGrabbedInfos(_pGrabbedInfos.begin(), itRemoveFirst); | ||
| _pBody->ResetGrabbed(pConstGrabbedInfos); | ||
| } | ||
|
|
||
| private: | ||
| OpenRAVE::KinBodyPtr _pBody; | ||
| OpenRAVE::KinBody::KinBodyStateSaver _grabbedStateSaver; | ||
| OpenRAVE::RobotBasePtr _pBodyRobot; | ||
| std::string _activeManipName; ///< the name of the current active manipulator of pBody at the time of removal. | ||
| }; | ||
| } | ||
|
|
||
| void RobotBase::RobotStateSaver::_RestoreRobot(boost::shared_ptr<RobotBase> probot) | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eisoku9618
Any problem if we use
RobotStateSaver'sSave_ActiveManipulatorhere?maybe you need to restore by the name, not by the pointer?