Skip to content
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

[models] Move allegro_hand_description into drake_models #21184

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Mar 24, 2024

Towards #13942.

Twin PR: RobotLocomotion/models#40


This change is Reviewable

Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

+@rpoyner-tri for both reviews, please +(status: single reviewer ok).

Reviewable status: 1 unresolved discussion, LGTM missing from assignee rpoyner-tri(platform)


examples/allegro_hand/test/parse_test.cc line 22 at r2 (raw file):

TEST_P(ParseTest, Quantities) {
  const std::string file_extension = GetParam();
  const std::string url_right =

FYI This test file is moved and edited, not created from scratch, but sadly git isn't noticing the rename.

The only change to the test was fixing these package URLs.


tools/workspace/drake_models/repository.bzl line 9 at r1 (raw file):

        name = name,
        repository = "RobotLocomotion/models",
        commit = "bc0447998cbba9046323581775d4ea7f5d00a1a2",

Working

Replace with the real commit sha from master, once we have it.

Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri 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: 2 unresolved discussions, LGTM missing from assignee rpoyner-tri(platform)


BUILD.bazel line 43 at r2 (raw file):

        "//bindings/pydrake/multibody:models",
        "//examples/acrobot:models",
        "//examples/allegro_hand:models",

nit Actually this line doesn't accomplish anything. I'll remove it in the next push.

@jwnimmer-tri jwnimmer-tri force-pushed the mv-allegro-hand-models branch from 0d11bc2 to 5a5523b Compare March 24, 2024 19:04
@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review March 24, 2024 19:04
Copy link
Contributor

@rpoyner-tri rpoyner-tri 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 12 of 14 files at r1, 2 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: 2 unresolved discussions


tools/workspace/drake_models/test/parse_test.py line 65 at r3 (raw file):

            "package://drake_models/atlas/robotiq.urdf",
            "package://drake_models/atlas/robotiq_simple.urdf",
            # We don't any tracking issue for fixing the allegro models,

typo

Suggestion:

don't have any 

@jwnimmer-tri jwnimmer-tri force-pushed the mv-allegro-hand-models branch from 5a5523b to 754a78a Compare March 25, 2024 16:00
@jwnimmer-tri jwnimmer-tri force-pushed the mv-allegro-hand-models branch from 754a78a to 874e328 Compare March 25, 2024 16:08
Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignee rpoyner-tri(platform)

@jwnimmer-tri jwnimmer-tri merged commit 02992bd into RobotLocomotion:master Mar 25, 2024
9 of 10 checks passed
@jwnimmer-tri jwnimmer-tri deleted the mv-allegro-hand-models branch March 25, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low release notes: fix This pull request contains fixes (no new features) status: single reviewer ok https://drake.mit.edu/reviewable.html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants