RussTedrake / manipulation

Course notes for MIT manipulation class
BSD 3-Clause "New" or "Revised" License
392 stars 115 forks source link

Switch to Drake's `AddSimIiwaDriver` and enable missing control modes for iiwa hardware #293

Closed nepfaff closed 3 months ago

nepfaff commented 4 months ago

So far, we ignored the iiwa driver control_mode. This PR uses the control mode when setting up LCM connections with the iiwa hardware. In particular, iiwa.position is no longer exported when the iiwa is in torque_only mode.

Note that this does not implement a simulated version for non-default (position_and_torque) control modes but adds a warning instead.

Question: This seems to be duplicating much of what BuildIiwaControl is doing. Is there a reason for this?


This change is Reviewable

nepfaff commented 4 months ago

+@RussTedrake for review please. This change is breaking if someone specified torque_only mode in their scenario file (very unlikely).

nepfaff commented 4 months ago

I talked with Jeremy, and we don't want to use BuildIiwaControl in its current state for hardware control. So this change is needed

nepfaff commented 4 months ago

We should be able to use Drake's AddSimIiwaDriver as proposed here. However, I'm struggling with the binding, so blocked until TRI is back in the office to help.

RussTedrake commented 4 months ago

I can likely help with the binding…?

On Sat, Feb 24, 2024 at 8:04 PM, Nicholas Pfaff @.***> wrote:

We should be able to use Drake's AddSimIiwaDriver https://github.com/RobotLocomotion/drake/blob/master/manipulation/kuka_iiwa/build_iiwa_control.cc#L147 as proposed here. However, I'm struggling with the binding, so blocked until TRI is back in the office to help.

— Reply to this email directly, view it on GitHub https://github.com/RussTedrake/manipulation/pull/293#issuecomment-1962775262, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRE2NHWQCXIUAKAWAPTJBDYVKEZFAVCNFSM6AAAAABDT2NTSOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSG43TKMRWGI . You are receiving this because you were assigned.Message ID: @.***>

nepfaff commented 4 months ago

I expect this to be fine once the Drake PR has landed and the Drake nightly version has been added here. Both simulation and hardware work beautifully locally.

nepfaff commented 4 months ago

This should be ready now s.t. being able to peg the correct drake version. It might have broken some of the manipulation course notebooks due to the new default port naming/ port presence:

nepfaff commented 4 months ago

Everything is passing locally now. Should be ready for review.

RussTedrake commented 4 months ago

fyi -- i've just added you to the manipulation-solutions repo. all of the notebooks here that are in an exercises subfolder are actually installed from the solutions repo, so also need to change upstream. The recommendation is that you check out the manipulation-solutions repo into e.g. a solutions subdirectory of this repo (so that bazel runs those tests, too.

RussTedrake commented 4 months ago

did you manage to make the updates in solutions for me, too?

nepfaff commented 4 months ago

did you manage to make the updates in solutions for me, too?

As per slack, I never received the invitation. Would you mind re-sending it?

nepfaff commented 3 months ago

Should we merge this now, or wait? (I've recently fixed the CI failures on mac, but by upgrading to sonoma; the monterey CI will continue to fail).

_Reviewable_ status: 24 of 25 files reviewed, all discussions resolved (waiting on @nepfaff)

Looks like I can access the solution repo. I just didn't get a notification and wasn't sure where it was located. I should probably fix the solution repo before merging this. What is the usual workflow here? I have this on my backlog, but I'm quite swamped at the moment. I should be able to do it in two weeks.

RussTedrake commented 3 months ago

The mac failure is, I think, not caused by you. The pip failure is because I'll need to roll new pip wheels after this merges.