gazebosim / sdformat

Simulation Description Format (SDFormat) parser and description files.
http://sdformat.org
Apache License 2.0
152 stars 90 forks source link

Deprecate camera pose #1431

Open iche033 opened 4 weeks ago

iche033 commented 4 weeks ago

🦟 Bug fix

Summary

Deprecate //sensor/camera/pose as it's redundant with //sensor/pose, see https://github.com/gazebosim/gz-sim/pull/2433#issuecomment-2150894010.

I deprecated the relevant functions in Camera DOM class. As for deprecating the actual //sensor/camera/pose SDF element, I noticed that the pose element is included in the sdf/1.12/camera.sdf file using <include filename="pose.sdf" required="0"/> so I can't update it's description to say that it's being deprecated. One option is to copy the entire conent of pose.sdf into camera.sdf and update its description - let me know if this is the preferred method. Update: marked as deprecated by setting required to -1 (<include filename="pose.sdf" required="-1"/>)

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

scpeters commented 4 weeks ago

As for deprecating the actual //sensor/camera/pose SDF element, I noticed that the pose element is included in the sdf/1.12/camera.sdf file using <include filename="pose.sdf" required="0"/>

does it work to set required="-1" in the include element?

<include filename="pose.sdf" required="-1"/>
iche033 commented 4 weeks ago

does it work to set required="-1" in the include element?

oh I didn't know about this! Just found what -1 means in the parser code. I think that's what I need, thanks. Set required to -1 in c8f2521

scpeters commented 4 weeks ago

I looked back in gazebo-classic, and we used the //camera/pose for multicameras (such as a stereo camera) in order to specify the location of each sub-camera:

do we have support for stereo-cameras yet in gz-sim? cc @azeey

iche033 commented 4 weeks ago

do we have support for stereo-cameras yet in gz-sim?

Gazebo-classic uses a multicamera sensor so that it can guarantee the cameras are synchronized / updated in the same timestep. This is not necessary in gz-sim. To use stereo or multi-cameras in gz-sim, the users just need to create 2 (or more) cameras with the same update rate - this will guarantee that the cameras are updated in the same timestep.

iche033 commented 4 weeks ago

I originally thought //camera/pose is used for rotating the camera to optical frame but no you're right it's for multicamera sensors. If there are other consumers of sdformat (other than gazebo-classic and gz-sim) that support multicamera + //camera/pose, maybe we should not deprecate this. We just won't support this field in gz-sim.