RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.35k stars 1.27k forks source link

Inconsistent input port name "query_object" on three visualizer systems #17523

Open rpoyner-tri opened 2 years ago

rpoyner-tri commented 2 years ago

Original issue title: [examples] Redundant aliased output port "geometry_query", "query_object" on ManipulationStation


What happened?

Since #14292, ManipulationStation (a diagram) has had two string-name aliases for its contained scene_graph.get_query_output_port(): "geometry_query", and "query_object". This is confusing, since example programs use one or the other in baffling combinations.

Of the two, "query_object" is older, and is documented in the system diagram. "geometry_query" is undocumented, but is used in some example code.

One possible reading of the history of "geometry_query" code changes is that it was meant to replace"query_object", and that "query_object" should have been deprecated and removed at the same time as "pose_bundle".

In any case, we should either document the currently undocumented alias, or remove one of them and update documentation to match.

Version

No response

What operating system are you using?

No response

What installation option are you using?

compiled from source code using Bazel

Relevant log output

No response

rpoyner-tri commented 2 years ago

Assigned temporarily to component lead @jwnimmer-tri . Possible interested parties include @RussTedrake , @SeanCurtis-TRI .

jwnimmer-tri commented 2 years ago

My quick skim of the code (throughout Drake, not just this one example) is that geometry_query is the canonical port name string, and query_object is the canonical name for the value on that port (of type geometry::QueryObject). If that seems true, then it's probably worth a quick fix to our sample code so that obeys that convention uniformly.

I don't think it's worth taking the time to deprecate anything here, at least not soon. The general plan as I understand it is to rewrite the whole manipulation_station example Python at some point. Maybe even using hardware_station_simulation libraries.

RussTedrake commented 2 years ago

I agree we should clean up the usages of query_object vs geometry_query across the codebase. Sorry if I've added to the noise!

FWIW -- The python manipulation station has been here: https://github.com/RussTedrake/manipulation/blob/master/manipulation_station.ipynb and has survived being used by numerous student projects. We could port it whenever we decide to pull that trigger. (before teaching manipulation this fall might be a natural time)

jwnimmer-tri commented 3 weeks ago

There are only three places still worth fixing:

I will re-title the issue to match.