Conform to ros format for header field frame_id of sensor msgs

[doisyg]

Should partially solves gazebosim/gz-sim#340
The frame_id field of the header of a sensor msg will now be the name given in the sdf sensor name attribute instead of the full chain of links.
Example with <sensor name='laser' type='gpu_lidar'>
Before:

$ rostopic echo --noarr /scan 
header: 
  seq: 384
  stamp: 
    secs: 12
    nsecs: 800000000
  frame_id: "spawned/base_footprint/laser"
angle_min: -2.0899999141693115
angle_max: 2.0899999141693115
angle_increment: 0.0014933905331417918
time_increment: 0.0
scan_time: 0.0
range_min: 0.05000000074505806
range_max: 30.0
ranges: "<array type: float32, length: 2800>"
intensities: "<array type: float32, length: 2800>"

After:

header: 
  seq: 127
  stamp: 
    secs: 4
    nsecs: 234000000
  frame_id: "laser"
angle_min: -2.0899999141693115
angle_max: 2.0899999141693115
angle_increment: 0.0014933905331417918
time_increment: 0.0
scan_time: 0.0
range_min: 0.05000000074505806
range_max: 30.0
ranges: "<array type: float32, length: 2800>"
intensities: "<array type: float32, length: 2800>"

However, in case of multi robot simulation, one should take care of properly prefixing the frame_id names to ensure each stays globally unique. See for instance, an example of a multiple camera launch file with a macro for proper prefixing of the frame_id field in the format tf_prefix_frame_id_name and without /:
https://github.com/IntelRealSense/realsense-ros/blob/development/realsense2_camera/launch/rs_multiple_devices.launch

[azeey]

This would break existing usage in SubT where multiple robots that each have a sensor with the same name use the prefix as a way to differentiate themselves. For this to work, the conversion from :: to / is expected to convert the whole string, not just the substring after the last ::.

As an alternative to this PR, maybe we can add an option to ign-sensors/ign-gazebo to report frame ids using just the name of the sensor instead of the fully qualified name.

[doisyg]

As an alternative to this PR, maybe we can add an option to ign-sensors/ign-gazebo to report frame ids using just the name of the sensor instead of the fully qualified name.

I agree with the alternative, how would you see this option exposed ? As an sdf attribute of the sensor plugging (which will apply this option to all sensors) or as new sdf attribute under the <sensor> one (which will apply this option per sensor) ? Or something else?

[doisyg]

I would like to move forward on this topic, @chapulina , any advice on how to tackle it ? Implementing @azeey suggestion ? But how ?

[chapulina]

how would you see this option exposed ?

How about adding support for a custom ignition:frame_id SDF element that's supported by ign-sensors? If the element is not specified, the behaviour remains the same as now (i.e. with the fully scoped name).

For example:

        <sensor name="imu" type="imu">
          <update_rate>100</update_rate>
          <topic>imu</topic>
          <ignition:frame_id>imu</ignition:frame_id>
        </sensor>

Then ign-sensors can parse that special element for all sensors, and offer FrameId and SetFrameId APIs, and all sensors must be updated to use that when populating their headers.

[chapulina]

@doisyg , are you still interested in working on this?

[doisyg]

Still interested in the feature, still interested in working on it if nobody takes care of it, but I don't immediately have time

[j-rivero]

@ros-pull-request-builder retest this please

[ahcorde]

I think this PR/discussion is related with this other PR https://github.com/ignitionrobotics/ign-sensors/pull/195/files

[ahcorde]

I think this PR/discussion is related with this other PR https://github.com/ignitionrobotics/ign-sensors/pull/195/files

Reading the discussion I will work on a more generic solution in ign-sensors

[ahcorde]

I udpated the PR gazebosim/gz-sensors#195 here is my solution

I couldn't use ignition:frame_id I used ignition_frame_id instead

[chapulina]

@doisyg , what's the status of this PR? We just synced Citadel to packages.ros.org, so if you were waiting on any releases, this should be unblocked.

[doisyg]

Closing in favor of gazebosim/gz-sensors#195 and gazebosim/gz-sensors#218