Name the DROID sideviews by side, and give each one a camera dict - #715
Name the DROID sideviews by side, and give each one a camera dict#715v-positronic wants to merge 2 commits into
Conversation
1269df9 to
f4f07ff
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1269df9a4e
ℹ️ 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".
f4f07ff to
e706c6e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e706c6e09f
ℹ️ 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".
e706c6e to
5e3a55e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e3a55e8f0
ℹ️ 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".
The station carries two sideviews and the DROID embodiment binds one exterior. Which one it binds was decided by which serial the dict named, so the side a task asks for reached the rig as an instruction to a human to aim the camera. `sideview_left` and `sideview_right` name the two serials for their sides, and `droid_right` binds the other one into the single exterior slot. `droid_left` is the unsided default, which already binds the left sideview, so the left binding has one definition rather than two. Each carries the same keys as `droid`, so a policy trained on that observation set sees the set it expects and only the vantage moves. Which serial sits on which side is ASSUMED, not measured: the names are numbered rather than sided and no rig log carries a serial. The two alias lines are the one place to correct it. The test pins what a launch actually does — override `cameras` on an embodiment a caller has already overridden elsewhere — and that the other override survives it. Ticket: Positronic-Robotics/internal#1131 #refs
5e3a55e to
fac4d71
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fac4d713d8
ℹ️ 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".
| return {name: cfg.kwargs['serial_number'] for name, cfg in cameras.items()} | ||
|
|
||
|
|
||
| def test_each_sided_droid_carries_the_same_cameras_as_the_unsided_one(): |
There was a problem hiding this comment.
Rule misleading-name violated:
test_each_sided_droid_carries_the_same_cameras_as_the_unsided_one only compares dictionary keys, and the exterior camera is deliberately different between the sided configurations. Rename it to describe preservation of the observation-key schema rather than claiming the cameras are the same.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
| assert _serials(camera.droid_right)[keys.EXTERIOR_IMAGE] == camera.sideview_right.kwargs['serial_number'] | ||
|
|
||
|
|
||
| def test_the_launcher_reference_binds_the_side_without_disturbing_the_rest(): |
There was a problem hiding this comment.
Rule misleading-name violated:
test_the_launcher_reference_binds_the_side_without_disturbing_the_rest never exercises a launcher; it constructs configuronic objects directly and checks nested override/reference semantics. Rename it for that local behavior so the test does not promise launcher coverage it cannot provide.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
One compares the observation keys of a sided embodiment with the unsided one. The other checks that an @ reference overrides the cameras and leaves a sibling override standing. Shelved with the pull request. Ticket: Positronic-Robotics/internal#1131 #refs
1 participant
The station carries two sideviews and the DROID embodiment binds one exterior. Which one it
binds was decided by which serial the dict happened to name, so the side a task asks for reached
the rig as an instruction to a human to aim the camera. Removing that aiming is what this is for.
This is the positronic half. On its own it changes no behaviour: nothing passes the new names yet.
The platform half (the launcher and the MCP that exports the side) is a separate pull request, and
the rig runs a pinned console wheel, so neither reaches the rig until somebody installs it.
What it adds
sideview_left/sideview_right— the two serials, named for their sides.droid_right—droidwith the other sideview in the single exterior slot.droid_left— an alias ofdroid, which already binds the left one, so the left binding has onedefinition rather than two.
Each carries the same keys as
droid, so a policy trained on the DROID observation set sees theset it expects and only the vantage moves.
droidanddroid_3camnow reach their sideviewsthrough the sided names, so the mapping has one home.
The mapping is an assumption, and it is deliberately in one place
zed_2iis taken to be the LEFT camera andzed_2i_secondthe RIGHT one. That is a decision,not a measurement. The two names are numbered rather than sided, and no rig log carries a serial,
so nothing in the repository or on the rig records which is which. If the rig says otherwise, the
two
sideview_*lines are the only edit needed.Getting it wrong is not loud: the policy sees a mirrored view, which is out of distribution for a
DROID checkpoint, so episodes score badly for a reason that is not the policy.
The flag a caller uses
--embodiment.cameras=@positronic.cfg.hardware.camera.droid_right, never--embodiment=. Thewhole-embodiment form binds the side too and drops every other override pinned on the
embodiment — the console pins the arm's idle brake there, so it would be silently lost. The test
pins the narrow form against an embodiment already overridden elsewhere, and asserts that other
override survives.
That behaviour was verified rather than assumed, since the ticket flagged it as unknown:
configuronic resolves an
@reference to a plain dict, with no requirement that the target be aConfig—_import_object_from_pathreturns the module attribute verbatim andinstantiatewalks a dict, building each
Configinside it. A run through the real CLI path binds the rightserial, keeps the arm override, and leaves the module-level dict unmutated.
Tests
positronic/cfg/tests/test_camera.py— the first tests for this module. They assert the sideddicts are drop-in for the embodiment's
cameras.items()loop, that the two sides bind differentsideviews and share the wrist, that each side takes the serial the station declares for it, and
the launcher-reference behaviour above. Binding a side to the wrong camera fails three of the four.
What no test can catch is the assumption itself: whether the serial called
sideview_leftis theone physically on the left. That needs somebody at the rig.
Three accepted literals:
serial_number,camerasandrobot_armare read off a configuronicConfig's kwargs by the wrapped function's own parameter name, which is how configuronic storesthem. The latter two name parameters of the test's own local config function.
Ticket: Positronic-Robotics/internal#1131 — this does not close it; the platform half and the
console install remain.