Skip to content

fix(control): clear completed JTT state and remove idle holding - #3969

Queued
TomCC7 wants to merge 3 commits into
mainfrom
cc/fix/jtt-preempt
Queued

fix(control): clear completed JTT state and remove idle holding#3969
TomCC7 wants to merge 3 commits into
mainfrom
cc/fix/jtt-preempt

Conversation

@TomCC7

@TomCC7 TomCC7 commented Sep 8, 2026

Copy link
Copy Markdown
Member

Contribution path

Fixes a regression introduced in #3610.

Problem

  • JTT reused stale commanded positions after another task moved the joints, causing dangerous jumps on the next execution.
  • Idle holding duplicated the shared hardware interface's handling of retained joint targets.

Solution

  • Clear each joint's commanded state after its final output; initialize new executions from measured positions.
  • Keep commanded-state continuity during active execution and trajectory replacement.
  • Remove hold_position_when_idle and its G1 configuration.
  • Add regression tests verifying JTT does not pull joints back after another task moves them.

How to Test

  • Manually verified on hardware: teleop followed by trajectory execution no longer produces the dangerous motion.
  • Verified trajectory completion and teleop handoff in G1 MuJoCo simulation.
  • 128 unit tests passed:
uv run pytest dimos/control/tasks/trajectory_task/test_trajectory_task.py dimos/control/test_control.py dimos/control/test_coordinator_routing.py -q

AI assistance

Codex (GPT-6): diagnosis, implementation, tests, and PR description.

Checklist

  • I have read and approved the CLA.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.48485% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...trol/tasks/trajectory_task/test_trajectory_task.py 98.30% 0 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main    #3969      +/-   ##
==========================================
- Coverage   77.91%   77.91%   -0.01%     
==========================================
  Files        1398     1399       +1     
  Lines      132268   132293      +25     
  Branches    11487    11484       -3     
==========================================
+ Hits       103061   103079      +18     
- Misses      25943    25949       +6     
- Partials     3264     3265       +1     
Components Coverage Δ
Tests 94.72% <98.30%> (-0.01%) ⬇️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.26% <98.48%> (+<0.01%) ⬆️
OS-ubuntu-latest 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.10 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.11 74.75% <98.48%> (+<0.01%) ⬆️
Py-3.12 74.75% <98.48%> (+<0.01%) ⬆️
Py-3.13 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.14 74.76% <98.48%> (+<0.01%) ⬆️
Py-3.14t 74.76% <98.48%> (+<0.01%) ⬆️
SelfHosted-Large 30.71% <18.18%> (-0.01%) ⬇️
SelfHosted-Linux 35.76% <19.69%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/control/tasks/trajectory_task/trajectory_task.py 89.20% <100.00%> (+0.51%) ⬆️
dimos/control/test_control.py 96.74% <ø> (-0.10%) ⬇️
...nitree/g1/blueprints/basic/unitree_g1_groot_wbc.py 58.33% <100.00%> (+0.34%) ⬆️
...trol/tasks/trajectory_task/test_trajectory_task.py 98.30% <98.30%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TomCC7 TomCC7 changed the title fix(control): clear completed JTT commands before teleop handoff fix(control): clear completed JTT state and remove idle holding Sep 8, 2026
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

No merge-blocking issue was established in the changed trajectory control logic.

T-Rex validation blocked

The focused trajectory lifecycle check and narrow control suite could not reach the changed code because the temporary environment was missing dimos_lcm and pydantic_settings.

Confidence Score: 5/5

No established issue blocks merging.

No severity-bearing findings were established.

Files Needing Attention: No specific file requires follow-up; the focused control suite should be rerun in an environment with the required Python dependencies.

T-Rex T-Rex Logs

What T-Rex did

  • A focused trajectory lifecycle harness, including a trajectory factory, completion, replacement, and preemption, was prepared for before-and-after execution.
  • The base harness import attempt halted during imports because dimos_lcm was unavailable, preventing any control behavior from running.
  • The updated harness import attempt halted during imports for the same dimos_lcm blocker, before pre-execution work occurred.
  • The narrow control suite loading stopped while loading test configuration because pydantic_settings was unavailable, preventing test collection.
  • The harness and validation runner artifacts were uploaded for review, including the harness source and the run-validation script.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "test(control): assert JTT continuity aft..." | Re-trigger Greptile

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
@TomCC7
TomCC7 added this pull request to the merge queue Sep 8, 2026
Any commits made after this event will not be merged.
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 8, 2026
@mustafab0
mustafab0 added this pull request to the merge queue Sep 8, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants