Skip to content

Drive the Trossen station from the leader arm beside it - #703

Open
DarksaCY wants to merge 77 commits into
Positronic-Robotics:mainfrom
DarksaCY:trossen-leader
Open

Drive the Trossen station from the leader arm beside it#703
DarksaCY wants to merge 77 commits into
Positronic-Robotics:mainfrom
DarksaCY:trossen-leader

Conversation

@DarksaCY

@DarksaCY DarksaCY commented Sep 2, 2026

Copy link
Copy Markdown

What this adds

The station records demonstrations from a Meta Quest so far: a hand tracked in space, a pose solved for at
every tick. This adds the other way to drive it — the leader arm standing beside the follower. The joints
the leader reads are what the follower is asked to stand at, one for one, with nothing solved in between.
A pose has to be solved for, and near the edge of the workspace a solution may not exist; joints always do.

The pieces:

  • drivers/roboarm/trossen_leader.py — the arm the operator holds. It runs in external_effort, where
    every joint is back-drivable, and publishes the joints and the grip it is moved to. It carries the leader
    end effector: an arm read as a follower reports a gripper position 5.9 mm too low, which is past what the
    controller tolerates, and the controller then refuses position mode outright.
  • --leader in data_collection.main, which is what makes a rig leader-driven. The follower copies its
    leader once every joint of the two is within 0.1 rad — engaging on the fact that the arms stand together
    rather than on the operator's word for it.
  • The keys carry the session, because the operator's hand is on the arm: r records, the space key puts
    both arms at the start pose, h takes them to rest, q ends the run.
  • cfg.embodiment.trossen, cfg.eval.real.trossen and vendors.lerobot_0_3_3.codecs.trossen_joints, so a
    policy trained on what the station records can drive it. The action is the joints the leader asked for,
    which the follower's own driver takes back as they are.

What the rig taught it

Every one of these is a defect the station found, and each has a test:

  • Both arms travel to the start pose. The space key used to move the follower alone. It then met a
    leader standing somewhere else and took up the whole gap in one streamed step, which the arm makes as a
    jerk. The leader is driven to the same pose, in position mode, and is free in the hand again on arrival.
  • A reading taken before a travel is dropped. Nothing read the arms while the rig travelled, so those
    readings waited in their queues and the tick after the move read them as the present: the follower met a
    leader that had gone, and was sent to the pose the two had stood at.
  • A follower copies nothing until the session has put both arms where they start. A run opens with the
    arms close together, and one that engaged there moved on the first hand laid on the leader.
  • _COMMANDED_SHARE is three tenths. At a tenth the follower held 1 rad/s where the operator asked for
    2 to 3, fell as far as 0.8 rad behind on a turn of the wrist and took a second to take that up: nine such
    spells over one 89 s episode, 7% of its ticks. At three tenths, measured the same way over the next
    episode, the 95th percentile of the following error fell from 243 to 44 mrad and the spells are gone,
    with no overshoot (the arm passes a standing target by at most 36 mrad).
  • gripper_friction_constant takes the trigger's stiffness off the operator's hand. The gripper joint
    carries the most friction on the arm, and what the controller cancels of it is that joint's
    friction_constant_term. The station's leaders are calibrated at 5.77 N and run at 10.02. The term is
    configuration and outlives the process that wrote it, so the run hands the arm back as it took it.

How to read this

The branch stacks on work that is still open: #683 (the arm), #689 (the Linux video driver) and #692 (the
station's cameras), plus the wiring of those cameras into trossencfg, which waits for them to merge
before it can be a change of its own. The diff against main therefore carries all of it.

What is new here is the last ten commits:
DarksaCY/positronic@trossen-cameras...trossen-leader

One of those ten, Ask a device for the setpoint it is streamed now, is #702 on its own, because it fixes
every arm driver rather than this one. It stays on this branch so the station runs; it can come out once
#702 lands.

The Trossen arm driver needs `trossen-arm`, and the RealSense camera driver
needs `pyrealsense2`. Each vendor gets its own extra, as `yam` does.

PyPI has no macOS wheel and no source distribution for `pyrealsense2`, so
the marker keeps `uv lock` resolvable on macOS.
`XRInputSource.gamepad` is null for an input source that has no buttons, and
the frontend then sends an empty array. `WebXR` emitted that array, and
`_parse_buttons` reads index 4 from it. The `IndexError` stopped the
foreground control loop and the process with it.
The controller firmware runs the servo loop, so the driver streams joint
setpoints and reads back the pose the firmware reports. A synchronous move
hands the firmware a goal time and lets it plan the trajectory; a streamed
setpoint is applied without interpolation.

The gripper is the 7th joint of the same controller, so the arm driver
carries the `grip` and `target_grip` ports. Grip converts against the travel
the arm reports for that joint, not a constant.

Joint commands only. Cartesian commands come next.
A closed gripper reads a shade below the joint's lower limit, which the
conversion turns into a grip above 1.
The controller streams its telemetry over UDP and `get_robot_output` hands
back the last of it, so a link that drops does not raise — it repeats the
same reading. The arm read AVAILABLE with a stale pose, and the failure
surfaced only on the next write.

The controller's own clock is in that reading and stands still once the
stream stops, which is what now marks the link down. A write the link
refuses marks it too, and the setpoint stays unsent until the arm answers.

A run that ends on a dead link no longer raises out of the driver: an arm
that cannot be reached cannot be set idle either, and the handle still has
to go back.
Telemetry and commands travel separately: the controller streams the first
over UDP and takes the second over TCP. A dropped link takes the TCP session
with it, and the vendor driver does not open another, so an arm that answers
again still takes no command until this does.

The two halves are now tracked apart. Fresh telemetry no longer clears a
write the link refused, which is what a reconnected cable leaves behind. A
link that stays down for half a second gets a new session, and another every
two seconds while it stays down.

A new session holds the arm where it finds it: it ended up wherever the lost
one left it, and driving it back to the target that session held is a jump.
The firmware solves the kinematics, so a Cartesian goal goes to it as a pose
and the driver never sees joints for it. The controller speaks angle-axis
where positronic speaks a rotation.

A streamed target is capped to one tick's travel first. A teleoperator
reaching past what the arm can do produces targets that run away from it,
and the firmware plans a trajectory to each; a short plan is one it can
solve. The cap follows the short way round a turn, which the angle-axis the
command carries does not.

The firmware is also asked to check the path it plans before it starts one,
so a target it cannot reach is refused rather than failed part-way.

A Cartesian goal names the arm alone, so the fingers now take their own
call, and each half of the setpoint goes out only when it changes. A
synchronous Cartesian move is refused: arrival is judged from the joints the
controller reports, and a pose does not say which joints reach it.
At or below 0.001 s the firmware takes the goal as a step and asks the servo
for whatever acceleration closes the distance at once. One tick of goal time
makes it a linear segment instead, which the next setpoint replaces as it
arrives.
Past its velocity limit the controller faults and sets the arm to idle,
which drops it. The driver now reads what the joints run at, holds the arm
where it reads once any of them passes four fifths of its limit, and takes
no new target until they slow.

A fault that stands no longer fills the log a hundred times a second: a
complaint that repeats is said again every five seconds. Opening a new
session backs off the same way, because a fault the controller latches
outlives one, and every attempt stalls the control loop.
The firmware solves Cartesian goals, but each one on its own, knowing
nothing of the last. The arm rests on the lower limit of joints 1 and 2,
where half the directions have no solution at all and the rest sit against
the boundary; from there successive firmware solutions came from different
branches and the arm tore itself between them until a joint passed its
velocity limit and the controller dropped it.

FK and IK now run here, against the MJCF vendored from `trossen_arm_mujoco`
at `ee_site` — the frame the controller reports its own Cartesian position
in. Forward kinematics on that model and the pose the controller reports
agree to 0.13 mm and 0.01 degrees, measured on firmware 1.11.1. IK is
warm-started from where the arm stands, so the joints stay continuous, and
clamped and FK-verified before it is accepted.

Every command therefore reaches the arm as joints, held to a quarter of what
each may travel in a tick. A goal that is far away is walked to rather than
handed over, so the firmware never plans a path of its own, one goal time
covers every setpoint, and a synchronous move to a pose can be answered like
any other.
The same pose is reachable with the arm in more than one shape. The solver
was seeded from where the arm stands and then from two reach postures, so a
target the first seed missed was answered from one of the others — the same
pose, a different arm. Walking the joints there swung the end effector half a
metre from where it had been asked to go.

A streamed target now searches only from where the arm stands, and a solution
that sits further than a fifth of a radian from it is not one: a target a step
away has a solution a step away. A move somebody waits on may still change the
arm's shape, and is no longer paced to a step at a time — it was arriving at
the first step and answering that it had arrived.

The setpoint ramp now steps from the last setpoint rather than from the
reading, clamped to a few ticks ahead of it. Anchored to the reading it went
at whatever the servo followed it with, which left a synchronous move short of
its target until it timed out.
The arm's own material is nearly black, so a viewer opened on the model
alone shows it against a dark background and nothing can be made out.
Upstream's scene wraps a different arm, so this one is ours.
The controller takes a margin past what it reports — a gripper a millimetre
below its zero is driven without complaint — but far enough past, and
entering position mode faults it and drops the arm, whatever it is then told
to do. How much further is not something the SDK says, so the driver says
what it sees and lets the controller answer.
A position servo holds the arm up with a following error, so the arm stands
off from every pose it is given, and the standoff grows as it reaches out.
Three places measured against the reading and so carried that standoff into
what they asked for next.

The setpoint ramp is now a ramp, saturated by the following error the
controller allows rather than based on it: based on the reading, each tick
the arm closed some of the gap moved the setpoint up again, which walked the
arm rather than holding it. The allowance, and the tolerance a move counts as
arrived within, both come from what the controller says it permits — the
guessed ones were tighter than the droop, so no move ever arrived.

A streamed Cartesian target steps on from the pose last asked for, and its
solution is measured against the joints last asked for. Against the reading,
the room a step needs would have to cover the standoff too, which is room
enough to change the arm's shape in. The pose stepped on from is kept only
once a solution for it is found, so one that has none does not leave every
pose after it further out of reach than the last.
The driver had grown a trajectory generator: a rate limit, then an
acceleration limit, then a cap on how far the setpoint could lead the arm.
Each of those fought the following error a position servo holds the arm up
with, and together they left a move creeping towards a target it never
reached.

The controller plans a quintic for any goal time over 0.2 s, which starts and
stops the arm gently. A move now hands it the whole travel and the time to
make it in, and the driver keeps only what streaming needs: a step from the
joints last asked for, held to a tenth of what each joint may do.
`trossencfg` fills both the arm and gripper slots with the one driver, the way `yamcfg` does — the Trossen
controller carries the gripper on the same link. Its start pose is mid-range on every joint rather than the
zeros the arm rests at, where joints 1 and 2 sit on their lower limit and half the directions out have no
solution.
The D405s on the Trossen station stream colour at 640x480@30 through
`pyrealsense2`, which the `realsense` extra carries. Depth is a stream rather
than a retrieval: an unbound `depth` port never enables it, so the bandwidth a
camera takes is the bandwidth someone reads — four of them streaming both do
not fit one USB controller. Where depth is bound it comes through `rs.align`,
in meters, because a D405 resolves 0.1 mm and eight bits do not.

Frames are polled rather than waited on, so the loop keeps answering the world
while the camera has nothing ready, and silence longer than a second is taken
for a link that dropped: the pipeline is opened again, and a camera that stays
down raises once the recovery time is out.

`VideoWriter` moves out of the OpenCV driver's smoke, where the two drivers'
smokes would otherwise carry a copy each, and drains the encoder before it
closes the file.
`_parse_controller_data` returns one entry for each hand, and a hand that the
headset does not track is `None`. The dictionaries said nothing about that, so
the type of each value was `None` and every consumer of a pose or a button
array was an error.
`CLAUDE.md` asks for short sentences, the active voice and no metaphor. The
module text now follows it.
`main` wrapped the camera signal with a lambda, and `World.start` pickles the
`WebXR` control system that reads it. No configuration set
`stream_video_to_webxr`, so the path never ran.
A station without a sound device gives the operator no sign that the A button
reached the harness, or that the arm is in error and holds the commands. The
log now carries both.
A teleoperator whose pose has no solution gets one refusal each tick, at the
rate of the stream. The refusals carry the pose, which differs every tick, so
`complain` now takes the name of the fault and holds the rest back.
An interrupt reaches every process of a run at once, and it can land inside a
manager call. The connection then holds half a message, and the answers a
stopping handler writes read the tail of another one: `fail_queued` took a
grip value for a request envelope and raised `AttributeError` over the real
reason the run ended.
The arm holds itself up with a following error, so the pose it reads stands
off from the pose it was asked for: 0.07 rad at joint 1, measured at the
nominal pose. A teleoperator's target starts at what the arm reads, and the
streamed solution was capped 0.05 rad from the joints last asked for — less
than that error. Every target of a session was refused and the arm stood
still.

The cap is now the `position_tolerance` the controller reports for each joint,
which is the same error. What a tick may move is unchanged: `advance` paces
the setpoint at a tenth of each joint's velocity limit.
The fake settled exactly where it was told, and a real arm settles short of it
by the error it holds itself up with — which is what a streamed target is
measured against. `SaggingArm` carries the error the arm at the rig reads, and
the turn a teleoperator asks for is walked against it.
Measured at the rig with the controller moved along one axis at a time.
`scene.xml` is not from upstream: it puts the arm on a lit ground plane for a
viewer to open. Nothing in the code reads it. The vendored model stays, since
the driver solves its kinematics against it, and its README now states the
layout as it is and names the revision it came from.
A pipeline is a subcommand of the server, and the codec a checkpoint was trained
under has to be the one it is served with.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4029f8eb6e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/roboarm/trossen.py Outdated
Comment thread positronic/cfg/embodiment.py Outdated
Comment thread positronic/data_collection.py
Comment thread positronic/data_collection.py Outdated
`main` moved thirteen commits under this branch, and one of them changed where a
recording is named: `DsWriterAgent` takes a dataset factory, and `START` carries
the path an episode writes to (Positronic-Robotics#694).

The session controller takes `output_path` beside the poses and the
teleoperation source it already carried, and `main` builds the factory where it
used to enter a writer. The rest of the branch merged as it stands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b56d4c84cb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/data_collection.py
Comment thread positronic/tests/test_components.py Outdated
Six things, three of them findings this branch shares with the pull requests
under it and carries its own copy of until they merge.

A joint target reached `np.clip` without a shape, so a scalar broadcast across
all six limits and a NaN clipped to itself and went to the controller; a grip
went the same way. `_steadied` read a relative rotation through `as_rotvec`,
which reads a quaternion and its negative -- the same turn -- as turns a whole
revolution apart, and answered a hand that barely moved with a full radian.
`_optional_import` caught every `ImportError`, which turns a broken project
import into a skipped test.

The station's camera layout was written out twice, in `trossencfg` and in the
`trossen` embodiment, where the two must name the same views or a policy is
fed something it was never trained on. It is `camera.TROSSEN_STATION_VIEWS`
now, and both read it.

`DataCollectionController.run` had grown a wall: recording, tracking, readying,
parking, decoding and the error transition all inside one `while`/`try`. Each
is a named method now and the loop coordinates one tick, which is what the
`noqa: C901` on it was standing in for.

`SessionEvent` was declared after the controller that dispatches on it, and now
sits above it. The keys that ask for each stay beside `_wire`, which is what
reads them.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe5734f81d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/camera/linux_video.py Outdated
Comment thread positronic/data_collection.py Outdated
Comment thread positronic/data_collection.py Outdated
Comment thread positronic/drivers/roboarm/trossen.py Outdated
Comment thread positronic/drivers/roboarm/trossen_leader.py
Comment thread positronic/cfg/eval/real/trossen.py Outdated
`_go_to` put the leader in position mode before looking at the goal, so a
malformed `JointPosition` -- the wrong length, or a NaN -- reached
`set_all_positions` with the arm already servoing, and a NaN made the travel
time itself NaN. The goal is checked for six finite joints first, the way the
follower checks its own.

`_opened` sat above `_Arm`, four hundred lines from `Robot.run`, its only
caller; it moved down beside it. Its last `driver.cleanup()` ran unguarded in a
`finally`, where a failed link raises and replaces whatever was ending the run.

Planned Trossen trials carried no metadata, so their episodes recorded neither
`eval.trial_index` nor `eval.trial_count`; they carry both now, as the droid
trials do.

The Linux video driver, its tests and the D405 paths take what the review of
their own branches settled: a decoder holding a frame back is no longer counted
as a buffer of the wrong size, images of one buffer no longer share an adapter,
and the names say what they hold.
A V4L2 buffer short of a frame is what a busy bus hands over: measured on the
station's four D405, one per camera over three minutes of capture and five to
eight per camera per minute while all four are also being encoded. The waiver
records that, so the count is read as traffic rather than as a swallowed fault.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 109d35cbf6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +203 to +205
# TODO: a fault the controller latches is cleared by a new session, not by
# asking for the mode again; reopen one, the way the follower's `recover` does.
driver.set_all_modes(trossen_arm.Mode.external_effort)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reopen the leader session after a latched fault

When the controller latches a fault, this recovery path keeps using the failed session even though the adjacent comment states that only a new session clears the fault. set_all_modes therefore cannot restore the leader, and the station will retry the same unusable handle indefinitely until the process is restarted; clean up and reconfigure the driver as the follower's recovery path does.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Known, and marked in the code: the TODO two lines above this says a latched fault is cleared by a new session, not by asking for the mode again, and that the leader has to reopen one the way the follower's recover does. The leader has not been run against a latched fault at the rig, so the recovery is written for the mode change that is measured and the rest is named rather than guessed.

Comment thread positronic/drivers/camera/linux_video.py Outdated
Comment thread positronic/data_collection.py
Comment thread positronic/drivers/roboarm/trossen_leader.py Outdated
Comment thread positronic/drivers/roboarm/trossen.py Outdated
Comment thread positronic/drivers/roboarm/trossen.py
`_abandon`, `_recorded`, `_readied`, `_parked` and `_drove` all acted and
answered with a bare `bool`, so what each verdict meant was invisible at the
call site. They are commands now, and `run` sets `recording` and `in_error`
where a reader can see it.

`_readied` and `_parked` did nothing but give up the recording, stop the
tracking and call `_ready` or `_park`; the pairs are one method each, and the
recording is given up at the caller. `_drove` split into the state the arm
reads, `_in_error`, and the emission the caller makes with it. `_recorded` is
`_record`, an instruction like its neighbours.

The Trossen driver takes what Positronic-Robotics#683 settled: a move the link drops under is
failed by the new session instead of running out its 15 s deadline. The Linux
video driver takes what Positronic-Robotics#689 settled: a buffer that decodes to several images
hands over the newest, because the frame port holds one. The D405 paths carry
the waiver that says why they are strings.
…r safely

`_drop_readings` let go of the session events, the poses and the arm readings a
travel arrived under, but not the buttons. Those reach the controller on a
queue, so a press made in front of a moving arm was read as new on the first
tick after the move and started a recording or another move.

`_opened` in the leader driver closed its session unguarded in a `finally`,
where an unreachable arm raises and replaces the reason the run is ending; it
is logged now, as the follower's is.

`_connect` sat above `_Arm`, four hundred lines from `Robot`, its only user; it
moved down beside `_opened`.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: adcedc28b4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/webxr.py Outdated
Comment on lines +46 to +49
if buttons.size < _TELEOP_BUTTONS:
raise ValueError(
f'The {side} controller sends {buttons.size} buttons; teleoperation needs {_TELEOP_BUTTONS}'
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep teleoperation-specific validation out of WebXR

Rule overspecific violated:
_parse_controller_data now rejects any tracked input source with fewer than six gamepad buttons, and the WebSocket handler then closes the connection, even when a consumer only needs controller_positions—for example, hand tracking or a pose-only controller. Relay the available controls here and let the teleoperation consumer validate or map the button layout instead of baking DataCollectionController's B-button index into the generic WebXR driver.

AGENTS.md reference: AGENTS.md:L7-L8

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same answer as the thread on #683: this check is here because the reviewer asked for it here, with this raise text (#683 (comment)). Moving the mapping into DataCollectionController is worth doing and is not ours to do against that direction. It does ask for one row of at least six now, rather than counting by size.

Comment thread positronic/data_collection.py
Comment thread positronic/drivers/roboarm/trossen_leader.py
# Conflicts:
#	positronic/drivers/roboarm/yam.py
#	positronic/keys.py
`main` split `positronic.keys`: the robot model keys went to
`positronic.drivers.roboarm.keys` and the trial keys to `positronic.eval.keys`,
where `EVAL_TRIAL_INDEX` is `TRIAL_INDEX`. The Trossen driver, the embodiment
and the Trossen eval read them from there, and `ROBOT` joins the roboarm keys.

`_Tracker.update` asks `_operator_position` directly rather than through the
`umi_mode` property, which no checker can see through, and `where` in the
tracker test is bound before its loop.
`mp_pipes` answers with one receiver or a list of them, and the test read
`.read()` off the union.
Twelve errors the baseline grandfathered are gone from this branch. The ratchet
only refuses growth, so CI would prune them on its own; the branch carries the
pruned file instead.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe039d2816

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread pimm/world.py Outdated
Comment thread positronic/data_collection.py Outdated
Comment thread positronic/drivers/roboarm/trossen.py Outdated
Comment thread positronic/drivers/roboarm/trossen.py
A delta is consumed once and names a distance, but a streamed pose is paced to
one step of `_MAX_STEP_M`. So a delta longer than 15 mm arrived as 15 mm and
the rest of it was lost: `PoseDeltaAction` emits up to 5 cm, and the Trossen
evaluation this branch carries undertravelled by every command.

`_Arm` keeps what a delta still owes and takes another step each tick nothing
supersedes it, until it lands or the arm cannot reach further. A pose keeps the
behaviour it had: it says where the arm is wanted now, the next one supersedes
it, and an arm whose stream stops holds where it stands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fef8bc77fb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/roboarm/trossen_leader.py
Comment thread positronic/data_collection.py Outdated
`_Follow.met` read as a question and latched `on` while answering it. It is
`take_up`, an instruction, and `run` reads `on` where a reader can see it.

`_drop_readings` let the button messages go but never showed them to the
handler, which reports an edge against the last reading it saw: a button
released before a travel and held during it read as pressed the moment the
travel ended. `_forget_presses` takes the buttons as they stand.

A joint-space target left `_anchor` on the pose a Cartesian stream had asked
for, so the first setpoint after the rig was readied drove the arm back towards
the last episode's target. Joints end the anchor now, and so does a move
somebody waits on. `stand_down` also left a move in flight that nothing would
finish; it fails it, as the reconnect does.

`recover` suppressed the failure to close the old session without a word; it
logs the address and the reason.

The interrupt guard sat inside `_emit_queue` and `_read_queue`, which the
public `emit` and `read` reach only after `_ensure_mode` and `transport_mode`
have called the manager -- and the shared memory path never reached it. Both
public methods check first, and a queue that answers with anything but a
`Message` says the connection is torn. The SIGINT handler goes: a transport
records the interrupt it takes itself, so a main-process control system notes
its own.

`_parse_controller_data` counted buttons by `size`, so a nested payload of six
passed, and the payload's field names are spelled once beside the parser.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17719aee41

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread pimm/world.py
# Set in a process that has taken an interrupt. An interrupt can land inside a call to the manager, and
# that connection then holds half a message: the next call over it returns what another one asked for, so
# a reader takes a value from a channel it never subscribed to. Nothing may be sent or read after it.
_interrupted = False

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset the interrupt latch for fresh worlds

Rule hidden-dependency violated:
MultiprocessEmitter and MultiprocessReceiver silently depend on no earlier World having set this process-global latch. If a caller catches a KeyboardInterrupt raised inside a transport and later creates a fresh World in the same process, _interrupted remains true, so every new multiprocess channel drops emissions and returns no data even though its connections are uncorrupted. Scope the latch to the affected world/transports, or reset it when constructing a new world.

AGENTS.md reference: AGENTS.md:L7-L8

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, and left. This is the module-global _interrupted that #691 owns, and the same finding stands open on that PR (#691). Scoping it to a manager means threading that state into both transports and through World.connect, which is a change about World, not about ending a run on an interrupt. The process that takes SIGINT is going down; a caller that catches it and builds a fresh World is the case, and it is worth its own change.

Comment thread positronic/drivers/roboarm/trossen.py Outdated
…umber

`hold_grip` took the whole range at once and `_put_goal` sent it with the
streamed goal time of 10 ms, which asks the finger drive for metres a second
where the controller reports 0.25. The wanted grip and the setpoint are two
things now, and `advance` walks one to the other.

`_take_control` judged the arm by the velocities `read` saw a tick before, so a
session opened on a moving arm entered position mode anyway.

`_go_to` checked the shape and the finiteness of a leader goal but not the
range: a configured pose outside the controller's limits went over verbatim
while the follower clipped the same pose, and the two could then not meet. The
leader clips to what it reports, and says when it did.

`_steadied` kept whatever it was given, so one pose that is not a number sat in
the filter and every pose after it came back NaN.

`_in_error` read as a question and emitted the operator's sound while answering
it. It is `_note_error`, an instruction, and the state it follows is
`arm_in_error`, which `run` reads.
Standing down holds the arm where it reads, which is not where the last
Cartesian setpoint asked it to be. The next step measured from that pose would
drive it back towards a target the arm was stopped short of.
`_FakeTrossen` duck-types the slice of the vendor driver this driver uses, and
`get_error_information` is not in it.
`FakeDevice.opened` was set in the constructor, so it held the last device
built and the assertion that the driver opened one could not fail.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23c595f37d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/roboarm/trossen.py
Comment thread positronic/drivers/roboarm/trossen.py Outdated
…control

This branch carries the Trossen driver as a copy, so what Positronic-Robotics#683 settled has to be
brought over. It gains:

The whole model in `robot_meta` -- the vendored URDF, its meshes,
`joint_names` and `control_frame` at `default`, which sits where the MJCF puts
`ee_site` and where the controller reports its pose. No `gripper`: the viewer
drives one to `grip * travel`, and this arm's carriage joints close at zero and
open at their positive end, which no single travel expresses.

A run that opens by taking the arm off its rest pose and closes by putting it
back on it, and that waits for the arm to be in position mode before it writes
anything -- a goal sent before the mode change is refused, and the refusal
reads as a dead command channel.

A joint target clipped into range says so at WARNING; a delta's destination is
kept only once the command is accepted; `_short_way` and `_apart` belong to
`_Arm`; the fake's limits are a `NamedTuple`; and the mesh dictionary is
`roboarm_keys.MESHES`.

`_COMMANDED_SHARE` stays 0.3 and `_connect` stays beside `Robot`, which are
this branch's own.
The key the viewer reads and every model writes is `roboarm_keys.MESHES`, and
the URDF this branch now ships is pinned to the MJCF the driver solves against:
`default` and `ee_site` agree to 0.1 mm over eight joint vectors.
`Element.get` answers `str | None`, so the set of filenames was `set[str | None]`
and `sorted` had nothing to compare. The default makes it `set[str]".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70abd19484

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

yield driver
finally:
try:
driver.set_all_modes(trossen_arm.Mode.idle)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the leader supported until it is parked

Rule hidden-dependency violated:
When the operator presses q without first pressing h, Leader.run exits while the arm may be anywhere, and _opened immediately switches it from gravity-compensated external_effort to idle; the adjacent startup log confirms that the controller was holding the arm's weight, so it can fall on shutdown. Make shutdown move the leader to its rest pose before selecting idle, or retain a supporting mode unless rest has been confirmed.

AGENTS.md reference: AGENTS.md:L7-L8

Useful? React with 👍 / 👎.


# The controller holds the arm only at rest; `_opened` sets it idle, and idle anywhere else
# is an arm that falls.
yield from arm.travel_to(_REST_JOINTS, 'rest', at_teardown=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep control when the teardown move fails

Rule hidden-dependency violated:
When the rest travel times out or the link drops, travel_to logs and returns without reaching _REST_JOINTS, but this call still exits into _opened, which unconditionally selects idle; as the preceding comment states, idle away from rest makes this arm fall. Return an explicit travel outcome and select idle only after confirmed arrival, otherwise leave the arm holding safely.

AGENTS.md reference: AGENTS.md:L7-L8

Useful? React with 👍 / 👎.

Comment on lines +63 to +65
driver.configure(
trossen_arm.Model.wxai_v0, trossen_arm.StandardEndEffector.wxai_v0_leader, ip, True, _CONNECT_TIMEOUT_S
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clean up a partially configured leader

When driver.configure(...) raises after opening any SDK or controller resources, _connect never returns, so _opened is never entered and this driver receives no cleanup() call. The follower's equivalent connection factory explicitly handles this partial-initialization case; protect configuration here the same way, cleaning the driver while preserving the original configuration failure.

Useful? React with 👍 / 👎.

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.

1 participant