[workspace] Upgrade drake_models to latest commit

[jwnimmer-tri]

Towards #13942.

Twin PR: RobotLocomotion/models#48

Unlike other model-moves I've done, here we are only bumping the models sha, not actually deleting the duplicated files yet. Because there are so many places that use these models, it will be a lot easier to bump the sha, then fix all of the call sites, then delete this repo's copy of the files.

---

This change is 

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers

---

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

        name = name,
        repository = "RobotLocomotion/models",
        commit = "7ede868f6dd76f1a68c9dc34da991e16ba61dfca",

Working

Needs a real sha from master

[jwnimmer-tri]

Anzu CI passed

[jwnimmer-tri]

Reviewed 1 of 1 files at r1, 19 of 20 files at r2, 9 of 9 files at r3, all commit messages.
Reviewable status: 1 unresolved discussion, needs platform reviewer assigned, needs at least two assigned reviewers

[jwnimmer-tri]

+@rpoyner-tri for feature review (to match the twin PR).

[rpoyner-tri]

Reviewed 1 of 1 files at r1, 19 of 20 files at r2, 9 of 9 files at r3, all commit messages.
Reviewable status: 2 unresolved discussions, needs at least two assigned reviewers

---

manipulation/models/README.md line 11 at r3 (raw file):

# Drake Developers

Do not many any further changes to these model files.

typo

Suggestion:

Do not make any 

[jwnimmer-tri]

Reviewed 8 of 8 files at r4, all commit messages.
Reviewable status: needs at least two assigned reviewers

---

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

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

Needs a real sha from master

Done

[jwnimmer-tri]

+@ggould-tri for platform review per schedule, please (high priority).

[rpoyner-tri]

Reviewed 1 of 8 files at r4.
Reviewable status: LGTM missing from assignee ggould-tri(platform)

[ggould-tri]

Reviewed 16 of 20 files at r2, 5 of 9 files at r3, 8 of 8 files at r4, all commit messages.
Reviewable status: 2 unresolved discussions

---

examples/manipulation_station/manipulation_station.cc line 466 at r5 (raw file):

        sdf_url, "iiwa", plant_->world_frame(), "iiwa_link_0", X_WI, plant_);
    RegisterIiwaControllerModel(
        PackageMap{}.ResolveUrl(sdf_url), iiwa_instance, plant_->world_frame(),

minor: This curious idiom is rule-of-three, suggesting that we probably need a proper named function for it.

Code quote:

PackageMap{}.ResolveUrl

---

manipulation/models/jaco_description/BUILD.bazel line 53 at r5 (raw file):

# === test/ ===

drake_cc_googletest(

Here, and 3x below: This test goes away here, but does not (and afaict cannot) appear in the models repo. Is this a deliberate loss of test coverage?

It seems like the test could probably be relocated rather than killed outright.

[jwnimmer-tri]

Reviewable status: 2 unresolved discussions

---

manipulation/models/jaco_description/BUILD.bazel line 53 at r5 (raw file):

Previously, ggould-tri wrote…

Here, and 3x below: This test goes away here, but does not (and afaict cannot) appear in the models repo. Is this a deliberate loss of test coverage?

It seems like the test could probably be relocated rather than killed outright.

The https://github.com/RobotLocomotion/drake/blob/master/tools/workspace/drake_models/test/parse_test.py checks that all models in the drake_models repo parse without any warning or errors. That covers the majority of what was being tested in these programs.

There were a few tests that count how many bodies etc. are in the model (like the jaco one here), and it's true that we've lost those. I'd argue that they were unnecessary in the first place.

If you think not, I'll open a tracking issue to resurrect those tests in the models repo. (It does have CI, and we do plant to do parse-checks there RobotLocomotion/models#50.)

[jwnimmer-tri]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: 1 unresolved discussion

---

examples/manipulation_station/manipulation_station.cc line 466 at r5 (raw file):

Previously, ggould-tri wrote…

minor: This curious idiom is rule-of-three, suggesting that we probably need a proper named function for it.

I'd argue that this is already exactly the function we need -- we want to look up the given URL, using the default package map.

Anyway, the real problem is the poor API design in this class, so I've added a TODO. I'm not inclined to invest too much time in this file, since the plan is to delete/rewrite it from scratch.

[ggould-tri]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),ggould-tri(platform)

---

manipulation/models/jaco_description/BUILD.bazel line 53 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

The https://github.com/RobotLocomotion/drake/blob/master/tools/workspace/drake_models/test/parse_test.py checks that all models in the drake_models repo parse without any warning or errors. That covers the majority of what was being tested in these programs.

There were a few tests that count how many bodies etc. are in the model (like the jaco one here), and it's true that we've lost those. I'd argue that they were unnecessary in the first place.

If you think not, I'll open a tracking issue to resurrect those tests in the models repo. (It does have CI, and we do plant to do parse-checks there RobotLocomotion/models#50.)

OK: Seems fine to me, the remaining test coverage you describe seems adequate.