adds new embodiment of Agibot A2D #292
Conversation
alexmillane
left a comment
alexmillane
left a comment
There was a problem hiding this comment.
Thanks for adding this! This is really useful.
I have a couple of comments.
| debug_vis=False, | ||
| target_frames=[ | ||
| FrameTransformerCfg.FrameCfg( | ||
| prim_path="{ENV_REGEX_NS}/Robot/gripper_center", |
There was a problem hiding this comment.
Below we have {ENV_REGEX_NS}/Robot/right_gripper_center for the right gripper.
Suggestion to make this {ENV_REGEX_NS}/Robot/left_gripper_center
There was a problem hiding this comment.
@alexmillane this prim_path is copied from the Agibot usd asset, and I think I will be a little difficult to revise the usd asset already uploaded to AWS S3 bucket. But if you think this is necessary, I can contact the Isaac Sim engineer to revise the asset.
There was a problem hiding this comment.
Ah I see. My mistake. This is fine! Proceed!
| mimic_arm_mode: MimicArmMode | None = None, | ||
| ): | ||
| super().__init__(enable_cameras, initial_pose) | ||
| self.mimic_arm_mode = mimic_arm_mode or self.default_mimic_arm_mode |
There was a problem hiding this comment.
It seems in this robot the mimic_arm_mode is being used for more then mimic. It's defining which arm of the robot is being controlled.
Suggestion to rename the enum MimicArmMode -> ArmMode and mimic_arm_mode -> arm_mode.
Does that make sense to you?
There was a problem hiding this comment.
Absolutely, agree with you @alexmillane. Shall I just rename the MimicArmMode in this MR or create a new one?
There was a problem hiding this comment.
Let's just do it in this one. We've got a couple of MRs in flight.
| class AgibotRightArmSceneCfg(AgibotLeftArmSceneCfg): | ||
| """Scene configuration for the Agibot right arm.""" | ||
|
|
||
| def __post_init__(self): | ||
| # update the ee_frame for the right arm | ||
| self.ee_frame = FrameTransformerCfg( | ||
| prim_path="{ENV_REGEX_NS}/Robot/base_link", | ||
| debug_vis=False, | ||
| target_frames=[ | ||
| FrameTransformerCfg.FrameCfg( | ||
| prim_path="{ENV_REGEX_NS}/Robot/right_gripper_center", | ||
| name="right_end_effector", | ||
| offset=OffsetCfg( | ||
| pos=[0.0, 0.0, 0.0], | ||
| ), | ||
| ), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
This is a bit of a strange use of inheritance. Suggestion to instead create a function which takes in arm_mode and constructs a AgibotSceneCfg appropriately.
Maybe:
@configclass
class AgibotSceneCfg:
robot = AGIBOT_A2D_CFG.replace(prim_path="{ENV_REGEX_NS}/Robot")
arm_mode: ArmMode
eef_frame: MISSING
def __post_init__(self):
if self.arm_mode == ArmMode.LEFT:
eef = ...
Then above in AgibotEmbodiment you can do:
scene_config = AgibotSceneCfg(arm_mode=ArmMode.LEFT)
And we don't need these two classes with inheritance etc.
| @configclass | ||
| class AgibotRightArmActionsCfg: | ||
| """Action configuration for the Agibot robot.""" | ||
|
|
||
| arm_action = RMPFlowActionCfg( | ||
| asset_name="robot", | ||
| joint_names=["right_arm_joint.*"], | ||
| body_name="right_gripper_center", | ||
| controller=AGIBOT_RIGHT_ARM_RMPFLOW_CFG, | ||
| scale=1.0, | ||
| body_offset=RMPFlowActionCfg.OffsetCfg(pos=[0.0, 0.0, 0.0]), | ||
| articulation_prim_expr="/World/envs/env_.*/Robot", | ||
| use_relative_mode=True, | ||
| ) | ||
|
|
||
| gripper_action = mdp.AbsBinaryJointPositionActionCfg( | ||
| asset_name="robot", | ||
| threshold=0.5, | ||
| joint_names=["right_hand_joint1", "right_.*_Support_Joint"], | ||
| open_command_expr={"right_hand_joint1": 0.994, "right_.*_Support_Joint": 0.994}, | ||
| close_command_expr={"right_hand_joint1": 0.0, "right_.*_Support_Joint": 0.0}, | ||
| ) |
There was a problem hiding this comment.
As above. Suggestion to make a single class which takes an ArmMode as input and constructs the config struct appropriately.
| # Process rigid objects | ||
| for obj_name, obj_state in rigid_object_states.items(): | ||
| pos_obj_base, quat_obj_base = PoseUtils.subtract_frame_transforms( | ||
| root_pos, root_quat, obj_state["root_pose"][env_ids, :3], obj_state["root_pose"][env_ids, 3:7] | ||
| ) | ||
| rot_obj_base = PoseUtils.matrix_from_quat(quat_obj_base) | ||
| object_pose_matrix[obj_name] = PoseUtils.make_pose(pos_obj_base, rot_obj_base) | ||
|
|
||
| # Process articulated objects (except robot) | ||
| for art_name, art_state in articulation_states.items(): | ||
| if art_name != "robot": # Skip robot | ||
| pos_obj_base, quat_obj_base = PoseUtils.subtract_frame_transforms( | ||
| root_pos, root_quat, art_state["root_pose"][env_ids, :3], art_state["root_pose"][env_ids, 3:7] | ||
| ) | ||
| rot_obj_base = PoseUtils.matrix_from_quat(quat_obj_base) | ||
| object_pose_matrix[art_name] = PoseUtils.make_pose(pos_obj_base, rot_obj_base) | ||
|
|
||
| return object_pose_matrix |
There was a problem hiding this comment.
In franka.py and gr1t2.py we call out to a function get_rigid_and_articulated_object_poses. Could we do the same here to remove duplication of this functionality, or is this code specific to Agibot?
Edit: Actually I notice that this class inherits from FrankaMimicEnv. I guess, then, there's a reason we're overriding this function?
There was a problem hiding this comment.
@alexmillane I modified this func is to align with the observations definition. Since in AgibotObservationCfg, our eef_pos and eef_quat is related to the robot_base_frame, which I thought would make more sense if the robot is not located at the env origin or is a mobile robot. So in this MimicEnv, we need to also get_object_poses in robot_base_frame. I think reset both ObsCfg and MimicEnv to track eef_pose in env_world_frame is OK for me, which will simplify this MimicEnv code, just reuse FrankaMimicEnv. What is your suggestion?
There was a problem hiding this comment.
Ok, my mistake. I couldn't see the reason in the code, but from your description, this is required.
Looks good to me!
- since the property of 'arm_mode' in Agibot is not for building mimic_env_cfg only, we rename MimicArmMode class to ArmMode class
Summary
Adds a new embodiment of Agibot
Detailed description