[geometry] Add MeshcatParams option for initial_properties #20920
Conversation
jwnimmer-tri
added
status: do not merge
priority: medium
status: do not review
release notes: feature
jwnimmer-tri
force-pushed
the
meshcat-initial-properties
branch
from
805d4b9
to
42dc777
Compare
jwnimmer-tri
force-pushed
the
meshcat-initial-properties
branch
2 times, most recently
from
16c2cbd
to
677f013
Compare
jwnimmer-tri
force-pushed
the
meshcat-initial-properties
branch
2 times, most recently
from
4892c0e
to
aef09f9
Compare
There was a problem hiding this comment.
Reviewed 3 of 7 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers
There was a problem hiding this comment.
+@SeanCurtis-TRI for feature review.
Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform), needs at least two assigned reviewers
There was a problem hiding this comment.
+@rpoyner-tri for out of band platform review, please.
Reviewed 3 of 7 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee rpoyner-tri(platform)
geometry/meshcat_params.h line 70 at r2 (raw file):
}; /** Configures the initial conditions for Meshcat. These properties will be
nit: I suspect this documentation needs to offer more. It seems that the average user is not one who has looked at either meshcat or drake::...::Meshcat so, understanding the semantics of this particular set of fields may be overly opaque.
Ideally a cross reference to "setting properties for meshcat" would be best. But, for now, I suspect people won't even have a hint of how they would successfully use this.
jwnimmer-tri
force-pushed
the
meshcat-initial-properties
branch
from
aef09f9
to
23ed212
Compare
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee rpoyner-tri(platform)
geometry/meshcat_params.h line 70 at r2 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
nit: I suspect this documentation needs to offer more. It seems that the average user is not one who has looked at either
meshcatordrake::...::Meshcatso, understanding the semantics of this particular set of fields may be overly opaque.Ideally a cross reference to "setting properties for meshcat" would be best. But, for now, I suspect people won't even have a hint of how they would successfully use this.
I added more words.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform)
geometry/meshcat_params.h line 70 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
I added more words.
I like the change - it tightens Drake's meshcat ecosystem, but it doesn't quite touch the heart of my concern. But I realized that the heart of my concern belongs outside this PR. As ever, how does a user discover what a meaningful tuple would be?
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform)
There was a problem hiding this comment.
Reviewed 1 of 7 files at r1, 6 of 6 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status:complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)
Towards #20927.
This works with Python YAML loading, but not yet C++ YAML loading. C++ loading support will appear in a follow-up.
This change is