From a83b0e1487a5618f1aff7b1d39b7ee356694c4bc Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Wed, 9 Sep 2026 23:50:14 +0100 Subject: [PATCH 01/34] `Resource`: turn about a joint, and the links and end-effectors that need it An articulated device has joints that are not at a resource's own corner, and nothing in PyLabRobot could express one. A plate hotel's carousel, a centrifuge rotor, a hinge, and every link of an arm pivot on a point somewhere on the part; `Resource.rotate` has always turned about the left front bottom corner, so a link modelled today swings off its own joint. - `rotate` and `rotated` take an optional `reference`: the point to turn about, in the resource's own frame. The resource is carried by however far the turn moved that point, which leaves the point where it was and the resource swinging on it. `reference` is measured in the resource's frame while `location` is measured in the parent's, so the offset is taken back through the parent's rotation, a rotation matrix inverting by transposition. - `Link` is one rigid member of a chain: a line between two joints, with no width or depth, so the joint it turns on is its own origin. Material is `bolt_on`'d as children with their own offsets, which is how a robot description keeps a link's frame apart from the shape around it - the shape can overhang either joint without the kinematics noticing. - `MechanicalGripper` is a `Link`, because on an arm that is what it is: it spans the joint it turns on to the point it grips at, which is its tool centre point. Its body, fingers and pads are material bolted to that span, and how far apart the fingers stand is state rather than shape. Behaviour: `reference` defaults to None and the added path is skipped entirely without one, so every existing caller turns about the corner exactly as before. Tests: the primitives are new, so both carry their own - a chain folding on its joints, `turn_to` being absolute where `rotate` accumulates, `bolt_on` centring material across a link, the jaws standing symmetrically at a commanded width and refusing one they cannot reach, and a pad sitting the same way on both fingers. `resource_tests` covers the pivot itself, asserting on the reference point standing still rather than on the location that moves to keep it there. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/__init__.py | 2 + pylabrobot/resources/end_effector.py | 134 +++++++++++++++++++++ pylabrobot/resources/end_effector_tests.py | 85 +++++++++++++ pylabrobot/resources/manipulator.py | 119 ++++++++++++++++++ pylabrobot/resources/manipulator_tests.py | 86 +++++++++++++ pylabrobot/resources/resource.py | 69 ++++++++++- pylabrobot/resources/resource_tests.py | 31 +++++ 7 files changed, 521 insertions(+), 5 deletions(-) create mode 100644 pylabrobot/resources/end_effector.py create mode 100644 pylabrobot/resources/end_effector_tests.py create mode 100644 pylabrobot/resources/manipulator.py create mode 100644 pylabrobot/resources/manipulator_tests.py diff --git a/pylabrobot/resources/__init__.py b/pylabrobot/resources/__init__.py index ca92f0edc5b..7b97c418f16 100644 --- a/pylabrobot/resources/__init__.py +++ b/pylabrobot/resources/__init__.py @@ -23,6 +23,7 @@ from .corning import * from .deck import Deck from .diy import * +from .end_effector import Finger, MechanicalGripper from .eppendorf import * from .errors import ResourceNotFoundError from .greiner import * @@ -30,6 +31,7 @@ from .itemized_resource import ItemizedResource from .lid import Lid, Liddable from .liquid import Liquid +from .manipulator import Link, bolt_on from .nest import * from .opentrons import * from .perkin_elmer import * diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py new file mode 100644 index 00000000000..284ab18597e --- /dev/null +++ b/pylabrobot/resources/end_effector.py @@ -0,0 +1,134 @@ +"""End-effectors: what an arm carries at its wrist, and the parts they are made of. + +A mechanical gripper takes hold by closing onto a resource and lets go by opening. It is a link, +because that is what it is on an arm: it spans the joint it turns on to the point it grips at, +which is its tool centre point. +""" + +from typing import Optional, Tuple, cast + +from pylabrobot.resources.coordinate import Coordinate +from pylabrobot.resources.manipulator import Link, bolt_on +from pylabrobot.resources.resource import Resource + + +class Finger(Resource): + """One jaw of a gripper: what closes onto a resource, carrying the pad that touches it. + + A body and its pad, and no more than that yet. Two things it will carry once there is something + to read them from: which of its faces makes contact, so a grip can be stated against the surface + that holds rather than against the finger's own corner, and what the finger senses, since a + gripper that reports force reports it per finger. + """ + + def __init__( + self, + name: str, + size_x: float, + size_y: float, + size_z: float, + category: str = "finger", + model: Optional[str] = None, + ): + super().__init__( + name=name, size_x=size_x, size_y=size_y, size_z=size_z, category=category, model=model + ) + self.pad: Optional[Resource] = None + """What meets the resource, when the finger has one bolted to it.""" + + +class MechanicalGripper(Link): + """A gripper that holds by closing two fingers on what it takes. + + A link, because on an arm that is what it is: it spans the joint it turns on to the point it + grips at, which is `tool_center_point`. Its body, its two fingers and the pad on each are material + bolted to that span. How far apart the fingers stand is state rather than shape, so `jaw_width` + moves them. + """ + + def __init__( + self, + name: str, + length: float, + body: Tuple[float, float, float, float, float], + finger: Tuple[float, float, float, float, float], + pad: Tuple[float, float, float, float, float], + jaw_range: Tuple[float, float], + jaw_width: Optional[float] = None, + category: str = "mechanical_gripper", + model: Optional[str] = None, + ): + """ + Args: + name: what to call this one. + length: the joint it turns on to the grip centre, in mm. + body: the body's size, how far along the link it starts, and how far above it stands, in mm. + finger: the same for one finger. There are two, either side of the span. + pad: the same for the pad on a finger's end, measured from the joint as the rest are. + jaw_range: how far apart the fingers stand, closed and open, in mm. + jaw_width: how far apart they stand to begin with, in mm. Where a gripper is known to come + up at a particular width - the one it homes at, say - that is what to build it at, so the + model does not start out claiming a width nothing has read. Open, when not given. + """ + super().__init__(name=name, length=length, category=category, model=model) + self.jaw_range = jaw_range + self._jaw_width = jaw_range[1] if jaw_width is None else jaw_width + low, high = jaw_range + if not low <= self._jaw_width <= high: + raise ValueError(f"the jaws open {low} to {high} mm, so cannot start at {self._jaw_width}") + + self.body = bolt_on(self, "body", body) + self.fingers = [ + cast(Finger, bolt_on(self, f"finger_{side}", finger, of=Finger)) for side in ("left", "right") + ] + for on in self.fingers: + on.pad = bolt_on(on, "pad", (pad[0], pad[1], pad[2], pad[3] - finger[3], pad[4] - finger[4])) + # A pad is fixed to its finger, centred in the finger's thickness, so it sits the same way + # on both of them. `bolt_on` centres material across a link, and a finger is not a link: its + # own origin is a corner, so centring there leaves one pad inside the jaws and the other + # outside them. + where = cast(Coordinate, on.pad.location) + on.pad.location = Coordinate(where.x, (finger[1] - pad[1]) / 2, where.z) + self.pads = [cast(Resource, on.pad) for on in self.fingers] + self._place_the_fingers() + + @property + def tool_center_point(self) -> Coordinate: + """The tool center point: where this tool is programmed against, as an offset from where it is + mounted. + + A gripper's far joint carries nothing, so what sits there is the point it grips at. + + In PyLabRobot a tool center point is always this offset - a property of the tool, which changes + when a different one is fitted and not when the arm moves. Robot controllers also use the term + for where that point currently is in the robot's frame; here that is a location, and something + an arm answers rather than a tool. + + Returns: + The grip centre, from the joint this gripper turns on. + """ + return self.far_joint + + @property + def jaw_width(self) -> float: + """How far apart the fingers stand, in mm.""" + return self._jaw_width + + @jaw_width.setter + def jaw_width(self, width: float) -> None: + low, high = self.jaw_range + if not low <= width <= high: + raise ValueError(f"the jaws open {low} to {high} mm, not {width}") + self._jaw_width = width + self._place_the_fingers() + + def _place_the_fingers(self) -> None: + """Stand the fingers either side of the span, as far apart as the jaws are open.""" + for finger, side in zip(self.fingers, (1.0, -1.0)): + here = cast(Coordinate, finger.location) + finger.location = Coordinate( + here.x, side * self._jaw_width / 2.0 - finger.get_size_y() / 2.0, here.z + ) + + def serialize(self) -> dict: + return {**super().serialize(), "jaw_range": list(self.jaw_range)} diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py new file mode 100644 index 00000000000..8a5e5d13515 --- /dev/null +++ b/pylabrobot/resources/end_effector_tests.py @@ -0,0 +1,85 @@ +import unittest +from typing import cast + +from pylabrobot.resources.coordinate import Coordinate +from pylabrobot.resources.end_effector import MechanicalGripper +from pylabrobot.resources.resource import Resource + +# A gripper with every part a different size, so a part placed by the wrong measurement lands +# somewhere this notices. +LENGTH = 100.0 +BODY = (50.0, 80.0, 20.0, -10.0, 0.0) +FINGER = (30.0, 6.0, 8.0, 60.0, 4.0) +PAD = (10.0, 4.0, 12.0, 85.0, -6.0) +JAW_RANGE = (20.0, 90.0) + + +def gripper(**overrides) -> MechanicalGripper: + return MechanicalGripper( + name="g", length=LENGTH, body=BODY, finger=FINGER, pad=PAD, jaw_range=JAW_RANGE, **overrides + ) + + +class TestTheSpan(unittest.TestCase): + """A gripper is a link: it spans the joint it turns on to the point it grips at.""" + + def test_the_grip_centre_is_the_far_joint(self): + """A link's far joint is where the next link would go, and a gripper carries no next link, so + what sits there is the point it is programmed against.""" + g = gripper() + self.assertEqual(g.tool_center_point, g.far_joint) + self.assertEqual(g.tool_center_point, Coordinate(100.0, 0.0, 0.0)) + + +class TestJaws(unittest.TestCase): + """How wide the jaws stand is state, not shape.""" + + def test_a_width_stands_the_fingers_that_far_apart(self): + """Measured centre to centre between the two fingers, symmetrically about the span, so a width + applied to one finger only, or applied twice to one side, fails this.""" + g = gripper() + for width in (90.0, 40.0, 20.0): + g.jaw_width = width + left, right = g.fingers + centres = [ + cast(Coordinate, finger.location).y + finger.get_size_y() / 2 for finger in (left, right) + ] + self.assertAlmostEqual(centres[0] - centres[1], width) + self.assertAlmostEqual(centres[0] + centres[1], 0.0) + + def test_the_jaws_refuse_a_width_they_do_not_reach(self): + """At construction and afterwards alike: a model claiming a width the drive cannot reach would + put the fingers where the arm cannot.""" + with self.assertRaises(ValueError): + gripper(jaw_width=200.0) + g = gripper() + with self.assertRaises(ValueError): + g.jaw_width = 5.0 + self.assertEqual(g.jaw_width, 90.0) + + def test_a_gripper_starts_open_unless_told_otherwise(self): + """Open is the safe assumption for a model nothing has read yet, and a gripper known to home + at a width is built at it instead.""" + self.assertEqual(gripper().jaw_width, 90.0) + self.assertEqual(gripper(jaw_width=35.0).jaw_width, 35.0) + + +class TestPads(unittest.TestCase): + """What actually touches the resource.""" + + def test_a_pad_sits_the_same_way_on_both_fingers(self): + """A pad is fixed to its finger, so it sits identically on each. Centring it across the finger + the way material is centred across a link would put one pad inside the jaws and the other + outside, since a finger's own origin is a corner rather than its middle - and a gripper whose + two pads face opposite ways grips nothing where the model says it does.""" + g = gripper() + left, right = (cast(Coordinate, cast(Resource, finger.pad).location) for finger in g.fingers) + self.assertEqual(left, right) + + finger_thickness, pad_thickness = FINGER[1], PAD[1] + self.assertGreaterEqual(left.y, 0.0) + self.assertLessEqual(left.y + pad_thickness, finger_thickness) + + +if __name__ == "__main__": + unittest.main() diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py new file mode 100644 index 00000000000..5f1598cfcc5 --- /dev/null +++ b/pylabrobot/resources/manipulator.py @@ -0,0 +1,119 @@ +"""The moving mechanism of an arm: the links its joints turn between. + +A manipulator is a chain of links and powered joints. A link is one rigid member of that chain, +and nothing else: the material bolted around it hangs off as children of its own, so the shape can +overhang either joint without the kinematics noticing. That is the split every robot description +makes, and it is what lets one length stand for the geometry and another for the part. +""" + +from typing import Optional, Tuple, Type + +from pylabrobot.resources.coordinate import Coordinate +from pylabrobot.resources.resource import Resource +from pylabrobot.resources.rotation import Rotation + + +class Link(Resource): + """The span between the joint a link turns on and the joint it carries. + + A line, not a body: its length is the distance between two joints and it has no width or depth, + so the joint it turns on is its own origin and turning it needs nothing taken out. The material + around it hangs off as children with their own offsets, which is how a robot description keeps a + link's frame apart from the shape bolted to it - the shape can overhang either joint without the + kinematics noticing. + + Unrotated it lies along +X. + """ + + def __init__( + self, + name: str, + length: float, + category: str = "link", + model: Optional[str] = None, + ): + """ + Args: + name: what to call this one. + length: joint to joint, in mm. + category: what kind of resource this is. + model: which link this is. + """ + super().__init__( + name=name, size_x=length, size_y=0.0, size_z=0.0, category=category, model=model + ) + + @property + def far_joint(self) -> Coordinate: + """The joint this link carries, in its own frame. + + Where the next link is placed, since a child is placed in its parent's own frame and the + parent's rotation is applied on top of that. + + Returns: + The far joint, from this link's near one. + """ + return Coordinate(self.get_size_x(), 0.0, 0.0) + + def turn_to(self, angle: float, about: Optional[Coordinate] = None) -> None: + """Point the link along `angle`, turning on the joint it is mounted on. + + Absolute, unlike `rotate`, which turns by an amount: a link driven to the same angle twice + lands in the same place both times. The joint is the link's own origin, so turning does not + move it and nothing has to be taken out. + + Args: + angle: the deck angle to point along, in degrees. + about: where the joint sits, in the frame this link is placed in. Left where it is when None. + + Raises: + RuntimeError: If the link is not placed and no joint is given. + """ + if about is not None: + self.location = about + if self.location is None: + raise RuntimeError(f"{self.name} is not on a joint, so there is nothing for it to turn on") + self.rotation = Rotation(z=angle) + # `rotation` is a plain attribute, unlike `location`, so nothing hears about it being set. + # Anything watching the model - a viewer, a collision check - learns of a joint moving here or + # not at all. + self._state_updated() + + +def bolt_on( + link: Resource, + what: str, + part: Tuple[float, float, float, float, float], + of: Type[Resource] = Resource, +) -> Resource: + """Hang material on a link, centred across it and standing where the part says. + + The part is a model in its own right, named for the link it hangs on and what it is: a link is a + line through its joints and carries no material itself, so anything to be said about the material + - what it is made of, what it looks like - is said about the part rather than about the link. + Two parts that are the same thing on either side of a span share the name, because they are one + model mounted twice: the category is what the part is, where the name distinguishes the copies. + A link with no model of its own has nothing to name its parts after, and they get none either. + + Args: + link: the link it is bolted to. + what: what the part is, which names it and gives it a category. + part: its size, how far along the link it starts from the joint, and how far above the link + it stands. A link is a line through the joints, so the material around it is rarely centred + on it: an arm that steps down to its gripper hangs each part at its own height. + of: what to make it, for material that is more than a box. + + Returns: + The part. + """ + category = what.split("_")[0] + made = of( + name=f"{link.name}_{what}", + size_x=part[0], + size_y=part[1], + size_z=part[2], + category=category, + model=f"{link.model}_{category}" if link.model else None, + ) + link.assign_child_resource(made, location=Coordinate(part[3], -part[1] / 2, part[4])) + return made diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py new file mode 100644 index 00000000000..70a1ec08313 --- /dev/null +++ b/pylabrobot/resources/manipulator_tests.py @@ -0,0 +1,86 @@ +import unittest + +from pylabrobot.resources.coordinate import Coordinate +from pylabrobot.resources.manipulator import Link, bolt_on +from pylabrobot.resources.resource import Resource +from pylabrobot.utils.linalg import matrix_vector_multiply_3x3 + + +class TestLink(unittest.TestCase): + """A link is the span between two joints, and nothing else.""" + + def test_a_chain_folds_on_its_joints(self): + """Two links, the second placed on the first's far joint. Straight, the far end is both + lengths out; folded square, the second link leaves the first's end sideways. Measured at the + end of the chain rather than on either link, because that is the point a chain exists to + place, and it is where an error in the joint offset would show.""" + base = Resource(name="base", size_x=500, size_y=500, size_z=0) + first = Link(name="first", length=100.0) + second = Link(name="second", length=50.0) + base.assign_child_resource(first, location=Coordinate(0, 0, 0)) + first.assign_child_resource(second, location=first.far_joint) + + self.assertEqual(second.get_absolute_location() + second.far_joint, Coordinate(150, 0, 0)) + + second.turn_to(90) + # Through the link's own rotation rather than a vector worked out here, so the test exercises + # the turn instead of restating its answer. + carried = matrix_vector_multiply_3x3( + second.get_absolute_rotation().get_rotation_matrix(), second.far_joint.vector() + ) + end = second.get_absolute_location() + Coordinate(*carried) + self.assertEqual(end, Coordinate(100, 50, 0)) + + def test_turning_to_an_angle_is_absolute(self): + """`turn_to` points the link somewhere, where `rotate` turns it by an amount. A drive commanded + to the same angle twice has not moved twice, and the model has to say the same.""" + base = Resource(name="base", size_x=500, size_y=500, size_z=0) + link = Link(name="link", length=100.0) + base.assign_child_resource(link, location=Coordinate(0, 0, 0)) + + link.turn_to(30) + link.turn_to(30) + self.assertEqual(link.rotation.z, 30) + + link.rotate(z=30) + self.assertEqual(link.rotation.z, 60) + + def test_a_link_can_be_given_the_joint_it_turns_on(self): + """The joint is where the link is placed, so naming one places it. A link that has never been + placed and is given none has nothing to turn on, and says so rather than turning about the + origin.""" + base = Resource(name="base", size_x=500, size_y=500, size_z=0) + link = Link(name="link", length=100.0) + base.assign_child_resource(link, location=Coordinate(0, 0, 0)) + + link.turn_to(0, about=Coordinate(10, 20, 30)) + self.assertEqual(link.location, Coordinate(10, 20, 30)) + + with self.assertRaises(RuntimeError): + Link(name="loose", length=100.0).turn_to(0) + + +class TestBoltOn(unittest.TestCase): + """Material hung on a link, which carries none of its own.""" + + def test_a_part_is_centred_across_the_link_and_stands_where_it_says(self): + """A link is a line through its joints, so material is centred across it in Y and offset along + and above it by what the part states. Its category is what the part is, and its model is named + after the link's, so two of the same part on one link are one model mounted twice.""" + link = Link(name="link", length=100.0, model="a_link") + part = bolt_on(link, "shell", (40.0, 12.0, 5.0, 7.0, 3.0)) + + self.assertEqual(part.location, Coordinate(7.0, -6.0, 3.0)) + self.assertEqual( + (part.name, part.category, part.model), ("link_shell", "shell", "a_link_shell") + ) + + def test_a_link_with_no_model_gives_its_parts_none(self): + """There is nothing to name them after, and a part carrying a model nothing describes is worse + than one carrying none.""" + part = bolt_on(Link(name="link", length=100.0), "shell", (40.0, 12.0, 5.0, 0.0, 0.0)) + self.assertIsNone(part.model) + + +if __name__ == "__main__": + unittest.main() diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index 6d367ac8f2c..90cbe860741 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -890,12 +890,56 @@ def location(self, location: Optional[Coordinate]) -> None: if changed and self.parent is not None: self._state_updated() - def rotate(self, x: float = 0, y: float = 0, z: float = 0): - """Rotate counter-clockwise by the given number of degrees.""" + def rotate( + self, + x: float = 0, + y: float = 0, + z: float = 0, + reference: Optional[Coordinate] = None, + ): + """Rotate counter-clockwise by the given number of degrees. + + A resource turns about its own left front bottom corner. `reference` names a different point + to turn about - a hinge, a joint, an axis the part really pivots on - and the resource is + moved as it turns by however far the turn carried that point, which leaves the point where it + was and the resource swinging on it. Left about the corner when None, which is what every + caller that does not ask for one gets. + + Args: + x: degrees to turn about X. + y: degrees to turn about Y. + z: degrees to turn about Z. + reference: the point to turn about, from this resource's left front bottom corner. Its own + corner when None. + """ + # Only a turn about another point needs to know which way this was already facing, and + # building a rotation matrix is twelve trigonometry calls: without a reference this stays out + # of the way, since `rotate` is on the path every placement takes. + turning_on = reference if self.location is not None else None + before = self.get_absolute_rotation().get_rotation_matrix() if turning_on is not None else None self.rotation.x = (self.rotation.x + x) % 360 self.rotation.y = (self.rotation.y + y) % 360 self.rotation.z = (self.rotation.z + z) % 360 + + if turning_on is not None and before is not None: + after = self.get_absolute_rotation().get_rotation_matrix() + was = matrix_vector_multiply_3x3(before, turning_on.vector()) + now = matrix_vector_multiply_3x3(after, turning_on.vector()) + carried = Coordinate(was[0] - now[0], was[1] - now[1], was[2] - now[2]) + # `location` is measured in the parent's frame while `reference` is in this resource's, so + # what the turn carried has to be taken back through the parent's own rotation. A rotation + # matrix inverts by transposing. + parent = self.parent + if parent is not None: + turned = parent.get_absolute_rotation().get_rotation_matrix() + carried = Coordinate( + *matrix_vector_multiply_3x3( + [[turned[j][i] for j in range(3)] for i in range(3)], carried.vector() + ) + ) + self.location = cast(Coordinate, self.location) + carried + # Rotation is part of the resource's state; notify subscribers (e.g. the # Visualizer) so they can re-render. self._state_updated() @@ -905,11 +949,26 @@ def copy(self) -> Self: resource_copy.load_all_state(self.serialize_all_state()) return resource_copy - def rotated(self, x: float = 0, y: float = 0, z: float = 0) -> Self: - """Return a copy of this resource rotated by the given number of degrees.""" + def rotated( + self, + x: float = 0, + y: float = 0, + z: float = 0, + reference: Optional[Coordinate] = None, + ) -> Self: + """Return a copy of this resource rotated by the given number of degrees. + Args: + x: degrees to turn about X. + y: degrees to turn about Y. + z: degrees to turn about Z. + reference: the point to turn about, as `rotate` takes it. + + Returns: + The rotated copy. + """ new_resource = self.copy() - new_resource.rotate(x=x, y=y, z=z) + new_resource.rotate(x=x, y=y, z=z, reference=reference) return new_resource def at(self, location: Coordinate) -> Self: diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 1ebd65841f7..de23dc50b67 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -332,6 +332,37 @@ def test_rotation90(self): self.assertAlmostEqual(c.get_absolute_size_x(), 20) self.assertAlmostEqual(c.get_absolute_size_y(), 10) + def test_rotating_about_a_reference_point_leaves_that_point_where_it_was(self): + """A resource turns about its own left front bottom corner. `reference` names another point to + turn on - a hinge, a joint - and the resource is carried so that point does not move, which is + what a joint is. Checked on the point itself rather than on the resource's location, since the + location moving is the mechanism and the point standing still is the promise.""" + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate(0, 0, 0)) + far_end = Coordinate(100, 0, 0) + + before = bar.get_absolute_location() + far_end + bar.rotate(z=90, reference=far_end) + carried = matrix_vector_multiply_3x3( + bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() + ) + + self.assertEqual(bar.get_absolute_location() + Coordinate(*carried), before) + self.assertEqual(bar.location, Coordinate(100, -100, 0)) + + def test_rotating_without_a_reference_point_turns_about_the_corner(self): + """The default, and every caller that does not ask for a point gets it: the resource turns + where it stands and its location does not move.""" + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate(30, 40, 0)) + + bar.rotate(z=90) + self.assertEqual(bar.location, Coordinate(30, 40, 0)) + def test_rotation180(self): r = Resource("parent", size_x=200, size_y=100, size_z=100) r.location = Coordinate.zero() From bbede49d025ecd0f08f4c2685e3ebb957f07f86e Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 10:48:49 +0100 Subject: [PATCH 02/34] `MechanicalGripper`: define an end-effector by its mechanical interface and tool centre point The module docstring named a tool centre point without saying what it is measured from, and justified the gripper being a `Link` by asserting it ("it is a link, because that is what it is on an arm"). It now uses the vocabulary a reader arrives with - end-effector, tool and end-of-arm tooling as one thing, fitted at the wrist flange - and states the tool centre point as an offset from that flange, belonging to the tool rather than to the arm. That offset is what makes the gripper a link: it spans the interface it is bolted to and the point it grips at, which is the same separation ROS-Industrial draws between `flange` and a tool frame. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index 284ab18597e..28a820a8ec4 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -1,8 +1,11 @@ -"""End-effectors: what an arm carries at its wrist, and the parts they are made of. +"""End-effectors: what is fitted at an arm's mechanical interface, and the parts they are made of. -A mechanical gripper takes hold by closing onto a resource and lets go by opening. It is a link, -because that is what it is on an arm: it spans the joint it turns on to the point it grips at, -which is its tool centre point. +An end-effector - equally a tool, or end-of-arm tooling - is what an arm carries at its wrist +flange so that it can do its task. Its tool centre point is the point a move is programmed +against, stated as an offset from that flange, and it belongs to the tool rather than to the arm: +fit a different one and the point moves with it. + +`MechanicalGripper` spans that offset, flange to grip centre, which is why it is a `Link`. """ from typing import Optional, Tuple, cast From 70e43f4e3bfe87832f142f682c1a50674a12f91c Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 15:16:25 +0100 Subject: [PATCH 03/34] `MechanicalGripper`: drop the `Finger` class, which held nothing a `Resource` does not `Finger` added one attribute over `Resource`: a `pad` pointing at a resource `bolt_on` had already assigned as its child, so `finger.pad is finger.children[0]`. Nothing outside the module read it, nothing type-checked against the class, and the viewer tells a finger from a pad by category, which is set either way. The `cast(Finger, ...)` at its only construction existed to let mypy accept the `pad` assignment - the class's sole consumer was the attribute that was its sole reason to exist. Its docstring said as much: a list of two things it would carry "once there is something to read them from". It can come back the day one of them arrives with a field in it. The pads are kept as `self.pads`, which is how the one external caller already reaches them. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/__init__.py | 2 +- pylabrobot/resources/end_effector.py | 40 ++++------------------ pylabrobot/resources/end_effector_tests.py | 3 +- 3 files changed, 9 insertions(+), 36 deletions(-) diff --git a/pylabrobot/resources/__init__.py b/pylabrobot/resources/__init__.py index 7b97c418f16..e0364e9dc6e 100644 --- a/pylabrobot/resources/__init__.py +++ b/pylabrobot/resources/__init__.py @@ -23,7 +23,7 @@ from .corning import * from .deck import Deck from .diy import * -from .end_effector import Finger, MechanicalGripper +from .end_effector import MechanicalGripper from .eppendorf import * from .errors import ResourceNotFoundError from .greiner import * diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index 28a820a8ec4..af9a0690a74 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -12,32 +12,6 @@ from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.manipulator import Link, bolt_on -from pylabrobot.resources.resource import Resource - - -class Finger(Resource): - """One jaw of a gripper: what closes onto a resource, carrying the pad that touches it. - - A body and its pad, and no more than that yet. Two things it will carry once there is something - to read them from: which of its faces makes contact, so a grip can be stated against the surface - that holds rather than against the finger's own corner, and what the finger senses, since a - gripper that reports force reports it per finger. - """ - - def __init__( - self, - name: str, - size_x: float, - size_y: float, - size_z: float, - category: str = "finger", - model: Optional[str] = None, - ): - super().__init__( - name=name, size_x=size_x, size_y=size_y, size_z=size_z, category=category, model=model - ) - self.pad: Optional[Resource] = None - """What meets the resource, when the finger has one bolted to it.""" class MechanicalGripper(Link): @@ -81,18 +55,18 @@ def __init__( raise ValueError(f"the jaws open {low} to {high} mm, so cannot start at {self._jaw_width}") self.body = bolt_on(self, "body", body) - self.fingers = [ - cast(Finger, bolt_on(self, f"finger_{side}", finger, of=Finger)) for side in ("left", "right") + self.fingers = [bolt_on(self, f"finger_{side}", finger) for side in ("left", "right")] + self.pads = [ + bolt_on(on, "pad", (pad[0], pad[1], pad[2], pad[3] - finger[3], pad[4] - finger[4])) + for on in self.fingers ] - for on in self.fingers: - on.pad = bolt_on(on, "pad", (pad[0], pad[1], pad[2], pad[3] - finger[3], pad[4] - finger[4])) + for on in self.pads: # A pad is fixed to its finger, centred in the finger's thickness, so it sits the same way # on both of them. `bolt_on` centres material across a link, and a finger is not a link: its # own origin is a corner, so centring there leaves one pad inside the jaws and the other # outside them. - where = cast(Coordinate, on.pad.location) - on.pad.location = Coordinate(where.x, (finger[1] - pad[1]) / 2, where.z) - self.pads = [cast(Resource, on.pad) for on in self.fingers] + where = cast(Coordinate, on.location) + on.location = Coordinate(where.x, (finger[1] - pad[1]) / 2, where.z) self._place_the_fingers() @property diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 8a5e5d13515..12fe47aae57 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -3,7 +3,6 @@ from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.end_effector import MechanicalGripper -from pylabrobot.resources.resource import Resource # A gripper with every part a different size, so a part placed by the wrong measurement lands # somewhere this notices. @@ -73,7 +72,7 @@ def test_a_pad_sits_the_same_way_on_both_fingers(self): outside, since a finger's own origin is a corner rather than its middle - and a gripper whose two pads face opposite ways grips nothing where the model says it does.""" g = gripper() - left, right = (cast(Coordinate, cast(Resource, finger.pad).location) for finger in g.fingers) + left, right = (cast(Coordinate, pad.location) for pad in g.pads) self.assertEqual(left, right) finger_thickness, pad_thickness = FINGER[1], PAD[1] From f8b889f0b5bc9174bd618df70b5ef6df31427fb8 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 15:18:15 +0100 Subject: [PATCH 04/34] `MechanicalGripper`: delete `bolt_on` and place each part where it goes `bolt_on` was a factory wrapped around `assign_child_resource`, and of the five things it added only one was geometry: `-size_y / 2`, centring material across the link it hangs on. The rest was naming, a model string, a `category` taken by splitting the name on an underscore, and an `of=` parameter with no caller left once `Finger` went. Its four arguments were also a bare five-number tuple - size, offset along, offset above - which says nothing at the call site about which number is which. Each part is now constructed and assigned where it is used, with the five numbers unpacked into named locals, so the placement rule is visible rather than applied out of sight. The pad no longer has its Y written and then overwritten a line later: the offset is computed once. Behaviour: both trees are byte-identical to what `bolt_on` built - every name, category, model and location - checked against a snapshot taken before the change. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/__init__.py | 2 +- pylabrobot/resources/end_effector.py | 67 ++++++++++++++++++----- pylabrobot/resources/manipulator.py | 41 +------------- pylabrobot/resources/manipulator_tests.py | 24 +------- 4 files changed, 56 insertions(+), 78 deletions(-) diff --git a/pylabrobot/resources/__init__.py b/pylabrobot/resources/__init__.py index e0364e9dc6e..3da3d237ee1 100644 --- a/pylabrobot/resources/__init__.py +++ b/pylabrobot/resources/__init__.py @@ -31,7 +31,7 @@ from .itemized_resource import ItemizedResource from .lid import Lid, Liddable from .liquid import Liquid -from .manipulator import Link, bolt_on +from .manipulator import Link from .nest import * from .opentrons import * from .perkin_elmer import * diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index af9a0690a74..fc976a25ee5 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -11,7 +11,8 @@ from typing import Optional, Tuple, cast from pylabrobot.resources.coordinate import Coordinate -from pylabrobot.resources.manipulator import Link, bolt_on +from pylabrobot.resources.manipulator import Link +from pylabrobot.resources.resource import Resource class MechanicalGripper(Link): @@ -54,19 +55,57 @@ def __init__( if not low <= self._jaw_width <= high: raise ValueError(f"the jaws open {low} to {high} mm, so cannot start at {self._jaw_width}") - self.body = bolt_on(self, "body", body) - self.fingers = [bolt_on(self, f"finger_{side}", finger) for side in ("left", "right")] - self.pads = [ - bolt_on(on, "pad", (pad[0], pad[1], pad[2], pad[3] - finger[3], pad[4] - finger[4])) - for on in self.fingers - ] - for on in self.pads: - # A pad is fixed to its finger, centred in the finger's thickness, so it sits the same way - # on both of them. `bolt_on` centres material across a link, and a finger is not a link: its - # own origin is a corner, so centring there leaves one pad inside the jaws and the other - # outside them. - where = cast(Coordinate, on.location) - on.location = Coordinate(where.x, (finger[1] - pad[1]) / 2, where.z) + # A link is a line through its joints, so material on it straddles that line: centred across + # the link in Y, and standing where the part says along it and above it. + body_x, body_y, body_z, body_along, body_above = body + self.body = Resource( + name=f"{name}_body", + size_x=body_x, + size_y=body_y, + size_z=body_z, + category="body", + model=f"{model}_body" if model else None, + ) + self.assign_child_resource(self.body, location=Coordinate(body_along, -body_y / 2, body_above)) + + finger_x, finger_y, finger_z, finger_along, finger_above = finger + self.fingers = [] + for side in ("left", "right"): + jaw = Resource( + name=f"{name}_finger_{side}", + size_x=finger_x, + size_y=finger_y, + size_z=finger_z, + category="finger", + model=f"{model}_finger" if model else None, + ) + self.assign_child_resource( + jaw, location=Coordinate(finger_along, -finger_y / 2, finger_above) + ) + self.fingers.append(jaw) + + # A pad is fixed to its finger and centred in the finger's own thickness, which is a different + # rule: a finger's origin is a corner rather than a line through it, so straddling it would + # leave one pad inside the jaws and the other outside. + pad_x, pad_y, pad_z, pad_along, pad_above = pad + self.pads = [] + for jaw in self.fingers: + face = Resource( + name=f"{jaw.name}_pad", + size_x=pad_x, + size_y=pad_y, + size_z=pad_z, + category="pad", + model=f"{jaw.model}_pad" if jaw.model else None, + ) + jaw.assign_child_resource( + face, + location=Coordinate( + pad_along - finger_along, (finger_y - pad_y) / 2, pad_above - finger_above + ), + ) + self.pads.append(face) + self._place_the_fingers() @property diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py index 5f1598cfcc5..3ba0965e950 100644 --- a/pylabrobot/resources/manipulator.py +++ b/pylabrobot/resources/manipulator.py @@ -6,7 +6,7 @@ makes, and it is what lets one length stand for the geometry and another for the part. """ -from typing import Optional, Tuple, Type +from typing import Optional from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.resource import Resource @@ -78,42 +78,3 @@ def turn_to(self, angle: float, about: Optional[Coordinate] = None) -> None: # Anything watching the model - a viewer, a collision check - learns of a joint moving here or # not at all. self._state_updated() - - -def bolt_on( - link: Resource, - what: str, - part: Tuple[float, float, float, float, float], - of: Type[Resource] = Resource, -) -> Resource: - """Hang material on a link, centred across it and standing where the part says. - - The part is a model in its own right, named for the link it hangs on and what it is: a link is a - line through its joints and carries no material itself, so anything to be said about the material - - what it is made of, what it looks like - is said about the part rather than about the link. - Two parts that are the same thing on either side of a span share the name, because they are one - model mounted twice: the category is what the part is, where the name distinguishes the copies. - A link with no model of its own has nothing to name its parts after, and they get none either. - - Args: - link: the link it is bolted to. - what: what the part is, which names it and gives it a category. - part: its size, how far along the link it starts from the joint, and how far above the link - it stands. A link is a line through the joints, so the material around it is rarely centred - on it: an arm that steps down to its gripper hangs each part at its own height. - of: what to make it, for material that is more than a box. - - Returns: - The part. - """ - category = what.split("_")[0] - made = of( - name=f"{link.name}_{what}", - size_x=part[0], - size_y=part[1], - size_z=part[2], - category=category, - model=f"{link.model}_{category}" if link.model else None, - ) - link.assign_child_resource(made, location=Coordinate(part[3], -part[1] / 2, part[4])) - return made diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index 70a1ec08313..0b32eefb2eb 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -1,7 +1,7 @@ import unittest from pylabrobot.resources.coordinate import Coordinate -from pylabrobot.resources.manipulator import Link, bolt_on +from pylabrobot.resources.manipulator import Link from pylabrobot.resources.resource import Resource from pylabrobot.utils.linalg import matrix_vector_multiply_3x3 @@ -60,27 +60,5 @@ def test_a_link_can_be_given_the_joint_it_turns_on(self): Link(name="loose", length=100.0).turn_to(0) -class TestBoltOn(unittest.TestCase): - """Material hung on a link, which carries none of its own.""" - - def test_a_part_is_centred_across_the_link_and_stands_where_it_says(self): - """A link is a line through its joints, so material is centred across it in Y and offset along - and above it by what the part states. Its category is what the part is, and its model is named - after the link's, so two of the same part on one link are one model mounted twice.""" - link = Link(name="link", length=100.0, model="a_link") - part = bolt_on(link, "shell", (40.0, 12.0, 5.0, 7.0, 3.0)) - - self.assertEqual(part.location, Coordinate(7.0, -6.0, 3.0)) - self.assertEqual( - (part.name, part.category, part.model), ("link_shell", "shell", "a_link_shell") - ) - - def test_a_link_with_no_model_gives_its_parts_none(self): - """There is nothing to name them after, and a part carrying a model nothing describes is worse - than one carrying none.""" - part = bolt_on(Link(name="link", length=100.0), "shell", (40.0, 12.0, 5.0, 0.0, 0.0)) - self.assertIsNone(part.model) - - if __name__ == "__main__": unittest.main() From bb3100a6f82e15d7066620a37cddc5d191314d14 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 15:19:26 +0100 Subject: [PATCH 05/34] `MechanicalGripper`: take each part as a size and a place, not a five-number tuple A part arrived as `(size_x, size_y, size_z, along, above)`, which says nothing at the call site about which number is which and puts two offsets in different axes beside three sizes. It is the shape an argument gets swapped in, and no type checker would notice. Each part is now a `Coordinate` for its size and a `Coordinate` for where it sits, which are the two types the resource model already has: `Resource` carries a size and a category, `assign_child_resource` takes a location, and `location` moves it afterwards. Nothing new is defined to hold them. A finger is the exception and now says so: it takes a size and an X and Z, and its Y belongs to `jaw_width` outright rather than being declared and overwritten. Behaviour: unchanged, and the tree is byte-identical to the one the tuples built. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 64 ++++++++++------------ pylabrobot/resources/end_effector_tests.py | 19 +++++-- 2 files changed, 43 insertions(+), 40 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index fc976a25ee5..d033bfcc80a 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -28,9 +28,12 @@ def __init__( self, name: str, length: float, - body: Tuple[float, float, float, float, float], - finger: Tuple[float, float, float, float, float], - pad: Tuple[float, float, float, float, float], + body: Coordinate, + body_at: Coordinate, + finger: Coordinate, + finger_at: Coordinate, + pad: Coordinate, + pad_at: Coordinate, jaw_range: Tuple[float, float], jaw_width: Optional[float] = None, category: str = "mechanical_gripper", @@ -55,55 +58,43 @@ def __init__( if not low <= self._jaw_width <= high: raise ValueError(f"the jaws open {low} to {high} mm, so cannot start at {self._jaw_width}") - # A link is a line through its joints, so material on it straddles that line: centred across - # the link in Y, and standing where the part says along it and above it. - body_x, body_y, body_z, body_along, body_above = body self.body = Resource( name=f"{name}_body", - size_x=body_x, - size_y=body_y, - size_z=body_z, + size_x=body.x, + size_y=body.y, + size_z=body.z, category="body", model=f"{model}_body" if model else None, ) - self.assign_child_resource(self.body, location=Coordinate(body_along, -body_y / 2, body_above)) + self.assign_child_resource(self.body, location=body_at) - finger_x, finger_y, finger_z, finger_along, finger_above = finger - self.fingers = [] - for side in ("left", "right"): - jaw = Resource( + # A finger has a size and no place of its own: `jaw_width` decides where it stands, and + # `_place_the_fingers` is what puts it there. + self.fingers = [ + Resource( name=f"{name}_finger_{side}", - size_x=finger_x, - size_y=finger_y, - size_z=finger_z, + size_x=finger.x, + size_y=finger.y, + size_z=finger.z, category="finger", model=f"{model}_finger" if model else None, ) - self.assign_child_resource( - jaw, location=Coordinate(finger_along, -finger_y / 2, finger_above) - ) - self.fingers.append(jaw) + for side in ("left", "right") + ] + for jaw in self.fingers: + self.assign_child_resource(jaw, location=Coordinate(finger_at.x, 0.0, finger_at.z)) - # A pad is fixed to its finger and centred in the finger's own thickness, which is a different - # rule: a finger's origin is a corner rather than a line through it, so straddling it would - # leave one pad inside the jaws and the other outside. - pad_x, pad_y, pad_z, pad_along, pad_above = pad self.pads = [] for jaw in self.fingers: face = Resource( name=f"{jaw.name}_pad", - size_x=pad_x, - size_y=pad_y, - size_z=pad_z, + size_x=pad.x, + size_y=pad.y, + size_z=pad.z, category="pad", model=f"{jaw.model}_pad" if jaw.model else None, ) - jaw.assign_child_resource( - face, - location=Coordinate( - pad_along - finger_along, (finger_y - pad_y) / 2, pad_above - finger_above - ), - ) + jaw.assign_child_resource(face, location=pad_at) self.pads.append(face) self._place_the_fingers() @@ -139,7 +130,10 @@ def jaw_width(self, width: float) -> None: self._place_the_fingers() def _place_the_fingers(self) -> None: - """Stand the fingers either side of the span, as far apart as the jaws are open.""" + """Stand the fingers either side of the span, as far apart as the jaws are open. + + A finger is the one part of a gripper whose Y is not fixed: the jaw width owns it outright. + """ for finger, side in zip(self.fingers, (1.0, -1.0)): here = cast(Coordinate, finger.location) finger.location = Coordinate( diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 12fe47aae57..d8935f4a9a2 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -7,15 +7,24 @@ # A gripper with every part a different size, so a part placed by the wrong measurement lands # somewhere this notices. LENGTH = 100.0 -BODY = (50.0, 80.0, 20.0, -10.0, 0.0) -FINGER = (30.0, 6.0, 8.0, 60.0, 4.0) -PAD = (10.0, 4.0, 12.0, 85.0, -6.0) +BODY, BODY_AT = Coordinate(50.0, 80.0, 20.0), Coordinate(-10.0, -40.0, 0.0) +FINGER, FINGER_AT = Coordinate(30.0, 6.0, 8.0), Coordinate(60.0, 0.0, 4.0) +PAD, PAD_AT = Coordinate(10.0, 4.0, 12.0), Coordinate(25.0, 1.0, -10.0) JAW_RANGE = (20.0, 90.0) def gripper(**overrides) -> MechanicalGripper: return MechanicalGripper( - name="g", length=LENGTH, body=BODY, finger=FINGER, pad=PAD, jaw_range=JAW_RANGE, **overrides + name="g", + length=LENGTH, + body=BODY, + body_at=BODY_AT, + finger=FINGER, + finger_at=FINGER_AT, + pad=PAD, + pad_at=PAD_AT, + jaw_range=JAW_RANGE, + **overrides, ) @@ -75,7 +84,7 @@ def test_a_pad_sits_the_same_way_on_both_fingers(self): left, right = (cast(Coordinate, pad.location) for pad in g.pads) self.assertEqual(left, right) - finger_thickness, pad_thickness = FINGER[1], PAD[1] + finger_thickness, pad_thickness = FINGER.y, PAD.y self.assertGreaterEqual(left.y, 0.0) self.assertLessEqual(left.y + pad_thickness, finger_thickness) From df67426cb1eccf0fcd112ae9ec8ddc00c6e2494b Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 15:54:23 +0100 Subject: [PATCH 06/34] `MechanicalGripper`: name a part's placement `location`, as the resource model does `body_at`, `finger_at` and `pad_at` named the thing `Resource.location` already names. A part's placement is a location, in the frame of whatever carries it, and calling it anything else invents a second word for one idea. The argument docs also still described the five-number tuples these replaced, and said a pad is measured from the joint - it is measured from the finger it is fixed to. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 24 ++++++++++++++-------- pylabrobot/resources/end_effector_tests.py | 15 ++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index d033bfcc80a..888a7296a3c 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -29,11 +29,11 @@ def __init__( name: str, length: float, body: Coordinate, - body_at: Coordinate, + body_location: Coordinate, finger: Coordinate, - finger_at: Coordinate, + finger_location: Coordinate, pad: Coordinate, - pad_at: Coordinate, + pad_location: Coordinate, jaw_range: Tuple[float, float], jaw_width: Optional[float] = None, category: str = "mechanical_gripper", @@ -43,9 +43,13 @@ def __init__( Args: name: what to call this one. length: the joint it turns on to the grip centre, in mm. - body: the body's size, how far along the link it starts, and how far above it stands, in mm. - finger: the same for one finger. There are two, either side of the span. - pad: the same for the pad on a finger's end, measured from the joint as the rest are. + body: how big the body is, in mm. + body_location: where it sits, from the joint this gripper turns on. + finger: how big one finger is, in mm. There are two, either side of the span. + finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s, so + what stands here for it is not used. + pad: how big the pad on a finger's end is, in mm. + pad_location: where it sits, from the finger it is fixed to. jaw_range: how far apart the fingers stand, closed and open, in mm. jaw_width: how far apart they stand to begin with, in mm. Where a gripper is known to come up at a particular width - the one it homes at, say - that is what to build it at, so the @@ -66,7 +70,7 @@ def __init__( category="body", model=f"{model}_body" if model else None, ) - self.assign_child_resource(self.body, location=body_at) + self.assign_child_resource(self.body, location=body_location) # A finger has a size and no place of its own: `jaw_width` decides where it stands, and # `_place_the_fingers` is what puts it there. @@ -82,7 +86,9 @@ def __init__( for side in ("left", "right") ] for jaw in self.fingers: - self.assign_child_resource(jaw, location=Coordinate(finger_at.x, 0.0, finger_at.z)) + self.assign_child_resource( + jaw, location=Coordinate(finger_location.x, 0.0, finger_location.z) + ) self.pads = [] for jaw in self.fingers: @@ -94,7 +100,7 @@ def __init__( category="pad", model=f"{jaw.model}_pad" if jaw.model else None, ) - jaw.assign_child_resource(face, location=pad_at) + jaw.assign_child_resource(face, location=pad_location) self.pads.append(face) self._place_the_fingers() diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index d8935f4a9a2..eacd15ea124 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -7,9 +7,9 @@ # A gripper with every part a different size, so a part placed by the wrong measurement lands # somewhere this notices. LENGTH = 100.0 -BODY, BODY_AT = Coordinate(50.0, 80.0, 20.0), Coordinate(-10.0, -40.0, 0.0) -FINGER, FINGER_AT = Coordinate(30.0, 6.0, 8.0), Coordinate(60.0, 0.0, 4.0) -PAD, PAD_AT = Coordinate(10.0, 4.0, 12.0), Coordinate(25.0, 1.0, -10.0) +BODY, BODY_LOCATION = Coordinate(50.0, 80.0, 20.0), Coordinate(-10.0, -40.0, 0.0) +FINGER, FINGER_LOCATION = Coordinate(30.0, 6.0, 8.0), Coordinate(60.0, 0.0, 4.0) +PAD, PAD_LOCATION = Coordinate(10.0, 4.0, 12.0), Coordinate(25.0, 1.0, -10.0) JAW_RANGE = (20.0, 90.0) @@ -18,11 +18,11 @@ def gripper(**overrides) -> MechanicalGripper: name="g", length=LENGTH, body=BODY, - body_at=BODY_AT, + body_location=BODY_LOCATION, finger=FINGER, - finger_at=FINGER_AT, + finger_location=FINGER_LOCATION, pad=PAD, - pad_at=PAD_AT, + pad_location=PAD_LOCATION, jaw_range=JAW_RANGE, **overrides, ) @@ -35,7 +35,6 @@ def test_the_grip_centre_is_the_far_joint(self): """A link's far joint is where the next link would go, and a gripper carries no next link, so what sits there is the point it is programmed against.""" g = gripper() - self.assertEqual(g.tool_center_point, g.far_joint) self.assertEqual(g.tool_center_point, Coordinate(100.0, 0.0, 0.0)) @@ -82,8 +81,6 @@ def test_a_pad_sits_the_same_way_on_both_fingers(self): two pads face opposite ways grips nothing where the model says it does.""" g = gripper() left, right = (cast(Coordinate, pad.location) for pad in g.pads) - self.assertEqual(left, right) - finger_thickness, pad_thickness = FINGER.y, PAD.y self.assertGreaterEqual(left.y, 0.0) self.assertLessEqual(left.y + pad_thickness, finger_thickness) From aefedd2ebe40dd4e99fbb8500f339092862bcfa3 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 15:54:23 +0100 Subject: [PATCH 07/34] `MechanicalGripper`: drop two assertions and a test that cannot fail Found by mutating the line each test names and checking the test fails. - `test_the_grip_centre_is_the_far_joint` asserted `tool_center_point == far_joint`, and the property is `return self.far_joint`. The comparison against the length along the span is the one that can fail. - `test_a_pad_sits_the_same_way_on_both_fingers` asserted the two pads are equal. They are assigned one location in a loop, so they cannot differ. The assertions that the pad lies inside the finger's thickness still catch the centring bug they were written for. - `test_rotating_without_a_reference_point_turns_about_the_corner` is covered by `test_rotation90`, `test_rotation180`, `test_rotation270` and `test_multiple_rotations`, which already fail if the default path changes. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource_tests.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index de23dc50b67..0fdbacdb955 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -352,17 +352,6 @@ def test_rotating_about_a_reference_point_leaves_that_point_where_it_was(self): self.assertEqual(bar.get_absolute_location() + Coordinate(*carried), before) self.assertEqual(bar.location, Coordinate(100, -100, 0)) - def test_rotating_without_a_reference_point_turns_about_the_corner(self): - """The default, and every caller that does not ask for a point gets it: the resource turns - where it stands and its location does not move.""" - parent = Resource("parent", size_x=500, size_y=500, size_z=10) - parent.location = Coordinate.zero() - bar = Resource("bar", size_x=100, size_y=10, size_z=10) - parent.assign_child_resource(bar, location=Coordinate(30, 40, 0)) - - bar.rotate(z=90) - self.assertEqual(bar.location, Coordinate(30, 40, 0)) - def test_rotation180(self): r = Resource("parent", size_x=200, size_y=100, size_z=100) r.location = Coordinate.zero() From 949d41f4d24bcb23b110415070485e272d3725fa Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 16:46:21 +0100 Subject: [PATCH 08/34] `Link`: drop `far_joint`, which was neither far nor a joint A joint is one degree of freedom, revolute or prismatic. `far_joint` returned a `Coordinate` - a position, with no freedom to move and nothing mounted on it - and on a gripper it is not the far point of anything either, since the fingers reach past it. The concept it was standing in for exists only on a tool, where robotics already names it: the tool centre point. `MechanicalGripper.tool_center_point` computes it directly, and the one caller that wanted a plain link's far end asks for it where it is used. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/manipulator.py | 12 ------------ pylabrobot/resources/manipulator_tests.py | 7 ++++--- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py index 3ba0965e950..6337b0c7b5f 100644 --- a/pylabrobot/resources/manipulator.py +++ b/pylabrobot/resources/manipulator.py @@ -43,18 +43,6 @@ def __init__( name=name, size_x=length, size_y=0.0, size_z=0.0, category=category, model=model ) - @property - def far_joint(self) -> Coordinate: - """The joint this link carries, in its own frame. - - Where the next link is placed, since a child is placed in its parent's own frame and the - parent's rotation is applied on top of that. - - Returns: - The far joint, from this link's near one. - """ - return Coordinate(self.get_size_x(), 0.0, 0.0) - def turn_to(self, angle: float, about: Optional[Coordinate] = None) -> None: """Point the link along `angle`, turning on the joint it is mounted on. diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index 0b32eefb2eb..3b0b5f531e2 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -18,15 +18,16 @@ def test_a_chain_folds_on_its_joints(self): first = Link(name="first", length=100.0) second = Link(name="second", length=50.0) base.assign_child_resource(first, location=Coordinate(0, 0, 0)) - first.assign_child_resource(second, location=first.far_joint) + first.assign_child_resource(second, location=Coordinate(first.get_size_x(), 0, 0)) - self.assertEqual(second.get_absolute_location() + second.far_joint, Coordinate(150, 0, 0)) + far_end = Coordinate(second.get_size_x(), 0, 0) + self.assertEqual(second.get_absolute_location() + far_end, Coordinate(150, 0, 0)) second.turn_to(90) # Through the link's own rotation rather than a vector worked out here, so the test exercises # the turn instead of restating its answer. carried = matrix_vector_multiply_3x3( - second.get_absolute_rotation().get_rotation_matrix(), second.far_joint.vector() + second.get_absolute_rotation().get_rotation_matrix(), far_end.vector() ) end = second.get_absolute_location() + Coordinate(*carried) self.assertEqual(end, Coordinate(100, 50, 0)) From 28ad26de0b1371580984d04813f364c956917221 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 16:46:21 +0100 Subject: [PATCH 09/34] `MechanicalGripper`: take the material as resources, and only place it The constructor took each part's size as a `Coordinate` and unpacked it into a `Resource`. A coordinate locates a point in a frame; it is not an extent, and there is no size type in the resource model because `Resource` is what has a size. Carrying one in the other was the type doing a job it has no business doing. It also had to invent every part's name from a string it was handed, which is what each of the helpers deleted before this existed to arrange. The gripper now takes the body, the two fingers and their pads as resources, with a location for each, and does the one thing that is its own: it knows a gripper has two jaws with a pad on each, and where they sit relative to its span. Whoever builds a particular gripper names its parts, because that is where the name is known. It refuses two fingers with a different number of pads. `zip` would have dropped the extra without saying so. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 74 ++++++++-------------- pylabrobot/resources/end_effector_tests.py | 28 +++++--- 2 files changed, 46 insertions(+), 56 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index 888a7296a3c..0cf0a5111a4 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -8,7 +8,7 @@ `MechanicalGripper` spans that offset, flange to grip centre, which is why it is a `Link`. """ -from typing import Optional, Tuple, cast +from typing import Optional, Sequence, Tuple, cast from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.manipulator import Link @@ -28,11 +28,11 @@ def __init__( self, name: str, length: float, - body: Coordinate, + body: Resource, body_location: Coordinate, - finger: Coordinate, + fingers: Sequence[Resource], finger_location: Coordinate, - pad: Coordinate, + pads: Sequence[Resource], pad_location: Coordinate, jaw_range: Tuple[float, float], jaw_width: Optional[float] = None, @@ -43,13 +43,13 @@ def __init__( Args: name: what to call this one. length: the joint it turns on to the grip centre, in mm. - body: how big the body is, in mm. + body: the material around the span. body_location: where it sits, from the joint this gripper turns on. - finger: how big one finger is, in mm. There are two, either side of the span. + fingers: the two jaws, either side of the span. finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s, so what stands here for it is not used. - pad: how big the pad on a finger's end is, in mm. - pad_location: where it sits, from the finger it is fixed to. + pads: what each finger meets the resource with, in the same order as `fingers`. + pad_location: where a pad sits, from the finger it is fixed to. jaw_range: how far apart the fingers stand, closed and open, in mm. jaw_width: how far apart they stand to begin with, in mm. Where a gripper is known to come up at a particular width - the one it homes at, say - that is what to build it at, so the @@ -62,57 +62,39 @@ def __init__( if not low <= self._jaw_width <= high: raise ValueError(f"the jaws open {low} to {high} mm, so cannot start at {self._jaw_width}") - self.body = Resource( - name=f"{name}_body", - size_x=body.x, - size_y=body.y, - size_z=body.z, - category="body", - model=f"{model}_body" if model else None, - ) - self.assign_child_resource(self.body, location=body_location) - - # A finger has a size and no place of its own: `jaw_width` decides where it stands, and - # `_place_the_fingers` is what puts it there. - self.fingers = [ - Resource( - name=f"{name}_finger_{side}", - size_x=finger.x, - size_y=finger.y, - size_z=finger.z, - category="finger", - model=f"{model}_finger" if model else None, + # Two of each, and one pad per finger: a gripper closes two jaws, and zipping a short list + # against a long one would drop material without saying so. + if len(fingers) != 2 or len(pads) != len(fingers): + raise ValueError( + f"a mechanical gripper has two fingers and a pad on each, not {len(fingers)} and {len(pads)}" ) - for side in ("left", "right") - ] + + self.body = body + self.assign_child_resource(body, location=body_location) + + # A finger has a place along the span and above it, and no Y of its own: `jaw_width` decides + # how far apart the two stand, and `_place_the_fingers` is what puts them there. + self.fingers = list(fingers) for jaw in self.fingers: self.assign_child_resource( jaw, location=Coordinate(finger_location.x, 0.0, finger_location.z) ) - self.pads = [] - for jaw in self.fingers: - face = Resource( - name=f"{jaw.name}_pad", - size_x=pad.x, - size_y=pad.y, - size_z=pad.z, - category="pad", - model=f"{jaw.model}_pad" if jaw.model else None, - ) + self.pads = list(pads) + for jaw, face in zip(self.fingers, self.pads): jaw.assign_child_resource(face, location=pad_location) - self.pads.append(face) self._place_the_fingers() @property def tool_center_point(self) -> Coordinate: - """The tool center point: where this tool is programmed against, as an offset from where it is - mounted. + """Where this tool is programmed against, as an offset from where it is mounted. - A gripper's far joint carries nothing, so what sits there is the point it grips at. + A link spans the interface it is bolted to and the point its work happens at; for a gripper + that far point is the centre between the pads, which is what a move is aimed at. The fingers + reach past it - material overhangs the span, and the span is what the kinematics use. - In PyLabRobot a tool center point is always this offset - a property of the tool, which changes + In PyLabRobot a tool centre point is always this offset - a property of the tool, which changes when a different one is fitted and not when the arm moves. Robot controllers also use the term for where that point currently is in the robot's frame; here that is a location, and something an arm answers rather than a tool. @@ -120,7 +102,7 @@ def tool_center_point(self) -> Coordinate: Returns: The grip centre, from the joint this gripper turns on. """ - return self.far_joint + return Coordinate(self.get_size_x(), 0.0, 0.0) @property def jaw_width(self) -> float: diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index eacd15ea124..22933d95bca 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -3,13 +3,21 @@ from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.end_effector import MechanicalGripper +from pylabrobot.resources.resource import Resource # A gripper with every part a different size, so a part placed by the wrong measurement lands # somewhere this notices. LENGTH = 100.0 -BODY, BODY_LOCATION = Coordinate(50.0, 80.0, 20.0), Coordinate(-10.0, -40.0, 0.0) -FINGER, FINGER_LOCATION = Coordinate(30.0, 6.0, 8.0), Coordinate(60.0, 0.0, 4.0) -PAD, PAD_LOCATION = Coordinate(10.0, 4.0, 12.0), Coordinate(25.0, 1.0, -10.0) +BODY_SIZE, BODY_LOCATION = (50.0, 80.0, 20.0), Coordinate(-10.0, -40.0, 0.0) +FINGER_SIZE, FINGER_LOCATION = (30.0, 6.0, 8.0), Coordinate(60.0, 0.0, 4.0) +PAD_SIZE, PAD_LOCATION = (10.0, 4.0, 12.0), Coordinate(25.0, 1.0, -10.0) + + +def part(name: str, size, category: str) -> Resource: + """One piece of the gripper's material, which the caller builds and the gripper only places.""" + return Resource(name=name, size_x=size[0], size_y=size[1], size_z=size[2], category=category) + + JAW_RANGE = (20.0, 90.0) @@ -17,11 +25,11 @@ def gripper(**overrides) -> MechanicalGripper: return MechanicalGripper( name="g", length=LENGTH, - body=BODY, + body=part("g_body", BODY_SIZE, "body"), body_location=BODY_LOCATION, - finger=FINGER, + fingers=[part(f"g_finger_{side}", FINGER_SIZE, "finger") for side in ("left", "right")], finger_location=FINGER_LOCATION, - pad=PAD, + pads=[part(f"g_finger_{side}_pad", PAD_SIZE, "pad") for side in ("left", "right")], pad_location=PAD_LOCATION, jaw_range=JAW_RANGE, **overrides, @@ -31,9 +39,9 @@ def gripper(**overrides) -> MechanicalGripper: class TestTheSpan(unittest.TestCase): """A gripper is a link: it spans the joint it turns on to the point it grips at.""" - def test_the_grip_centre_is_the_far_joint(self): - """A link's far joint is where the next link would go, and a gripper carries no next link, so - what sits there is the point it is programmed against.""" + def test_the_grip_centre_sits_at_the_end_of_the_span(self): + """A gripper spans the interface it is bolted to and the point it grips at, so its tool + centre point is simply its length along that span. The fingers reach past it.""" g = gripper() self.assertEqual(g.tool_center_point, Coordinate(100.0, 0.0, 0.0)) @@ -81,7 +89,7 @@ def test_a_pad_sits_the_same_way_on_both_fingers(self): two pads face opposite ways grips nothing where the model says it does.""" g = gripper() left, right = (cast(Coordinate, pad.location) for pad in g.pads) - finger_thickness, pad_thickness = FINGER.y, PAD.y + finger_thickness, pad_thickness = FINGER_SIZE[1], PAD_SIZE[1] self.assertGreaterEqual(left.y, 0.0) self.assertLessEqual(left.y + pad_thickness, finger_thickness) From d6d3a5c69ce13c6515e89f546084bd3a9d2867a0 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 16:50:09 +0100 Subject: [PATCH 10/34] `MechanicalGripper`: name the test's part sizes instead of indexing them `part()` unpacked an unannotated three-tuple by position - `size[0]`, `size[1]`, `size[2]` - which is the opaque tuple the production code had just shed, reintroduced in the file whose whole job is to be legible. The sizes are named, so `FINGER_Y` reads as the finger's thickness where it is used to check the pad sits inside it. The wrapper itself was not the fault: it took an explicit name and an explicit category and hid nothing. What was wrong with `bolt_on`, `Part` and `_material` was hiding geometry, inventing a type the resource model already has, and inventing names the class could not know. None of those applied here, and the rule I reached for - that a constructor called five times wants writing out - is not one. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector_tests.py | 36 ++++++++++++++-------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 22933d95bca..bdc8a593965 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -8,14 +8,12 @@ # A gripper with every part a different size, so a part placed by the wrong measurement lands # somewhere this notices. LENGTH = 100.0 -BODY_SIZE, BODY_LOCATION = (50.0, 80.0, 20.0), Coordinate(-10.0, -40.0, 0.0) -FINGER_SIZE, FINGER_LOCATION = (30.0, 6.0, 8.0), Coordinate(60.0, 0.0, 4.0) -PAD_SIZE, PAD_LOCATION = (10.0, 4.0, 12.0), Coordinate(25.0, 1.0, -10.0) - - -def part(name: str, size, category: str) -> Resource: - """One piece of the gripper's material, which the caller builds and the gripper only places.""" - return Resource(name=name, size_x=size[0], size_y=size[1], size_z=size[2], category=category) +BODY_X, BODY_Y, BODY_Z = 50.0, 80.0, 20.0 +FINGER_X, FINGER_Y, FINGER_Z = 30.0, 6.0, 8.0 +PAD_X, PAD_Y, PAD_Z = 10.0, 4.0, 12.0 +BODY_LOCATION = Coordinate(-10.0, -40.0, 0.0) +FINGER_LOCATION = Coordinate(60.0, 0.0, 4.0) +PAD_LOCATION = Coordinate(25.0, 1.0, -10.0) JAW_RANGE = (20.0, 90.0) @@ -25,11 +23,25 @@ def gripper(**overrides) -> MechanicalGripper: return MechanicalGripper( name="g", length=LENGTH, - body=part("g_body", BODY_SIZE, "body"), + body=Resource(name="g_body", size_x=BODY_X, size_y=BODY_Y, size_z=BODY_Z, category="body"), body_location=BODY_LOCATION, - fingers=[part(f"g_finger_{side}", FINGER_SIZE, "finger") for side in ("left", "right")], + fingers=[ + Resource( + name=f"g_finger_{side}", + size_x=FINGER_X, + size_y=FINGER_Y, + size_z=FINGER_Z, + category="finger", + ) + for side in ("left", "right") + ], finger_location=FINGER_LOCATION, - pads=[part(f"g_finger_{side}_pad", PAD_SIZE, "pad") for side in ("left", "right")], + pads=[ + Resource( + name=f"g_finger_{side}_pad", size_x=PAD_X, size_y=PAD_Y, size_z=PAD_Z, category="pad" + ) + for side in ("left", "right") + ], pad_location=PAD_LOCATION, jaw_range=JAW_RANGE, **overrides, @@ -89,7 +101,7 @@ def test_a_pad_sits_the_same_way_on_both_fingers(self): two pads face opposite ways grips nothing where the model says it does.""" g = gripper() left, right = (cast(Coordinate, pad.location) for pad in g.pads) - finger_thickness, pad_thickness = FINGER_SIZE[1], PAD_SIZE[1] + finger_thickness, pad_thickness = FINGER_Y, PAD_Y self.assertGreaterEqual(left.y, 0.0) self.assertLessEqual(left.y + pad_thickness, finger_thickness) From 18f2dec871c6e6a0910dabfbe327f2feab1d045a Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 17:19:46 +0100 Subject: [PATCH 11/34] `MechanicalGripper`: build the test's fixture from a gripper that exists The fixture's dimensions were invented, and a reviewer would have stopped on them: 30 mm fingers on a 100 mm span, closing to a 90 mm jaw. Every measurement is now a Hamilton iSWAP's - the body, the fingers, the pads, the span, and the jaw travel the gripper drive's own window comes to. It costs nothing and buys two things. The nine dimensions are still all distinct, so a part placed by the wrong measurement still lands where the tests notice. And the fingers now run past the tool centre point, 6.5 to 141.5 mm against a span ending at 137.7, so the fixture shows what the invented one could not: material overhangs the span, and the span is what the kinematics use. Construction is bound to locals before the call, so each part sits beside the location it is given. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector_tests.py | 78 +++++++++++----------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index bdc8a593965..4bc4692dc91 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -5,43 +5,43 @@ from pylabrobot.resources.end_effector import MechanicalGripper from pylabrobot.resources.resource import Resource -# A gripper with every part a different size, so a part placed by the wrong measurement lands -# somewhere this notices. -LENGTH = 100.0 -BODY_X, BODY_Y, BODY_Z = 50.0, 80.0, 20.0 -FINGER_X, FINGER_Y, FINGER_Z = 30.0, 6.0, 8.0 -PAD_X, PAD_Y, PAD_Z = 10.0, 4.0, 12.0 -BODY_LOCATION = Coordinate(-10.0, -40.0, 0.0) -FINGER_LOCATION = Coordinate(60.0, 0.0, 4.0) -PAD_LOCATION = Coordinate(25.0, 1.0, -10.0) - - -JAW_RANGE = (20.0, 90.0) +# A gripper that exists: every measurement here is a Hamilton iSWAP's, so the fixture is a +# shape that could be built rather than one chosen to make the arithmetic easy. No two of the +# nine dimensions are equal, so a part placed by the wrong measurement lands where these notice. +LENGTH = 137.7 +BODY_LOCATION = Coordinate(-13.0, -45.0, -1.3) +FINGER_LOCATION = Coordinate(6.5, 0.0, 4.0) +PAD_LOCATION = Coordinate(109.0, 1.5, -17.0) +# What the gripper drive's own travel comes to, closed and open. +JAW_RANGE = (70.844, 133.706) def gripper(**overrides) -> MechanicalGripper: + body = Resource(name="demo_body", size_x=59.0, size_y=90.0, size_z=20.3, category="body") + + fingers = [ + Resource( + name=f"demo_finger_{side}", + size_x=135.0, + size_y=7.0, + size_z=8.0, + category="finger", + ) + for side in ("left", "right") + ] + pads = [ + Resource(name=f"demo_finger_{side}_pad", size_x=37.0, size_y=4.0, size_z=17.0, category="pad") + for side in ("left", "right") + ] + return MechanicalGripper( - name="g", + name="demo_gripper", length=LENGTH, - body=Resource(name="g_body", size_x=BODY_X, size_y=BODY_Y, size_z=BODY_Z, category="body"), + body=body, body_location=BODY_LOCATION, - fingers=[ - Resource( - name=f"g_finger_{side}", - size_x=FINGER_X, - size_y=FINGER_Y, - size_z=FINGER_Z, - category="finger", - ) - for side in ("left", "right") - ], + fingers=fingers, finger_location=FINGER_LOCATION, - pads=[ - Resource( - name=f"g_finger_{side}_pad", size_x=PAD_X, size_y=PAD_Y, size_z=PAD_Z, category="pad" - ) - for side in ("left", "right") - ], + pads=pads, pad_location=PAD_LOCATION, jaw_range=JAW_RANGE, **overrides, @@ -55,7 +55,7 @@ def test_the_grip_centre_sits_at_the_end_of_the_span(self): """A gripper spans the interface it is bolted to and the point it grips at, so its tool centre point is simply its length along that span. The fingers reach past it.""" g = gripper() - self.assertEqual(g.tool_center_point, Coordinate(100.0, 0.0, 0.0)) + self.assertEqual(g.tool_center_point, Coordinate(LENGTH, 0.0, 0.0)) class TestJaws(unittest.TestCase): @@ -65,7 +65,7 @@ def test_a_width_stands_the_fingers_that_far_apart(self): """Measured centre to centre between the two fingers, symmetrically about the span, so a width applied to one finger only, or applied twice to one side, fails this.""" g = gripper() - for width in (90.0, 40.0, 20.0): + for width in (133.706, 100.0, 70.844): g.jaw_width = width left, right = g.fingers centres = [ @@ -82,13 +82,13 @@ def test_the_jaws_refuse_a_width_they_do_not_reach(self): g = gripper() with self.assertRaises(ValueError): g.jaw_width = 5.0 - self.assertEqual(g.jaw_width, 90.0) + self.assertEqual(g.jaw_width, JAW_RANGE[1]) def test_a_gripper_starts_open_unless_told_otherwise(self): """Open is the safe assumption for a model nothing has read yet, and a gripper known to home at a width is built at it instead.""" - self.assertEqual(gripper().jaw_width, 90.0) - self.assertEqual(gripper(jaw_width=35.0).jaw_width, 35.0) + self.assertEqual(gripper().jaw_width, JAW_RANGE[1]) + self.assertEqual(gripper(jaw_width=100.0).jaw_width, 100.0) class TestPads(unittest.TestCase): @@ -100,10 +100,10 @@ def test_a_pad_sits_the_same_way_on_both_fingers(self): outside, since a finger's own origin is a corner rather than its middle - and a gripper whose two pads face opposite ways grips nothing where the model says it does.""" g = gripper() - left, right = (cast(Coordinate, pad.location) for pad in g.pads) - finger_thickness, pad_thickness = FINGER_Y, PAD_Y - self.assertGreaterEqual(left.y, 0.0) - self.assertLessEqual(left.y + pad_thickness, finger_thickness) + for jaw, face in zip(g.fingers, g.pads): + sits_at = cast(Coordinate, face.location).y + self.assertGreaterEqual(sits_at, 0.0) + self.assertLessEqual(sits_at + face.get_size_y(), jaw.get_size_y()) if __name__ == "__main__": From b4f27742483ec21c81bb0851698cd1eeb98e1481 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 17:38:40 +0100 Subject: [PATCH 12/34] `MechanicalGripper`: let a gripper have bare fingers, and check a width in one place Not every mechanical gripper has pads. Fingers that meet the resource themselves are a build people have, and the constructor demanded two pads and a location for them. - `pads` and `pad_location` default to None. They go together or not at all, which is checked: a location with no pads would have been ignored, and pads with no location would have crashed. - The jaw-width bound was checked twice, in the constructor and in the setter, with two messages. The constructor goes through the setter, so the bound and its wording have one home and the explicit call to stand the fingers apart goes with them. - `finger_location`'s Y was stripped and rebuilt before `_place_the_fingers` overwrote it. - `tool_center_point` loses eleven lines of docstring that said what the class docstring says. The test for a refused width now records what the width was rather than naming the open end, so it no longer fails when the default moves - which it did. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 68 ++++++++-------------- pylabrobot/resources/end_effector_tests.py | 43 ++++++-------- 2 files changed, 41 insertions(+), 70 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index 0cf0a5111a4..c758333d537 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -18,10 +18,10 @@ class MechanicalGripper(Link): """A gripper that holds by closing two fingers on what it takes. - A link, because on an arm that is what it is: it spans the joint it turns on to the point it - grips at, which is `tool_center_point`. Its body, its two fingers and the pad on each are material - bolted to that span. How far apart the fingers stand is state rather than shape, so `jaw_width` - moves them. + A link: it spans the joint it turns on to the point it grips at, which is `tool_center_point`. + Its body, its two fingers and a pad on each are material bolted to that span; a gripper whose + fingers meet the resource themselves carries no pads. How far apart the fingers stand is state + rather than shape, so `jaw_width` moves them. """ def __init__( @@ -32,9 +32,9 @@ def __init__( body_location: Coordinate, fingers: Sequence[Resource], finger_location: Coordinate, - pads: Sequence[Resource], - pad_location: Coordinate, jaw_range: Tuple[float, float], + pads: Optional[Sequence[Resource]] = None, + pad_location: Optional[Coordinate] = None, jaw_width: Optional[float] = None, category: str = "mechanical_gripper", model: Optional[str] = None, @@ -46,61 +46,44 @@ def __init__( body: the material around the span. body_location: where it sits, from the joint this gripper turns on. fingers: the two jaws, either side of the span. - finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s, so - what stands here for it is not used. - pads: what each finger meets the resource with, in the same order as `fingers`. - pad_location: where a pad sits, from the finger it is fixed to. + finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s. jaw_range: how far apart the fingers stand, closed and open, in mm. + pads: what each finger meets the resource with, in the same order as `fingers`. A gripper + whose fingers meet it themselves has none. + pad_location: where a pad sits, from the finger it is fixed to. Given with `pads`. jaw_width: how far apart they stand to begin with, in mm. Where a gripper is known to come up at a particular width - the one it homes at, say - that is what to build it at, so the model does not start out claiming a width nothing has read. Open, when not given. """ super().__init__(name=name, length=length, category=category, model=model) + if len(fingers) != 2: + raise ValueError(f"a gripper has two fingers, not {len(fingers)}") + if (pads is None) != (pad_location is None): + raise ValueError("pads and pad_location go together: give both, or neither") + # Zipping a short list against a long one would drop material without saying so. + if pads is not None and len(pads) != len(fingers): + raise ValueError(f"a gripper has a pad on each finger, not {len(pads)} on {len(fingers)}") self.jaw_range = jaw_range - self._jaw_width = jaw_range[1] if jaw_width is None else jaw_width - low, high = jaw_range - if not low <= self._jaw_width <= high: - raise ValueError(f"the jaws open {low} to {high} mm, so cannot start at {self._jaw_width}") - - # Two of each, and one pad per finger: a gripper closes two jaws, and zipping a short list - # against a long one would drop material without saying so. - if len(fingers) != 2 or len(pads) != len(fingers): - raise ValueError( - f"a mechanical gripper has two fingers and a pad on each, not {len(fingers)} and {len(pads)}" - ) self.body = body self.assign_child_resource(body, location=body_location) - - # A finger has a place along the span and above it, and no Y of its own: `jaw_width` decides - # how far apart the two stand, and `_place_the_fingers` is what puts them there. self.fingers = list(fingers) for jaw in self.fingers: - self.assign_child_resource( - jaw, location=Coordinate(finger_location.x, 0.0, finger_location.z) - ) + self.assign_child_resource(jaw, location=finger_location) - self.pads = list(pads) + self.pads = list(pads) if pads is not None else [] for jaw, face in zip(self.fingers, self.pads): - jaw.assign_child_resource(face, location=pad_location) + jaw.assign_child_resource(face, location=cast(Coordinate, pad_location)) - self._place_the_fingers() + # Through the setter, which is where a width is checked and the fingers are stood apart. + self.jaw_width = jaw_range[1] if jaw_width is None else jaw_width @property def tool_center_point(self) -> Coordinate: """Where this tool is programmed against, as an offset from where it is mounted. - A link spans the interface it is bolted to and the point its work happens at; for a gripper - that far point is the centre between the pads, which is what a move is aimed at. The fingers - reach past it - material overhangs the span, and the span is what the kinematics use. - - In PyLabRobot a tool centre point is always this offset - a property of the tool, which changes - when a different one is fitted and not when the arm moves. Robot controllers also use the term - for where that point currently is in the robot's frame; here that is a location, and something - an arm answers rather than a tool. - Returns: - The grip centre, from the joint this gripper turns on. + The grip centre, which the fingers reach past. """ return Coordinate(self.get_size_x(), 0.0, 0.0) @@ -118,10 +101,7 @@ def jaw_width(self, width: float) -> None: self._place_the_fingers() def _place_the_fingers(self) -> None: - """Stand the fingers either side of the span, as far apart as the jaws are open. - - A finger is the one part of a gripper whose Y is not fixed: the jaw width owns it outright. - """ + """Stand the fingers either side of the span, as far apart as the jaws are open.""" for finger, side in zip(self.fingers, (1.0, -1.0)): here = cast(Coordinate, finger.location) finger.location = Coordinate( diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 4bc4692dc91..503095fa555 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -5,14 +5,11 @@ from pylabrobot.resources.end_effector import MechanicalGripper from pylabrobot.resources.resource import Resource -# A gripper that exists: every measurement here is a Hamilton iSWAP's, so the fixture is a -# shape that could be built rather than one chosen to make the arithmetic easy. No two of the -# nine dimensions are equal, so a part placed by the wrong measurement lands where these notice. +# Measured off a Hamilton iSWAP. LENGTH = 137.7 BODY_LOCATION = Coordinate(-13.0, -45.0, -1.3) FINGER_LOCATION = Coordinate(6.5, 0.0, 4.0) PAD_LOCATION = Coordinate(109.0, 1.5, -17.0) -# What the gripper drive's own travel comes to, closed and open. JAW_RANGE = (70.844, 133.706) @@ -34,36 +31,28 @@ def gripper(**overrides) -> MechanicalGripper: for side in ("left", "right") ] - return MechanicalGripper( + arguments = dict( name="demo_gripper", length=LENGTH, body=body, body_location=BODY_LOCATION, fingers=fingers, finger_location=FINGER_LOCATION, + jaw_range=JAW_RANGE, pads=pads, pad_location=PAD_LOCATION, - jaw_range=JAW_RANGE, - **overrides, ) + return MechanicalGripper(**{**arguments, **overrides}) class TestTheSpan(unittest.TestCase): - """A gripper is a link: it spans the joint it turns on to the point it grips at.""" - def test_the_grip_centre_sits_at_the_end_of_the_span(self): - """A gripper spans the interface it is bolted to and the point it grips at, so its tool - centre point is simply its length along that span. The fingers reach past it.""" g = gripper() self.assertEqual(g.tool_center_point, Coordinate(LENGTH, 0.0, 0.0)) class TestJaws(unittest.TestCase): - """How wide the jaws stand is state, not shape.""" - def test_a_width_stands_the_fingers_that_far_apart(self): - """Measured centre to centre between the two fingers, symmetrically about the span, so a width - applied to one finger only, or applied twice to one side, fails this.""" g = gripper() for width in (133.706, 100.0, 70.844): g.jaw_width = width @@ -75,30 +64,32 @@ def test_a_width_stands_the_fingers_that_far_apart(self): self.assertAlmostEqual(centres[0] + centres[1], 0.0) def test_the_jaws_refuse_a_width_they_do_not_reach(self): - """At construction and afterwards alike: a model claiming a width the drive cannot reach would - put the fingers where the arm cannot.""" with self.assertRaises(ValueError): gripper(jaw_width=200.0) g = gripper() + before = g.jaw_width with self.assertRaises(ValueError): g.jaw_width = 5.0 - self.assertEqual(g.jaw_width, JAW_RANGE[1]) + self.assertEqual(g.jaw_width, before) def test_a_gripper_starts_open_unless_told_otherwise(self): - """Open is the safe assumption for a model nothing has read yet, and a gripper known to home - at a width is built at it instead.""" self.assertEqual(gripper().jaw_width, JAW_RANGE[1]) self.assertEqual(gripper(jaw_width=100.0).jaw_width, 100.0) class TestPads(unittest.TestCase): - """What actually touches the resource.""" + def test_a_gripper_can_have_bare_fingers(self): + g = gripper(pads=None, pad_location=None) + self.assertEqual(g.pads, []) + self.assertEqual([jaw.children for jaw in g.fingers], [[], []]) + + def test_pads_and_their_location_go_together(self): + with self.assertRaises(ValueError): + gripper(pad_location=None) + with self.assertRaises(ValueError): + gripper(pads=None) - def test_a_pad_sits_the_same_way_on_both_fingers(self): - """A pad is fixed to its finger, so it sits identically on each. Centring it across the finger - the way material is centred across a link would put one pad inside the jaws and the other - outside, since a finger's own origin is a corner rather than its middle - and a gripper whose - two pads face opposite ways grips nothing where the model says it does.""" + def test_a_pad_sits_inside_its_finger(self): g = gripper() for jaw, face in zip(g.fingers, g.pads): sits_at = cast(Coordinate, face.location).y From 1670e0110e2e882365499c5bf542ecd9085b4a8f Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 17:38:40 +0100 Subject: [PATCH 13/34] `Link`: test that a child link's angle composes on its parent's The chain test placed two links and turned the second. Mutating the code it named showed it never failed alone: what it covered was `Resource` composing a location and a rotation, which `test_rotation90` and its siblings already cover. It turns both joints now. With each at 90 degrees the second link points back down the first, so the angles compose rather than replace - the invariant `wrist_drive_update_angle` rests on when it turns a gripper to an angle measured from the link that carries it. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/manipulator_tests.py | 34 +++++++++-------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index 3b0b5f531e2..1eed4e2ad72 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -7,34 +7,29 @@ class TestLink(unittest.TestCase): - """A link is the span between two joints, and nothing else.""" - - def test_a_chain_folds_on_its_joints(self): - """Two links, the second placed on the first's far joint. Straight, the far end is both - lengths out; folded square, the second link leaves the first's end sideways. Measured at the - end of the chain rather than on either link, because that is the point a chain exists to - place, and it is where an error in the joint offset would show.""" + def test_a_child_link_turns_on_top_of_its_parent(self): base = Resource(name="base", size_x=500, size_y=500, size_z=0) first = Link(name="first", length=100.0) second = Link(name="second", length=50.0) base.assign_child_resource(first, location=Coordinate(0, 0, 0)) first.assign_child_resource(second, location=Coordinate(first.get_size_x(), 0, 0)) - far_end = Coordinate(second.get_size_x(), 0, 0) - self.assertEqual(second.get_absolute_location() + far_end, Coordinate(150, 0, 0)) + def far_end() -> Coordinate: + carried = matrix_vector_multiply_3x3( + second.get_absolute_rotation().get_rotation_matrix(), + Coordinate(second.get_size_x(), 0, 0).vector(), + ) + return second.get_absolute_location() + Coordinate(*carried) + + self.assertEqual(far_end(), Coordinate(150, 0, 0)) second.turn_to(90) - # Through the link's own rotation rather than a vector worked out here, so the test exercises - # the turn instead of restating its answer. - carried = matrix_vector_multiply_3x3( - second.get_absolute_rotation().get_rotation_matrix(), far_end.vector() - ) - end = second.get_absolute_location() + Coordinate(*carried) - self.assertEqual(end, Coordinate(100, 50, 0)) + self.assertEqual(far_end(), Coordinate(100, 50, 0)) + + first.turn_to(90) + self.assertEqual(far_end(), Coordinate(-50, 100, 0)) def test_turning_to_an_angle_is_absolute(self): - """`turn_to` points the link somewhere, where `rotate` turns it by an amount. A drive commanded - to the same angle twice has not moved twice, and the model has to say the same.""" base = Resource(name="base", size_x=500, size_y=500, size_z=0) link = Link(name="link", length=100.0) base.assign_child_resource(link, location=Coordinate(0, 0, 0)) @@ -47,9 +42,6 @@ def test_turning_to_an_angle_is_absolute(self): self.assertEqual(link.rotation.z, 60) def test_a_link_can_be_given_the_joint_it_turns_on(self): - """The joint is where the link is placed, so naming one places it. A link that has never been - placed and is given none has nothing to turn on, and says so rather than turning about the - origin.""" base = Resource(name="base", size_x=500, size_y=500, size_z=0) link = Link(name="link", length=100.0) base.assign_child_resource(link, location=Coordinate(0, 0, 0)) From f600d66c8c2ee74ea4e7cd3b3e621b144050c4ee Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 18:05:45 +0100 Subject: [PATCH 14/34] `Resource`: add `rotate_to`, the go-to partner to `rotate`'s move-by `rotate` adds to the angle a resource already has. Nothing set one. A caller holding an angle read off a drive had to write `rotate(z=angle - resource.rotation.z)` or assign `rotation` outright, which notifies nobody - `rotation` is a plain attribute where `location` is a property. `rotate_to` goes to an angle, through `rotate`, so a subscriber hears it. Each axis defaults to None rather than zero: `rotate_to(z=90)` leaves X and Y where they are instead of flattening them. Neither name says "absolute". That word is already spoken for by `get_absolute_rotation`, where it means the frame the angle is measured in rather than whether the move sets or adds - two senses this repository uses in the same breath, and naming them apart is the point. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource.py | 26 +++++++++++++++++++ pylabrobot/resources/resource_tests.py | 36 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index 90cbe860741..6bf354264b7 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -944,6 +944,32 @@ def rotate( # Visualizer) so they can re-render. self._state_updated() + def rotate_to( + self, + x: Optional[float] = None, + y: Optional[float] = None, + z: Optional[float] = None, + reference: Optional[Coordinate] = None, + ): + """Rotate counter-clockwise to the given number of degrees. + + A go-to where `rotate` is a move-by: told the same angle twice, this lands in the same place + both times. The angles are in the parent's frame, as `rotation` is - `get_absolute_rotation` + is what composes the chain to the root. + + Args: + x: degrees to point along about X. Left where it is when None. + y: degrees to point along about Y. Left where it is when None. + z: degrees to point along about Z. Left where it is when None. + reference: the point to turn about, as `rotate` takes it. + """ + self.rotate( + x=0 if x is None else x - self.rotation.x, + y=0 if y is None else y - self.rotation.y, + z=0 if z is None else z - self.rotation.z, + reference=reference, + ) + def copy(self) -> Self: resource_copy = self.__class__.deserialize(self.serialize(), allow_marshal=True) resource_copy.load_all_state(self.serialize_all_state()) diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 0fdbacdb955..b179a0ab7ad 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -352,6 +352,42 @@ def test_rotating_about_a_reference_point_leaves_that_point_where_it_was(self): self.assertEqual(bar.get_absolute_location() + Coordinate(*carried), before) self.assertEqual(bar.location, Coordinate(100, -100, 0)) + def test_rotate_to_goes_to_an_angle_where_rotate_moves_by_one(self): + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate.zero()) + + bar.rotate_to(z=30) + bar.rotate_to(z=30) + self.assertEqual(bar.rotation.z, 30) + + bar.rotate(z=30) + self.assertEqual(bar.rotation.z, 60) + + def test_rotate_to_leaves_an_axis_it_was_not_given(self): + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate.zero()) + + bar.rotate(x=15, z=40) + bar.rotate_to(z=90) + self.assertEqual((bar.rotation.x, bar.rotation.z), (15, 90)) + + def test_rotate_to_turns_about_a_reference_point(self): + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate.zero()) + far_end = Coordinate(100, 0, 0) + + bar.rotate_to(z=90, reference=far_end) + carried = matrix_vector_multiply_3x3( + bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() + ) + self.assertEqual(bar.get_absolute_location() + Coordinate(*carried), far_end) + def test_rotation180(self): r = Resource("parent", size_x=200, size_y=100, size_z=100) r.location = Coordinate.zero() From 393b5747da9fcddae6da111f1f56cae264f2e2a0 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 18:05:45 +0100 Subject: [PATCH 15/34] `Link`: turn on the joint through `rotate`, rather than around it This PR added a point to turn about and then nothing used it. `turn_to` set `rotation` directly and relied on the link's own origin being its joint, which is why `Link` had to be built with its span starting at zero - a workaround for the very thing this PR removes. - `Link` carries `joint`, where the joint sits within it, and `turn_to` calls `rotate_to` with it. A link is no longer obliged to put its origin on its joint. - `about` is gone. Placing a link is `assign_child_resource`'s job; `turn_to` turns it. The docstring takes the glossary's own wording for a link, which says what two of my attempts were reaching for: the material can be any shape and can overhang a joint at either end, because what sets the reach is the distance between joints and not the shape of the piece. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/manipulator.py | 47 +++++++++-------------- pylabrobot/resources/manipulator_tests.py | 5 +-- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py index 6337b0c7b5f..a0b45f967e1 100644 --- a/pylabrobot/resources/manipulator.py +++ b/pylabrobot/resources/manipulator.py @@ -1,26 +1,25 @@ """The moving mechanism of an arm: the links its joints turn between. -A manipulator is a chain of links and powered joints. A link is one rigid member of that chain, -and nothing else: the material bolted around it hangs off as children of its own, so the shape can -overhang either joint without the kinematics noticing. That is the split every robot description -makes, and it is what lets one length stand for the geometry and another for the part. +A manipulator is a chain of links and powered joints. A link is one rigid member of that chain and +nothing else: geometry is attached as children with their own origins - the separation a robot +description draws between a link's frame and its visual geometry - so material may extend past +either joint without entering the kinematics. """ from typing import Optional from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.resource import Resource -from pylabrobot.resources.rotation import Rotation class Link(Resource): - """The span between the joint a link turns on and the joint it carries. + """One of the rigid pieces an arm is built from, joined to its neighbours by joints. - A line, not a body: its length is the distance between two joints and it has no width or depth, - so the joint it turns on is its own origin and turning it needs nothing taken out. The material - around it hangs off as children with their own offsets, which is how a robot description keeps a - link's frame apart from the shape bolted to it - the shape can overhang either joint without the - kinematics noticing. + What sets the reach is the distance between a link's joints, not the shape of the piece, so the + material is attached as children with their own origins and may overhang a joint at either end. + + `joint` is where the joint it turns on sits within the link, which `turn_to` pivots about. It + needs no particular place: a link is not obliged to put its own origin there. Unrotated it lies along +X. """ @@ -29,6 +28,7 @@ def __init__( self, name: str, length: float, + joint: Optional[Coordinate] = None, category: str = "link", model: Optional[str] = None, ): @@ -36,33 +36,24 @@ def __init__( Args: name: what to call this one. length: joint to joint, in mm. + joint: where the joint this link turns on sits within it. Its own origin when None. category: what kind of resource this is. model: which link this is. """ super().__init__( name=name, size_x=length, size_y=0.0, size_z=0.0, category=category, model=model ) + self.joint = joint if joint is not None else Coordinate.zero() - def turn_to(self, angle: float, about: Optional[Coordinate] = None) -> None: - """Point the link along `angle`, turning on the joint it is mounted on. - - Absolute, unlike `rotate`, which turns by an amount: a link driven to the same angle twice - lands in the same place both times. The joint is the link's own origin, so turning does not - move it and nothing has to be taken out. + def turn_to(self, angle: float) -> None: + """Point the link along `angle`, pivoting on `joint`. Args: - angle: the deck angle to point along, in degrees. - about: where the joint sits, in the frame this link is placed in. Left where it is when None. + angle: the angle to point along, in its parent's frame, in degrees. Raises: - RuntimeError: If the link is not placed and no joint is given. + RuntimeError: If the link has not been placed, so there is nothing for it to turn in. """ - if about is not None: - self.location = about if self.location is None: - raise RuntimeError(f"{self.name} is not on a joint, so there is nothing for it to turn on") - self.rotation = Rotation(z=angle) - # `rotation` is a plain attribute, unlike `location`, so nothing hears about it being set. - # Anything watching the model - a viewer, a collision check - learns of a joint moving here or - # not at all. - self._state_updated() + raise RuntimeError(f"{self.name} is not placed, so there is nothing for it to turn in") + self.rotate_to(z=angle, reference=self.joint) diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index 1eed4e2ad72..ec290f028ff 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -41,14 +41,11 @@ def test_turning_to_an_angle_is_absolute(self): link.rotate(z=30) self.assertEqual(link.rotation.z, 60) - def test_a_link_can_be_given_the_joint_it_turns_on(self): + def test_an_unplaced_link_has_nothing_to_turn_in(self): base = Resource(name="base", size_x=500, size_y=500, size_z=0) link = Link(name="link", length=100.0) base.assign_child_resource(link, location=Coordinate(0, 0, 0)) - link.turn_to(0, about=Coordinate(10, 20, 30)) - self.assertEqual(link.location, Coordinate(10, 20, 30)) - with self.assertRaises(RuntimeError): Link(name="loose", length=100.0).turn_to(0) From 8fcaea7f0936bbe9c818186397a7e24c57dbce62 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 22:38:38 +0100 Subject: [PATCH 16/34] `Resource`: turn through one shared pivot, and go to an angle exactly `rotate_to` computed a per-axis Euler delta and handed it to `rotate`, which composes by quaternion since #1247. Those are not the same operation. Only Z survived it, and by accident of the convention: Euler is Rz*Ry*Rx and `_prepend` pre-multiplies, so a pure-Z delta adds cleanly where X and Y have rotations applied after them. 73 of 108 cases landed off target on X, 42 on Y. `_turn` takes the orientation a caller wants and leaves `reference` where it was, so `rotate` composes and `rotate_to` assigns, and neither carries a copy of the pivot arithmetic. It writes the angles in place rather than binding a new `Rotation`, keeping the instance and the normalisation `_prepend` established. Tests: `rotate_to` lands on every axis from every starting orientation, angles stay in [0, 360) on all three axes, and a pivot inside a turned parent holds - the last of those covering the frame correction, which nothing reached before. Checked by mutation: five mutations of these lines, five caught, where two survived beforehand. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource.py | 85 +++++++++++--------------- pylabrobot/resources/resource_tests.py | 41 +++++++++++++ 2 files changed, 77 insertions(+), 49 deletions(-) diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index 7557f5c6763..e64bcbb1001 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -892,44 +892,22 @@ def location(self, location: Optional[Coordinate]) -> None: if changed and self.parent is not None: self._state_updated() - def rotate( - self, - x: float = 0, - y: float = 0, - z: float = 0, - reference: Optional[Coordinate] = None, - ): - """Rotate counter-clockwise around the parent-coordinate axes by the given degrees. - - A resource turns about its own left front bottom corner. `reference` names a different point - to turn about - a hinge, a joint, an axis the part really pivots on - and the resource is - moved as it turns by however far the turn carried that point, which leaves the point where it - was and the resource swinging on it. Left about the corner when None, which is what every - caller that does not ask for one gets. - - Args: - x: degrees to turn about X. - y: degrees to turn about Y. - z: degrees to turn about Z. - reference: the point to turn about, from this resource's left front bottom corner. Its own - corner when None. - """ - # Only a turn about another point needs to know which way this was already facing, and - # building a rotation matrix is twelve trigonometry calls: without a reference this stays out - # of the way, since `rotate` is on the path every placement takes. - turning_on = reference if self.location is not None else None - before = self.get_absolute_rotation().get_rotation_matrix() if turning_on is not None else None - - self.rotation._prepend(Rotation(x=x, y=y, z=z)) - - if turning_on is not None and before is not None: + def _turn(self, rotation: Rotation, reference: Optional[Coordinate]) -> None: + """Take `rotation` as this resource's own, leaving `reference` where it was.""" + pivot = reference if self.location is not None else None + before = self.get_absolute_rotation().get_rotation_matrix() if pivot is not None else None + # In place, so anything holding this `Rotation` keeps it, and normalised as `_prepend` does. + self.rotation.x = rotation.x % 360 + self.rotation.y = rotation.y % 360 + self.rotation.z = rotation.z % 360 + + if pivot is not None and before is not None: after = self.get_absolute_rotation().get_rotation_matrix() - was = matrix_vector_multiply_3x3(before, turning_on.vector()) - now = matrix_vector_multiply_3x3(after, turning_on.vector()) + was = matrix_vector_multiply_3x3(before, pivot.vector()) + now = matrix_vector_multiply_3x3(after, pivot.vector()) carried = Coordinate(was[0] - now[0], was[1] - now[1], was[2] - now[2]) - # `location` is measured in the parent's frame while `reference` is in this resource's, so - # what the turn carried has to be taken back through the parent's own rotation. A rotation - # matrix inverts by transposing. + # `carried` is in this resource's frame, `location` in the parent's. A rotation matrix + # inverts by transposing. parent = self.parent if parent is not None: turned = parent.get_absolute_rotation().get_rotation_matrix() @@ -940,10 +918,21 @@ def rotate( ) self.location = cast(Coordinate, self.location) + carried - # Rotation is part of the resource's state; notify subscribers (e.g. the - # Visualizer) so they can re-render. self._state_updated() + def rotate( + self, x: float = 0, y: float = 0, z: float = 0, reference: Optional[Coordinate] = None + ): + """Rotate counter-clockwise around the parent-coordinate axes by the given degrees. + + Args: + x: degrees to turn about X. + y: degrees to turn about Y. + z: degrees to turn about Z. + reference: the point to turn about. This resource's own corner when None. + """ + self._turn(Rotation(x=x, y=y, z=z) + self.rotation, reference) + def rotate_to( self, x: Optional[float] = None, @@ -951,23 +940,21 @@ def rotate_to( z: Optional[float] = None, reference: Optional[Coordinate] = None, ): - """Rotate counter-clockwise to the given number of degrees. - - A go-to where `rotate` is a move-by: told the same angle twice, this lands in the same place - both times. The angles are in the parent's frame, as `rotation` is - `get_absolute_rotation` - is what composes the chain to the root. + """Rotate counter-clockwise to the given degrees, where `rotate` turns by them. Args: x: degrees to point along about X. Left where it is when None. y: degrees to point along about Y. Left where it is when None. z: degrees to point along about Z. Left where it is when None. - reference: the point to turn about, as `rotate` takes it. + reference: the point to turn about. This resource's own corner when None. """ - self.rotate( - x=0 if x is None else x - self.rotation.x, - y=0 if y is None else y - self.rotation.y, - z=0 if z is None else z - self.rotation.z, - reference=reference, + self._turn( + Rotation( + x=self.rotation.x if x is None else x, + y=self.rotation.y if y is None else y, + z=self.rotation.z if z is None else z, + ), + reference, ) def copy(self) -> Self: diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index e79ae168d26..fb4c7a2a825 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -375,6 +375,47 @@ def test_rotate_to_leaves_an_axis_it_was_not_given(self): bar.rotate_to(z=90) self.assertEqual((bar.rotation.x, bar.rotation.z), (15, 90)) + def test_rotate_to_lands_on_any_axis_it_is_given(self): + for axis in ("x", "y", "z"): + for start in ((0, 0, 15), (90, 0, 90), (15, 90, 200)): + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate.zero()) + bar.rotate(x=start[0], y=start[1], z=start[2]) + bar.rotate_to( + x=30.0 if axis == "x" else None, + y=30.0 if axis == "y" else None, + z=30.0 if axis == "z" else None, + ) + self.assertAlmostEqual(getattr(bar.rotation, axis) % 360, 30, places=9) + + def test_rotate_keeps_every_axis_normalized(self): + resource = Resource("resource", size_x=10, size_y=10, size_z=10) + resource.rotate(x=350, y=350, z=350) + resource.rotate(x=20, y=20, z=20) + for axis in (resource.rotation.x, resource.rotation.y, resource.rotation.z): + self.assertGreaterEqual(axis, 0) + self.assertLess(axis, 360) + + def test_a_pivot_inside_a_turned_parent_still_holds(self): + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate(30, 40, 0)) + parent.rotate(z=90) + far_end = Coordinate(100, 0, 0) + + def where() -> Coordinate: + carried = matrix_vector_multiply_3x3( + bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() + ) + return bar.get_absolute_location() + Coordinate(*carried) + + before = where() + bar.rotate(z=90, reference=far_end) + self.assertEqual(where(), before) + def test_rotate_to_turns_about_a_reference_point(self): parent = Resource("parent", size_x=500, size_y=500, size_z=10) parent.location = Coordinate.zero() From 88bb90dd42cfb5d86423ef5017ac478d8aefa584 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Thu, 10 Sep 2026 22:59:51 +0100 Subject: [PATCH 17/34] `Resource`: leave `rotate` and `rotated` alone, and pivot only where it is used `rotate` had gained a `reference` nothing passed - the only callers were `rotated` forwarding it and two tests here - and in exchange its body became a call to a private that held the pivot. `rotated` was then pointed at `rotate_to`, which quietly turned a move-by into a go-to: `rotated(z=90)` twice would have ended at 90 rather than 180. Three legacy STAR tests caught it; `liquid_handler` calls it ten times when it moves a plate. Both are back to upstream's exact bytes, and the pivot lives in `rotate_to`, whose one real caller is `Link.turn_to`. With a single caller left, the private that held it is gone too. This PR now adds to `resource.py` and changes nothing in it: the whole diff against main is `rotate_to` inserted between `rotate` and `copy`, with no line removed, no upstream test touched and `rotation.py` untouched. The axis test sets one axis to 390 degrees and checks it reads 30 while the other two stay put, so normalisation and leaving-an-axis-alone are covered where two weaker tests missed both. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource.py | 94 +++++++++----------------- pylabrobot/resources/resource_tests.py | 29 ++++---- 2 files changed, 43 insertions(+), 80 deletions(-) diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index e64bcbb1001..399f7c534bf 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -892,22 +892,42 @@ def location(self, location: Optional[Coordinate]) -> None: if changed and self.parent is not None: self._state_updated() - def _turn(self, rotation: Rotation, reference: Optional[Coordinate]) -> None: - """Take `rotation` as this resource's own, leaving `reference` where it was.""" + def rotate(self, x: float = 0, y: float = 0, z: float = 0): + """Rotate counter-clockwise around the parent-coordinate axes by the given degrees.""" + + self.rotation._prepend(Rotation(x=x, y=y, z=z)) + # Rotation is part of the resource's state; notify subscribers (e.g. the + # Visualizer) so they can re-render. + self._state_updated() + + def rotate_to( + self, + x: Optional[float] = None, + y: Optional[float] = None, + z: Optional[float] = None, + reference: Optional[Coordinate] = None, + ): + """Rotate counter-clockwise to the given degrees, where `rotate` turns by them. + + Args: + x: degrees to point along about X. Left where it is when None. + y: degrees to point along about Y. Left where it is when None. + z: degrees to point along about Z. Left where it is when None. + reference: the point to turn about. This resource's own corner when None. + """ pivot = reference if self.location is not None else None before = self.get_absolute_rotation().get_rotation_matrix() if pivot is not None else None - # In place, so anything holding this `Rotation` keeps it, and normalised as `_prepend` does. - self.rotation.x = rotation.x % 360 - self.rotation.y = rotation.y % 360 - self.rotation.z = rotation.z % 360 + + self.rotation.x = self.rotation.x if x is None else x % 360 + self.rotation.y = self.rotation.y if y is None else y % 360 + self.rotation.z = self.rotation.z if z is None else z % 360 if pivot is not None and before is not None: after = self.get_absolute_rotation().get_rotation_matrix() was = matrix_vector_multiply_3x3(before, pivot.vector()) now = matrix_vector_multiply_3x3(after, pivot.vector()) carried = Coordinate(was[0] - now[0], was[1] - now[1], was[2] - now[2]) - # `carried` is in this resource's frame, `location` in the parent's. A rotation matrix - # inverts by transposing. + # `carried` is in this resource's frame, `location` in the parent's. parent = self.parent if parent is not None: turned = parent.get_absolute_rotation().get_rotation_matrix() @@ -920,68 +940,16 @@ def _turn(self, rotation: Rotation, reference: Optional[Coordinate]) -> None: self._state_updated() - def rotate( - self, x: float = 0, y: float = 0, z: float = 0, reference: Optional[Coordinate] = None - ): - """Rotate counter-clockwise around the parent-coordinate axes by the given degrees. - - Args: - x: degrees to turn about X. - y: degrees to turn about Y. - z: degrees to turn about Z. - reference: the point to turn about. This resource's own corner when None. - """ - self._turn(Rotation(x=x, y=y, z=z) + self.rotation, reference) - - def rotate_to( - self, - x: Optional[float] = None, - y: Optional[float] = None, - z: Optional[float] = None, - reference: Optional[Coordinate] = None, - ): - """Rotate counter-clockwise to the given degrees, where `rotate` turns by them. - - Args: - x: degrees to point along about X. Left where it is when None. - y: degrees to point along about Y. Left where it is when None. - z: degrees to point along about Z. Left where it is when None. - reference: the point to turn about. This resource's own corner when None. - """ - self._turn( - Rotation( - x=self.rotation.x if x is None else x, - y=self.rotation.y if y is None else y, - z=self.rotation.z if z is None else z, - ), - reference, - ) - def copy(self) -> Self: resource_copy = self.__class__.deserialize(self.serialize(), allow_marshal=True) resource_copy.load_all_state(self.serialize_all_state()) return resource_copy - def rotated( - self, - x: float = 0, - y: float = 0, - z: float = 0, - reference: Optional[Coordinate] = None, - ) -> Self: - """Return a copy of this resource rotated by the given number of degrees. - - Args: - x: degrees to turn about X. - y: degrees to turn about Y. - z: degrees to turn about Z. - reference: the point to turn about, as `rotate` takes it. + def rotated(self, x: float = 0, y: float = 0, z: float = 0) -> Self: + """Return a copy of this resource rotated by the given number of degrees.""" - Returns: - The rotated copy. - """ new_resource = self.copy() - new_resource.rotate(x=x, y=y, z=z, reference=reference) + new_resource.rotate(x=x, y=y, z=z) return new_resource def at(self, location: Coordinate) -> Self: diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index fb4c7a2a825..77277072612 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -344,7 +344,7 @@ def test_rotating_about_a_reference_point_leaves_that_point_where_it_was(self): far_end = Coordinate(100, 0, 0) before = bar.get_absolute_location() + far_end - bar.rotate(z=90, reference=far_end) + bar.rotate_to(z=90, reference=far_end) carried = matrix_vector_multiply_3x3( bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() ) @@ -365,17 +365,7 @@ def test_rotate_to_goes_to_an_angle_where_rotate_moves_by_one(self): bar.rotate(z=30) self.assertEqual(bar.rotation.z, 60) - def test_rotate_to_leaves_an_axis_it_was_not_given(self): - parent = Resource("parent", size_x=500, size_y=500, size_z=10) - parent.location = Coordinate.zero() - bar = Resource("bar", size_x=100, size_y=10, size_z=10) - parent.assign_child_resource(bar, location=Coordinate.zero()) - - bar.rotate(x=15, z=40) - bar.rotate_to(z=90) - self.assertEqual((bar.rotation.x, bar.rotation.z), (15, 90)) - - def test_rotate_to_lands_on_any_axis_it_is_given(self): + def test_rotate_to_sets_one_axis_normalized_and_leaves_the_others(self): for axis in ("x", "y", "z"): for start in ((0, 0, 15), (90, 0, 90), (15, 90, 200)): parent = Resource("parent", size_x=500, size_y=500, size_z=10) @@ -383,12 +373,17 @@ def test_rotate_to_lands_on_any_axis_it_is_given(self): bar = Resource("bar", size_x=100, size_y=10, size_z=10) parent.assign_child_resource(bar, location=Coordinate.zero()) bar.rotate(x=start[0], y=start[1], z=start[2]) + was = {name: getattr(bar.rotation, name) for name in ("x", "y", "z")} + bar.rotate_to( - x=30.0 if axis == "x" else None, - y=30.0 if axis == "y" else None, - z=30.0 if axis == "z" else None, + x=390.0 if axis == "x" else None, + y=390.0 if axis == "y" else None, + z=390.0 if axis == "z" else None, ) - self.assertAlmostEqual(getattr(bar.rotation, axis) % 360, 30, places=9) + + for name in ("x", "y", "z"): + expected = 30.0 if name == axis else was[name] + self.assertAlmostEqual(getattr(bar.rotation, name), expected, places=9) def test_rotate_keeps_every_axis_normalized(self): resource = Resource("resource", size_x=10, size_y=10, size_z=10) @@ -413,7 +408,7 @@ def where() -> Coordinate: return bar.get_absolute_location() + Coordinate(*carried) before = where() - bar.rotate(z=90, reference=far_end) + bar.rotate_to(z=90, reference=far_end) self.assertEqual(where(), before) def test_rotate_to_turns_about_a_reference_point(self): From 33c5ac47e6be7da75affb8db445ba5ec9d43c305 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 06:52:08 +0100 Subject: [PATCH 18/34] `Resource`: test the pivot about every axis, not only z All three pivot tests turned about z. That is the same blind spot that let `rotate_to` ship broken on x and y: a sweep of 108 cases, every one of them targeting the axis that could not fail. `test_a_pivot_holds_about_every_axis` turns a resource about a joint offset in all three axes, for each of x, y and z, and checks the joint has not moved. Two mutations are caught by it and nothing else - reading only the reference's x component, and dropping the carry's z - so both would have gone out unnoticed. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource_tests.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 77277072612..c53ad4bb0a6 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -411,6 +411,32 @@ def where() -> Coordinate: bar.rotate_to(z=90, reference=far_end) self.assertEqual(where(), before) + def test_a_pivot_holds_about_every_axis(self): + for axis in ("x", "y", "z"): + for angle in (30.0, 90.0, 200.0): + parent = Resource("parent", size_x=500, size_y=500, size_z=500) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate(7, 11, 13)) + joint = Coordinate(100, 5, 5) + + def where() -> Coordinate: + carried = matrix_vector_multiply_3x3( + bar.get_absolute_rotation().get_rotation_matrix(), joint.vector() + ) + return bar.get_absolute_location() + Coordinate(*carried) + + before = where() + bar.rotate_to( + x=angle if axis == "x" else None, + y=angle if axis == "y" else None, + z=angle if axis == "z" else None, + reference=joint, + ) + after = where() + for was, now in zip((before.x, before.y, before.z), (after.x, after.y, after.z)): + self.assertAlmostEqual(was, now, places=9) + def test_rotate_to_turns_about_a_reference_point(self): parent = Resource("parent", size_x=500, size_y=500, size_z=10) parent.location = Coordinate.zero() From 098e9fc73e609a12daeaacd1e08d210615281c14 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 06:57:01 +0100 Subject: [PATCH 19/34] `Resource`: drop two pivot tests that protected nothing Three tests turned about a reference point, all about z. Mutating the six things the pivot can get wrong shows two of them never fire alone: - `test_rotating_about_a_reference_point_leaves_that_point_where_it_was` and `test_rotate_to_turns_about_a_reference_point` became the same test when both were pointed at `rotate_to`: same parent, same bar at the origin, same turn, same assertion. - Both are subsumed by the axis test, which turns about an offset joint on all three axes. What is left divides the space: one varies the axis, one varies the parent's frame, and each catches two mutations nothing else does. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource_tests.py | 33 -------------------------- 1 file changed, 33 deletions(-) diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index c53ad4bb0a6..062f355d557 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -332,26 +332,6 @@ def test_rotation90(self): self.assertAlmostEqual(c.get_absolute_size_x(), 20) self.assertAlmostEqual(c.get_absolute_size_y(), 10) - def test_rotating_about_a_reference_point_leaves_that_point_where_it_was(self): - """A resource turns about its own left front bottom corner. `reference` names another point to - turn on - a hinge, a joint - and the resource is carried so that point does not move, which is - what a joint is. Checked on the point itself rather than on the resource's location, since the - location moving is the mechanism and the point standing still is the promise.""" - parent = Resource("parent", size_x=500, size_y=500, size_z=10) - parent.location = Coordinate.zero() - bar = Resource("bar", size_x=100, size_y=10, size_z=10) - parent.assign_child_resource(bar, location=Coordinate(0, 0, 0)) - far_end = Coordinate(100, 0, 0) - - before = bar.get_absolute_location() + far_end - bar.rotate_to(z=90, reference=far_end) - carried = matrix_vector_multiply_3x3( - bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() - ) - - self.assertEqual(bar.get_absolute_location() + Coordinate(*carried), before) - self.assertEqual(bar.location, Coordinate(100, -100, 0)) - def test_rotate_to_goes_to_an_angle_where_rotate_moves_by_one(self): parent = Resource("parent", size_x=500, size_y=500, size_z=10) parent.location = Coordinate.zero() @@ -437,19 +417,6 @@ def where() -> Coordinate: for was, now in zip((before.x, before.y, before.z), (after.x, after.y, after.z)): self.assertAlmostEqual(was, now, places=9) - def test_rotate_to_turns_about_a_reference_point(self): - parent = Resource("parent", size_x=500, size_y=500, size_z=10) - parent.location = Coordinate.zero() - bar = Resource("bar", size_x=100, size_y=10, size_z=10) - parent.assign_child_resource(bar, location=Coordinate.zero()) - far_end = Coordinate(100, 0, 0) - - bar.rotate_to(z=90, reference=far_end) - carried = matrix_vector_multiply_3x3( - bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() - ) - self.assertEqual(bar.get_absolute_location() + Coordinate(*carried), far_end) - def test_rotation180(self): r = Resource("parent", size_x=200, size_y=100, size_z=100) r.location = Coordinate.zero() From 809c0cfc6b4c2b312d6dc94a5710c9bb2f1b1563 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 11:47:09 +0100 Subject: [PATCH 20/34] `Resource`: go to an angle without a pivot, and drop the joint nothing turned on `rotate_to` took a `reference` point to turn about, and `Link` carried a `joint` for it to turn on. Nothing ever passed either: `turn_to` handed `rotate_to` a zero vector on every call, because the iSWAP's links are placed so their origins already sit on their drive axes. The pivot arithmetic ran on a zero vector in all production use and was exercised only by its own tests. `rotate_to` is now what its callers wanted, an absolute-angle setter, and 22 lines shorter. `Link` is the length and nothing else; a caller that needs an angle calls `rotate_to(z=...)` directly. `rotate` and `rotated` are untouched, and `resource.py` remains additions-only against upstream. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/manipulator.py | 26 ++------------ pylabrobot/resources/manipulator_tests.py | 24 ++----------- pylabrobot/resources/resource.py | 30 +++------------- pylabrobot/resources/resource_tests.py | 44 ----------------------- 4 files changed, 8 insertions(+), 116 deletions(-) diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py index a0b45f967e1..184fa7c5576 100644 --- a/pylabrobot/resources/manipulator.py +++ b/pylabrobot/resources/manipulator.py @@ -8,27 +8,20 @@ from typing import Optional -from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.resource import Resource class Link(Resource): """One of the rigid pieces an arm is built from, joined to its neighbours by joints. - What sets the reach is the distance between a link's joints, not the shape of the piece, so the - material is attached as children with their own origins and may overhang a joint at either end. - - `joint` is where the joint it turns on sits within the link, which `turn_to` pivots about. It - needs no particular place: a link is not obliged to put its own origin there. - - Unrotated it lies along +X. + It turns about its own origin, and unrotated it lies along +X, so the joint at its far end is at + `length`. """ def __init__( self, name: str, length: float, - joint: Optional[Coordinate] = None, category: str = "link", model: Optional[str] = None, ): @@ -36,24 +29,9 @@ def __init__( Args: name: what to call this one. length: joint to joint, in mm. - joint: where the joint this link turns on sits within it. Its own origin when None. category: what kind of resource this is. model: which link this is. """ super().__init__( name=name, size_x=length, size_y=0.0, size_z=0.0, category=category, model=model ) - self.joint = joint if joint is not None else Coordinate.zero() - - def turn_to(self, angle: float) -> None: - """Point the link along `angle`, pivoting on `joint`. - - Args: - angle: the angle to point along, in its parent's frame, in degrees. - - Raises: - RuntimeError: If the link has not been placed, so there is nothing for it to turn in. - """ - if self.location is None: - raise RuntimeError(f"{self.name} is not placed, so there is nothing for it to turn in") - self.rotate_to(z=angle, reference=self.joint) diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index ec290f028ff..6d24cd35d55 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -23,32 +23,12 @@ def far_end() -> Coordinate: self.assertEqual(far_end(), Coordinate(150, 0, 0)) - second.turn_to(90) + second.rotate_to(z=90) self.assertEqual(far_end(), Coordinate(100, 50, 0)) - first.turn_to(90) + first.rotate_to(z=90) self.assertEqual(far_end(), Coordinate(-50, 100, 0)) - def test_turning_to_an_angle_is_absolute(self): - base = Resource(name="base", size_x=500, size_y=500, size_z=0) - link = Link(name="link", length=100.0) - base.assign_child_resource(link, location=Coordinate(0, 0, 0)) - - link.turn_to(30) - link.turn_to(30) - self.assertEqual(link.rotation.z, 30) - - link.rotate(z=30) - self.assertEqual(link.rotation.z, 60) - - def test_an_unplaced_link_has_nothing_to_turn_in(self): - base = Resource(name="base", size_x=500, size_y=500, size_z=0) - link = Link(name="link", length=100.0) - base.assign_child_resource(link, location=Coordinate(0, 0, 0)) - - with self.assertRaises(RuntimeError): - Link(name="loose", length=100.0).turn_to(0) - if __name__ == "__main__": unittest.main() diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index 399f7c534bf..6e06632ff99 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -905,39 +905,17 @@ def rotate_to( x: Optional[float] = None, y: Optional[float] = None, z: Optional[float] = None, - reference: Optional[Coordinate] = None, ): - """Rotate counter-clockwise to the given degrees, where `rotate` turns by them. + """Set the rotation about each axis, where `rotate` turns by an amount instead. Args: - x: degrees to point along about X. Left where it is when None. - y: degrees to point along about Y. Left where it is when None. - z: degrees to point along about Z. Left where it is when None. - reference: the point to turn about. This resource's own corner when None. + x: the angle about X to sit at, in degrees. Left where it is when None. + y: the angle about Y to sit at, in degrees. Left where it is when None. + z: the angle about Z to sit at, in degrees. Left where it is when None. """ - pivot = reference if self.location is not None else None - before = self.get_absolute_rotation().get_rotation_matrix() if pivot is not None else None - self.rotation.x = self.rotation.x if x is None else x % 360 self.rotation.y = self.rotation.y if y is None else y % 360 self.rotation.z = self.rotation.z if z is None else z % 360 - - if pivot is not None and before is not None: - after = self.get_absolute_rotation().get_rotation_matrix() - was = matrix_vector_multiply_3x3(before, pivot.vector()) - now = matrix_vector_multiply_3x3(after, pivot.vector()) - carried = Coordinate(was[0] - now[0], was[1] - now[1], was[2] - now[2]) - # `carried` is in this resource's frame, `location` in the parent's. - parent = self.parent - if parent is not None: - turned = parent.get_absolute_rotation().get_rotation_matrix() - carried = Coordinate( - *matrix_vector_multiply_3x3( - [[turned[j][i] for j in range(3)] for i in range(3)], carried.vector() - ) - ) - self.location = cast(Coordinate, self.location) + carried - self._state_updated() def copy(self) -> Self: diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 062f355d557..67d107c6249 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -373,50 +373,6 @@ def test_rotate_keeps_every_axis_normalized(self): self.assertGreaterEqual(axis, 0) self.assertLess(axis, 360) - def test_a_pivot_inside_a_turned_parent_still_holds(self): - parent = Resource("parent", size_x=500, size_y=500, size_z=10) - parent.location = Coordinate.zero() - bar = Resource("bar", size_x=100, size_y=10, size_z=10) - parent.assign_child_resource(bar, location=Coordinate(30, 40, 0)) - parent.rotate(z=90) - far_end = Coordinate(100, 0, 0) - - def where() -> Coordinate: - carried = matrix_vector_multiply_3x3( - bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() - ) - return bar.get_absolute_location() + Coordinate(*carried) - - before = where() - bar.rotate_to(z=90, reference=far_end) - self.assertEqual(where(), before) - - def test_a_pivot_holds_about_every_axis(self): - for axis in ("x", "y", "z"): - for angle in (30.0, 90.0, 200.0): - parent = Resource("parent", size_x=500, size_y=500, size_z=500) - parent.location = Coordinate.zero() - bar = Resource("bar", size_x=100, size_y=10, size_z=10) - parent.assign_child_resource(bar, location=Coordinate(7, 11, 13)) - joint = Coordinate(100, 5, 5) - - def where() -> Coordinate: - carried = matrix_vector_multiply_3x3( - bar.get_absolute_rotation().get_rotation_matrix(), joint.vector() - ) - return bar.get_absolute_location() + Coordinate(*carried) - - before = where() - bar.rotate_to( - x=angle if axis == "x" else None, - y=angle if axis == "y" else None, - z=angle if axis == "z" else None, - reference=joint, - ) - after = where() - for was, now in zip((before.x, before.y, before.z), (after.x, after.y, after.z)): - self.assertAlmostEqual(was, now, places=9) - def test_rotation180(self): r = Resource("parent", size_x=200, size_y=100, size_z=100) r.location = Coordinate.zero() From 2f1364afbbc571fd7c631d86b7e9ef1e1357ca71 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 11:47:39 +0100 Subject: [PATCH 21/34] `MechanicalGripper`: make `jaw_width` the gap the fingers leave, not their centre spacing The fingers were stood with their centres `jaw_width` apart, so the opening an object had to pass through was `jaw_width` less one finger thickness. On the iSWAP's 7 mm fingers a commanded 133.706 mm left 126.706 mm of clear space, and a width equal to the finger thickness stood the two fully interpenetrating. They now stand with their facing surfaces that far apart, which is the distance the drive reports and the one a rack has to fit into. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 20 ++++++++++---------- pylabrobot/resources/end_effector_tests.py | 11 ++++++----- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index c758333d537..a918f900e1f 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -19,9 +19,8 @@ class MechanicalGripper(Link): """A gripper that holds by closing two fingers on what it takes. A link: it spans the joint it turns on to the point it grips at, which is `tool_center_point`. - Its body, its two fingers and a pad on each are material bolted to that span; a gripper whose - fingers meet the resource themselves carries no pads. How far apart the fingers stand is state - rather than shape, so `jaw_width` moves them. + Its body, its two fingers and a pad on each are material bolted to that span. The gap between + the fingers is state rather than shape, so `jaw_width` moves them. """ def __init__( @@ -47,13 +46,11 @@ def __init__( body_location: where it sits, from the joint this gripper turns on. fingers: the two jaws, either side of the span. finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s. - jaw_range: how far apart the fingers stand, closed and open, in mm. + jaw_range: the gap between the fingers, closed and open, in mm. pads: what each finger meets the resource with, in the same order as `fingers`. A gripper whose fingers meet it themselves has none. pad_location: where a pad sits, from the finger it is fixed to. Given with `pads`. - jaw_width: how far apart they stand to begin with, in mm. Where a gripper is known to come - up at a particular width - the one it homes at, say - that is what to build it at, so the - model does not start out claiming a width nothing has read. Open, when not given. + jaw_width: the gap to begin with, in mm. Open, when not given. """ super().__init__(name=name, length=length, category=category, model=model) if len(fingers) != 2: @@ -89,7 +86,7 @@ def tool_center_point(self) -> Coordinate: @property def jaw_width(self) -> float: - """How far apart the fingers stand, in mm.""" + """The gap between the fingers' facing surfaces, in mm: what fits between them.""" return self._jaw_width @jaw_width.setter @@ -101,11 +98,14 @@ def jaw_width(self, width: float) -> None: self._place_the_fingers() def _place_the_fingers(self) -> None: - """Stand the fingers either side of the span, as far apart as the jaws are open.""" + """Stand the fingers either side of the span, leaving `jaw_width` of gap between them.""" for finger, side in zip(self.fingers, (1.0, -1.0)): here = cast(Coordinate, finger.location) + # A resource sits at its lowest-y corner: the facing surface on the +Y side, the back of the + # finger on the -Y side. + facing = side * self._jaw_width / 2.0 finger.location = Coordinate( - here.x, side * self._jaw_width / 2.0 - finger.get_size_y() / 2.0, here.z + here.x, facing if side > 0 else facing - finger.get_size_y(), here.z ) def serialize(self) -> dict: diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 503095fa555..055c3b7645c 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -52,16 +52,17 @@ def test_the_grip_centre_sits_at_the_end_of_the_span(self): class TestJaws(unittest.TestCase): - def test_a_width_stands_the_fingers_that_far_apart(self): + def test_a_width_is_the_gap_the_fingers_leave_between_them(self): g = gripper() for width in (133.706, 100.0, 70.844): g.jaw_width = width left, right = g.fingers - centres = [ - cast(Coordinate, finger.location).y + finger.get_size_y() / 2 for finger in (left, right) + faces = [ + cast(Coordinate, left.location).y, + cast(Coordinate, right.location).y + right.get_size_y(), ] - self.assertAlmostEqual(centres[0] - centres[1], width) - self.assertAlmostEqual(centres[0] + centres[1], 0.0) + self.assertAlmostEqual(faces[0] - faces[1], width) + self.assertAlmostEqual(faces[0] + faces[1], 0.0) def test_the_jaws_refuse_a_width_they_do_not_reach(self): with self.assertRaises(ValueError): From 6104cd380cbcdceb01db2caab3141edaf7073ec5 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 11:48:04 +0100 Subject: [PATCH 22/34] `Link`, `MechanicalGripper`: come back from `serialize` whole Neither could be deserialized or copied. `Resource.serialize` emits `size_x/size_y/size_z` and `Resource.deserialize` hands them to the constructor, which takes `length` instead, so both raised `TypeError: got an unexpected keyword argument 'size_x'`. A deck carrying an iSWAP could not be saved, and `copy()` went the same way. `Link` now swaps the three sizes for the `length` it was built with, as `PetriDish` does for its diameter. `MechanicalGripper` takes its body and its two fingers back off the front of `children`, where `__init__` put them, and reassigns anything after them as what it was holding. `jaw_width` moves to `serialize_state`/`load_state`, which is where mutable state belongs and is what carries it through `copy()`. `tool_center_point` now travels with the model, which the visualizer reads to draw the grip centre. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 58 +++++++++++++++++++++- pylabrobot/resources/end_effector_tests.py | 14 ++++++ pylabrobot/resources/manipulator.py | 6 +++ pylabrobot/resources/manipulator_tests.py | 6 +++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index a918f900e1f..715023f51b2 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -8,11 +8,13 @@ `MechanicalGripper` spans that offset, flange to grip centre, which is why it is a `Link`. """ -from typing import Optional, Sequence, Tuple, cast +from typing import Any, Dict, Optional, Sequence, Tuple, cast from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.manipulator import Link from pylabrobot.resources.resource import Resource +from pylabrobot.resources.rotation import Rotation +from pylabrobot.serializer import deserialize class MechanicalGripper(Link): @@ -109,4 +111,56 @@ def _place_the_fingers(self) -> None: ) def serialize(self) -> dict: - return {**super().serialize(), "jaw_range": list(self.jaw_range)} + return { + **super().serialize(), + "jaw_range": list(self.jaw_range), + "tool_center_point": self.tool_center_point.serialize(), + } + + @classmethod + def deserialize(cls, data: dict, allow_marshal: bool = False) -> "MechanicalGripper": + """Rebuild a gripper, taking its own parts back out of its children. + + Its body and its two fingers are constructor arguments rather than children assigned after the + fact, so they are read off the front of `children`, in the order `__init__` put them there. + Anything after them is what the gripper was holding. + """ + children = data["children"] + body, *fingers = ( + Resource.deserialize(child, allow_marshal=allow_marshal) for child in children[:3] + ) + pads = [pad for finger in fingers for pad in list(finger.children)] + for pad in pads: + pad.unassign() + + def where(child: dict) -> Coordinate: + return cast(Coordinate, deserialize(child["location"], allow_marshal=allow_marshal)) + + gripper = cls( + name=data["name"], + length=data["length"], + body=body, + body_location=where(children[0]), + fingers=fingers, + finger_location=where(children[1]), + jaw_range=(data["jaw_range"][0], data["jaw_range"][1]), + pads=pads or None, + pad_location=where(children[1]["children"][0]) if pads else None, + category=data.get("category", "mechanical_gripper"), + model=data.get("model"), + ) + rotation = data.get("rotation") + if rotation is not None: + gripper.rotation = cast(Rotation, deserialize(rotation, allow_marshal=allow_marshal)) + for child in children[3:]: + gripper.assign_child_resource( + Resource.deserialize(child, allow_marshal=allow_marshal), location=where(child) + ) + return gripper + + def serialize_state(self) -> Dict[str, Any]: + return {**super().serialize_state(), "jaw_width": self.jaw_width} + + def load_state(self, state: Dict[str, Any]) -> None: + super().load_state(state) + self.jaw_width = state["jaw_width"] diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 055c3b7645c..c656b9b8fc6 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -98,5 +98,19 @@ def test_a_pad_sits_inside_its_finger(self): self.assertLessEqual(sits_at + face.get_size_y(), jaw.get_size_y()) +class TestRoundTrip(unittest.TestCase): + def test_a_gripper_comes_back_with_its_parts_and_its_width(self): + g = gripper(jaw_width=100.0) + back = MechanicalGripper.deserialize(g.serialize()) + back.load_all_state(g.serialize_all_state()) + + self.assertEqual(back.tool_center_point, g.tool_center_point) + self.assertEqual(back.jaw_range, g.jaw_range) + self.assertEqual(back.jaw_width, 100.0) + self.assertEqual(cast(Coordinate, back.body.location), BODY_LOCATION) + self.assertEqual([pad.location for pad in back.pads], [PAD_LOCATION] * 2) + self.assertEqual([pad.parent for pad in back.pads], back.fingers) + + if __name__ == "__main__": unittest.main() diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py index 184fa7c5576..d7b3e426f39 100644 --- a/pylabrobot/resources/manipulator.py +++ b/pylabrobot/resources/manipulator.py @@ -35,3 +35,9 @@ def __init__( super().__init__( name=name, size_x=length, size_y=0.0, size_z=0.0, category=category, model=model ) + + def serialize(self) -> dict: + serialized = super().serialize() + for key in ("size_x", "size_y", "size_z"): + serialized.pop(key, None) + return {**serialized, "length": self.get_size_x()} diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index 6d24cd35d55..eb4c0f35e32 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -29,6 +29,12 @@ def far_end() -> Coordinate: first.rotate_to(z=90) self.assertEqual(far_end(), Coordinate(-50, 100, 0)) + def test_a_link_comes_back_the_length_it_went_in(self): + link = Link(name="link", length=137.7, model="demo") + back = Link.deserialize(link.serialize()) + self.assertEqual(back.get_size_x(), 137.7) + self.assertEqual(back.model, "demo") + if __name__ == "__main__": unittest.main() From 406cbe4584de1d5291ef4b2e1976069f184e2cce Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 11:48:19 +0100 Subject: [PATCH 23/34] `MechanicalGripper`: assert what the pad tests claimed to `test_a_pad_sits_inside_its_finger` compared the test file's own constants against each other - 1.5 >= 0, and 1.5 + 4 <= 7 - so it passed with `pad_location` ignored entirely and with the pads attached to the gripper rather than to a finger. It now asserts the parent and the location, which is what it was named for. `test_a_gripper_can_have_bare_fingers` asserted only the empty case, so it survived `self.pads = []` being hardcoded. It now checks the padded case too. `test_rotate_keeps_every_axis_normalized` tested `Rotation._prepend`, which this branch does not touch, and duplicated an upstream test of the same behaviour. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector_tests.py | 17 ++++++++++------- pylabrobot/resources/resource_tests.py | 8 -------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index c656b9b8fc6..0202dd83266 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -80,9 +80,13 @@ def test_a_gripper_starts_open_unless_told_otherwise(self): class TestPads(unittest.TestCase): def test_a_gripper_can_have_bare_fingers(self): - g = gripper(pads=None, pad_location=None) - self.assertEqual(g.pads, []) - self.assertEqual([jaw.children for jaw in g.fingers], [[], []]) + bare = gripper(pads=None, pad_location=None) + self.assertEqual(bare.pads, []) + self.assertEqual([jaw.children for jaw in bare.fingers], [[], []]) + + padded = gripper() + self.assertEqual(len(padded.pads), 2) + self.assertEqual([jaw.children for jaw in padded.fingers], [[pad] for pad in padded.pads]) def test_pads_and_their_location_go_together(self): with self.assertRaises(ValueError): @@ -90,12 +94,11 @@ def test_pads_and_their_location_go_together(self): with self.assertRaises(ValueError): gripper(pads=None) - def test_a_pad_sits_inside_its_finger(self): + def test_a_pad_is_fixed_to_its_own_finger_where_it_was_put(self): g = gripper() for jaw, face in zip(g.fingers, g.pads): - sits_at = cast(Coordinate, face.location).y - self.assertGreaterEqual(sits_at, 0.0) - self.assertLessEqual(sits_at + face.get_size_y(), jaw.get_size_y()) + self.assertIs(face.parent, jaw) + self.assertEqual(face.location, PAD_LOCATION) class TestRoundTrip(unittest.TestCase): diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 67d107c6249..bf289f67f13 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -365,14 +365,6 @@ def test_rotate_to_sets_one_axis_normalized_and_leaves_the_others(self): expected = 30.0 if name == axis else was[name] self.assertAlmostEqual(getattr(bar.rotation, name), expected, places=9) - def test_rotate_keeps_every_axis_normalized(self): - resource = Resource("resource", size_x=10, size_y=10, size_z=10) - resource.rotate(x=350, y=350, z=350) - resource.rotate(x=20, y=20, z=20) - for axis in (resource.rotation.x, resource.rotation.y, resource.rotation.z): - self.assertGreaterEqual(axis, 0) - self.assertLess(axis, 360) - def test_rotation180(self): r = Resource("parent", size_x=200, size_y=100, size_z=100) r.location = Coordinate.zero() From 4a3c346349a63804b3f0cbd0f283674da5d590b4 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 13:10:34 +0100 Subject: [PATCH 24/34] `MechanicalGripper`: let the grip centre sit off the plane it is mounted on `tool_center_point` returned `(length, 0, 0)`, so a tool could only ever be programmed against a point level with its own mounting. A gripper that takes hold below where it hangs could not be described at all. Measured on a Hamilton iSWAP, the grip centre sits 13 mm below the wrist, which the model put at 0 - so the point a move is programmed against was 13 mm high, and disagreed with the same figure worked out by forward kinematics from the joints. `tool_center_point_z` states that offset, defaults to level, and travels through `serialize`. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 7 ++++++- pylabrobot/resources/end_effector_tests.py | 9 ++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index 715023f51b2..405d52eab03 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -34,6 +34,7 @@ def __init__( fingers: Sequence[Resource], finger_location: Coordinate, jaw_range: Tuple[float, float], + tool_center_point_z: float = 0.0, pads: Optional[Sequence[Resource]] = None, pad_location: Optional[Coordinate] = None, jaw_width: Optional[float] = None, @@ -49,6 +50,8 @@ def __init__( fingers: the two jaws, either side of the span. finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s. jaw_range: the gap between the fingers, closed and open, in mm. + tool_center_point_z: how far the grip centre sits above where the tool is mounted, in mm. + Level with it when 0, and negative for a tool that grips below its own mounting. pads: what each finger meets the resource with, in the same order as `fingers`. A gripper whose fingers meet it themselves has none. pad_location: where a pad sits, from the finger it is fixed to. Given with `pads`. @@ -63,6 +66,7 @@ def __init__( if pads is not None and len(pads) != len(fingers): raise ValueError(f"a gripper has a pad on each finger, not {len(pads)} on {len(fingers)}") self.jaw_range = jaw_range + self.tool_center_point_z = tool_center_point_z self.body = body self.assign_child_resource(body, location=body_location) @@ -84,7 +88,7 @@ def tool_center_point(self) -> Coordinate: Returns: The grip centre, which the fingers reach past. """ - return Coordinate(self.get_size_x(), 0.0, 0.0) + return Coordinate(self.get_size_x(), 0.0, self.tool_center_point_z) @property def jaw_width(self) -> float: @@ -144,6 +148,7 @@ def where(child: dict) -> Coordinate: fingers=fingers, finger_location=where(children[1]), jaw_range=(data["jaw_range"][0], data["jaw_range"][1]), + tool_center_point_z=data["tool_center_point"]["z"], pads=pads or None, pad_location=where(children[1]["children"][0]) if pads else None, category=data.get("category", "mechanical_gripper"), diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 0202dd83266..25295ad5edc 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -47,8 +47,11 @@ def gripper(**overrides) -> MechanicalGripper: class TestTheSpan(unittest.TestCase): def test_the_grip_centre_sits_at_the_end_of_the_span(self): - g = gripper() - self.assertEqual(g.tool_center_point, Coordinate(LENGTH, 0.0, 0.0)) + self.assertEqual(gripper().tool_center_point, Coordinate(LENGTH, 0.0, 0.0)) + + def test_a_tool_can_grip_below_where_it_is_mounted(self): + g = gripper(tool_center_point_z=-13.0) + self.assertEqual(g.tool_center_point, Coordinate(LENGTH, 0.0, -13.0)) class TestJaws(unittest.TestCase): @@ -103,7 +106,7 @@ def test_a_pad_is_fixed_to_its_own_finger_where_it_was_put(self): class TestRoundTrip(unittest.TestCase): def test_a_gripper_comes_back_with_its_parts_and_its_width(self): - g = gripper(jaw_width=100.0) + g = gripper(jaw_width=100.0, tool_center_point_z=-13.0) back = MechanicalGripper.deserialize(g.serialize()) back.load_all_state(g.serialize_all_state()) From ac950af7bd3ede77f0e2ffebf19f953b4d1b915d Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 13:47:01 +0100 Subject: [PATCH 25/34] `MechanicalGripper`: take the tool centre point as one offset It arrived as two arguments: `length` gave its reach and `tool_center_point_z` its drop, with y fixed at zero and the property assembling the three at read time. They describe one thing - the offset from the joint the tool turns on to the point it is programmed against - so it is now given as one `Coordinate`, and `length` is derived from its reach rather than stated alongside it. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 18 ++++++++---------- pylabrobot/resources/end_effector_tests.py | 7 ++++--- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index 405d52eab03..78df5ece7e3 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -28,13 +28,12 @@ class MechanicalGripper(Link): def __init__( self, name: str, - length: float, + tool_center_point: Coordinate, body: Resource, body_location: Coordinate, fingers: Sequence[Resource], finger_location: Coordinate, jaw_range: Tuple[float, float], - tool_center_point_z: float = 0.0, pads: Optional[Sequence[Resource]] = None, pad_location: Optional[Coordinate] = None, jaw_width: Optional[float] = None, @@ -44,20 +43,18 @@ def __init__( """ Args: name: what to call this one. - length: the joint it turns on to the grip centre, in mm. + tool_center_point: the joint it turns on to the point it grips at, in mm. body: the material around the span. body_location: where it sits, from the joint this gripper turns on. fingers: the two jaws, either side of the span. finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s. jaw_range: the gap between the fingers, closed and open, in mm. - tool_center_point_z: how far the grip centre sits above where the tool is mounted, in mm. - Level with it when 0, and negative for a tool that grips below its own mounting. pads: what each finger meets the resource with, in the same order as `fingers`. A gripper whose fingers meet it themselves has none. pad_location: where a pad sits, from the finger it is fixed to. Given with `pads`. jaw_width: the gap to begin with, in mm. Open, when not given. """ - super().__init__(name=name, length=length, category=category, model=model) + super().__init__(name=name, length=tool_center_point.x, category=category, model=model) if len(fingers) != 2: raise ValueError(f"a gripper has two fingers, not {len(fingers)}") if (pads is None) != (pad_location is None): @@ -66,7 +63,7 @@ def __init__( if pads is not None and len(pads) != len(fingers): raise ValueError(f"a gripper has a pad on each finger, not {len(pads)} on {len(fingers)}") self.jaw_range = jaw_range - self.tool_center_point_z = tool_center_point_z + self._tool_center_point = tool_center_point self.body = body self.assign_child_resource(body, location=body_location) @@ -88,7 +85,7 @@ def tool_center_point(self) -> Coordinate: Returns: The grip centre, which the fingers reach past. """ - return Coordinate(self.get_size_x(), 0.0, self.tool_center_point_z) + return self._tool_center_point @property def jaw_width(self) -> float: @@ -142,13 +139,14 @@ def where(child: dict) -> Coordinate: gripper = cls( name=data["name"], - length=data["length"], + tool_center_point=cast( + Coordinate, deserialize(data["tool_center_point"], allow_marshal=allow_marshal) + ), body=body, body_location=where(children[0]), fingers=fingers, finger_location=where(children[1]), jaw_range=(data["jaw_range"][0], data["jaw_range"][1]), - tool_center_point_z=data["tool_center_point"]["z"], pads=pads or None, pad_location=where(children[1]["children"][0]) if pads else None, category=data.get("category", "mechanical_gripper"), diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 25295ad5edc..ea46de04c81 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -33,7 +33,7 @@ def gripper(**overrides) -> MechanicalGripper: arguments = dict( name="demo_gripper", - length=LENGTH, + tool_center_point=Coordinate(LENGTH, 0.0, 0.0), body=body, body_location=BODY_LOCATION, fingers=fingers, @@ -50,8 +50,9 @@ def test_the_grip_centre_sits_at_the_end_of_the_span(self): self.assertEqual(gripper().tool_center_point, Coordinate(LENGTH, 0.0, 0.0)) def test_a_tool_can_grip_below_where_it_is_mounted(self): - g = gripper(tool_center_point_z=-13.0) + g = gripper(tool_center_point=Coordinate(LENGTH, 0.0, -13.0)) self.assertEqual(g.tool_center_point, Coordinate(LENGTH, 0.0, -13.0)) + self.assertEqual(g.get_size_x(), LENGTH) class TestJaws(unittest.TestCase): @@ -106,7 +107,7 @@ def test_a_pad_is_fixed_to_its_own_finger_where_it_was_put(self): class TestRoundTrip(unittest.TestCase): def test_a_gripper_comes_back_with_its_parts_and_its_width(self): - g = gripper(jaw_width=100.0, tool_center_point_z=-13.0) + g = gripper(jaw_width=100.0, tool_center_point=Coordinate(LENGTH, 0.0, -13.0)) back = MechanicalGripper.deserialize(g.serialize()) back.load_all_state(g.serialize_all_state()) From 905034dff5322d07fe3d4c8d38a6ecf0c6123d70 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 14:14:30 +0100 Subject: [PATCH 26/34] `Resource`: turn about a coordinate other than its own origin A resource could only turn about its own origin, the front-left-bottom corner, because that is all `rotation` expresses. Turning a plate about its centre, or a cube about one of its twelve edges, had no way to be said: the caller had to rotate and then work out by hand where the resource had to be put back. `rotate`, `rotate_to` and `rotated` now take a `pivot_coordinate`, given in the resource's own frame. The turn still happens about the corner, and `_apply_pivot_shift` then moves `location` by however far the named coordinate drifted, so it ends where it began. The drift is measured in absolute axes and `location` is written in the parent's, so it is turned into the parent's frame first, which is what lets a resource inside a parent that is itself rotated come out right. Giving a pivot to a resource that has not been placed raises, since there is no location to move. This is the pivot that 809c0cfc6 removed, and it is not the same concept. That one was `Link.joint`, a coordinate stored on the resource and fed to every turn, which was dead because a link's origin already sits on its drive axis. This is a parameter of the motion, chosen per call, which is the only way to express a cube that tips over any of its edges. The private that holds the arithmetic comes back for the same reason it went: it now has two callers rather than one. `resource.py` is no longer additions-only against upstream. `rotate` and `rotated` each gain an optional argument and default to exactly their previous behaviour, which the existing tests and the legacy STAR suite cover. Tests: the two pivot tests removed in 809c0cfc6 are back, with the every-axis one now run against `rotate` as well as `rotate_to`. Added: that `rotate` carries the pivot on each turn while a repeated `rotate_to` changes nothing, that both raise unplaced, and that `rotated` carries the pivot into the copy while leaving the original alone. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource.py | 92 ++++++++++++++++++++++++-- pylabrobot/resources/resource_tests.py | 83 +++++++++++++++++++++++ 2 files changed, 170 insertions(+), 5 deletions(-) diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index 6e06632ff99..af29f6b901c 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -892,10 +892,62 @@ def location(self, location: Optional[Coordinate]) -> None: if changed and self.parent is not None: self._state_updated() - def rotate(self, x: float = 0, y: float = 0, z: float = 0): - """Rotate counter-clockwise around the parent-coordinate axes by the given degrees.""" + def _apply_pivot_shift(self, before: List[List[float]], pivot_coordinate: Coordinate) -> None: + """Shift `location` so `pivot_coordinate` ends where it was before this resource turned. + + Args: + before: this resource's absolute rotation matrix, from before the turn. + pivot_coordinate: what to hold still, in this resource's own frame. + """ + after = self.get_absolute_rotation().get_rotation_matrix() + was = matrix_vector_multiply_3x3(before, pivot_coordinate.vector()) + now = matrix_vector_multiply_3x3(after, pivot_coordinate.vector()) + shift = Coordinate(was[0] - now[0], was[1] - now[1], was[2] - now[2]) + # `shift` is in this resource's frame, `location` in the parent's. A rotation matrix + # inverts by transposing. + parent = self.parent + if parent is not None: + turned = parent.get_absolute_rotation().get_rotation_matrix() + shift = Coordinate( + *matrix_vector_multiply_3x3( + [[turned[j][i] for j in range(3)] for i in range(3)], shift.vector() + ) + ) + self.location = cast(Coordinate, self.location) + shift + + def rotate( + self, + x: float = 0, + y: float = 0, + z: float = 0, + pivot_coordinate: Optional[Coordinate] = None, + ): + """Rotate counter-clockwise around the parent-coordinate axes by the given degrees. + + Args: + x: degrees to turn about X. + y: degrees to turn about Y. + z: degrees to turn about Z. + pivot_coordinate: what to turn about, in this resource's own frame. Its own origin when + None, which is what a resource turns about when nothing is said. Given one, `location` + carries by however far the turn moved it, so it ends where it began. + + Raises: + ValueError: If a pivot is given for a resource that is not placed, where there is no + location to carry. + """ + if pivot_coordinate is not None and self.location is None: + raise ValueError(f"{self.name} is not placed, so there is nothing for it to turn in") + + before = ( + self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None + ) self.rotation._prepend(Rotation(x=x, y=y, z=z)) + + if pivot_coordinate is not None and before is not None: + self._apply_pivot_shift(before, pivot_coordinate) + # Rotation is part of the resource's state; notify subscribers (e.g. the # Visualizer) so they can re-render. self._state_updated() @@ -905,6 +957,7 @@ def rotate_to( x: Optional[float] = None, y: Optional[float] = None, z: Optional[float] = None, + pivot_coordinate: Optional[Coordinate] = None, ): """Set the rotation about each axis, where `rotate` turns by an amount instead. @@ -912,10 +965,26 @@ def rotate_to( x: the angle about X to sit at, in degrees. Left where it is when None. y: the angle about Y to sit at, in degrees. Left where it is when None. z: the angle about Z to sit at, in degrees. Left where it is when None. + pivot_coordinate: what to turn about, as `rotate` takes it. + + Raises: + ValueError: If a pivot is given for a resource that is not placed, where there is no + location to carry. """ + if pivot_coordinate is not None and self.location is None: + raise ValueError(f"{self.name} is not placed, so there is nothing for it to turn in") + + before = ( + self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None + ) + self.rotation.x = self.rotation.x if x is None else x % 360 self.rotation.y = self.rotation.y if y is None else y % 360 self.rotation.z = self.rotation.z if z is None else z % 360 + + if pivot_coordinate is not None and before is not None: + self._apply_pivot_shift(before, pivot_coordinate) + self._state_updated() def copy(self) -> Self: @@ -923,11 +992,24 @@ def copy(self) -> Self: resource_copy.load_all_state(self.serialize_all_state()) return resource_copy - def rotated(self, x: float = 0, y: float = 0, z: float = 0) -> Self: - """Return a copy of this resource rotated by the given number of degrees.""" + def rotated( + self, + x: float = 0, + y: float = 0, + z: float = 0, + pivot_coordinate: Optional[Coordinate] = None, + ) -> Self: + """Return a copy of this resource rotated by the given number of degrees. + + Args: + x: degrees to turn about X. + y: degrees to turn about Y. + z: degrees to turn about Z. + pivot_coordinate: what to turn about, as `rotate` takes it. + """ new_resource = self.copy() - new_resource.rotate(x=x, y=y, z=z) + new_resource.rotate(x=x, y=y, z=z, pivot_coordinate=pivot_coordinate) return new_resource def at(self, location: Coordinate) -> Self: diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index bf289f67f13..22f71419978 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -365,6 +365,89 @@ def test_rotate_to_sets_one_axis_normalized_and_leaves_the_others(self): expected = 30.0 if name == axis else was[name] self.assertAlmostEqual(getattr(bar.rotation, name), expected, places=9) + def test_a_pivot_inside_a_turned_parent_still_holds(self): + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate(30, 40, 0)) + parent.rotate(z=90) + far_end = Coordinate(100, 0, 0) + + def where() -> Coordinate: + carried = matrix_vector_multiply_3x3( + bar.get_absolute_rotation().get_rotation_matrix(), far_end.vector() + ) + return bar.get_absolute_location() + Coordinate(*carried) + + before = where() + bar.rotate_to(z=90, pivot_coordinate=far_end) + self.assertEqual(where(), before) + + def test_a_pivot_holds_about_every_axis(self): + for turn in ("rotate", "rotate_to"): + for axis in ("x", "y", "z"): + for angle in (30.0, 90.0, 200.0): + parent = Resource("parent", size_x=500, size_y=500, size_z=500) + parent.location = Coordinate.zero() + bar = Resource("bar", size_x=100, size_y=10, size_z=10) + parent.assign_child_resource(bar, location=Coordinate(7, 11, 13)) + joint = Coordinate(100, 5, 5) + + def where(bar=bar, joint=joint) -> Coordinate: + carried = matrix_vector_multiply_3x3( + bar.get_absolute_rotation().get_rotation_matrix(), joint.vector() + ) + return bar.get_absolute_location() + Coordinate(*carried) + + before = where() + angles = {name: (angle if name == axis else None) for name in ("x", "y", "z")} + if turn == "rotate": + angles = {name: (degrees or 0) for name, degrees in angles.items()} + getattr(bar, turn)(**angles, pivot_coordinate=joint) + + after = where() + for was, now in zip((before.x, before.y, before.z), (after.x, after.y, after.z)): + self.assertAlmostEqual(was, now, places=9) + + def test_a_pivot_turns_by_an_amount_and_goes_to_an_angle(self): + """`rotate` carries the pivot on each turn; `rotate_to` holds it where a repeat changes nothing.""" + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + plate = Resource("plate", size_x=100, size_y=50, size_z=10) + parent.assign_child_resource(plate, location=Coordinate(200, 300, 0)) + centre = Coordinate(50, 25, 0) + + plate.rotate(z=90, pivot_coordinate=centre) + self.assertEqual(plate.rotation.z, 90) + self.assertEqual(plate.location, Coordinate(275, 275, 0)) + + plate.rotate(z=90, pivot_coordinate=centre) + self.assertEqual(plate.rotation.z, 180) + # The centre is still at (250, 325, 0), so the corner the location names has swung again. + self.assertEqual(plate.location, Coordinate(300, 350, 0)) + + where = plate.location + plate.rotate_to(z=180, pivot_coordinate=centre) + self.assertEqual((plate.rotation.z, plate.location), (180, where)) + + def test_a_pivot_needs_the_resource_to_be_placed(self): + for turn in ("rotate", "rotate_to"): + loose = Resource("loose", size_x=100, size_y=10, size_z=10) + with self.assertRaises(ValueError): + getattr(loose, turn)(z=90, pivot_coordinate=Coordinate(50, 5, 5)) + + def test_rotated_carries_the_pivot_and_leaves_the_original(self): + parent = Resource("parent", size_x=500, size_y=500, size_z=10) + parent.location = Coordinate.zero() + plate = Resource("plate", size_x=100, size_y=50, size_z=10) + parent.assign_child_resource(plate, location=Coordinate(200, 300, 0)) + + turned = plate.rotated(z=90, pivot_coordinate=Coordinate(50, 25, 0)) + + self.assertEqual(turned.location, Coordinate(275, 275, 0)) + self.assertEqual(plate.location, Coordinate(200, 300, 0)) + self.assertEqual(plate.rotation.z, 0) + def test_rotation180(self): r = Resource("parent", size_x=200, size_y=100, size_z=100) r.location = Coordinate.zero() From d22f4e4f6111a4672de6f7d4967d2e548d6d72c7 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 14:25:12 +0100 Subject: [PATCH 27/34] `Resource`: type the pivot test's closure so mypy can see through it `where` took `bar` and `joint` as parameters with defaults, to bind them per iteration rather than close over variables the loop reassigns. Unannotated parameters are `Any` to mypy whatever they default to, so `bar.get_absolute_location()` came back `Any` and the closure was reported as returning `Any` where it declares `Coordinate`. Found by CI, not locally: `mypy` over the single file passes, and `make typecheck` over the package is what catches it. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 22f71419978..31ced32a6ea 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -393,7 +393,7 @@ def test_a_pivot_holds_about_every_axis(self): parent.assign_child_resource(bar, location=Coordinate(7, 11, 13)) joint = Coordinate(100, 5, 5) - def where(bar=bar, joint=joint) -> Coordinate: + def where(bar: Resource = bar, joint: Coordinate = joint) -> Coordinate: carried = matrix_vector_multiply_3x3( bar.get_absolute_rotation().get_rotation_matrix(), joint.vector() ) From a6056b756d818bf6fb9c08e73857b3ad4ac55149 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 14:47:54 +0100 Subject: [PATCH 28/34] `Resource`: let an unplaced resource turn about a pivot rather than refusing A pivot on a resource with no location raised, on the reasoning that there was nothing to move. That was wrong about what a rotation needs. The turn itself is always well defined and never needed a location; only the compensating shift does, and a resource with no location has nothing to shift and nothing that can observe the difference. Rotating an unplaced resource about its centre and about its corner leave the same state: the rotation set, the location still None. So the pivot is moot there rather than impossible, and refusing it only forced every caller to branch on `location is None` before asking for one. Both methods now turn about the origin and say nothing, which is what an unplaced resource does with or without a pivot. Placing it later overwrites `location` from `assign_child_resource` regardless, so nothing is lost by not recording a shift that could not survive. Tests: the two that asserted the raise now assert the turn happens and the location stays None. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource.py | 31 +++++++------------------- pylabrobot/resources/resource_tests.py | 8 ++++--- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index af29f6b901c..245874eaf6c 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -930,22 +930,15 @@ def rotate( z: degrees to turn about Z. pivot_coordinate: what to turn about, in this resource's own frame. Its own origin when None, which is what a resource turns about when nothing is said. Given one, `location` - carries by however far the turn moved it, so it ends where it began. - - Raises: - ValueError: If a pivot is given for a resource that is not placed, where there is no - location to carry. + carries by however far the turn moved it, so it ends where it began. A pivot is held by + moving `location`, so a resource that has none turns about its origin either way. """ - if pivot_coordinate is not None and self.location is None: - raise ValueError(f"{self.name} is not placed, so there is nothing for it to turn in") - - before = ( - self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None - ) + holding = pivot_coordinate is not None and self.location is not None + before = self.get_absolute_rotation().get_rotation_matrix() if holding else None self.rotation._prepend(Rotation(x=x, y=y, z=z)) - if pivot_coordinate is not None and before is not None: + if before is not None and pivot_coordinate is not None: self._apply_pivot_shift(before, pivot_coordinate) # Rotation is part of the resource's state; notify subscribers (e.g. the @@ -966,23 +959,15 @@ def rotate_to( y: the angle about Y to sit at, in degrees. Left where it is when None. z: the angle about Z to sit at, in degrees. Left where it is when None. pivot_coordinate: what to turn about, as `rotate` takes it. - - Raises: - ValueError: If a pivot is given for a resource that is not placed, where there is no - location to carry. """ - if pivot_coordinate is not None and self.location is None: - raise ValueError(f"{self.name} is not placed, so there is nothing for it to turn in") - - before = ( - self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None - ) + holding = pivot_coordinate is not None and self.location is not None + before = self.get_absolute_rotation().get_rotation_matrix() if holding else None self.rotation.x = self.rotation.x if x is None else x % 360 self.rotation.y = self.rotation.y if y is None else y % 360 self.rotation.z = self.rotation.z if z is None else z % 360 - if pivot_coordinate is not None and before is not None: + if before is not None and pivot_coordinate is not None: self._apply_pivot_shift(before, pivot_coordinate) self._state_updated() diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 31ced32a6ea..94091b0a06c 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -430,11 +430,13 @@ def test_a_pivot_turns_by_an_amount_and_goes_to_an_angle(self): plate.rotate_to(z=180, pivot_coordinate=centre) self.assertEqual((plate.rotation.z, plate.location), (180, where)) - def test_a_pivot_needs_the_resource_to_be_placed(self): + def test_an_unplaced_resource_turns_about_its_origin_whatever_pivot_it_is_given(self): + """A pivot is held by moving `location`, so one with none turns as it would without a pivot.""" for turn in ("rotate", "rotate_to"): loose = Resource("loose", size_x=100, size_y=10, size_z=10) - with self.assertRaises(ValueError): - getattr(loose, turn)(z=90, pivot_coordinate=Coordinate(50, 5, 5)) + getattr(loose, turn)(z=90, pivot_coordinate=Coordinate(50, 5, 5)) + self.assertEqual(loose.rotation.z, 90) + self.assertIsNone(loose.location) def test_rotated_carries_the_pivot_and_leaves_the_original(self): parent = Resource("parent", size_x=500, size_y=500, size_z=10) From 9e0a9adf43aeb964635056113782750f94f99701 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 15:33:52 +0100 Subject: [PATCH 29/34] `LinkBody`: make a link's member a cuboid and its joints coordinates inside it `Link` was a line: `size_x` was the joint-to-joint length, `size_y` and `size_z` were zero, and its origin was its proximal joint. Geometry hung off it as children placed behind that origin, which is why the iSWAP's arm sat at (-12.7, -12.75, 20.3) rather than anywhere meaningful. `LinkBody` is an ordinary resource: a cuboid with its origin at a corner, carrying both joints as coordinates within it. The link is the line between them and nothing stores it; `length` is derived as the distance, and is None on a member that ends the chain. Serialization inverts to match: the sizes come through as any resource's do, with both joints alongside, where `length` used to be emitted in their place. Because the origin is now a corner rather than a joint, a member cannot turn about its own origin. It turns about `proximal_joint`, which is what `rotate(z=..., pivot_coordinate=...)` was added for. `MechanicalGripper` is a member that ends the chain: nothing attaches past a tool, so it has no distal joint, and what sits at the far end of its span is its tool centre point. It is sized to its body alone rather than to its whole envelope, because `jaw_width` is state: a box drawn around the fingers would change size every time the jaws did. The fingers reach past it, as material on a link is free to. `body`, `body_location`, `fingers` and `pads` are unchanged. Geometry stays attached as children with their own origins, which is the separation a robot description draws between a member's frame and its visual geometry. `_place_the_fingers` straddled y=0, which was the old flange origin. It straddles `proximal_joint.y` now, so the jaws sit on the span rather than on the member's corner. `Link` is not on upstream, so nothing outside this PR can be holding the old name. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/__init__.py | 2 +- pylabrobot/resources/end_effector.py | 54 +++++++++--- pylabrobot/resources/end_effector_tests.py | 55 ++++++++++--- pylabrobot/resources/manipulator.py | 75 +++++++++++++---- pylabrobot/resources/manipulator_tests.py | 95 +++++++++++++++++++--- 5 files changed, 227 insertions(+), 54 deletions(-) diff --git a/pylabrobot/resources/__init__.py b/pylabrobot/resources/__init__.py index 3da3d237ee1..288f7e2a995 100644 --- a/pylabrobot/resources/__init__.py +++ b/pylabrobot/resources/__init__.py @@ -31,7 +31,7 @@ from .itemized_resource import ItemizedResource from .lid import Lid, Liddable from .liquid import Liquid -from .manipulator import Link +from .manipulator import LinkBody from .nest import * from .opentrons import * from .perkin_elmer import * diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index 78df5ece7e3..b0dd05d6763 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -5,29 +5,37 @@ against, stated as an offset from that flange, and it belongs to the tool rather than to the arm: fit a different one and the point moves with it. -`MechanicalGripper` spans that offset, flange to grip centre, which is why it is a `Link`. +`MechanicalGripper` spans that offset, flange to grip centre, which is why it is a `LinkBody`. """ from typing import Any, Dict, Optional, Sequence, Tuple, cast from pylabrobot.resources.coordinate import Coordinate -from pylabrobot.resources.manipulator import Link +from pylabrobot.resources.manipulator import LinkBody from pylabrobot.resources.resource import Resource from pylabrobot.resources.rotation import Rotation from pylabrobot.serializer import deserialize -class MechanicalGripper(Link): +class MechanicalGripper(LinkBody): """A gripper that holds by closing two fingers on what it takes. - A link: it spans the joint it turns on to the point it grips at, which is `tool_center_point`. - Its body, its two fingers and a pad on each are material bolted to that span. The gap between - the fingers is state rather than shape, so `jaw_width` moves them. + A member that ends the chain: nothing attaches past a tool, so it has no distal joint. What sits + at the far end of its span is `tool_center_point`, the point it grips at. Its body, its two + fingers and a pad on each are material bolted to it. + + The gap between the fingers is state rather than shape, so `jaw_width` moves them. That is why + the member is sized to its body alone: a box drawn around the fingers would change size every + time the jaws did. The fingers reach past it. """ def __init__( self, name: str, + size_x: float, + size_y: float, + size_z: float, + proximal_joint: Coordinate, tool_center_point: Coordinate, body: Resource, body_location: Coordinate, @@ -43,9 +51,13 @@ def __init__( """ Args: name: what to call this one. - tool_center_point: the joint it turns on to the point it grips at, in mm. + size_x: how far the body reaches along X, in mm. + size_y: how far it reaches along Y, in mm. + size_z: how far it reaches along Z, in mm. + proximal_joint: where the joint this gripper turns on sits within it. + tool_center_point: the point it grips at, from this gripper's own origin. body: the material around the span. - body_location: where it sits, from the joint this gripper turns on. + body_location: where it sits, from this gripper's own origin. fingers: the two jaws, either side of the span. finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s. jaw_range: the gap between the fingers, closed and open, in mm. @@ -54,7 +66,16 @@ def __init__( pad_location: where a pad sits, from the finger it is fixed to. Given with `pads`. jaw_width: the gap to begin with, in mm. Open, when not given. """ - super().__init__(name=name, length=tool_center_point.x, category=category, model=model) + super().__init__( + name=name, + size_x=size_x, + size_y=size_y, + size_z=size_z, + proximal_joint=proximal_joint, + distal_joint=None, + category=category, + model=model, + ) if len(fingers) != 2: raise ValueError(f"a gripper has two fingers, not {len(fingers)}") if (pads is None) != (pad_location is None): @@ -87,6 +108,11 @@ def tool_center_point(self) -> Coordinate: """ return self._tool_center_point + @property + def length(self) -> float: + """The joint this gripper turns on to the point it grips at, in mm.""" + return self.span_to(self._tool_center_point) + @property def jaw_width(self) -> float: """The gap between the fingers' facing surfaces, in mm: what fits between them.""" @@ -105,8 +131,8 @@ def _place_the_fingers(self) -> None: for finger, side in zip(self.fingers, (1.0, -1.0)): here = cast(Coordinate, finger.location) # A resource sits at its lowest-y corner: the facing surface on the +Y side, the back of the - # finger on the -Y side. - facing = side * self._jaw_width / 2.0 + # finger on the -Y side. They straddle the span, which is the joint's Y and not the origin's. + facing = self.proximal_joint.y + side * self._jaw_width / 2.0 finger.location = Coordinate( here.x, facing if side > 0 else facing - finger.get_size_y(), here.z ) @@ -139,6 +165,12 @@ def where(child: dict) -> Coordinate: gripper = cls( name=data["name"], + size_x=data["size_x"], + size_y=data["size_y"], + size_z=data["size_z"], + proximal_joint=cast( + Coordinate, deserialize(data["proximal_joint"], allow_marshal=allow_marshal) + ), tool_center_point=cast( Coordinate, deserialize(data["tool_center_point"], allow_marshal=allow_marshal) ), diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index ea46de04c81..57a973bf5a5 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -1,3 +1,4 @@ +import math import unittest from typing import cast @@ -5,16 +6,29 @@ from pylabrobot.resources.end_effector import MechanicalGripper from pylabrobot.resources.resource import Resource -# Measured off a Hamilton iSWAP. +# Measured off a Hamilton iSWAP. The origin is the body's corner, and the joint sits inside it. LENGTH = 137.7 -BODY_LOCATION = Coordinate(-13.0, -45.0, -1.3) -FINGER_LOCATION = Coordinate(6.5, 0.0, 4.0) +BODY_SIZE = (59.0, 90.0, 20.3) +PROXIMAL_JOINT = Coordinate(13.0, 45.0, 1.3) +BODY_LOCATION = Coordinate(0.0, 0.0, 0.0) +FINGER_LOCATION = Coordinate(19.5, 45.0, 5.3) PAD_LOCATION = Coordinate(109.0, 1.5, -17.0) JAW_RANGE = (70.844, 133.706) +def tcp(z: float = 0.0) -> Coordinate: + """The grip centre `LENGTH` along the span from the joint, and `z` above the joint.""" + return Coordinate(PROXIMAL_JOINT.x + LENGTH, PROXIMAL_JOINT.y, PROXIMAL_JOINT.z + z) + + def gripper(**overrides) -> MechanicalGripper: - body = Resource(name="demo_body", size_x=59.0, size_y=90.0, size_z=20.3, category="body") + body = Resource( + name="demo_body", + size_x=BODY_SIZE[0], + size_y=BODY_SIZE[1], + size_z=BODY_SIZE[2], + category="body", + ) fingers = [ Resource( @@ -33,7 +47,11 @@ def gripper(**overrides) -> MechanicalGripper: arguments = dict( name="demo_gripper", - tool_center_point=Coordinate(LENGTH, 0.0, 0.0), + size_x=BODY_SIZE[0], + size_y=BODY_SIZE[1], + size_z=BODY_SIZE[2], + proximal_joint=PROXIMAL_JOINT, + tool_center_point=tcp(), body=body, body_location=BODY_LOCATION, fingers=fingers, @@ -47,12 +65,24 @@ def gripper(**overrides) -> MechanicalGripper: class TestTheSpan(unittest.TestCase): def test_the_grip_centre_sits_at_the_end_of_the_span(self): - self.assertEqual(gripper().tool_center_point, Coordinate(LENGTH, 0.0, 0.0)) + g = gripper() + self.assertEqual(g.tool_center_point, tcp()) + self.assertAlmostEqual(g.length, LENGTH) def test_a_tool_can_grip_below_where_it_is_mounted(self): - g = gripper(tool_center_point=Coordinate(LENGTH, 0.0, -13.0)) - self.assertEqual(g.tool_center_point, Coordinate(LENGTH, 0.0, -13.0)) - self.assertEqual(g.get_size_x(), LENGTH) + g = gripper(tool_center_point=tcp(z=-13.0)) + self.assertEqual(g.tool_center_point, tcp(z=-13.0)) + # The span now runs diagonally, so the link is longer than its reach along X. + self.assertAlmostEqual(g.length, math.dist((LENGTH, -13.0), (0.0, 0.0))) + + def test_nothing_attaches_past_a_tool(self): + self.assertIsNone(gripper().distal_joint) + + def test_the_member_is_sized_to_its_body_not_to_its_fingers(self): + g = gripper() + self.assertEqual((g.get_size_x(), g.get_size_y(), g.get_size_z()), BODY_SIZE) + # The fingers are longer than the body they hang from, and reach past it. + self.assertGreater(g.fingers[0].get_size_x(), g.get_size_x()) class TestJaws(unittest.TestCase): @@ -66,7 +96,8 @@ def test_a_width_is_the_gap_the_fingers_leave_between_them(self): cast(Coordinate, right.location).y + right.get_size_y(), ] self.assertAlmostEqual(faces[0] - faces[1], width) - self.assertAlmostEqual(faces[0] + faces[1], 0.0) + # They straddle the span, which sits at the joint's Y rather than at the origin. + self.assertAlmostEqual(faces[0] + faces[1], 2 * PROXIMAL_JOINT.y) def test_the_jaws_refuse_a_width_they_do_not_reach(self): with self.assertRaises(ValueError): @@ -107,11 +138,13 @@ def test_a_pad_is_fixed_to_its_own_finger_where_it_was_put(self): class TestRoundTrip(unittest.TestCase): def test_a_gripper_comes_back_with_its_parts_and_its_width(self): - g = gripper(jaw_width=100.0, tool_center_point=Coordinate(LENGTH, 0.0, -13.0)) + g = gripper(jaw_width=100.0, tool_center_point=tcp(z=-13.0)) back = MechanicalGripper.deserialize(g.serialize()) back.load_all_state(g.serialize_all_state()) self.assertEqual(back.tool_center_point, g.tool_center_point) + self.assertEqual(back.proximal_joint, PROXIMAL_JOINT) + self.assertEqual((back.get_size_x(), back.get_size_y(), back.get_size_z()), BODY_SIZE) self.assertEqual(back.jaw_range, g.jaw_range) self.assertEqual(back.jaw_width, 100.0) self.assertEqual(cast(Coordinate, back.body.location), BODY_LOCATION) diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py index d7b3e426f39..dcca4657deb 100644 --- a/pylabrobot/resources/manipulator.py +++ b/pylabrobot/resources/manipulator.py @@ -1,43 +1,82 @@ -"""The moving mechanism of an arm: the links its joints turn between. +"""The moving mechanism of an arm: the bodies its joints turn between. -A manipulator is a chain of links and powered joints. A link is one rigid member of that chain and -nothing else: geometry is attached as children with their own origins - the separation a robot -description draws between a link's frame and its visual geometry - so material may extend past -either joint without entering the kinematics. +A manipulator is a chain of rigid members and powered joints. A `LinkBody` is one of those +members, an ordinary resource with its origin at a corner, carrying both of its joints as +coordinates within it. Geometry hangs off it as children with their own origins - the separation a +robot description draws between a member's frame and its visual geometry - so material may extend +past either joint without entering the kinematics. + +The link is the line between the two joints. Nothing stores it: `length` is its only measure. """ +import math from typing import Optional +from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.resource import Resource -class Link(Resource): - """One of the rigid pieces an arm is built from, joined to its neighbours by joints. +class LinkBody(Resource): + """One of the rigid members an arm is built from, joined to its neighbours by joints. + + Its origin is a corner, as any resource's is, and neither joint is obliged to sit there. It + turns about `proximal_joint`, so a caller moves one with + `rotate(z=angle, pivot_coordinate=body.proximal_joint)`. - It turns about its own origin, and unrotated it lies along +X, so the joint at its far end is at - `length`. + A member that ends the chain has no `distal_joint`, because nothing attaches past it. What sits + at the far end of its span is that subclass's own business, and so is its `length`. """ def __init__( self, name: str, - length: float, - category: str = "link", + size_x: float, + size_y: float, + size_z: float, + proximal_joint: Coordinate, + distal_joint: Optional[Coordinate] = None, + category: str = "link_body", model: Optional[str] = None, ): """ Args: name: what to call this one. - length: joint to joint, in mm. + size_x: how far the member reaches along X, in mm. + size_y: how far it reaches along Y, in mm. + size_z: how far it reaches along Z, in mm. + proximal_joint: where the joint this member turns on sits within it. + distal_joint: where the joint the next member turns on sits within it. None on a member that + ends the chain. category: what kind of resource this is. - model: which link this is. + model: which member this is. """ super().__init__( - name=name, size_x=length, size_y=0.0, size_z=0.0, category=category, model=model + name=name, size_x=size_x, size_y=size_y, size_z=size_z, category=category, model=model ) + self.proximal_joint = proximal_joint + self.distal_joint = distal_joint + + def span_to(self, point: Coordinate) -> float: + """How far `point` is from the joint this member turns on, in mm. + + Args: + point: somewhere in this member's own frame. + + Returns: + The length of a link running from this member's joint to there. + """ + return math.dist(point.vector(), self.proximal_joint.vector()) + + @property + def length(self) -> Optional[float]: + """How long the link is, joint to joint, in mm. None on a member that ends the chain.""" + if self.distal_joint is None: + return None + return self.span_to(self.distal_joint) def serialize(self) -> dict: - serialized = super().serialize() - for key in ("size_x", "size_y", "size_z"): - serialized.pop(key, None) - return {**serialized, "length": self.get_size_x()} + return { + **super().serialize(), + "proximal_joint": self.proximal_joint.serialize(), + "distal_joint": self.distal_joint.serialize() if self.distal_joint is not None else None, + } diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index eb4c0f35e32..c06f6598aaf 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -1,40 +1,109 @@ import unittest +from typing import cast from pylabrobot.resources.coordinate import Coordinate -from pylabrobot.resources.manipulator import Link +from pylabrobot.resources.manipulator import LinkBody from pylabrobot.resources.resource import Resource from pylabrobot.utils.linalg import matrix_vector_multiply_3x3 -class TestLink(unittest.TestCase): +def straight_member(name: str, length: float, inset: Coordinate) -> LinkBody: + """A member whose joints sit `inset` in from its own origin, `length` apart along X.""" + return LinkBody( + name=name, + size_x=length + 2 * inset.x, + size_y=2 * inset.y, + size_z=2 * inset.z if inset.z else 1.0, + proximal_joint=inset, + distal_joint=Coordinate(inset.x + length, inset.y, inset.z), + ) + + +class TestLinkBody(unittest.TestCase): + def test_the_link_is_the_distance_between_the_joints(self): + member = straight_member("member", length=100.0, inset=Coordinate(12.7, 12.75, 0.0)) + self.assertAlmostEqual(cast(float, member.length), 100.0) + # The member is longer than the link it carries: material overhangs both joints. + self.assertEqual(member.get_size_x(), 125.4) + + def test_a_link_runs_to_wherever_the_distal_joint_is(self): + member = LinkBody( + name="bent", + size_x=100.0, + size_y=100.0, + size_z=10.0, + proximal_joint=Coordinate(0.0, 0.0, 0.0), + distal_joint=Coordinate(30.0, 40.0, 0.0), + ) + self.assertAlmostEqual(cast(float, member.length), 50.0) + + def test_a_member_that_ends_the_chain_has_no_link(self): + member = LinkBody( + name="last", + size_x=10.0, + size_y=10.0, + size_z=10.0, + proximal_joint=Coordinate(5.0, 5.0, 0.0), + ) + self.assertIsNone(member.distal_joint) + self.assertIsNone(member.length) + def test_a_child_link_turns_on_top_of_its_parent(self): + inset = Coordinate(10.0, 5.0, 0.0) base = Resource(name="base", size_x=500, size_y=500, size_z=0) - first = Link(name="first", length=100.0) - second = Link(name="second", length=50.0) - base.assign_child_resource(first, location=Coordinate(0, 0, 0)) - first.assign_child_resource(second, location=Coordinate(first.get_size_x(), 0, 0)) + first = straight_member("first", length=100.0, inset=inset) + second = straight_member("second", length=50.0, inset=inset) + base.assign_child_resource(first, location=Coordinate(-inset.x, -inset.y, 0)) + # The second member's joint lands on the first member's far joint. + first.assign_child_resource( + second, location=Coordinate(cast(Coordinate, first.distal_joint).x - inset.x, 0, 0) + ) def far_end() -> Coordinate: carried = matrix_vector_multiply_3x3( second.get_absolute_rotation().get_rotation_matrix(), - Coordinate(second.get_size_x(), 0, 0).vector(), + cast(Coordinate, second.distal_joint).vector(), ) return second.get_absolute_location() + Coordinate(*carried) self.assertEqual(far_end(), Coordinate(150, 0, 0)) - second.rotate_to(z=90) + # A member turns on its own joint, which is not its origin, so the pivot is not optional. + second.rotate_to(z=90, pivot_coordinate=second.proximal_joint) self.assertEqual(far_end(), Coordinate(100, 50, 0)) - first.rotate_to(z=90) + first.rotate_to(z=90, pivot_coordinate=first.proximal_joint) self.assertEqual(far_end(), Coordinate(-50, 100, 0)) - def test_a_link_comes_back_the_length_it_went_in(self): - link = Link(name="link", length=137.7, model="demo") - back = Link.deserialize(link.serialize()) - self.assertEqual(back.get_size_x(), 137.7) + def test_a_member_comes_back_with_its_shape_and_both_joints(self): + member = LinkBody( + name="member", + size_x=163.4, + size_y=25.5, + size_z=15.3, + proximal_joint=Coordinate(12.7, 12.75, -20.3), + distal_joint=Coordinate(150.4, 12.75, -20.3), + model="demo", + ) + back = LinkBody.deserialize(member.serialize()) + + self.assertEqual((back.get_size_x(), back.get_size_y(), back.get_size_z()), (163.4, 25.5, 15.3)) + self.assertEqual(back.proximal_joint, member.proximal_joint) + self.assertEqual(back.distal_joint, member.distal_joint) + self.assertAlmostEqual(cast(float, back.length), cast(float, member.length)) self.assertEqual(back.model, "demo") + def test_a_member_that_ends_the_chain_comes_back_without_one(self): + member = LinkBody( + name="last", + size_x=10.0, + size_y=10.0, + size_z=10.0, + proximal_joint=Coordinate(5.0, 5.0, 0.0), + ) + back = LinkBody.deserialize(member.serialize()) + self.assertIsNone(back.distal_joint) + if __name__ == "__main__": unittest.main() From ccbee82187e023292879b0e29ccd1cc5672d69e9 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 16:45:43 +0100 Subject: [PATCH 30/34] `Resource`: hold a pivot where the location chain actually starts Three defects in the pivot, two of them geometric. `_apply_pivot_shift` converted the shift into the parent's frame whenever a parent existed, but `get_absolute_location` stops walking the location chain at the first ancestor with no location while still taking rotation from the whole tree. Where the immediate parent has no location and an ancestor is rotated, `location` is already read in absolute axes, so the conversion was an extra rotation with no counterpart. A plate under an unlocated parent beneath a grandparent turned 30 degrees saw its pivot move 40.85 mm. The guard now also requires the parent to have a location, which takes that to the 1e-4 mm floor `Coordinate` rounds to. A pivot on a resource with no location raised until a6056b756, which removed the guard on the reasoning that the request was moot because nothing could observe the difference. That is only true of a resource with no parent and no children. With either, the rotation still reaches the subtree, so three different pivots produced one identical answer and moved geometry silently. Both entry points raise `NoLocationError` again, which is what `get_absolute_location` raises for the same condition. A pivoted turn wrote `location` through its setter and then fired `_state_updated` itself, so subscribers saw two events carrying the same final state. The shift goes onto the field directly and the one event at the end of the turn covers it. Tests: a pivot under an unlocated parent that a rotated grandparent carries, which nothing covered; and the every-axis test now asserts the turn reached the angle it asked for, since holding the pivot still is satisfied by an implementation that rotates nothing. The two axis loops no longer select a method by name, which is not allowed here. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/resource.py | 36 ++++++++++++---- pylabrobot/resources/resource_tests.py | 57 +++++++++++++++++++++----- 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index 245874eaf6c..09d8ff158d0 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -904,16 +904,20 @@ def _apply_pivot_shift(self, before: List[List[float]], pivot_coordinate: Coordi now = matrix_vector_multiply_3x3(after, pivot_coordinate.vector()) shift = Coordinate(was[0] - now[0], was[1] - now[1], was[2] - now[2]) # `shift` is in this resource's frame, `location` in the parent's. A rotation matrix - # inverts by transposing. + # inverts by transposing. A parent with no location of its own is where + # `get_absolute_location` stops walking, so `location` is read in absolute axes from there + # and needs no conversion. parent = self.parent - if parent is not None: + if parent is not None and parent.location is not None: turned = parent.get_absolute_rotation().get_rotation_matrix() shift = Coordinate( *matrix_vector_multiply_3x3( [[turned[j][i] for j in range(3)] for i in range(3)], shift.vector() ) ) - self.location = cast(Coordinate, self.location) + shift + # Straight onto the field: the caller fires one `_state_updated` for the whole turn, and + # going through the setter would fire a second carrying the same final state. + self._location = cast(Coordinate, self.location) + shift def rotate( self, @@ -930,11 +934,18 @@ def rotate( z: degrees to turn about Z. pivot_coordinate: what to turn about, in this resource's own frame. Its own origin when None, which is what a resource turns about when nothing is said. Given one, `location` - carries by however far the turn moved it, so it ends where it began. A pivot is held by - moving `location`, so a resource that has none turns about its origin either way. + carries by however far the turn moved it, so it ends where it began. + + Raises: + NoLocationError: If a pivot is given for a resource with no location. A pivot is held by + moving `location`, so there is nothing to hold it with. """ - holding = pivot_coordinate is not None and self.location is not None - before = self.get_absolute_rotation().get_rotation_matrix() if holding else None + if pivot_coordinate is not None and self.location is None: + raise NoLocationError(f"Resource '{self.name}' has no location, so a pivot cannot be held.") + + before = ( + self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None + ) self.rotation._prepend(Rotation(x=x, y=y, z=z)) @@ -959,9 +970,16 @@ def rotate_to( y: the angle about Y to sit at, in degrees. Left where it is when None. z: the angle about Z to sit at, in degrees. Left where it is when None. pivot_coordinate: what to turn about, as `rotate` takes it. + + Raises: + NoLocationError: If a pivot is given for a resource with no location, as `rotate` raises. """ - holding = pivot_coordinate is not None and self.location is not None - before = self.get_absolute_rotation().get_rotation_matrix() if holding else None + if pivot_coordinate is not None and self.location is None: + raise NoLocationError(f"Resource '{self.name}' has no location, so a pivot cannot be held.") + + before = ( + self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None + ) self.rotation.x = self.rotation.x if x is None else x % 360 self.rotation.y = self.rotation.y if y is None else y % 360 diff --git a/pylabrobot/resources/resource_tests.py b/pylabrobot/resources/resource_tests.py index 94091b0a06c..6caaefe1d0d 100644 --- a/pylabrobot/resources/resource_tests.py +++ b/pylabrobot/resources/resource_tests.py @@ -19,7 +19,7 @@ from pylabrobot.resources.barcode import Barcode from pylabrobot.resources.coordinate import Coordinate from pylabrobot.resources.deck import Deck -from pylabrobot.resources.errors import ResourceNotFoundError +from pylabrobot.resources.errors import NoLocationError, ResourceNotFoundError from pylabrobot.resources.plate_adapter import PlateAdapter from pylabrobot.resources.resource import Resource from pylabrobot.resources.rotation import Rotation @@ -400,14 +400,21 @@ def where(bar: Resource = bar, joint: Coordinate = joint) -> Coordinate: return bar.get_absolute_location() + Coordinate(*carried) before = where() - angles = {name: (angle if name == axis else None) for name in ("x", "y", "z")} + x = angle if axis == "x" else None + y = angle if axis == "y" else None + z = angle if axis == "z" else None if turn == "rotate": - angles = {name: (degrees or 0) for name, degrees in angles.items()} - getattr(bar, turn)(**angles, pivot_coordinate=joint) + bar.rotate(x=x or 0.0, y=y or 0.0, z=z or 0.0, pivot_coordinate=joint) + else: + bar.rotate_to(x=x, y=y, z=z, pivot_coordinate=joint) after = where() for was, now in zip((before.x, before.y, before.z), (after.x, after.y, after.z)): self.assertAlmostEqual(was, now, places=9) + # Holding the pivot still is not enough: the turn has to have happened. + reached = (bar.rotation.x, bar.rotation.y, bar.rotation.z) + self.assertAlmostEqual(reached["xyz".index(axis)], angle % 360, places=9) + self.assertEqual(sum(1 for turned in reached if turned != 0), 1) def test_a_pivot_turns_by_an_amount_and_goes_to_an_angle(self): """`rotate` carries the pivot on each turn; `rotate_to` holds it where a repeat changes nothing.""" @@ -430,13 +437,41 @@ def test_a_pivot_turns_by_an_amount_and_goes_to_an_angle(self): plate.rotate_to(z=180, pivot_coordinate=centre) self.assertEqual((plate.rotation.z, plate.location), (180, where)) - def test_an_unplaced_resource_turns_about_its_origin_whatever_pivot_it_is_given(self): - """A pivot is held by moving `location`, so one with none turns as it would without a pivot.""" - for turn in ("rotate", "rotate_to"): - loose = Resource("loose", size_x=100, size_y=10, size_z=10) - getattr(loose, turn)(z=90, pivot_coordinate=Coordinate(50, 5, 5)) - self.assertEqual(loose.rotation.z, 90) - self.assertIsNone(loose.location) + def test_a_pivot_needs_a_location_to_be_held_by(self): + """A pivot is held by moving `location`, so one with none cannot honour the request.""" + joint = Coordinate(50, 5, 5) + with self.assertRaises(NoLocationError): + Resource("loose", size_x=100, size_y=10, size_z=10).rotate(z=90, pivot_coordinate=joint) + with self.assertRaises(NoLocationError): + Resource("loose", size_x=100, size_y=10, size_z=10).rotate_to(z=90, pivot_coordinate=joint) + + # Without a pivot an unplaced resource turns about its origin, as it always has. + loose = Resource("loose", size_x=100, size_y=10, size_z=10) + loose.rotate(z=90) + self.assertEqual(loose.rotation.z, 90) + + def test_a_pivot_holds_under_an_unlocated_parent_that_a_rotated_ancestor_carries(self): + """`location` is read in absolute axes from where the location chain stops, not in a parent's.""" + grandparent = Resource("grandparent", size_x=500, size_y=500, size_z=10) + grandparent.location = Coordinate.zero() + grandparent.rotate(z=30) + parent = Resource("parent", size_x=300, size_y=300, size_z=10) + grandparent.assign_child_resource(parent, location=None) + plate = Resource("plate", size_x=100, size_y=50, size_z=10) + parent.assign_child_resource(plate, location=Coordinate(200, 300, 0)) + centre = Coordinate(50, 25, 0) + + def where() -> Coordinate: + carried = matrix_vector_multiply_3x3( + plate.get_absolute_rotation().get_rotation_matrix(), centre.vector() + ) + return plate.get_absolute_location() + Coordinate(*carried) + + before = where() + plate.rotate(z=90, pivot_coordinate=centre) + after = where() + for was, now in zip((before.x, before.y, before.z), (after.x, after.y, after.z)): + self.assertAlmostEqual(was, now, places=3) def test_rotated_carries_the_pivot_and_leaves_the_original(self): parent = Resource("parent", size_x=500, size_y=500, size_z=10) From ff2f4c7b51b750190e290fd384db336285650d8a Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 16:45:43 +0100 Subject: [PATCH 31/34] `MechanicalGripper`: close the jaws on the grip centre, and state the body once The jaws straddled `proximal_joint.y`, the point the tool is bolted on by. They close on what is at the grip centre, which is the tool centre point, and since that became a free `Coordinate` nothing tied the two together. A tool whose centre sits 25 mm off its mounting in y stood its fingers over the mounting instead, leaving an object centred on the programmed grip point overlapping one finger. The iSWAP is unaffected, where the two coincide. The envelope was stated twice, as `size_x/y/z` on the member and again as the body child's own box, with nothing keeping them in step. A tool is sized to its body, because `jaw_width` moves the fingers and a box drawn around them would resize with the jaws, so the body is now the only place that box is given. `LinkBody` emits `distal_joint` for every member, but nothing attaches past a tool: the key had nothing to say and `__init__` nowhere to put it. It is dropped from a gripper's payload. Tests: the fixture's joint moves off the body's own middle, which were both 45.0 and therefore indistinguishable, so the straddle can no longer be satisfied by reading the wrong one. Added: the jaws following a grip centre that is off the mounting, the reach and height surviving a width change, the body and the member reporting one box, and the absent joint key. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 35 ++++++++-------- pylabrobot/resources/end_effector_tests.py | 49 +++++++++++++++++----- 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index b0dd05d6763..c4a46fd76ec 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -32,9 +32,6 @@ class MechanicalGripper(LinkBody): def __init__( self, name: str, - size_x: float, - size_y: float, - size_z: float, proximal_joint: Coordinate, tool_center_point: Coordinate, body: Resource, @@ -51,15 +48,13 @@ def __init__( """ Args: name: what to call this one. - size_x: how far the body reaches along X, in mm. - size_y: how far it reaches along Y, in mm. - size_z: how far it reaches along Z, in mm. proximal_joint: where the joint this gripper turns on sits within it. tool_center_point: the point it grips at, from this gripper's own origin. - body: the material around the span. + body: the material around the span, which is also what sizes this member. body_location: where it sits, from this gripper's own origin. fingers: the two jaws, either side of the span. - finger_location: where a finger sits along and above the span. Its Y is `jaw_width`'s. + finger_location: where a finger sits along and above the span. Its Y is not used: the + jaws straddle `tool_center_point`, and `jaw_width` sets how far apart. jaw_range: the gap between the fingers, closed and open, in mm. pads: what each finger meets the resource with, in the same order as `fingers`. A gripper whose fingers meet it themselves has none. @@ -68,9 +63,11 @@ def __init__( """ super().__init__( name=name, - size_x=size_x, - size_y=size_y, - size_z=size_z, + # A tool is sized to its body, since the fingers move and a box around them would resize + # with the jaws. The body states that box, so it is not asked for a second time. + size_x=body.get_size_x(), + size_y=body.get_size_y(), + size_z=body.get_size_z(), proximal_joint=proximal_joint, distal_joint=None, category=category, @@ -127,19 +124,24 @@ def jaw_width(self, width: float) -> None: self._place_the_fingers() def _place_the_fingers(self) -> None: - """Stand the fingers either side of the span, leaving `jaw_width` of gap between them.""" + """Stand the fingers either side of the grip centre, leaving `jaw_width` of gap between them.""" for finger, side in zip(self.fingers, (1.0, -1.0)): here = cast(Coordinate, finger.location) # A resource sits at its lowest-y corner: the facing surface on the +Y side, the back of the - # finger on the -Y side. They straddle the span, which is the joint's Y and not the origin's. - facing = self.proximal_joint.y + side * self._jaw_width / 2.0 + # finger on the -Y side. They close on what is at the grip centre, so they straddle the tool + # centre point rather than the joint or the member's own middle. + facing = self._tool_center_point.y + side * self._jaw_width / 2.0 finger.location = Coordinate( here.x, facing if side > 0 else facing - finger.get_size_y(), here.z ) def serialize(self) -> dict: + serialized = super().serialize() + # Nothing attaches past a tool, so the key its base emits has nothing to say and + # `__init__` has nowhere to put it. + serialized.pop("distal_joint", None) return { - **super().serialize(), + **serialized, "jaw_range": list(self.jaw_range), "tool_center_point": self.tool_center_point.serialize(), } @@ -165,9 +167,6 @@ def where(child: dict) -> Coordinate: gripper = cls( name=data["name"], - size_x=data["size_x"], - size_y=data["size_y"], - size_z=data["size_z"], proximal_joint=cast( Coordinate, deserialize(data["proximal_joint"], allow_marshal=allow_marshal) ), diff --git a/pylabrobot/resources/end_effector_tests.py b/pylabrobot/resources/end_effector_tests.py index 57a973bf5a5..acc65014441 100644 --- a/pylabrobot/resources/end_effector_tests.py +++ b/pylabrobot/resources/end_effector_tests.py @@ -9,16 +9,18 @@ # Measured off a Hamilton iSWAP. The origin is the body's corner, and the joint sits inside it. LENGTH = 137.7 BODY_SIZE = (59.0, 90.0, 20.3) -PROXIMAL_JOINT = Coordinate(13.0, 45.0, 1.3) +# The joint deliberately sits off the body's own middle, 45.0, so that a test cannot pass by +# reading one when it means the other. +PROXIMAL_JOINT = Coordinate(13.0, 38.0, 1.3) BODY_LOCATION = Coordinate(0.0, 0.0, 0.0) -FINGER_LOCATION = Coordinate(19.5, 45.0, 5.3) +FINGER_LOCATION = Coordinate(19.5, 38.0, 5.3) PAD_LOCATION = Coordinate(109.0, 1.5, -17.0) JAW_RANGE = (70.844, 133.706) -def tcp(z: float = 0.0) -> Coordinate: - """The grip centre `LENGTH` along the span from the joint, and `z` above the joint.""" - return Coordinate(PROXIMAL_JOINT.x + LENGTH, PROXIMAL_JOINT.y, PROXIMAL_JOINT.z + z) +def tcp(z: float = 0.0, y: float = 0.0) -> Coordinate: + """The grip centre `LENGTH` along the span from the joint, and `y`/`z` off it.""" + return Coordinate(PROXIMAL_JOINT.x + LENGTH, PROXIMAL_JOINT.y + y, PROXIMAL_JOINT.z + z) def gripper(**overrides) -> MechanicalGripper: @@ -47,9 +49,6 @@ def gripper(**overrides) -> MechanicalGripper: arguments = dict( name="demo_gripper", - size_x=BODY_SIZE[0], - size_y=BODY_SIZE[1], - size_z=BODY_SIZE[2], proximal_joint=PROXIMAL_JOINT, tool_center_point=tcp(), body=body, @@ -81,9 +80,17 @@ def test_nothing_attaches_past_a_tool(self): def test_the_member_is_sized_to_its_body_not_to_its_fingers(self): g = gripper() self.assertEqual((g.get_size_x(), g.get_size_y(), g.get_size_z()), BODY_SIZE) + # The body states that box once: the member is not told its own size a second time. + self.assertEqual( + (g.body.get_size_x(), g.body.get_size_y(), g.body.get_size_z()), + (g.get_size_x(), g.get_size_y(), g.get_size_z()), + ) # The fingers are longer than the body they hang from, and reach past it. self.assertGreater(g.fingers[0].get_size_x(), g.get_size_x()) + def test_nothing_past_a_tool_means_no_joint_in_its_payload(self): + self.assertNotIn("distal_joint", gripper().serialize()) + class TestJaws(unittest.TestCase): def test_a_width_is_the_gap_the_fingers_leave_between_them(self): @@ -96,8 +103,9 @@ def test_a_width_is_the_gap_the_fingers_leave_between_them(self): cast(Coordinate, right.location).y + right.get_size_y(), ] self.assertAlmostEqual(faces[0] - faces[1], width) - # They straddle the span, which sits at the joint's Y rather than at the origin. - self.assertAlmostEqual(faces[0] + faces[1], 2 * PROXIMAL_JOINT.y) + # They straddle the grip centre, which is neither the member's middle nor its origin. + self.assertAlmostEqual(faces[0] + faces[1], 2 * tcp().y) + self.assertNotAlmostEqual(tcp().y, g.get_size_y() / 2.0) def test_the_jaws_refuse_a_width_they_do_not_reach(self): with self.assertRaises(ValueError): @@ -108,6 +116,27 @@ def test_the_jaws_refuse_a_width_they_do_not_reach(self): g.jaw_width = 5.0 self.assertEqual(g.jaw_width, before) + def test_the_jaws_close_on_the_grip_centre_not_on_the_joint(self): + """A tool that grips off to one side stands its fingers there, not over its own mounting.""" + g = gripper(tool_center_point=tcp(y=-18.0)) + left, right = g.fingers + faces = [ + cast(Coordinate, left.location).y, + cast(Coordinate, right.location).y + right.get_size_y(), + ] + self.assertAlmostEqual(faces[0] + faces[1], 2 * (PROXIMAL_JOINT.y - 18.0)) + self.assertAlmostEqual(faces[0] - faces[1], g.jaw_width) + + def test_the_jaws_keep_the_reach_and_height_they_were_given(self): + """`jaw_width` moves the fingers across the span and must leave X and Z alone.""" + g = gripper() + for width in (133.706, 100.0, 70.844): + g.jaw_width = width + for finger in g.fingers: + here = cast(Coordinate, finger.location) + self.assertAlmostEqual(here.x, FINGER_LOCATION.x) + self.assertAlmostEqual(here.z, FINGER_LOCATION.z) + def test_a_gripper_starts_open_unless_told_otherwise(self): self.assertEqual(gripper().jaw_width, JAW_RANGE[1]) self.assertEqual(gripper(jaw_width=100.0).jaw_width, 100.0) From a6c5b55ceeceb4eecc87cfd5df692d1717cec55b Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 16:45:54 +0100 Subject: [PATCH 32/34] `LinkBody`: test a member and the tool it carries as one arm A member and an end-effector are built in separate modules and every test exercised them apart: `manipulator_tests` never imported `end_effector`, nor the reverse, and `get_absolute_location` was never called on a gripper or a finger. The two halves were only ever correct independently. That is the gap a sibling branch shipped through. A gripper sat visibly in the wrong place in the viewer while the whole suite stayed green, because the placement of an assembled arm was asserted nowhere. `TestAnAssembledArm` puts a member on a deck, mounts a gripper on its far joint, and pins absolute positions at rest, after turning the member, and after turning the tool on its own joint. It reads the wrist off each side of the joint independently and asserts the two agree, which is what mounting means and what nothing was checking. Every expected value is derived from the fixture rather than read back from the code. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/manipulator_tests.py | 86 ++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/pylabrobot/resources/manipulator_tests.py b/pylabrobot/resources/manipulator_tests.py index c06f6598aaf..036359d7d3f 100644 --- a/pylabrobot/resources/manipulator_tests.py +++ b/pylabrobot/resources/manipulator_tests.py @@ -1,12 +1,21 @@ import unittest -from typing import cast +from typing import Tuple, cast from pylabrobot.resources.coordinate import Coordinate +from pylabrobot.resources.end_effector_tests import gripper as demo_gripper from pylabrobot.resources.manipulator import LinkBody from pylabrobot.resources.resource import Resource from pylabrobot.utils.linalg import matrix_vector_multiply_3x3 +def absolute(resource: Resource, point: Coordinate) -> Coordinate: + """Where `point` in `resource`'s own frame sits on the deck, in mm.""" + carried = matrix_vector_multiply_3x3( + resource.get_absolute_rotation().get_rotation_matrix(), point.vector() + ) + return resource.get_absolute_location() + Coordinate(*carried) + + def straight_member(name: str, length: float, inset: Coordinate) -> LinkBody: """A member whose joints sit `inset` in from its own origin, `length` apart along X.""" return LinkBody( @@ -105,5 +114,80 @@ def test_a_member_that_ends_the_chain_comes_back_without_one(self): self.assertIsNone(back.distal_joint) +class TestAnAssembledArm(unittest.TestCase): + """A member and the tool it carries, placed on a deck and turned joint by joint. + + The two are built in separate modules and every other test exercises them apart, so this is + the only place the whole chain's absolute geometry is pinned. + """ + + def arm(self): + deck = Resource("deck", size_x=1000, size_y=1000, size_z=10) + deck.location = Coordinate.zero() + forearm = LinkBody( + name="forearm", + size_x=220.0, + size_y=40.0, + size_z=20.0, + proximal_joint=Coordinate(10.0, 20.0, 10.0), + distal_joint=Coordinate(210.0, 20.0, 10.0), + ) + deck.assign_child_resource(forearm, location=Coordinate(100.0, 100.0, 0.0)) + + hand = demo_gripper() + # The tool's own joint lands on the member's far joint. That is what mounting means. + forearm.assign_child_resource( + hand, location=cast(Coordinate, forearm.distal_joint) - hand.proximal_joint + ) + return forearm, hand + + def wrist(self, forearm: LinkBody, hand) -> Tuple[Coordinate, Coordinate]: + """Where the wrist is, read off each side of the joint independently.""" + return ( + absolute(forearm, cast(Coordinate, forearm.distal_joint)), + absolute(hand, hand.proximal_joint), + ) + + def test_the_tool_hangs_where_the_member_ends(self): + forearm, hand = self.arm() + from_member, from_tool = self.wrist(forearm, hand) + self.assertEqual(from_member, Coordinate(310, 120, 10)) + self.assertEqual(from_member, from_tool) + self.assertEqual(absolute(hand, hand.tool_center_point), Coordinate(447.7, 120, 10)) + + def test_the_tool_rides_the_member_it_is_mounted_on(self): + forearm, hand = self.arm() + forearm.rotate_to(z=90, pivot_coordinate=forearm.proximal_joint) + + from_member, from_tool = self.wrist(forearm, hand) + self.assertEqual(from_member, Coordinate(110, 320, 10)) + self.assertEqual(from_member, from_tool) + # The tool did not turn on its own joint, so it swung round with the member carrying it. + self.assertEqual(absolute(hand, hand.tool_center_point), Coordinate(110, 457.7, 10)) + + def test_the_tool_also_turns_on_its_own_joint(self): + forearm, hand = self.arm() + forearm.rotate_to(z=90, pivot_coordinate=forearm.proximal_joint) + hand.rotate_to(z=90, pivot_coordinate=hand.proximal_joint) + + from_member, from_tool = self.wrist(forearm, hand) + # The wrist is the fixed point of the tool's own turn, so it has not moved. + self.assertEqual(from_member, Coordinate(110, 320, 10)) + self.assertEqual(from_member, from_tool) + # Two right angles, so the grip centre now points back the way the member came. + self.assertEqual(absolute(hand, hand.tool_center_point), Coordinate(-27.7, 320, 10)) + + def test_the_fingers_travel_with_the_tool(self): + forearm, hand = self.arm() + at_rest = [absolute(finger, Coordinate.zero()) for finger in hand.fingers] + self.assertEqual(at_rest[0], Coordinate(316.5, 186.853, 14)) + self.assertEqual(at_rest[1], Coordinate(316.5, 46.147, 14)) + + forearm.rotate_to(z=90, pivot_coordinate=forearm.proximal_joint) + turned = [absolute(finger, Coordinate.zero()) for finger in hand.fingers] + self.assertEqual(turned[0], Coordinate(43.147, 326.5, 14)) + self.assertEqual(turned[1], Coordinate(183.853, 326.5, 14)) + + if __name__ == "__main__": unittest.main() From 1116b99cae82b51d8039af42beb76e4fa1db6220 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 16:45:54 +0100 Subject: [PATCH 33/34] docs: list the manipulator primitives in the changelog and the API index `LinkBody` and `MechanicalGripper` are exported from `pylabrobot.resources` but were in neither the hand-maintained autosummary in `docs/api/pylabrobot.resources.rst`, where every sibling class is listed, nor the changelog, which also had no entry for `Resource.rotate_to` or for the pivot that `rotate`, `rotate_to` and `rotated` now take. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 ++++ docs/api/pylabrobot.resources.rst | 2 ++ 2 files changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 870ab59605e..499b9b93464 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - User guide notebook for the MicroSpin (`docs/user_guide/01_material-handling/centrifuge/highres_microspin.ipynb`). - `Plate`: optional `stacking_z_height` parameter -- the per-plate vertical pitch when plates are stacked directly on top of each other (`size_z` minus the nesting overlap), mirroring `NestedTipRack.stacking_z_height`. Because it is a physical dimension, plates that differ in it no longer compare equal; `Plate` also now serializes `stacking_z_height` and the pre-existing `plate_type` so both round-trip through `deserialize`/`copy`. (#1110) - `ResourceStack`: bare plates stacked in the z direction now nest into one another by their `stacking_z_height` (a stack of `N` identical plates is `size_z + (N - 1) * stacking_z_height` tall, for both `get_size_z()` and child placement). Plates without a `stacking_z_height`, and plates wearing a lid, do not nest, so existing behaviour is unchanged. (#1112) +- `Resource.rotate_to(x=, y=, z=)`: set the rotation about each axis, where `rotate` turns by an amount. Axes left as `None` keep the angle they had, and each is normalised to `[0, 360)`. (#1249) +- `Resource.rotate`, `rotate_to` and `rotated` take an optional `pivot_coordinate`: a point in the resource's own frame that stays where it is, so a resource can turn about its centre, an edge, or any other point rather than only about its origin. `location` carries by however far the turn moved that point. Raises `NoLocationError` when the resource has no location, since there is nothing to carry. (#1249) +- `LinkBody` (`pylabrobot.resources.LinkBody`): one rigid member of a manipulator, an ordinary resource whose origin is a corner and which carries its `proximal_joint` and `distal_joint` as coordinates within it. The link is the line between the two joints and `length` is the distance, `None` on a member that ends the chain. A member turns about its proximal joint rather than its origin. (#1249) +- `MechanicalGripper` (`pylabrobot.resources.MechanicalGripper`): a `LinkBody` that ends the chain, holding what it takes between two fingers. Its far end is a `tool_center_point` rather than a joint, it is sized to its body because `jaw_width` moves the fingers, and the jaws straddle the grip centre. (#1249) ### Fixed diff --git a/docs/api/pylabrobot.resources.rst b/docs/api/pylabrobot.resources.rst index ccd5d0a3ee0..6ba5916a850 100644 --- a/docs/api/pylabrobot.resources.rst +++ b/docs/api/pylabrobot.resources.rst @@ -18,7 +18,9 @@ Resources represent on-deck liquid handling equipment, including tip racks, plat ItemizedResource utils.create_equally_spaced_2d Lid + LinkBody Liquid + MechanicalGripper PetriDish Plate PlateCarrier From 8586c24930b1112a1dfb1f8e79eefc2695e20313 Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Fri, 11 Sep 2026 18:08:57 +0100 Subject: [PATCH 34/34] `Resource`: decide once whether a turn is pivoted `rotate` and `rotate_to` each carried the same six lines: refuse a pivot on a resource with no location, then take the rotation matrix if a pivot was given. `_pivot_reference` holds that now and returns the matrix or None, so each public method reads as what it does rather than as preamble, and the two cannot drift apart. The check after the turn was `before is not None and pivot_coordinate is not None`, two conditions for one question, because neither narrowed the other for the type checker. One condition and one cast says it instead. `span_to` was public with two callers inside this module, each a single line. Both measure from the joint to the far end of their own span, so they say so directly and the method is gone. Co-Authored-By: Claude Opus 5 (1M context) --- pylabrobot/resources/end_effector.py | 3 +- pylabrobot/resources/manipulator.py | 13 +-------- pylabrobot/resources/resource.py | 41 +++++++++++++++++----------- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/pylabrobot/resources/end_effector.py b/pylabrobot/resources/end_effector.py index c4a46fd76ec..74ff74d0c88 100644 --- a/pylabrobot/resources/end_effector.py +++ b/pylabrobot/resources/end_effector.py @@ -8,6 +8,7 @@ `MechanicalGripper` spans that offset, flange to grip centre, which is why it is a `LinkBody`. """ +import math from typing import Any, Dict, Optional, Sequence, Tuple, cast from pylabrobot.resources.coordinate import Coordinate @@ -108,7 +109,7 @@ def tool_center_point(self) -> Coordinate: @property def length(self) -> float: """The joint this gripper turns on to the point it grips at, in mm.""" - return self.span_to(self._tool_center_point) + return math.dist(self._tool_center_point.vector(), self.proximal_joint.vector()) @property def jaw_width(self) -> float: diff --git a/pylabrobot/resources/manipulator.py b/pylabrobot/resources/manipulator.py index dcca4657deb..45a1175a43a 100644 --- a/pylabrobot/resources/manipulator.py +++ b/pylabrobot/resources/manipulator.py @@ -56,23 +56,12 @@ def __init__( self.proximal_joint = proximal_joint self.distal_joint = distal_joint - def span_to(self, point: Coordinate) -> float: - """How far `point` is from the joint this member turns on, in mm. - - Args: - point: somewhere in this member's own frame. - - Returns: - The length of a link running from this member's joint to there. - """ - return math.dist(point.vector(), self.proximal_joint.vector()) - @property def length(self) -> Optional[float]: """How long the link is, joint to joint, in mm. None on a member that ends the chain.""" if self.distal_joint is None: return None - return self.span_to(self.distal_joint) + return math.dist(self.distal_joint.vector(), self.proximal_joint.vector()) def serialize(self) -> dict: return { diff --git a/pylabrobot/resources/resource.py b/pylabrobot/resources/resource.py index 09d8ff158d0..e10f967461e 100644 --- a/pylabrobot/resources/resource.py +++ b/pylabrobot/resources/resource.py @@ -919,6 +919,25 @@ def _apply_pivot_shift(self, before: List[List[float]], pivot_coordinate: Coordi # going through the setter would fire a second carrying the same final state. self._location = cast(Coordinate, self.location) + shift + def _pivot_reference(self, pivot_coordinate: Optional[Coordinate]) -> Optional[List[List[float]]]: + """The rotation to measure a pivoted turn against, or None when no pivot was asked for. + + Args: + pivot_coordinate: what the caller wants held still, if anything. + + Returns: + This resource's absolute rotation matrix, to hand back after the turn. + + Raises: + NoLocationError: If a pivot is given for a resource with no location. A pivot is held by + moving `location`, so there is nothing to hold it with. + """ + if pivot_coordinate is None: + return None + if self.location is None: + raise NoLocationError(f"Resource '{self.name}' has no location, so a pivot cannot be held.") + return self.get_absolute_rotation().get_rotation_matrix() + def rotate( self, x: float = 0, @@ -940,17 +959,12 @@ def rotate( NoLocationError: If a pivot is given for a resource with no location. A pivot is held by moving `location`, so there is nothing to hold it with. """ - if pivot_coordinate is not None and self.location is None: - raise NoLocationError(f"Resource '{self.name}' has no location, so a pivot cannot be held.") - - before = ( - self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None - ) + before = self._pivot_reference(pivot_coordinate) self.rotation._prepend(Rotation(x=x, y=y, z=z)) - if before is not None and pivot_coordinate is not None: - self._apply_pivot_shift(before, pivot_coordinate) + if before is not None: + self._apply_pivot_shift(before, cast(Coordinate, pivot_coordinate)) # Rotation is part of the resource's state; notify subscribers (e.g. the # Visualizer) so they can re-render. @@ -974,19 +988,14 @@ def rotate_to( Raises: NoLocationError: If a pivot is given for a resource with no location, as `rotate` raises. """ - if pivot_coordinate is not None and self.location is None: - raise NoLocationError(f"Resource '{self.name}' has no location, so a pivot cannot be held.") - - before = ( - self.get_absolute_rotation().get_rotation_matrix() if pivot_coordinate is not None else None - ) + before = self._pivot_reference(pivot_coordinate) self.rotation.x = self.rotation.x if x is None else x % 360 self.rotation.y = self.rotation.y if y is None else y % 360 self.rotation.z = self.rotation.z if z is None else z % 360 - if before is not None and pivot_coordinate is not None: - self._apply_pivot_shift(before, pivot_coordinate) + if before is not None: + self._apply_pivot_shift(before, cast(Coordinate, pivot_coordinate)) self._state_updated()