Skip to content

Resource: turn about a joint, and the links and end-effectors that need it - #1249

Open
BioCam wants to merge 35 commits into
PyLabRobot:mainfrom
BioCam:resources-manipulator-primitives
Open

Resource: turn about a joint, and the links and end-effectors that need it#1249
BioCam wants to merge 35 commits into
PyLabRobot:mainfrom
BioCam:resources-manipulator-primitives

Conversation

@BioCam

@BioCam BioCam commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

An articulated device has joints that are not at a resource's own corner, and nothing in PyLabRobot can express one. A carousel, a rotor, a hinge, every link of an arm - all pivot on a point somewhere on the part, and Resource.rotate has always turned about the left front bottom corner, so a link modelled today swings off its joint rather than on it.

  • rotate, rotate_to and rotated take an optional pivot_coordinate: the point to turn about, in the resource's own frame. The resource is carried by however far the turn moved that point, so the point stays put and the resource swings on it. The offset is taken back through the parent's rotation, since pivot_coordinate is measured in the resource's frame and location in the parent's. A pivot on a resource with no location raises NoLocationError, because a pivot is held by moving location and there is nothing to move.
  • rotate_to is new: it sets the rotation about each axis where rotate turns by an amount. Axes left as None keep the angle they had, and each is normalised to [0, 360).
  • LinkBody is one rigid member of a chain. It is an ordinary resource, so its origin is a corner, and it carries proximal_joint and distal_joint as coordinates within it. The link is the line between those two joints and nothing stores it; length is the distance, and is None on a member that ends the chain. Because the origin is a corner rather than a joint, a member turns about proximal_joint, which is what the pivot above is for. Material hangs off it as children with their own origins, which keeps a member's frame apart from the shape around it.
  • MechanicalGripper is a LinkBody that ends the chain: nothing attaches past a tool, so it has no distal joint, and what sits at the far end of its span is its tool centre point. Body, fingers and pads are bolted to it, and how far apart the fingers stand is state rather than shape. It is sized to its body alone, because jaw_width moves the fingers and a box drawn around them would resize with the jaws. The jaws straddle the grip centre.

Behaviour: unchanged without a pivot. pivot_coordinate defaults to None, and both the matrix build and the offset are skipped without one, so rotate costs what it always did at any depth.

Scope against main: this is no longer additions-only to resource.py. rotate and rotated each gained the optional argument, so both signatures and both one-line docstrings changed and rotated forwards it - five upstream lines, none deleted outright. rotate_to and the two new classes are additions.

Tests: the new primitives carry their own - a chain folding on its joints, the link measured between its joints, the jaws standing symmetrically about the grip centre and refusing a width they cannot reach, and a pad sitting the same way on both fingers. resource_tests covers the pivot about every axis, under a rotated parent, and under an unlocated parent that a rotated ancestor carries, asserting both that the pivot point stands still and that the turn reached the angle it asked for. TestAnAssembledArm puts a member on a deck with a gripper on its far joint and pins absolute positions through two turns, reading the wrist off each side of the joint independently, since the two classes are otherwise only ever tested apart. Full suite passes, 3238 tests.

One pre-existing wart worth naming so it is not read as new here: Resource.deserialize resolves the subclass and then calls its constructor rather than dispatching to a custom deserialize, so a MechanicalGripper cannot be rebuilt through the generic path at the root of a tree. TipSpot and NimbusDeck behave the same way on main. Nested grippers round-trip correctly, since a parent deserializes children through child_cls.deserialize.

This also touches the same lines of rotate as #1247, so whichever lands second carries a small merge.

🤖 Generated with Claude Code

…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) <noreply@anthropic.com>
@BioCam
BioCam requested a review from a team as a code owner September 9, 2026 23:46
@BioCam
BioCam requested a review from rickwierenga September 9, 2026 23:47
BioCam and others added 9 commits September 10, 2026 10:48
…ce 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) <noreply@anthropic.com>
…esource` 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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
…-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) <noreply@anthropic.com>
…rce 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
@BioCam
BioCam force-pushed the resources-manipulator-primitives branch from a22a9e0 to d6d3a5c Compare September 10, 2026 15:52
BioCam and others added 3 commits September 10, 2026 17:19
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) <noreply@anthropic.com>
…th 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
BioCam added a commit to BioCam/pylabrobot that referenced this pull request Sep 10, 2026
Mirrors what is on `resources-manipulator-primitives`, opened as PyLabRobot#1249: `Resource.rotate` gains a
point to turn about, `Link` is one rigid member of a chain, and `MechanicalGripper` is the tool
that spans a mechanical interface to a tool centre point.

Here so the STAR driver builds against the same code the PR carries. When PyLabRobot#1249 lands upstream
this becomes a no-op.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BioCam and others added 2 commits September 10, 2026 18:05
`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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@rickwierenga

Copy link
Copy Markdown
Member

I merged #1247

PyLabRobot#1247 replaced `rotate`'s three Euler additions with `rotation._prepend`, which composes by
quaternion product rather than by adding components. This branch wraps those same three lines with
the pivot, so the two met head on.

Resolved by keeping the pivot and taking upstream's composition: the reference point's travel is
measured from the rotation matrices before and after, so it does not care how the orientation was
arrived at - and under a real composition the "after" matrix is now correct where Euler addition
would have been wrong for a turn about anything but one axis.

`rotate_to` also composes rather than assigns, so it was worth checking it still lands where it is
told. Swept 108 starting orientations against four targets: every one arrives exactly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BioCam and others added 8 commits September 10, 2026 22:38
`rotate_to` computed a per-axis Euler delta and handed it to `rotate`, which composes by quaternion
since PyLabRobot#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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…g turned on

`rotate_to` took a `reference` point to turn about, and `Link` carried a `joint` for it to turn on. Nothing ever passed either: `turn_to` handed `rotate_to` a zero vector on every call, because the iSWAP's links are placed so their origins already sit on their drive axes. The pivot arithmetic ran on a zero vector in all production use and was exercised only by its own tests.

`rotate_to` is now what its callers wanted, an absolute-angle setter, and 22 lines shorter. `Link` is the length and nothing else; a caller that needs an angle calls `rotate_to(z=...)` directly.

`rotate` and `rotated` are untouched, and `resource.py` remains additions-only against upstream.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…their centre spacing

The fingers were stood with their centres `jaw_width` apart, so the opening an object had to pass through was `jaw_width` less one finger thickness. On the iSWAP's 7 mm fingers a commanded 133.706 mm left 126.706 mm of clear space, and a width equal to the finger thickness stood the two fully interpenetrating.

They now stand with their facing surfaces that far apart, which is the distance the drive reports and the one a rack has to fit into.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither could be deserialized or copied. `Resource.serialize` emits `size_x/size_y/size_z` and `Resource.deserialize` hands them to the constructor, which takes `length` instead, so both raised `TypeError: got an unexpected keyword argument 'size_x'`. A deck carrying an iSWAP could not be saved, and `copy()` went the same way.

`Link` now swaps the three sizes for the `length` it was built with, as `PetriDish` does for its diameter. `MechanicalGripper` takes its body and its two fingers back off the front of `children`, where `__init__` put them, and reassigns anything after them as what it was holding. `jaw_width` moves to `serialize_state`/`load_state`, which is where mutable state belongs and is what carries it through `copy()`.

`tool_center_point` now travels with the model, which the visualizer reads to draw the grip centre.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`test_a_pad_sits_inside_its_finger` compared the test file's own constants against each other - 1.5 >= 0, and 1.5 + 4 <= 7 - so it passed with `pad_location` ignored entirely and with the pads attached to the gripper rather than to a finger. It now asserts the parent and the location, which is what it was named for.

`test_a_gripper_can_have_bare_fingers` asserted only the empty case, so it survived `self.pads = []` being hardcoded. It now checks the padded case too.

`test_rotate_keeps_every_axis_normalized` tested `Rotation._prepend`, which this branch does not touch, and duplicated an upstream test of the same behaviour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BioCam and others added 11 commits September 11, 2026 13:10
…ted on

`tool_center_point` returned `(length, 0, 0)`, so a tool could only ever be programmed against a point level with its own mounting. A gripper that takes hold below where it hangs could not be described at all.

Measured on a Hamilton iSWAP, the grip centre sits 13 mm below the wrist, which the model put at 0 - so the point a move is programmed against was 13 mm high, and disagreed with the same figure worked out by forward kinematics from the joints.

`tool_center_point_z` states that offset, defaults to level, and travels through `serialize`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It arrived as two arguments: `length` gave its reach and `tool_center_point_z` its drop, with y fixed at zero and the property assembling the three at read time. They describe one thing - the offset from the joint the tool turns on to the point it is programmed against - so it is now given as one `Coordinate`, and `length` is derived from its reach rather than stated alongside it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A resource could only turn about its own origin, the front-left-bottom corner, because that is all
`rotation` expresses. Turning a plate about its centre, or a cube about one of its twelve edges,
had no way to be said: the caller had to rotate and then work out by hand where the resource had
to be put back.

`rotate`, `rotate_to` and `rotated` now take a `pivot_coordinate`, given in the resource's own
frame. The turn still happens about the corner, and `_apply_pivot_shift` then moves `location` by
however far the named coordinate drifted, so it ends where it began. The drift is measured in
absolute axes and `location` is written in the parent's, so it is turned into the parent's frame
first, which is what lets a resource inside a parent that is itself rotated come out right.

Giving a pivot to a resource that has not been placed raises, since there is no location to move.

This is the pivot that 809c0cf removed, and it is not the same concept. That one was
`Link.joint`, a coordinate stored on the resource and fed to every turn, which was dead because a
link's origin already sits on its drive axis. This is a parameter of the motion, chosen per call,
which is the only way to express a cube that tips over any of its edges. The private that holds
the arithmetic comes back for the same reason it went: it now has two callers rather than one.

`resource.py` is no longer additions-only against upstream. `rotate` and `rotated` each gain an
optional argument and default to exactly their previous behaviour, which the existing tests and
the legacy STAR suite cover.

Tests: the two pivot tests removed in 809c0cf are back, with the every-axis one now run against
`rotate` as well as `rotate_to`. Added: that `rotate` carries the pivot on each turn while a
repeated `rotate_to` changes nothing, that both raise unplaced, and that `rotated` carries the
pivot into the copy while leaving the original alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`where` took `bar` and `joint` as parameters with defaults, to bind them per iteration rather than
close over variables the loop reassigns. Unannotated parameters are `Any` to mypy whatever they
default to, so `bar.get_absolute_location()` came back `Any` and the closure was reported as
returning `Any` where it declares `Coordinate`.

Found by CI, not locally: `mypy` over the single file passes, and `make typecheck` over the
package is what catches it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…efusing

A pivot on a resource with no location raised, on the reasoning that there was nothing to move.
That was wrong about what a rotation needs. The turn itself is always well defined and never
needed a location; only the compensating shift does, and a resource with no location has nothing
to shift and nothing that can observe the difference. Rotating an unplaced resource about its
centre and about its corner leave the same state: the rotation set, the location still None.

So the pivot is moot there rather than impossible, and refusing it only forced every caller to
branch on `location is None` before asking for one. Both methods now turn about the origin and
say nothing, which is what an unplaced resource does with or without a pivot. Placing it later
overwrites `location` from `assign_child_resource` regardless, so nothing is lost by not
recording a shift that could not survive.

Tests: the two that asserted the raise now assert the turn happens and the location stays None.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…inside it

`Link` was a line: `size_x` was the joint-to-joint length, `size_y` and `size_z` were zero, and its
origin was its proximal joint. Geometry hung off it as children placed behind that origin, which is
why the iSWAP's arm sat at (-12.7, -12.75, 20.3) rather than anywhere meaningful.

`LinkBody` is an ordinary resource: a cuboid with its origin at a corner, carrying both joints as
coordinates within it. The link is the line between them and nothing stores it; `length` is derived
as the distance, and is None on a member that ends the chain. Serialization inverts to match: the
sizes come through as any resource's do, with both joints alongside, where `length` used to be
emitted in their place.

Because the origin is now a corner rather than a joint, a member cannot turn about its own origin.
It turns about `proximal_joint`, which is what `rotate(z=..., pivot_coordinate=...)` was added for.

`MechanicalGripper` is a member that ends the chain: nothing attaches past a tool, so it has no
distal joint, and what sits at the far end of its span is its tool centre point. It is sized to its
body alone rather than to its whole envelope, because `jaw_width` is state: a box drawn around the
fingers would change size every time the jaws did. The fingers reach past it, as material on a link
is free to.

`body`, `body_location`, `fingers` and `pads` are unchanged. Geometry stays attached as children
with their own origins, which is the separation a robot description draws between a member's frame
and its visual geometry.

`_place_the_fingers` straddled y=0, which was the old flange origin. It straddles `proximal_joint.y`
now, so the jaws sit on the span rather than on the member's corner.

`Link` is not on upstream, so nothing outside this PR can be holding the old name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects in the pivot, two of them geometric.

`_apply_pivot_shift` converted the shift into the parent's frame whenever a parent existed, but
`get_absolute_location` stops walking the location chain at the first ancestor with no location
while still taking rotation from the whole tree. Where the immediate parent has no location and an
ancestor is rotated, `location` is already read in absolute axes, so the conversion was an extra
rotation with no counterpart. A plate under an unlocated parent beneath a grandparent turned 30
degrees saw its pivot move 40.85 mm. The guard now also requires the parent to have a location,
which takes that to the 1e-4 mm floor `Coordinate` rounds to.

A pivot on a resource with no location raised until a6056b7, which removed the guard on the
reasoning that the request was moot because nothing could observe the difference. That is only
true of a resource with no parent and no children. With either, the rotation still reaches the
subtree, so three different pivots produced one identical answer and moved geometry silently.
Both entry points raise `NoLocationError` again, which is what `get_absolute_location` raises for
the same condition.

A pivoted turn wrote `location` through its setter and then fired `_state_updated` itself, so
subscribers saw two events carrying the same final state. The shift goes onto the field directly
and the one event at the end of the turn covers it.

Tests: a pivot under an unlocated parent that a rotated grandparent carries, which nothing
covered; and the every-axis test now asserts the turn reached the angle it asked for, since
holding the pivot still is satisfied by an implementation that rotates nothing. The two axis loops
no longer select a method by name, which is not allowed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… body once

The jaws straddled `proximal_joint.y`, the point the tool is bolted on by. They close on what is
at the grip centre, which is the tool centre point, and since that became a free `Coordinate`
nothing tied the two together. A tool whose centre sits 25 mm off its mounting in y stood its
fingers over the mounting instead, leaving an object centred on the programmed grip point
overlapping one finger. The iSWAP is unaffected, where the two coincide.

The envelope was stated twice, as `size_x/y/z` on the member and again as the body child's own
box, with nothing keeping them in step. A tool is sized to its body, because `jaw_width` moves the
fingers and a box drawn around them would resize with the jaws, so the body is now the only place
that box is given.

`LinkBody` emits `distal_joint` for every member, but nothing attaches past a tool: the key had
nothing to say and `__init__` nowhere to put it. It is dropped from a gripper's payload.

Tests: the fixture's joint moves off the body's own middle, which were both 45.0 and therefore
indistinguishable, so the straddle can no longer be satisfied by reading the wrong one. Added: the
jaws following a grip centre that is off the mounting, the reach and height surviving a width
change, the body and the member reporting one box, and the absent joint key.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A member and an end-effector are built in separate modules and every test exercised them apart:
`manipulator_tests` never imported `end_effector`, nor the reverse, and `get_absolute_location`
was never called on a gripper or a finger. The two halves were only ever correct independently.

That is the gap a sibling branch shipped through. A gripper sat visibly in the wrong place in the
viewer while the whole suite stayed green, because the placement of an assembled arm was asserted
nowhere.

`TestAnAssembledArm` puts a member on a deck, mounts a gripper on its far joint, and pins absolute
positions at rest, after turning the member, and after turning the tool on its own joint. It reads
the wrist off each side of the joint independently and asserts the two agree, which is what
mounting means and what nothing was checking. Every expected value is derived from the fixture
rather than read back from the code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`LinkBody` and `MechanicalGripper` are exported from `pylabrobot.resources` but were in neither
the hand-maintained autosummary in `docs/api/pylabrobot.resources.rst`, where every sibling class
is listed, nor the changelog, which also had no entry for `Resource.rotate_to` or for the pivot
that `rotate`, `rotate_to` and `rotated` now take.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rotate` and `rotate_to` each carried the same six lines: refuse a pivot on a resource with no
location, then take the rotation matrix if a pivot was given. `_pivot_reference` holds that now
and returns the matrix or None, so each public method reads as what it does rather than as
preamble, and the two cannot drift apart.

The check after the turn was `before is not None and pivot_coordinate is not None`, two conditions
for one question, because neither narrowed the other for the type checker. One condition and one
cast says it instead.

`span_to` was public with two callers inside this module, each a single line. Both measure from
the joint to the far end of their own span, so they say so directly and the method is gone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants