From a83b0e1487a5618f1aff7b1d39b7ee356694c4bc Mon Sep 17 00:00:00 2001 From: Camillo Moschner Date: Wed, 9 Sep 2026 23:50:14 +0100 Subject: [PATCH 01/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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/19] `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()