sdformat icon indicating copy to clipboard operation
sdformat copied to clipboard

Deprecate `//sensor/camera/pose`

Open FirefoxMetzger opened this issue 2 years ago • 6 comments

A camera is the only sensor that allows specifying a pose element on top of //sensor/pose. I wonder if there is any utility in this or if it could be depreciated.

I can see how this might be relevant to the idea of specifying that a camera is attached at some position, and origin/center of the lense model has some offset to that. However, I think this case can easily be covered by setting the position of the //sensor to that position and introducing a fixed link from the sensor mount to the lense origin. This is likely a more accurate description anyway since the mount point is connected to a rigid body (the camera casing) which itself is attached to the lense system.

@azeey Would I have to write a dedicated proposal for this, or should this live here?

FirefoxMetzger avatar Sep 23 '21 09:09 FirefoxMetzger

osrf/gazebo currently uses //sensor/camera/pose to specify the pose of each camera within a MultiCameraSensor, which can contain multiple //sensor/camera elements:

  • https://github.com/osrf/gazebo/blob/gazebo11/test/worlds/multicamera_noisy_test.world#L63-L78
  • https://github.com/osrf/gazebo/blob/gazebo11/gazebo/sensors/MultiCameraSensor.cc#L134-L158

the multicamera is not yet implemented in ign-sensors

scpeters avatar Sep 28 '21 00:09 scpeters

@scpeters Thanks for the info; that's interesting.

So I assume the advantage of a MultiCameraSensor over multiple Camera sensors on the same link is that you get one topic that publishes messages that aggregate all frames for each camera and that they are guaranteed to be synchronized?

FirefoxMetzger avatar Sep 28 '21 07:09 FirefoxMetzger

@scpeters Thanks for the info; that's interesting.

So I assume the advantage of a MultiCameraSensor over multiple Camera sensors on the same link is that you get one topic that publishes messages that aggregate all frames for each camera and that they are guaranteed to be synchronized?

yes, exactly. This allows you to have synchronized stereo images, for example

scpeters avatar Oct 04 '21 18:10 scpeters

Hm, fair. In that case, do you know the placement rules for //sensor/camera/pose, @scpeters ? I.e., which frame is //sensor/camera/pose @relative_to by default?

Intuitively, I would think //sensor; however, //sensor is actually not frame-bearing (it doesn't create an implicit frame). (If it does, we should add that to the spec somewhere :D) This makes //sensor/camera/pose the only place in the spec where a pose can be specified @relative_to something that is not a frame itself.

FirefoxMetzger avatar Oct 04 '21 19:10 FirefoxMetzger

I believe the //sensor/camera/pose should be relative to the //sensor/pose by default, and we actually don't currently provide an API for resolving a camera pose if //camera/pose/@relative_to is set. I see two options:

  • deprecate and then remove //camera/pose/@relative_to. Then //camera/pose would only be relative to //sensor/pose, as //inertial/pose is only relative to its //link/pose
  • add SemanticPose API to sdf::Camera

scpeters avatar Aug 17 '22 22:08 scpeters

I would favor adding SemanticPose API to sdf::Camera, but as @FirefoxMetzger mentioned, that would mean we have to make //sensor an implicit frame. I think the spec would be more consistent if all (even //inertial/pose some day) //pose elements behaved the same way.

azeey avatar Aug 18 '22 18:08 azeey