Skip to content

feat(control): publish accepted hardware position commands - #4391

Closed
TomCC7 wants to merge 6 commits into
cc/feat/webcam-capture-configfrom
cc/feat/accepted-position-commands
Closed

TomCC7 wants to merge 6 commits into
cc/feat/webcam-capture-configfrom
cc/feat/accepted-position-commands

Conversation

@TomCC7

@TomCC7 TomCC7 commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

What is this feature?

Publish applied_joint_position_command JointState events after successful POSITION/SERVO_POSITION hardware writes. Events contain only accepted joint targets and the control snapshot timestamp; publication occurs outside the hardware lock.

Why do we need this?

Dataset actions need accepted command history rather than measured state or commands that hardware rejected. This is a prerequisite of #4343, stacked after #4390 and immediately before the integration PR.

How to Test

uv run pytest dimos/control/test_control.py dimos/control/test_per_robot_joint_states.py

97 control tests passed, including accepted/rejected writes and sparse target publication; commit hooks passed. Real hardware execution has not been verified.

Which issue(s) does this PR close?

None. This is a review slice of #4343, following the Manipulation SDK preview. It does not close or replace the integration PR.

Checklist

  • I have read and approved the CLA.

AI assistance: Codex assisted with the split, validation, and description.

Inherited review fixes

Inherit the #4395 complete-dataset publication and inspection fixes, and the #4396 recorder-local port declarations. Core Module and Blueprint definitions are unchanged, and the collection stack audit script is removed. This layer retains its existing control behavior and PR base.

The final assembled stack passed 278 host tests with two native-recorder tests excluded by the default marker filter. The LeRobot dataprep and MCAP suite passed 28 tests using cached LeRobot 0.6.0 and datasets 4.7.0; this is not a full locked-environment run. Scoped mypy passed for six scoped production files, and changed-file pre-commit passed. No dependencies were installed, no hardware was operated, and CI has not been checked.

Inherited training-key conversion

Inherit #4395's fixed LeRobot feature naming and MCAP-to-dataset E2E regression. Profiles retain their source declarations; conversion produces standard training-facing keys and records the mapping for inspection. No rename configuration is introduced. The final stack passed 278 host tests and 28 conversion/MCAP tests in the documented cached environment.

Rebased stack integration

Inherit #4395/#4396 rebased onto main, the recorder lifecycle fix, timestamp-boundary regression, and the Python-generated profile-to-dataset E2E. This layer keeps its original changes and existing history.

Current validation: 280 host tests passed with two pre-existing native-recorder tests excluded by the host marker filter. Thirty LeRobot conversion/E2E tests passed, including both Python-generated profile cases. Python tests used cached LeRobot 0.6.0 and datasets 4.7.0, not a complete locked isolated environment. The updated Python E2E passed scoped mypy; pre-commit passed. No physical hardware or policy training was used. CI has not been polled.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

go2 replay realtime

Details
Benchmark suite Current: 1d029a8 Previous: cdfaef0 Ratio
first frame wall 9.3 s 9.891 s 0.94
first frame cpu 18.938 s 21.456 s 0.88
run cpu 29.88 s 33.75 s 0.89
run cpu (user) 22.164 s 26.257 s 0.84
run cpu (system) 7.715 s 7.493 s 1.03
peak memory 2712.355 MB 2699.734 MB 1.00
peak threads 404 threads 394 threads 1.03
disk read 8.137 MB 20.871 MB 0.39
disk write 11.137 MB 11.098 MB 1.00
network (transport) 2713.989 MB 2722.56 MB 1.00
network (external rx) 0.114 MB 0.139 MB 0.82
network (external tx) 0.154 MB 0.342 MB 0.45

This comment was automatically generated by workflow using github-action-benchmark.

@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
6866 1 6865 59
View the top 1 failed test(s) by shortest run time
dimos.codebase_checks.test_blueprint_kwargs::test_blueprint_atom_kwargs_match_module_config[learning-collect-webxr-piper]
Stack Traces | 0.005s run time
blueprint_name = 'learning-collect-webxr-piper'
monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x170499f90>

    @pytest.mark.parametrize("blueprint_name", _blueprint_params())
    def test_blueprint_atom_kwargs_match_module_config(
        blueprint_name: str, monkeypatch: pytest.MonkeyPatch
    ) -> None:
        """Fail when blueprint kwargs cannot be consumed by their target module."""
        # The multi-robot blueprints read ROBOT_IPS at import time.
        monkeypatch.setattr(global_config, "robot_ips", "192.0.2.10,192.0.2.11")
        blueprint = _get_blueprint_or_skip(blueprint_name)
    
        violations: list[str] = []
        for atom in blueprint.blueprints:
            unknown_kwargs = sorted(set(atom.kwargs) - _allowed_kwarg_names(atom.module))
            if unknown_kwargs:
                violations.append(
                    f"{atom.module.__module__}.{atom.module.__name__}: unknown kwargs {unknown_kwargs}"
                )
    
        if violations:
            listing = "\n".join(f"  - {violation}" for violation in violations)
>           raise AssertionError(
                f"Blueprint {blueprint_name!r} passes unknown module kwargs:\n{listing}\n\n"
                "Blueprint kwargs are forwarded into the module constructor. For modules "
                "with an explicit `config` annotation, use fields from that config model; "
                "for legacy modules with direct constructor parameters, use the declared "
                "`__init__` keyword names."
            )
E           AssertionError: Blueprint 'learning-collect-webxr-piper' passes unknown module kwargs:
E             - dimos.imitation.collection.recorder.CollectionRecorder: unknown kwargs ['input_types', 'recording_schema']
E           
E           Blueprint kwargs are forwarded into the module constructor. For modules with an explicit `config` annotation, use fields from that config model; for legacy modules with direct constructor parameters, use the declared `__init__` keyword names.

atom       = BlueprintAtom(kwargs={'enable_pointcloud': False}, module=<class 'dimos.hardware.sensors.camera.realsense.camera.RealS...me='tf', type=<class 'dimos.msgs.tf2_msgs.TFMessage.TFMessage'>, direction='out')), module_refs=(), instance_name=None)
blueprint  = Blueprint(blueprints=(BlueprintAtom(kwargs={'input_types': {'color_image': <class 'dimos.msgs.sensor_msgs.Image.Image'...'left_gripper_command'): 'left_gripper_command'}), requirement_checks=(), configurator_checks=(), hosted_placements=())
blueprint_name = 'learning-collect-webxr-piper'
listing    = "  - dimos.imitation.collection.recorder.CollectionRecorder: unknown kwargs ['input_types', 'recording_schema']"
monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x170499f90>
unknown_kwargs = []
violations = ["dimos.imitation.collection.recorder.CollectionRecorder: unknown kwargs ['input_types', 'recording_schema']"]

dimos/codebase_checks/test_blueprint_kwargs.py:97: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

go2 replay realtime (arm64)

Details
Benchmark suite Current: 55e5f9e Previous: d793e80 Ratio
peak memory 1740.984 MB 1716.062 MB 1.01
peak threads 374 threads 367 threads 1.02
network (transport) 2730.918 MB 2724.058 MB 1.00
disk write 1.684 MB 1.738 MB 0.97
instructions 137.561 G 137.102 G 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@TomCC7
TomCC7 changed the base branch from main to cc/feat/webcam-capture-config October 6, 2026 09:22
@TomCC7
TomCC7 added this pull request to stack #4448 October 6, 2026 09:22
@TomCC7
TomCC7 force-pushed the cc/feat/webcam-capture-config branch from b780b3a to 7468103 Compare October 28, 2026 04:52
@TomCC7
TomCC7 force-pushed the cc/feat/accepted-position-commands branch from eea747e to 3bb478a Compare October 28, 2026 04:52
Consolidate reviewed layer changes from eea747e.
Preserve the existing PR history and inherit the updated parent layer.
@TomCC7

TomCC7 commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

I think we already have support for this in main?

@TomCC7 TomCC7 closed this Oct 28, 2026
@TomCC7
TomCC7 removed this pull request from stack #4448 October 28, 2026 19:43

This branch was previously deployed

1 inactive deployment
cachix — 55e5f9e8 Deployed Oct 28, 2026 by TomCC7 via cachix-build #14735
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