-
Notifications
You must be signed in to change notification settings - Fork 389
Description
Description
I noticed an inconsistency between the URDF link names and the Python attribute names for the Panda robot's gripper fingers.
Current situation:
- URDF link names:
panda_leftfinger,panda_rightfinger - Python attributes:
finger1_link,finger2_link
While the numeric indexing (1, 2) is functional, it creates a semantic gap where users need to remember which number corresponds to which finger (left or right).
Proposed Enhancement
I suggest adding more semantically clear attribute names that align with the URDF naming:
Option 1 (Recommended):
- Add new attributes:
left_finger_link,right_finger_link - Keep existing
finger1_link,finger2_linkas aliases for backward compatibility
Option 2 (Alternative):
- Use shorter names:
lfinger_link,rfinger_link
Benefits
- Improved readability: Code becomes self-documenting - no need to remember the finger1/finger2 mapping
- Consistency: Aligns Python attribute names with URDF naming conventions
- Better maintainability: Easier for new contributors and users to understand the code
- Backward compatibility: Existing code continues to work if we keep aliases
Example
Current code:
l_contact_forces = self.scene.get_pairwise_contact_forces(
self.finger1_link, object # Which finger is this?
)Proposed code:
l_contact_forces = self.scene.get_pairwise_contact_forces(
self.left_finger_link, object # Clear and intuitive
)Implementation Suggestion
In panda.py, the _after_init method could be updated to:
def _after_init(self):
# Use semantic names as primary
self.left_finger_link = sapien_utils.get_obj_by_name(
self.robot.get_links(), "panda_leftfinger"
)
self.right_finger_link = sapien_utils.get_obj_by_name(
self.robot.get_links(), "panda_rightfinger"
)
# Keep backward compatibility
self.finger1_link = self.left_finger_link
self.finger2_link = self.right_finger_link
# Similar updates for finger pads
self.left_finger_pad_link = sapien_utils.get_obj_by_name(
self.robot.get_links(), "panda_leftfinger_pad"
)
self.right_finger_pad_link = sapien_utils.get_obj_by_name(
self.robot.get_links(), "panda_rightfinger_pad"
)
# Backward compatibility
self.finger1pad_link = self.left_finger_pad_link
self.finger2pad_link = self.right_finger_pad_link
self.tcp = sapien_utils.get_obj_by_name(
self.robot.get_links(), self.ee_link_name
)Context
I'm a graduate student studying the codebase. I noticed this naming pattern and thought it could be improved for better code clarity.
I'm happy to working on a PR for this enhancement if the maintainers agree with this proposal.
Thank you for maintaining this excellent robotics framework! 🚀