Skip to content

IiwaDriver: Create controller MBP from scenario file #317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 7, 2024

Conversation

siddancha
Copy link
Contributor

@siddancha siddancha commented Jun 6, 2024

Fixes #316

  • Create the controller plant for the IiwaDriver by reusing the user-provided scenario, instead of creating a custom iiwa (hardcoded to iiwa7) and the WSG schunk.
  • This is enabled by being able to freeze child instances i.e. removing non-weld joints in the child instance and replacing them with weld joints. This is in-turn made possible by this recently landed PR: ([multibody] Adds RemoveJoint() RobotLocomotion/drake#21397)

This change is Reviewable

This is enabled by being able to freeze child instances
@siddancha
Copy link
Contributor Author

siddancha commented Jun 6, 2024

There is currently an issue with plant.GetJointActuatorIndices(child_instance) that returns an empty list. Therefore, the actuators aren't actually removed in:

# remove joint actuators
for actuator_index in plant.GetJointActuatorIndices(child_instance):  # list is empty
    actuator = plant.get_joint_actuator(actuator_index)
    plant.RemoveJointActuator(actuator)

I needed to explicitly add the following to get it to work locally:

for actuator in [
        plant.GetJointActuatorByName('left_finger_sliding_joint', child_instance),
        plant.GetJointActuatorByName('right_finger_sliding_joint', child_instance)
    ]:
    plant.RemoveJointActuator(actuator)

Seems like a potential bug MultibodyPlant::GetJointActuatorIndices(child_instance), and is what is causing the tests to fail. Will dig into this and potentially raise an issue on the drake's Github.

@siddancha
Copy link
Contributor Author

Added GetJointActuatorIndices bug report and a simple reproduction here: RobotLocomotion/drake#21547

@siddancha
Copy link
Contributor Author

siddancha commented Jun 7, 2024

Realized from bug report (RobotLocomotion/drake#21547) that GetJointActuatorIndices(model_instance) cannot be used pre-finalize. Using @jwnimmer-tri 's solution to call plant.GetJointActuatorIndices() and remove the actuators corresponding to the joints to be removed.

@siddancha siddancha marked this pull request as ready for review June 7, 2024 05:06
siddancha added 2 commits June 7, 2024 01:16
Also make this slightly more efficient
@RussTedrake
Copy link
Owner

thanks! ironically, i already did all this in https://github.com/RussTedrake/manipulation/blob/master/book/mobile/exercises/mobile_base_ik.ipynb
will review this morning.

Copy link
Owner

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

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

one small request. otherwise LGTM. Thanks!

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @siddancha)


manipulation/station.py line 395 at r3 (raw file):

        frame_on_child = joint.frame_on_child()
        plant.RemoveJoint(joint)
        plant.WeldFrames(frame_on_parent, frame_on_child)

per my mobile_ik example, I preferred the extra line using the WeldJoint constructor so that I could keep the joint name the same.

Also, if there are mimic joints anywhere in the model, this will throw. But I think that's ok for now.

Copy link
Contributor Author

@siddancha siddancha left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @RussTedrake)


manipulation/station.py line 395 at r3 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

per my mobile_ik example, I preferred the extra line using the WeldJoint constructor so that I could keep the joint name the same.

Also, if there are mimic joints anywhere in the model, this will throw. But I think that's ok for now.

Makes sense! I just switched to the WeldJoint constructor.

@siddancha siddancha requested a review from RussTedrake June 7, 2024 11:17
Copy link
Owner

@RussTedrake RussTedrake left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @siddancha)


pyproject.toml line 5 at r4 (raw file):

# Use e.g. 2023.10.4.rc0 if I need to release a release candidate.
# Use e.g. 2023.10.4.post1 if I need to rerelease on the same day.
version = "2024.06.07"

btw -- i presume that this means you would like me to roll some pip wheels once this lands?

@RussTedrake RussTedrake merged commit 9223750 into RussTedrake:master Jun 7, 2024
5 of 6 checks passed
Copy link
Contributor Author

@siddancha siddancha left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion


pyproject.toml line 5 at r4 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

btw -- i presume that this means you would like me to roll some pip wheels once this lands?

If it's not a hassle/concern? Otherwise I can just also do pip install git+https://github.com/RussTedrake/manipulation@master and not rely on PyPI releases.

@siddancha siddancha deleted the support-iiwa-14 branch June 7, 2024 11:36
@RussTedrake
Copy link
Owner

pyproject.toml line 5 at r4 (raw file):

Previously, siddancha (Siddharth Ancha) wrote…

If it's not a hassle/concern? Otherwise I can just also do pip install git+https://github.com/RussTedrake/manipulation@master and not rely on PyPI releases.

Done. (i've pushed to pypi)

Copy link
Contributor Author

@siddancha siddancha left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


pyproject.toml line 5 at r4 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

Done. (i've pushed to pypi)

Thank you!

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.

IiwaDriver in sim does not support iiwa14
2 participants