Add optional optical frame id to camera sensors

[MarqRazz]

🎉 New feature

Closes #175

Summary

This adds the ability for a CameraSensor or RgbdCameraSensor to use the optional sdf specified optical frame_id that will be used when publishing the camera_info, rgb_image, and point_cloud topic's from a simulated sensor. Note: this PR depends on an open PR in sdformat.
This feature request was noted in CameraSensors.cc and will help close #175 in ign-sensors.

Test it

Here are the instructions on how I setup my test environment:

• I started with the following Docker image from Open Robotics. osrf/ros:humble-desktop
• Clone an example repo that I created. You can see that I set optical_frame_id here. You will also need to clone some dependencies with vcs import src --skip-existing --input src/rgbd_camera_ignition/rgbd_camera_ignition.repos
• Build Ignition Gazebo from source including sdformat#1109 and this PR.
• Run Gazebo and Rviz with ros2 launch rgbd_camera_ignition rgbd_ignition.launch.py

Rviz should appear with the point cloud being rendered in the 6-DOF scene and the Camera widget properly overlaying it over the RGB image.

I even made a custom frustum mesh (not included in my repo) to ensure the camera was rendering in the correct location in Gazebo and Rviz.

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

[ahcorde]

can you add a test?

[MarqRazz]

can you add a test?

I can but nothing will pass till the SDF PR is merged. Any feedback there?

[MarqRazz]

@ahcorde after looking into this some more it seems like adding a proper test could be a sizable amount of work. The current tests in Camera_TEST.cc do not create a valid sensor object. You can see when running the tests the Manager is unable to create a valid sensor:

11: [ RUN      ] Camera_TEST.CreateCamera
11: [Err] [CameraSensor.cc:385] Camera doesn't exist.
11: [Err] [CameraSensor.cc:274] Attempting to a load a Camera sensor, but received a none
11: [Err] [CameraSensor.cc:280] Attempting to a load a Camera sensor, but received a null sensor.
11: [Err] [SensorFactory.hh:144] Failed to load sensor [cam_name] of type[not_camera]
11: [Err] [Manager.hh:80] Failed to create sensor.
11: [       OK ] Camera_TEST.CreateCamera (121 ms)
11: [ RUN      ] Camera_TEST.Topic
11: [Err] [Sensor.cc:278] Failed to set sensor topic [@@@]
11: [Err] [SensorFactory.hh:144] Failed to load sensor [TestCamera] of type[camera]
11: [Err] [Manager.hh:80] Failed to create sensor.
11: [       OK ] Camera_TEST.Topic (174 ms)

I attempted to load a camera from the SDF stream but it fails to create a valid publisher.
Here is what I have in my test:

    const std::string topic;
    auto cameraSdf = CameraToSdf(type, name, updateRate, topic, alwaysOn,
        visualize);

    auto camera = mgr.CreateSensor<ignition::sensors::CameraSensor>(cameraSdf);
    camera->Load(cameraSdf);

and here's the error I get...

11: Node::Advertise(): Error advertising topic [/camera]. Did you forget to start the discovery service?
11: [Err] [CameraSensor.cc:295] Unable to create publisher on topic[/camera].

Do you have any suggestions on how I can go about creating a valid CameraSensor that I can perform checks against?

[MarqRazz]

@ahcorde Do you have any suggestions on how I can go about creating a valid CameraSensor that I can perform checks against?

How can I trigger the tests to run again now that 1109 was merged?

[MarqRazz]

I need this merged for a project I'm working on! Are the new tests mandatory @ahcorde?

[ahcorde]

we need a sdformat release to merge this, I will talk with the maintainer ⛑️

[MarqRazz]

Thank you!!

[livanov93]

@ahcorde Any news on this? Can we merge it? What are the main concerns why we shoudn't?

[ahcorde]

@osrf-jenkins run tests please!

[scpeters]

we need a special branch of gzdev in order to use the prerelases, as well as a pull request with a special branch name: #273

[scpeters]

I'm trying to get CI working; in the meantime, I think the required version of sdformat12 should be updated to 12.6.0

• https://github.com/gazebosim/gz-sensors/blob/ign-sensors6/CMakeLists.txt#L89

[MarqRazz]

Sorry for the small delay! Please let me know if there is anything else I can help with here!

[scpeters]

Sorry for the small delay! Please let me know if there is anything else I can help with here!

I've updated the test branch in #273 with your latest changes and CI is running again

[scpeters]

@ahcorde CI looks ok with prereleases; this is ready for additional review. Once you are happy we can request a stable release of sdformat12

[ahcorde]

I merged because the CI was fine, but I;m getting an error in ign-sensor6. Maybe the stable release?

[scpeters]

@ahcorde CI looks ok with prereleases; this is ready for additional review. Once you are happy we can request a stable release of sdformat12

the CI for this branch is using prereleases, we need to make a stable release before this will work