gz-sim icon indicating copy to clipboard operation
gz-sim copied to clipboard

Documentation is out-of-date for some Link APIs that return std::optional

Open scpeters opened this issue 11 months ago • 2 comments

The behavior of several APIs of the gz::sim::Link class was updated in 9bf542eb39cb931e1f74c9bf0963216a14123c14 (part of #2081) without updating the documentation in include/gz/sim/Link.hh

For example, the documentation of Link::WorldPose() states that std::nullopt will be returned if the entity does not have a WorldPose component, but since 9bf542eb39cb931e1f74c9bf0963216a14123c14 sim::worldPose() is called now called to compute a value if the WorldPose component is not present so that it will have a default-constructed Pose3d() instead of std::nullopt.

At a minimum, we should update the documentation for released branches, and should consider changing the API to return a Pose3d directly instead of optional<Pose3d> if nullopt can never be returned.

scpeters avatar May 09 '25 21:05 scpeters

Is there a reason we chose to do this? Returning a default constructed pose seems very unintuitive. What if the link is really at (0.0,0.0,0.0). The reason these APIs return nullopt is so that we know there is an error or is it that we are guaranteeing we will always provide a worldPose?

arjo129 avatar May 14 '25 02:05 arjo129

Is there a reason we chose to do this? Returning a default constructed pose seems very unintuitive. What if the link is really at (0.0,0.0,0.0). The reason these APIs return nullopt is so that we know there is an error or is it that we are guaranteeing we will always provide a worldPose?

I don't know exactly why it was changed, but it was changed as part of #2081, so I assume it made it easier to support systems written in python

scpeters avatar May 14 '25 02:05 scpeters