Teach QuadrotorPlant about SceneGraph #10776
Conversation
There was a problem hiding this comment.
+@sammy-tri for feature review, please.
Reviewable status: all discussions resolved, LGTM missing from assignee sammy-tri, platform LGTM missing (waiting on @sammy-tri)
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1.
Reviewable status: all discussions resolved, LGTM missing from assignee seancurtis-tri, platform LGTM from [sammy-tri] (waiting on @SeanCurtis-TRI)
examples/quadrotor/quadrotor_plant.h, line 51 at r1 (raw file):
geometry::SourceId source_id() const { return source_id_; } protected:
BTW not new in this PR but the protected section on a class which is marked final probably isn't all that meaningful.
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1.
Reviewable status: 1 unresolved discussion, platform LGTM from [sammy-tri, seancurtis-tri] (waiting on @RussTedrake)
examples/quadrotor/quadrotor_plant.h, line 18 at r1 (raw file):
/// The Quadrotor - an underactuated aerial vehicle. This version of the /// Quadrotor is implemented to match the dynamics of the plant specified in /// the `quadrotor.urdf` model file.
FYI This system doesn't have our wonderful @system doxygen, but has new a new port. Is that enough to motivate adding it?
examples/quadrotor/quadrotor_plant.h, line 49 at r1 (raw file):
} geometry::SourceId source_id() const { return source_id_; }
FYI Making this public lets other systems hijack this system's geometry. Definitely an adversarial thing to do and they get what they deserve, but...
examples/quadrotor/quadrotor_plant.h, line 94 at r1 (raw file):
// Geometry source identifier for this system to interact with SceneGraph. geometry::SourceId source_id_{}; // The id for the pendulum (arm + point mass) frame.
nit: This comment seems copypasta for a quadrotor. Taken from pendulum, perhaps?
examples/quadrotor/quadrotor_plant.cc, line 164 at r1 (raw file):
// Use (temporary) MultibodyPlant to parse the urdf and setup the // scene_graph.
BTW Do me a favor and put a TODO on this with my name to update this when we add MBP-independent parsing.
RussTedrake
force-pushed
the
quadrotor_scenegraph
branch
from
a07dbdb to
03e482d
Compare
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, platform LGTM from [sammy-tri, seancurtis-tri] (waiting on @sammy-tri and @SeanCurtis-TRI)
examples/quadrotor/quadrotor_plant.h, line 18 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
FYI This system doesn't have our wonderful
@systemdoxygen, but has new a new port. Is that enough to motivate adding it?
Done.
examples/quadrotor/quadrotor_plant.h, line 49 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
FYI Making this public lets other systems hijack this system's geometry. Definitely an adversarial thing to do and they get what they deserve, but...
but i need it to be able to connect to the SceneGraph. the alternative, i guess, would be to provide a ConnectToSceneGraph(builder, scene_graph) method, but it feels worse to force every small system to gain a dependency on DiagramBuilder?
examples/quadrotor/quadrotor_plant.h, line 51 at r1 (raw file):
Previously, sammy-tri (Sam Creasey) wrote…
BTW not new in this PR but the
protectedsection on a class which is markedfinalprobably isn't all that meaningful.
agreed. i thought the same, but didn't actually fix. have done it now.
examples/quadrotor/quadrotor_plant.h, line 94 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
nit: This comment seems copypasta for a quadrotor. Taken from pendulum, perhaps?
Done
examples/quadrotor/quadrotor_plant.cc, line 164 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
BTW Do me a favor and put a TODO on this with my name to update this when we add MBP-independent parsing.
Done. (gave you the other one, too. ;-)
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2.
Reviewable status:
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2.
Reviewable status:
examples/quadrotor/quadrotor_plant.h, line 49 at r1 (raw file):
Previously, RussTedrake (Russ Tedrake) wrote…
but i need it to be able to connect to the SceneGraph. the alternative, i guess, would be to provide a
ConnectToSceneGraph(builder, scene_graph)method, but it feels worse to force every small system to gain a dependency on DiagramBuilder?
Good point. I'll have to ponder that.
Related to #10755.
This change is