Skip to content
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

Feature/implement grasp detector in grab #1239

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6642e75
Added grasp detection after grab and retry grab
P-ict0 Sep 13, 2022
802f766
Fixed typo in state machine
P-ict0 Sep 13, 2022
0d35c55
Fixed underscore
P-ict0 Sep 13, 2022
2be6ad1
Add position detector
PetervDooren Sep 13, 2022
6d53e0f
change min position after testing
PetervDooren Sep 13, 2022
1461fa4
Improved import
P-ict0 Sep 14, 2022
f79701c
Added outcome object_not_grasped of Grab
P-ict0 Sep 27, 2022
78798df
Added outcome to manipulate_machine
P-ict0 Sep 27, 2022
97150dd
Go to failure if retry fails
P-ict0 Oct 1, 2022
e86de40
Added type hinting
P-ict0 Oct 1, 2022
c466380
Completed type hinting
P-ict0 Oct 1, 2022
66232e5
Added retry parameter and announcements from robot
P-ict0 Oct 1, 2022
8c17a90
Added outcome to cleanup
P-ict0 Oct 1, 2022
b277700
add gripperpositiondetector to mockbot
PetervDooren Oct 1, 2022
7d91602
Changed value after testing
P-ict0 Oct 1, 2022
a763de0
Fixed retry
P-ict0 Oct 1, 2022
b56e24f
Merge branch 'feature/implement_grasp_detector_in_grab' of github.com…
P-ict0 Oct 1, 2022
5bacddf
add outcome for the object not grasped
PetervDooren Oct 1, 2022
f31614e
Add value -0.75 to grasp
GustavoDCC Oct 11, 2022
dca7493
Add attribute and old docstring minimum_position to gripper_position_…
GustavoDCC Oct 16, 2022
b893518
Add attribute minimum_position to GripperPositionDetector in mockbot.py
GustavoDCC Oct 16, 2022
7631ff9
Add minimum_position from the gripper_position_detector to compare in…
GustavoDCC Oct 16, 2022
c6d2d77
(grab.py) Added docs
P-ict0 Apr 29, 2023
a6b42fe
(temp) change retry to true
P-ict0 Apr 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion challenge_cleanup/src/challenge_cleanup/self_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,9 @@ def __init__(self, robot, selected_entity_designator, room_des):

smach.StateMachine.add("GRAB",
Grab(robot, selected_entity_designator, arm_designator),
transitions={"done": "SAY_GRAB_SUCCESS", "failed": "ARM_RESET"})
transitions={"done": "SAY_GRAB_SUCCESS",
"failed": "ARM_RESET",
"object_not_grasped": "ARM_RESET"})

smach.StateMachine.add("ARM_RESET", ArmToJointConfig(robot, arm_designator, "reset"),
transitions={"succeeded": "SAY_GRAB_FAILED", "failed": "SAY_GRAB_FAILED"})
Expand Down
3 changes: 2 additions & 1 deletion challenge_manipulation/src/manipulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,8 @@ def lock(userdata=None):
smach.StateMachine.add( "GRAB_ITEM",
Grab(robot, self.current_item, self.empty_arm_designator),
transitions={ 'done' :'STORE_ITEM',
'failed' :'SAY_GRAB_FAILED'})
'failed' :'SAY_GRAB_FAILED',
'object_not_grasped': 'SAY_GRAB_FAILED'})

smach.StateMachine.add( "SAY_GRAB_FAILED",
states.Say(robot, ["I couldn't grab this thing"], mood="sad"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class DefaultGrabDesignator(ds.Designator):
""" Designator to pick the closest item on top of the table to grab. This is used for testing

"""

def __init__(self, robot, surface_designator, area_description):
""" Constructor

Expand Down Expand Up @@ -116,7 +117,8 @@ def lock(userdata=None):
smach.StateMachine.add("GRAB_ITEM",
states.Grab(robot, self.grab_designator, self.empty_arm_designator),
transitions={'done': 'UNLOCK_ITEM_SUCCEED',
'failed': 'UNLOCK_ITEM_FAIL'})
'failed': 'UNLOCK_ITEM_FAIL',
'object_not_grasped': 'UNLOCK_ITEM_FAIL'})

@smach.cb_interface(outcomes=["unlocked"])
def lock(userdata=None):
Expand All @@ -138,6 +140,7 @@ def lock(userdata=None):
class PlaceSingleItem(smach.State):
""" Tries to place an object. A 'place' statemachine is constructed dynamically since this makes it easier to
build a statemachine (have we succeeded in grasping the objects?)"""

def __init__(self, robot, place_designator):
""" Constructor

Expand Down
6 changes: 6 additions & 0 deletions robot_skills/src/robot_skills/arm/arms.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ def handover_detector(self):
self._test_die(hasattr(self._arm, 'handover_detector'), "This arm does not have a handover_detector")
return self._arm.handover_detector

@property
def gripper_position_detector(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does a gripper_position_detector do? We already know where the gripper is with the joint_states/TF.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It reads the joint state of the gripper

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So how open the fingers are?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. And upon further inspection we can use the get_joint_states function of the robot object for this but this allows us to already specify which joint corresponds to the fingers. That is something that can potentially be improved. But for now we want to see if this gives us the desired behaviour. Lets not do premature optimization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a different can of worms and not for this project.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handover_detector can be ignored in this project, but gripper_position_detector should be modelled correctly. It doesn't mean it should be done correctly right now in this PR. But it should be done correctly soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#stopdeverloederingvanonzecode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should create an issue about this. With a deadline this should be fixed.

Copy link
Contributor

@PetervDooren PetervDooren Oct 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an issue to look into the grasp position detector robot part #1248 . It does not have high priority though. So I don't think we need to add a deadline

self._test_die(hasattr(self._arm, 'gripper_position_detector'),
"This arm does not have a gripper_position_detector")
return self._arm.gripper_position_detector

def has_gripper_type(self, gripper_type=None):
"""
Query whether the arm has the provided specific type of gripper.
Expand Down
7 changes: 7 additions & 0 deletions robot_skills/src/robot_skills/mockbot.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ def __init__(self, robot_name, tf_buffer, get_joint_states, name):
# add parts
self.gripper = Gripper(robot_name, tf_buffer)
self.handover_detector = HandoverDetector(robot_name, tf_buffer)
self.gripper_position_detector = GripperPositionDetector(robot_name, tf_buffer)

def collect_gripper_types(self, gripper_type):
return gripper_type
Expand All @@ -176,6 +177,12 @@ def __init__(self, robot_name, tf_buffer, *args, **kwargs):
self.handover_to_robot = AlteredMagicMock()


class GripperPositionDetector(MockedRobotPart):
def __init__(self, robot_name, tf_buffer, *args, **kwargs):
super(GripperPositionDetector, self).__init__(robot_name, tf_buffer)
self.detect = AlteredMagicMock(return_value=0.0)


class Base(MockedRobotPart):
def __init__(self, robot_name, tf_buffer, *args, **kwargs):
super(Base, self).__init__(robot_name, tf_buffer)
Expand Down
3 changes: 2 additions & 1 deletion robot_smach_states/src/robot_smach_states/clear.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ def __init__(self, robot, source_location, source_navArea, target_location, targ
smach.StateMachine.add('GRAB',
Grab(robot, selected_entity_designator, arm_des),
transitions={'done': 'INSPECT_TARGET',
'failed': 'failed'}
'failed': 'failed',
'object_not_grasped': 'failed'}
)

smach.StateMachine.add('INSPECT_TARGET',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class ActiveGraspDetector(smach.State):
REQUIRED_ARM_PROPERTIES = {"required_gripper_types": [GripperTypes.GRASPING], }

def __init__(self, robot: Robot, arm_designator: ArmDesignator, threshold_difference: float = 0.075,
minimum_position: float = -0.82, max_torque: float = 0.15) -> None:
minimum_position: float = -0.75, max_torque: float = 0.15) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are still very much magic numbers. They have the downside that they need to be tuned constantly. But I think that may be for another time

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this an issue. If we need keep changing this, this is something we need to work on with high priority.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an issue to look into the grasp position detector robot part #1248 . It does not have high priority though. So I don't think we need to add a deadline

"""
State for detecting whether the robot is holding something using the gripper position.

Expand Down
66 changes: 55 additions & 11 deletions robot_smach_states/src/robot_smach_states/manipulation/grab.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@
from ..manipulation.grasp_point_determination import GraspPointDeterminant
from ..util.designators.arm import ArmDesignator
from ..util.designators.core import Designator
from ..manipulation.active_grasp_detector import ActiveGraspDetector
from ..human_interaction import Say


class PrepareEdGrasp(smach.State):
REQUIRED_ARM_PROPERTIES = {"required_gripper_types": [GripperTypes.GRASPING],
"required_trajectories": ["prepare_grasp"], }

def __init__(self, robot, arm, grab_entity):
# type: (Robot, ArmDesignator, Designator) -> None
def __init__(self, robot: Robot, arm: ArmDesignator, grab_entity: Designator) -> None:
"""
Set the arm in the appropriate position before actually grabbing

Expand Down Expand Up @@ -80,7 +81,8 @@ class PickUp(smach.State):
REQUIRED_ARM_PROPERTIES = {"required_gripper_types": [GripperTypes.GRASPING],
"required_goals": ["carrying_pose"], }

def __init__(self, robot, arm, grab_entity, check_occupancy=False):
def __init__(self, robot: Robot, arm: ArmDesignator, grab_entity: Designator,
check_occupancy: bool = False) -> None:
"""
Pick up an item given an arm and an entity to be picked up

Expand All @@ -98,7 +100,7 @@ def __init__(self, robot, arm, grab_entity, check_occupancy=False):
self._gpd = GraspPointDeterminant(robot)
self._check_occupancy = check_occupancy

assert self.robot.get_arm(**self.REQUIRED_ARM_PROPERTIES) is not None,\
assert self.robot.get_arm(**self.REQUIRED_ARM_PROPERTIES) is not None, \
"None of the available arms meets all this class's requirements: {}".format(self.REQUIRED_ARM_PROPERTIES)

def execute(self, userdata=None):
Expand Down Expand Up @@ -255,7 +257,7 @@ def execute(self, userdata=None):

return result

def associate(self, original_entity):
def associate(self, original_entity: Entity) -> Entity:
"""
Tries to associate the original entity with one of the entities in the world model. This is useful if
after an update, the original entity is no longer present in the world model. If no good map can be found,
Expand Down Expand Up @@ -290,7 +292,7 @@ class ResetOnFailure(smach.State):

REQUIRED_ARM_PROPERTIES = {"required_gripper_types": [GripperTypes.GRASPING], }

def __init__(self, robot, arm):
def __init__(self, robot: Robot, arm: ArmDesignator):
"""
Constructor

Expand Down Expand Up @@ -320,20 +322,25 @@ def execute(self, userdata=None):


class Grab(smach.StateMachine):
def __init__(self, robot, item, arm):
def __init__(self, robot: Robot, item: Designator, arm: ArmDesignator, retry: bool = False) -> None:
"""
Let the given robot move to an entity and grab that entity using some arm
Let the given robot move to an entity and grab that entity using some arm. Performs grasp detection and retries
if it's not holding anything

:param robot: Robot to use
:param item: Designator that resolves to the item to grab. E.g. EntityByIdDesignator
:param arm: Designator that resolves to the arm to use for grabbing. Eg. UnoccupiedArmDesignator
:param retry: On True the robot will retry the grab if it fails to grasp the object
"""
smach.StateMachine.__init__(self, outcomes=['done', 'failed'])
smach.StateMachine.__init__(self, outcomes=['done', 'failed', 'object_not_grasped'])

# Check types or designator resolve types
check_type(item, Entity)
check_type(arm, PublicArm)

# Check retry
grasp_failed_next_state = 'SAY_RETRY' if retry else 'object_not_grasped'

with self:
smach.StateMachine.add('RESOLVE_ARM', ResolveArm(arm, self),
transitions={'succeeded': 'NAVIGATE_TO_GRAB',
Expand All @@ -349,10 +356,47 @@ def __init__(self, robot, item, arm):
'failed': 'RESET_FAILURE'})

smach.StateMachine.add('GRAB', PickUp(robot, arm, item),
transitions={'succeeded': 'done',
transitions={'succeeded': 'GRASP_DETECTOR',
'failed': 'RESET_FAILURE'})

smach.StateMachine.add('GRASP_DETECTOR', ActiveGraspDetector(robot, arm),
transitions={'true': 'done',
'false': 'SAY_GRASP_NOT_SUCCEEDED',
'failed': 'done',
'cannot_determine': 'SAY_GRASP_NOT_SUCCEEDED'})
PetervDooren marked this conversation as resolved.
Show resolved Hide resolved

smach.StateMachine.add('SAY_GRASP_NOT_SUCCEEDED', Say(robot, "I failed grasping the object"),
transitions={'spoken': grasp_failed_next_state})

smach.StateMachine.add('SAY_RETRY', Say(robot, "I will retry to grab it"),
transitions={'spoken': 'RETRY_NAVIGATE_TO_GRAB'})

smach.StateMachine.add('RETRY_NAVIGATE_TO_GRAB', NavigateToGrasp(robot, arm, item),
transitions={'unreachable': 'RESET_FAILURE',
'goal_not_defined': 'RESET_FAILURE',
'arrived': 'RETRY_PREPARE_GRASP'})

smach.StateMachine.add('RETRY_PREPARE_GRASP', PrepareEdGrasp(robot, arm, item),
transitions={'succeeded': 'RETRY_GRAB',
'failed': 'RESET_FAILURE'})

smach.StateMachine.add('RETRY_GRAB', PickUp(robot, arm, item),
transitions={'succeeded': 'RETRY_GRASP_DETECTOR',
'failed': 'RESET_FAILURE'})

smach.StateMachine.add("RESET_FAILURE", ResetOnFailure(robot, arm),
smach.StateMachine.add('RETRY_GRASP_DETECTOR', ActiveGraspDetector(robot, arm),
transitions={'true': 'done',
'false': 'SAY_RETRY_NOT_SUCCEEDED',
'failed': 'done',
'cannot_determine': 'SAY_RETRY_NOT_SUCCEEDED'})

smach.StateMachine.add('SAY_RETRY_NOT_SUCCEEDED', Say(robot, "I failed grasping the object again"),
transitions={'spoken': 'RESET_NOT_GRASPED'})

smach.StateMachine.add('RESET_FAILURE', ResetOnFailure(robot, arm),
transitions={'done': 'failed'})

smach.StateMachine.add('RESET_NOT_GRASPED', ResetOnFailure(robot, arm),
transitions={'done': 'object_not_grasped'})

check_arm_requirements(self, robot)