Remove pick tuning panel from teleop cockpit - #555
Conversation
…ing panel Add a wrist-camera fine-alignment stage to the pick_any_object grasp, and the Teleop tuning panel that drives it. After the base parks the object in the head-camera pick box, the arm goes to an operator-posed search position (up high, elbow ~90°, wrist camera looking down), Gemini marks the object, and the arm visual-servos straight down onto it: center-in-box first, then descend one step, repeating until a stop height where the blind ladder finishes the grasp. Tracking is HSV color segmentation + CamShift, not optical flow: during the descent the object grows ~2.5x in the wrist image and fabric deforms, which slides LK patches onto the carpet. A likelihood-ratio color model (object vs surrounding floor) back-projected each frame is scale- and deformation-proof, and its score honestly reports occlusion/loss instead of tracking carpet. The pick_any_object skill and its runtime helpers (arm_rest_position, gripper_open, gripper_close) move from the gitignored workspace/custom_skills/ into workspace/innate_skills/ so the pipeline is tracked and works from a fresh checkout. Bare-name skill chaining still resolves them (local/ then innate-os/). Panel (Teleop, key "o"): live top-down + side grasp views, per-stage sliders that publish overrides the running skill applies mid-run, and a draggable wrist box + detection marker over the arm-camera video for aiming the servo. Topics: skill -> /pick_any_object/debug (stage events), panel -> /pick_any_object/tuning (parameter overrides).
Introduce workspace/skill_lib/ — plain Python modules skills import directly, no Skill classes, no roster entries. Functions take the interfaces they need as explicit arguments. The hardware lessons now live exactly once: - skill_lib/arm.py: verified gripper open (overcurrent-trip reboot recovery), close with the strength-cap lesson, health-checked cartesian moves (FK verify, recover + retry, raise ArmUnhealthy), servo recovery, reach clamps. - skill_lib/geometry.py: head-camera <-> floor pinhole projection, moved verbatim from pick_any_object (pure math; the webapp panel mirrors it). gripper_open / gripper_close become thin wrappers over the lib — and gripper_open GAINS the trip-recovery verification that previously only pick_any_object had. pick_any_object drops ~150 lines of private helpers (_move_arm, _recover_arm, geometry, inline reach clamps) for lib calls, and calls the lib directly instead of round-tripping gripper skills through the invoker. Import contract: skills import the lib at module top — the loader puts the repo root on sys.path only while a skill module executes (namespace-package import, verified live). catalog.reload_all/reload_selective now evict cached workspace.skill_lib modules so a normal skills reload picks up lib edits too. No behavior or compatibility changes otherwise: skill IDs, innate.skills chaining, panel debug events, and TUNABLE keys are unchanged. Verified live on the robot: reload loads all 23 skills, pick_any_object acks its params through the lib imports, and gripper_close/gripper_open ran end-to-end via the skill CLI (claw physically cycled and returned to open).
refactor(skills): shared skill_lib for arm primitives + camera geometry
Replace the direct google-genai client in pick_any_object with the Innate
ProxyClient. Vision requests now go to the proxy's OpenAI-compatible endpoint
(service "gemini", POST /v1/chat/completions) with the image as a data URI —
the proxy holds the upstream API key; the robot authenticates with its
service key. Drops the GEMINI_API_KEY load path and the genai dependency from
this skill.
Verified against the live proxy that the service name, endpoint, and vision
body shape are all accepted (they pass path validation and body parsing and
reach the proxy's auth check). The native Gemini REST path
(models/{model}:generateContent) is NOT usable through the proxy — it rejects
the ":" in the path — so OpenAI-compat is the route.
KNOWN BLOCKER (deploy step, not a code issue): this robot's service key is
not yet authorized for the "gemini" service — the proxy returns 403 until an
admin grants access. Detection will fail with 403 until then; grant the key
"gemini" access, then verify a real pick end-to-end.
feat(pick): route vision calls through the Innate proxy
…ation refactor
pick_any_object:
- _grasp_verified now sends BOTH head + wrist cameras to Gemini so a
held object (visible in the mirrored wrist view) isn't misread as
dropped; degrades to head-only if the wrist frame is absent.
- cancellation via SkillCancelled + _checkpoint() replaces the
cancelled-as-None plumbing (one raise, one except in execute); fixes
cancel-during-search being misreported as FAILURE.
- extracted the pure vision math and proxy vision call into
workspace/skill_lib/{vision,gemini}.py; gemini.ask_image now takes
one image or a list (multi-image support).
Also folded in (per 'everything'):
- pyright: return-type annotations on Skill base (types.py) +
pyrightconfig.json; /etc/innate.env perms enforcement in
post_update.sh; webapp pick tuning panel rename to pickOverlay.js;
in-progress pyright cleanups across other innate_skills.
The pickOverlay rewrite pinned the boxes to hardcoded TUNABLE defaults; the old panel drew them from params synced off the debug topic (and drag published updates back). Any live-tuned value left the drawn box lying about where the skill actually aims. Seed a live P dict with the defaults and resync it from every run_start and params event (the skill broadcasts its full dict on both), including mid-run tuning acks. Non-numeric payload values are ignored. Verified headlessly (node + DOM stubs, real module): box lands within 0.05px of Python floor_to_pixel for default and tuned params, moves on a mid-run params ack, size follows box_half_px, hides on run_end.
…, overlay params - post_update: rename superseded workspace/custom_skills copies (arm_rest_position, gripper_open, gripper_close, pick_any_object) to *.py.superseded on update, so the gitignored local/ copy can't shadow the now-shipped skill via catalog precedence - ruff check + ruff format clean (CI format check); 'workspace' added to isort known-first-party - pickOverlay: draw boxes from the skill's broadcast params (run_start/params debug events) instead of hardcoded mirrors of TUNABLE — live tuning now redraws - drop orphaned .arm-follower CSS (the readout UI is not in this PR) - pick_any_object: guard head interface at entry like manipulation/mobility; replace -O-strippable assert in _sweet_box with a raise - catalog: document the reload_selective two-lib-copies invariant
custom_skills/ holds user-authored skills only. These four skills were never tracked outside this PR, so no release ever shipped them under the legacy skills/ dir and the skills/ -> custom_skills/ migration could not have deposited them. A copy only exists where someone hand-placed one, so the migration was dead code on every robot it would ever run on. Dev robots used during development do have the stale copies; those get removed directly rather than via a shipped migration.
Drop comments that narrate the code, recount how a constant was arrived at, or duplicate a docstring that lives with the implementation. Keep the ones stating constraints the code can't show (signed wrist gains, close_strength trip limit, color-model-not-LK, no cancel checkpoint mid-grip). Also repoint pickOverlay.js at skill_lib/geometry.py for the camera geometry it mirrors — it still named pick_any_object.py, where that code no longer is.
_open_gripper_checked discarded open_checked's result, so a gripper that stayed shut still went on to the wrist descent and close/twist/lift, squeezing an already-tripped servo. Return the result and abort with ArmUnhealthy instead. open_checked could not report that failure either: after the reboot+retry it returned the retry command's own status, and a tripped servo accepts the command and no-ops silently — which is the whole reason the j6 check exists. Re-verify j6 after the retry so the function keeps the promise in its docstring. Healthy, recovers-after-reboot, and no-j6-reading paths are unchanged.
…the skill's box
- pickPanel.js: per-stage sliders for all 45 pick_any_object TUNABLE knobs,
toggled with 'o' (button next to profiling). Each slider publishes a one-key
partial dict on /pick_any_object/tuning; the skill's params ack drives the
displayed values. On open the panel publishes {} — a pure echo request — so
it syncs to the skill's live values with no defaults mirrored in JS; sliders
stay disabled until that first ack.
- pickOverlay.js: the head-camera pick box is now drawn from the skill's `box`
telemetry ([cu,cv,half,accept] on run_start/params/servo/position) instead
of reprojecting sweet_x/box_y/tilt_deg through a JS copy of the camera
model — camPose/floorToPixel and the mirrored URDF constants are gone. The
wrist goal box is draggable when the arm camera is primary: dropping it
publishes wrist_box_u/v as a live tuning override (throttled ~8 Hz while
dragging, final on release; param echoes are ignored mid-drag so the ack of
an in-flight value can't yank the box). Marker DOM is built by one helper.
- arm_rest_position: drop the redundant __init__ — the Skill base class
creates the cancel latch lazily and its setter ignores False.
_rest_arm(keep_grip=False) sent all-zero joints, a completely different posture from the documented folded rest that risks driving the gripper into the floor. REST_POSITION moves to skill_lib/arm.py (single source of truth, hot-reloads with the lib) and both arm_rest_position and the pick skill's fold-home now use it. A sub-skill call was not an option here: after a cancel the invoker short-circuits child runs, so the finally-block rest would never execute. Also cap the close_strength slider at 0.6 — both arm.py and the TUNABLE default agree >~0.6 overcurrent-trips the gripper servo.
The wrist-servo descent was failing on ~0.051m FK residuals against a 0.05m bar — a marginal miss the reboot recovery can't fix, aborting the whole pick. 0.07m gives margin at the arm's stretched reach poses.
Bench testing found 0.8 grips more reliably than 0.6. Raises the default, the panel slider cap, and updates the trip-warning comments that previously called 0.6 the ceiling.
Move the odometry-closed rotate/drive loops and pixel P-servo out of pick_any_object into skill_lib/mobility.py (interfaces passed explicitly, gains as kwargs, optional cancel predicate + dbg hook), mirroring arm.py. Also fold the arm rest loop into armlib.rest and switch move_checked to per-axis tolerances (tol_xy/tol_z).
…/innate-os into theo/pick-wrist-servo # Conflicts: # webapp/js/teleop/pickOverlay.js # workspace/innate_skills/arm_rest_position.py # workspace/innate_skills/pick_any_object.py # workspace/skill_lib/arm.py
…ch-pose move, strength docs - _rest_arm(keep_grip=False) folds to armlib.REST again instead of the all-zero posture; the earlier fix (e407a38) was clobbered by the robot-side wip commit. Zero-config after a failed descent can sweep the gripper through the floor. - _goto_search_pose now goes through armlib.go so a rejected search-pose command raises ArmFailed instead of silently corrupting the wrist descent from the REST position. - gripper_close.py strength docstring updated to the live-tested ~0.8 ceiling (was a stale <=0.6, contradicting arm.py and the tuned defaults). Slider max (0.8) and TUNABLE default (0.70) stay as tuned. - pickTunePanel.js comments flagged by Greptile are moot: the panel file was removed in favor of pickOverlay.js. - The unchecked-gripper-open finding was already fixed at HEAD.
Drop the slider panel (wrench button / 'o' toggle) for live-tuning pick_any_object params — we no longer want to expose that option. The pick overlay and the skill-side tuning topic stay, since the overlay's draggable wrist box still publishes to it.
Greptile SummaryThis PR adds a visual pick-and-grasp workflow and shared robot-control helpers. The main changes are:
Confidence Score: 4/5The pick workflow can remain cancelled across runs, retain stale ROS callbacks after reload, and consume unsafe tuning values.
workspace/innate_skills/pick_any_object.py
|
| Filename | Overview |
|---|---|
| workspace/innate_skills/pick_any_object.py | Adds the pick workflow, but run cancellation state, ROS subscription cleanup, and tuning validation can fail during normal use. |
| webapp/js/teleop/pickOverlay.js | Adds camera-aware aim overlays, wrist-target dragging, and browser-side cleanup. |
| workspace/skill_lib/arm.py | Adds shared arm movement, gripper, recovery, and verification helpers. |
| ros2_ws/src/brain/brain_client/brain_client/skills/catalog.py | Evicts cached skill-library modules before full and selective skill reloads. |
| scripts/update/post_update.sh | Normalizes ownership and permissions for the system environment file. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as Teleop overlay
participant ROS as ROS topics
participant Current as Current pick skill
participant Retired as Retired pick skill
participant Robot as Robot interfaces
UI->>ROS: Publish wrist tuning
ROS->>Current: _on_tuning(payload)
ROS->>Retired: Stale _on_tuning(payload)
Current->>ROS: Params event
Retired->>ROS: Competing params event
Current->>Robot: Motion using live values
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as Teleop overlay
participant ROS as ROS topics
participant Current as Current pick skill
participant Retired as Retired pick skill
participant Robot as Robot interfaces
UI->>ROS: Publish wrist tuning
ROS->>Current: _on_tuning(payload)
ROS->>Retired: Stale _on_tuning(payload)
Current->>ROS: Params event
Retired->>ROS: Competing params event
Current->>Robot: Motion using live values
Comments Outside Diff (3)
-
workspace/innate_skills/pick_any_object.py, line 1845 (link)Cancellation State Carries Between Runs
Unlike the other new arm skills, this entry point never resets
_cancelled. After one call tocancel(), the same singleton skill instance starts every later execution with the latch still set; the first cancellation checkpoint then aborts the new pick even though nobody cancelled that run. -
workspace/innate_skills/pick_any_object.py, line 1279 (link)Reload Leaves Stale ROS Callbacks
This subscription is created on the shared server node with a bound method, but the skill has no
shutdown()cleanup. Replacing the skill during reload therefore leaves the retired instance subscribed; later tuning messages update every old instance and emit competingparamsevents while the subscriptions and instances accumulate. -
workspace/innate_skills/pick_any_object.py, line 1287 (link)Unbounded Tuning Reaches Motion Code
Any ROS publisher can replace motion and timing parameters with unrestricted numeric values. Inputs such as negative sleep durations, zero or negative step counts, non-finite gains, or reversed velocity bounds then flow into sleeps, loops, and physical-motion helpers, causing exceptions, skipped positioning, or incorrectly directed movement.
Reviews (1): Last reviewed commit: "Remove pick tuning panel from teleop coc..." | Re-trigger Greptile
|
Folded into #542 (merge |
Summary
webapp/js/teleop/pickPanel.js— the per-stage slider panel (wrench button,oshortcut) for live-tuningpick_any_objectparams; we no longer want to expose this optionmain.js.picktune-panel*) fromapp.cssLeft intact on purpose: the pick overlay (
pickOverlay.js) and its.picktune-*CSS, thePICK_DEBUG_TOPIC/PICK_TUNING_TOPICconstants, and the skill-side tuning handler — the overlay's draggable wrist box still uses them.Test plan
grep -rn 'pickPanel\|picktune-panel\|createPickPanel' webapp/returns nothingono longer toggles anything; pick overlay still works