-
Notifications
You must be signed in to change notification settings - Fork 28
Refactor robot models to use RobotModel with urdf parameter #639
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
Changes from all commits
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1833,7 +1833,31 @@ def self_collision_check(self): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class RobotModel(CascadedLink): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, link_list=None, joint_list=None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| root_link=None): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| root_link=None, urdf=None, include_mimic_joints=True): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Initialize a RobotModel instance. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Parameters | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ---------- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| link_list : list of Link, optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Initial list of links to register in the model. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| If None, an empty list is used. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joint_list : list of Joint, optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Initial list of joints to register in the model. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| If None, an empty list is used. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| root_link : Link, optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Root link of the robot model. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| urdf : str, optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| URDF input used to build the robot model. The value is interpreted | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| automatically: if the input looks like XML content (starts with | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| '<?xml' or '<robot'), it is treated as a string containing URDF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| XML; otherwise, if ``os.path.isfile`` returns True, it is treated | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| as a file system path to a URDF file; otherwise it is treated as | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| URDF XML string. If None (the default), the model is not | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| initialized from URDF. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| include_mimic_joints : bool, optional (default: True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Whether to include mimic joints defined in the URDF when loading | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| the model. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| link_list = link_list or [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joint_list = joint_list or [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| super(RobotModel, self).__init__(link_list, joint_list) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1853,6 +1877,43 @@ def __init__(self, link_list=None, joint_list=None, | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._relevance_predicate_table = \ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._compute_relevance_predicate_table() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if urdf is not None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self._load_from_urdf(urdf, include_mimic_joints=include_mimic_joints) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _load_from_urdf(self, urdf_input, include_mimic_joints=True): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Load URDF from a string or a file path. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Automatically detects if the input is a URDF string or a file path. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| If the input looks like XML content (starts with '<?xml' or '<robot'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it is treated as URDF XML string. Otherwise, if the input is an | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| existing file path, it is loaded as a file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Parameters | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ---------- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| urdf_input : str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Either the URDF model description as a string, or the path to a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| URDF file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| include_mimic_joints : bool, optional (default: True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| If True, mimic joints are included in the resulting | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `RobotModel`'s `joint_list`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # First, check if the input looks like URDF/XML content | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(urdf_input, str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| stripped = urdf_input.lstrip() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if stripped.startswith('<?xml') or stripped.startswith('<robot'): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.load_urdf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| urdf_input, include_mimic_joints=include_mimic_joints) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # If it doesn't look like XML, check if it's a file path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if os.path.isfile(urdf_input): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with open(urdf_input, 'r') as f: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.load_urdf_file( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| file_obj=f, include_mimic_joints=include_mimic_joints) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.load_urdf(urdf_input, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| include_mimic_joints=include_mimic_joints) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1909
to
+1915
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if os.path.isfile(urdf_input): | |
| with open(urdf_input, 'r') as f: | |
| self.load_urdf_file( | |
| file_obj=f, include_mimic_joints=include_mimic_joints) | |
| else: | |
| self.load_urdf(urdf_input, | |
| include_mimic_joints=include_mimic_joints) | |
| # First, check if the input looks like URDF/XML content. | |
| if isinstance(urdf_input, six.string_types): | |
| stripped = urdf_input.lstrip() | |
| if stripped.startswith('<?xml') or stripped.startswith('<robot'): | |
| self.load_urdf( | |
| urdf_input, include_mimic_joints=include_mimic_joints | |
| ) | |
| return | |
| # If it doesn't look like XML, fall back to treating it as a file path | |
| # when possible; otherwise, pass it to load_urdf as before. | |
| if isinstance(urdf_input, six.string_types) and os.path.isfile(urdf_input): | |
| with open(urdf_input, 'r') as f: | |
| self.load_urdf_file( | |
| file_obj=f, include_mimic_joints=include_mimic_joints | |
| ) | |
| else: | |
| self.load_urdf( | |
| urdf_input, include_mimic_joints=include_mimic_joints | |
| ) |
Copilot
AI
Jan 5, 2026
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.
The _load_from_urdf method has incomplete input validation. If urdf_input is not a string (e.g., accidentally passed as a file object or other type), the code will skip the XML detection block (line 1901) and attempt to call os.path.isfile(urdf_input) on line 1909 with a non-string argument, which could cause a TypeError or unexpected behavior. Consider adding explicit type checking at the start of the method to provide a clearer error message if urdf_input is not a string.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,18 +4,28 @@ | |
| from ..coordinates import CascadedCoords | ||
| from ..data import panda_urdfpath | ||
| from ..model import RobotModel | ||
| from .urdf import RobotModelFromURDF | ||
|
|
||
|
|
||
| class Panda(RobotModelFromURDF): | ||
| class Panda(RobotModel): | ||
|
|
||
| """Panda Robot Model. | ||
|
|
||
| https://frankaemika.github.io/docs/control_parameters.html | ||
| """ | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super(Panda, self).__init__(*args, **kwargs) | ||
| def __init__(self, urdf=None, urdf_file=None): | ||
| # For backward compatibility, support both urdf and urdf_file | ||
| if urdf is not None and urdf_file is not None: | ||
| raise ValueError( | ||
| "'urdf' and 'urdf_file' cannot be given at the same time" | ||
| ) | ||
| if urdf is not None: | ||
| urdf_input = urdf | ||
| elif urdf_file is not None: | ||
| urdf_input = urdf_file | ||
| else: | ||
| urdf_input = panda_urdfpath() | ||
| super(Panda, self).__init__(urdf=urdf_input) | ||
|
Comment on lines
+16
to
+28
|
||
|
|
||
| # End effector coordinate frame | ||
| # Based on franka_ros configuration: | ||
|
|
@@ -27,10 +37,6 @@ def __init__(self, *args, **kwargs): | |
| self.rarm_end_coords.rotate(np.deg2rad(-90), 'y') | ||
| self.reset_pose() | ||
|
|
||
| @cached_property | ||
| def default_urdf_path(self): | ||
| return panda_urdfpath() | ||
|
|
||
| def reset_pose(self): | ||
| angle_vector = [ | ||
| 0.03942226991057396, | ||
|
|
||
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.
The
__init__method signature now includes new parametersurdfandinclude_mimic_jointsbut lacks documentation. Consider adding a docstring to document these parameters, especially since this is a public API method. The docstring should explain whaturdfaccepts (file path or URDF string), whatinclude_mimic_jointsdoes, and the defaults for all parameters.