Triggered Camera

[WilliamLewww]

🎉 Triggered Camera

Related Pull Requests:

• Triggered Camera gz-sensors#194

Reference issue:

• Triggered cameras (take photos instead of videos) gz-sensors#185

Summary

Adds the SDF required to enable triggered cameras:

<camera>
  <triggered>true</triggered>
  <trigger_topic>/camera/trigger</trigger_topic>
  <horizontal_fov>1.05</horizontal_fov>
  <image>
    <width>256</width>
    <height>257</height>
  </image>
  <clip>
    <near>0.1</near>
    <far>10.0</far>
  </clip>
</camera>

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• 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]

Missing tests

[nkoenig]

Can you also add to the Camera_TEST.cc to cover these new features?

[scpeters]

gazebo_ros_pkgs doesn't allow the trigger topic name to be configurable. I think we can do the same, unless someone thinks it is important to add this

[nkoenig]

gazebo_ros_pkgs doesn't allow the trigger topic name to be configurable. I think we can do the same, unless someone thinks it is important to add this

I think you're referring to the SetTrigger function, which would be needed when creating the DOM programmatically.

[scpeters]

gazebo_ros_pkgs doesn't allow the trigger topic name to be configurable. I think we can do the same, unless someone thinks it is important to add this

I think you're referring to the SetTrigger function, which would be needed when creating the DOM programmatically.

I was referring to the fact that gazebo_ros_pkgs creates the trigger topic with a hard-coded image_trigger topic name:

• https://github.com/ros-simulation/gazebo_ros_pkgs/blob/ros2/gazebo_plugins/src/gazebo_ros_camera.cpp#L272
• Triggered Camera gz-sensors#194 (comment)

My question is whether the trigger topic name should be configurable

[nkoenig]

gazebo_ros_pkgs doesn't allow the trigger topic name to be configurable. I think we can do the same, unless someone thinks it is important to add this

I think you're referring to the SetTrigger function, which would be needed when creating the DOM programmatically.

I was referring to the fact that gazebo_ros_pkgs creates the trigger topic with a hard-coded image_trigger topic name:

• https://github.com/ros-simulation/gazebo_ros_pkgs/blob/ros2/gazebo_plugins/src/gazebo_ros_camera.cpp#L272
• Triggered Camera ign-sensors#194 (comment)

My question is whether the trigger topic name should be configurable

Ahh, thanks. Yeah, that would be nice.

[scpeters]

make a prerelease once this is merged to help test gazebosim/gz-sensors#194

[codecov-commenter]

Codecov Report

Merging #846 (8c3bd2b) into sdf12 (5e3605f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 8c3bd2b differs from pull request most recent head 65375c0. Consider uploading reports for the commit 65375c0 to get more accurate results

@@            Coverage Diff             @@
##            sdf12     #846      +/-   ##
==========================================
+ Coverage   90.88%   90.89%   +0.01%     
==========================================
  Files          78       78              
  Lines       12636    12650      +14     
==========================================
+ Hits        11484    11498      +14     
  Misses       1152     1152              

Impacted Files	Coverage Δ
src/Camera.cc	87.55% <100.00%> (+0.37%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e3605f...65375c0. Read the comment docs.

[scpeters]

did we say the trigger topic name should be configurable?

[WilliamLewww]

did we say the trigger topic name should be configurable?

Oh yea, I'll go add that change!

<camera>
  <triggered>true</triggered>
  <trigger_topic>/camera/trigger</trigger_topic>
  <horizontal_fov>1.05</horizontal_fov>
  <image>
    <width>256</width>
    <height>257</height>
  </image>
  <clip>
    <near>0.1</near>
    <far>10.0</far>
  </clip>
</camera>

[WilliamLewww]

Re-requesting review to complete auto-merge.

[azeey]

I disabled the auto-merge. In general, we prefer to have a manual merge so that the author has a chance to modify the commit message such that it concisely captures the core ideas of the pull request.

From https://ignitionrobotics.org/docs/all/contributing#process

Make sure the commit message concisely captures the core ideas of the pull request and contains all authors' signatures.

[chapulina]

I disabled the auto-merge. In general, we prefer to have a manual merge so that the author has a chance to modify the commit message such that it concisely captures the core ideas of the pull request.

The auto-merge option allows editing the commit message, right? If the auto-merge is not working for us, we should disable it, but I think it should be compatible with our contribution process.

[azeey]

The auto-merge option allows editing the commit message, right? If the auto-merge is not working for us, we should disable it, but I think it should be compatible with our contribution process.

Ah, it does. My mistake.

[scpeters]

I think @ahcorde concerns about the tests have been addressed, so I'll merge

tests have been added

[osrf-triage]

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1