Resource: turn about a joint, and the links and end-effectors that need it - #1249
Open
BioCam wants to merge 35 commits into
Open
Resource: turn about a joint, and the links and end-effectors that need it#1249BioCam wants to merge 35 commits into
Resource: turn about a joint, and the links and end-effectors that need it#1249BioCam wants to merge 35 commits into
Conversation
…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>
…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
force-pushed
the
resources-manipulator-primitives
branch
from
September 10, 2026 15:52
a22a9e0 to
d6d3a5c
Compare
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>
`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>
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>
rickwierenga
approved these changes
Sep 10, 2026
`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>
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.rotatehas always turned about the left front bottom corner, so a link modelled today swings off its joint rather than on it.rotate,rotate_toandrotatedtake an optionalpivot_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, sincepivot_coordinateis measured in the resource's frame andlocationin the parent's. A pivot on a resource with no location raisesNoLocationError, because a pivot is held by movinglocationand there is nothing to move.rotate_tois new: it sets the rotation about each axis whererotateturns by an amount. Axes left asNonekeep the angle they had, and each is normalised to[0, 360).LinkBodyis one rigid member of a chain. It is an ordinary resource, so its origin is a corner, and it carriesproximal_jointanddistal_jointas coordinates within it. The link is the line between those two joints and nothing stores it;lengthis the distance, and isNoneon a member that ends the chain. Because the origin is a corner rather than a joint, a member turns aboutproximal_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.MechanicalGripperis aLinkBodythat 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, becausejaw_widthmoves 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_coordinatedefaults toNone, and both the matrix build and the offset are skipped without one, sorotatecosts what it always did at any depth.Scope against
main: this is no longer additions-only toresource.py.rotateandrotatedeach gained the optional argument, so both signatures and both one-line docstrings changed androtatedforwards it - five upstream lines, none deleted outright.rotate_toand 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_testscovers 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.TestAnAssembledArmputs 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.deserializeresolves the subclass and then calls its constructor rather than dispatching to a customdeserialize, so aMechanicalGrippercannot be rebuilt through the generic path at the root of a tree.TipSpotandNimbusDeckbehave the same way onmain. Nested grippers round-trip correctly, since a parent deserializes children throughchild_cls.deserialize.This also touches the same lines of
rotateas #1247, so whichever lands second carries a small merge.🤖 Generated with Claude Code