From 19dae9a26fc9f11237d68fb0a03e824a02880837 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Sun, 5 Jul 2026 14:15:55 +1000 Subject: [PATCH 1/2] fix(models): validate addconfiguration() input, fix 5 broken named configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BaseRobot.addconfiguration() stored its q argument completely unvalidated, unlike its sibling addconfiguration_attr() (which uses getunit(..., dim=self.n)). A malformed stored config (wrong length, or a 0-d/scalar array) would silently succeed at addconfiguration() time and only crash later, deep in configurations_str() (i.e. whenever someone calls print(robot)), with a confusing IndexError far from the actual mistake. Fixed by validating through getvector(q, self.n), matching the existing q/qd/qdd setter pattern in the same file. This immediately surfaced 5 real, pre-existing bugs it had been silently swallowing: - px150/rx150/rx200: qz was np.zeros(7) but these robots have 8 joints (5 arm + gripper_prop + 2 finger links) — off-by-one, qr already had the correct 8 elements. - Jaco (j2n6s200): qr/qz both hardcoded to 6 elements (arm only), but the robot has 10 joints (6 arm + 2 fingers x [finger, finger_tip] = 4 gripper joints). No tag exists in the source xacro, so all 10 are genuinely independent joints as far as the kinematic model goes — extended both configs with 4 trailing zeros (gripper open), matching neither qr nor qz being intended to specify gripper state. - LWR4: qr was np.array(7), a 0-d array holding the scalar 7, not a vector — never usable. Docstring turned out to be copy-pasted verbatim from Puma560.py (identical "qs"/"qn" descriptions) without ever implementing qs/qn, and qr was left as a broken placeholder. Removed qr and the docstring's qs/qn claims entirely rather than inventing unverified "ready"/"nominal" pose values for this DH parameterization — left only the working qz. Verified: full test suite (630 tests) passes, plus explicit checks that each fixed model prints/loads correctly and the 6 other Trossen arms (px100/wx200/wx250/wx250s/vx300/vx300s) already had correct, self-consistent qr/qz lengths. Co-Authored-By: Claude Sonnet 5 --- src/roboticstoolbox/models/DH/LWR4.py | 7 +------ src/roboticstoolbox/models/URDF/Jaco.py | 6 ++++-- src/roboticstoolbox/models/URDF/px150.py | 2 +- src/roboticstoolbox/models/URDF/rx150.py | 2 +- src/roboticstoolbox/models/URDF/rx200.py | 2 +- src/roboticstoolbox/robot/BaseRobot.py | 4 ++-- 6 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/roboticstoolbox/models/DH/LWR4.py b/src/roboticstoolbox/models/DH/LWR4.py index 9118bf894..5001a001f 100755 --- a/src/roboticstoolbox/models/DH/LWR4.py +++ b/src/roboticstoolbox/models/DH/LWR4.py @@ -21,10 +21,7 @@ class LWR4(DHRobot): Defined joint configurations are: - - qz, zero joint angle configuration, 'L' shaped configuration - - qr, vertical 'READY' configuration - - qs, arm is stretched out in the X direction - - qn, arm is at a nominal non-singular configuration + - qz, zero joint angle configuration .. note:: SI units are used. @@ -73,10 +70,8 @@ def __init__(self): # tool = xyzrpy_to_trans(0, 0, d7, 0, 0, -np.pi/4) - self.qr = np.array(7) self.qz = np.zeros(7) - self.addconfiguration("qr", self.qr) self.addconfiguration("qz", self.qz) diff --git a/src/roboticstoolbox/models/URDF/Jaco.py b/src/roboticstoolbox/models/URDF/Jaco.py index 1e79633b1..87ed61b4f 100644 --- a/src/roboticstoolbox/models/URDF/Jaco.py +++ b/src/roboticstoolbox/models/URDF/Jaco.py @@ -35,8 +35,10 @@ def __init__(self): gripper_link_index=9, ) - self.qr = np.array([0, 45, 60, 0, 0, 0]) * np.pi / 180 - self.qz = np.zeros(6) + # 6 arm joints + 4 gripper joints (2 fingers x [finger, finger_tip]), + # gripper joints left at 0 (fully open) for both named configurations + self.qr = np.array([0, 45, 60, 0, 0, 0, 0, 0, 0, 0]) * np.pi / 180 + self.qz = np.zeros(10) self.addconfiguration("qr", self.qr) self.addconfiguration("qz", self.qz) diff --git a/src/roboticstoolbox/models/URDF/px150.py b/src/roboticstoolbox/models/URDF/px150.py index 0d0a4fbf9..f1fef2e75 100644 --- a/src/roboticstoolbox/models/URDF/px150.py +++ b/src/roboticstoolbox/models/URDF/px150.py @@ -37,7 +37,7 @@ def __init__(self): ) self.qr = np.array([0, -0.3, 0, -2.2, 0, 2.0, np.pi / 4, 0]) - self.qz = np.zeros(7) + self.qz = np.zeros(8) self.addconfiguration("qr", self.qr) self.addconfiguration("qz", self.qz) diff --git a/src/roboticstoolbox/models/URDF/rx150.py b/src/roboticstoolbox/models/URDF/rx150.py index 3cb394ebb..a0ff57605 100644 --- a/src/roboticstoolbox/models/URDF/rx150.py +++ b/src/roboticstoolbox/models/URDF/rx150.py @@ -37,7 +37,7 @@ def __init__(self): ) self.qr = np.array([0, -0.3, 0, -2.2, 0, 2.0, np.pi / 4, 0]) - self.qz = np.zeros(7) + self.qz = np.zeros(8) self.addconfiguration("qr", self.qr) self.addconfiguration("qz", self.qz) diff --git a/src/roboticstoolbox/models/URDF/rx200.py b/src/roboticstoolbox/models/URDF/rx200.py index 8424f731b..6382837eb 100644 --- a/src/roboticstoolbox/models/URDF/rx200.py +++ b/src/roboticstoolbox/models/URDF/rx200.py @@ -37,7 +37,7 @@ def __init__(self): ) self.qr = np.array([0, -0.3, 0, -2.2, 0, 2.0, np.pi / 4, 0]) - self.qz = np.zeros(7) + self.qz = np.zeros(8) self.addconfiguration("qr", self.qr) self.addconfiguration("qz", self.qz) diff --git a/src/roboticstoolbox/robot/BaseRobot.py b/src/roboticstoolbox/robot/BaseRobot.py index 8ebc703f2..1d9673bb6 100644 --- a/src/roboticstoolbox/robot/BaseRobot.py +++ b/src/roboticstoolbox/robot/BaseRobot.py @@ -1725,7 +1725,7 @@ def addconfiguration_attr(self, name: str, q: ArrayLike, unit: str = "rad"): self._configs[name] = v setattr(self, name, v) - def addconfiguration(self, name: str, q: NDArray): + def addconfiguration(self, name: str, q: ArrayLike): """ Add a named joint configuration @@ -1751,7 +1751,7 @@ def addconfiguration(self, name: str, q: NDArray): """ - self._configs[name] = q + self._configs[name] = np.array(getvector(q, self.n)) def configurations_str(self, border="thin"): deg = 180 / np.pi From 41ef99170fceee69c96de785d7176e95ad80cb6f Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Mon, 20 Jul 2026 16:30:42 +1000 Subject: [PATCH 2/2] fix(models): fix 3 more addconfiguration() length mismatches surfaced on current main The resurrected addconfiguration() validation (previous commit, originally authored 2026-07-05) only covered the 5 broken configs it found at the time. Re-running it against current main's model set surfaces 3 more: - Hyper/Hyper3d: self.qr = np.array(N) is the exact same 0-d-scalar-not- a-vector bug already fixed for LWR4 in the previous commit (np.array(N) holds the scalar N, not an N-element vector). Neither class's own docstring lists qr as a defined configuration (only qz) - same as LWR4, this was dead/broken code, not a documented feature. Removed, matching LWR4's precedent, rather than inventing an unverified pose. - DH.AL5D: all of its DH parameter arrays (a, d, alpha, offset, center_of_mass, moments_of_inertia, joint_limits) are defined with 4 entries, but the link-building loop was `for j in range(3)`, building only 3 links while "home" already had 4 elements - a genuine off-by- one, confirmed against the real 4-DOF Lynxmotion AL5D. Fixed to range(4). Also fixed the References link, which used mismatched quote characters instead of proper reST backtick syntax and so never rendered as a link. Verified: full suite green (620 passed, 72 skipped); AL5D/Hyper/Hyper3d construct, print, and fkine correctly with their corrected joint counts. Co-Authored-By: Claude Sonnet 5 --- src/roboticstoolbox/models/DH/AL5D.py | 4 ++-- src/roboticstoolbox/models/DH/Hyper.py | 2 -- src/roboticstoolbox/models/DH/Hyper3d.py | 2 -- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/roboticstoolbox/models/DH/AL5D.py b/src/roboticstoolbox/models/DH/AL5D.py index 8da0fb246..942becc11 100644 --- a/src/roboticstoolbox/models/DH/AL5D.py +++ b/src/roboticstoolbox/models/DH/AL5D.py @@ -33,7 +33,7 @@ class AL5D(DHRobot): :References: - - 'Reference of the robot '_ + - `Reference of the robot `_ .. codeauthor:: Tassos Natsakis """ # noqa @@ -85,7 +85,7 @@ def __init__(self, symbolic=False): links = [] - for j in range(3): + for j in range(4): link = RevoluteMDH( d=d[j], a=a[j], diff --git a/src/roboticstoolbox/models/DH/Hyper.py b/src/roboticstoolbox/models/DH/Hyper.py index 1839b5f89..7609a600a 100644 --- a/src/roboticstoolbox/models/DH/Hyper.py +++ b/src/roboticstoolbox/models/DH/Hyper.py @@ -69,10 +69,8 @@ def __init__(self, N=10, a=None, symbolic=False): links, name="Hyper" + str(N), keywords=("symbolic",), symbolic=symbolic ) - self.qr = np.array(N) self.qz = np.zeros(N) - self.addconfiguration("qr", self.qr) self.addconfiguration("qz", self.qz) diff --git a/src/roboticstoolbox/models/DH/Hyper3d.py b/src/roboticstoolbox/models/DH/Hyper3d.py index 39605d433..1db199722 100644 --- a/src/roboticstoolbox/models/DH/Hyper3d.py +++ b/src/roboticstoolbox/models/DH/Hyper3d.py @@ -69,10 +69,8 @@ def __init__(self, N=10, a=None, symbolic=False): links, name="Hyper3d" + str(N), keywords=("symbolic",), symbolic=symbolic ) - self.qr = np.array(N) self.qz = np.zeros(N) - self.addconfiguration("qr", self.qr) self.addconfiguration("qz", self.qz)