Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BoundingBox Sensor #136

Merged
merged 37 commits into from Jun 17, 2022
Merged

Conversation

AmrElsersy
Copy link
Contributor

@AmrElsersy AmrElsersy commented Jun 16, 2021

Signed-off-by: AmrElsersy amrelsersay@gmail.com

🎉 New feature

related to #135

Summary

BoundingBox Camera Sensor that allow users to use it easily in SDF, and It publishes the sensor data (bounding boxes) to ign-transport.

It published both BoundingBoxes Msg & Image (with drawn boxes on it for visualization)

Depends on

Gazebo #853
SDF #592
Rendering #334
Msgs #165

Demo

demo

Test it

Will add a tutorial later.

Format

        <sensor name="boundingbox_camera" type="boundingbox_camera">
          <box_type>visible_box</box_type>
          <camera>
            <horizontal_fov>1.047</horizontal_fov>
            <image>
              <width>800</width>
              <height>600</height>
            </image>
            <clip>
              <near>0.01</near>
              <far>1000</far>
            </clip>
          </camera>
          <always_on>1</always_on>
          <update_rate>30</update_rate>
          <visualize>true</visualize>
          <topic>camera</topic>
        </sensor>

It requires visuals annotating in the sdf via Gazeb #853

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

Notes

Full Bounding Boxes mode dosn't work yet here, But it works in rendering::BoundingBoxCamera, I think because the view matrix of the ogre camera dosn't change with the change of the node's pose ... still searching :D

Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
@AmrElsersy AmrElsersy requested a review from iche033 as a code owner June 16, 2021 20:15
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Jun 16, 2021
@osrf-triage osrf-triage added this to Inbox in Core development Jun 16, 2021
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
@adlarkin adlarkin self-requested a review June 17, 2021 15:37
@chapulina chapulina moved this from Inbox to In review in Core development Jun 17, 2021
AmrElsersy and others added 6 commits July 15, 2021 08:17
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
@adlarkin adlarkin added the needs upstream release Blocked by a release of an upstream library label Jul 18, 2021
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
@AmrElsersy AmrElsersy requested a review from adlarkin July 22, 2021 12:23
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
include/ignition/sensors/BoundingBoxCameraSensor.hh Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
src/BoundingBoxCameraSensor.cc Outdated Show resolved Hide resolved
Comment on lines 426 to 429
auto oriented3DBox = new msgs::Oriented3DBox();
auto center = new msgs::Vector3d();
auto size = new msgs::Vector3d();
auto rotation = new msgs::Quaternion();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a chance for memory leaks here? It looks like these variables are added to the message, but when/how are they de-allocated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know yet a way about how to deallocate it
is it deleted when we call the MSG_NAME.clear() function ?

I mean the whole msg, the AnnotatedAxisAligned2DBox_V

Comment on lines 469 to 471
auto axisAlignedBox = new msgs::AxisAligned2DBox();
auto min_corner = new msgs::Vector2d();
auto max_corner = new msgs::Vector2d();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question as my previous comment about memory leaks

Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
test/integration/boundingbox_camera_plugin.cc Outdated Show resolved Hide resolved
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
…into BoundingBox

Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
@adlarkin adlarkin mentioned this pull request Sep 1, 2021
8 tasks
Signed-off-by: AmrElsersy <amrelsersay@gmail.com>
@chapulina chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2021
@chapulina
Copy link
Contributor

gazebosim/gz-rendering#334 won't make it in time for code freeze, so I'll remove this PR from beta, which means the sensor won't be available on Fortress from day one. This PR is backwards compatible though, so we can get it into a minor release later with more time.

@chapulina chapulina removed the beta Targeting beta release of upcoming collection label Sep 21, 2021
@chapulina chapulina moved this from In review to In progress in Core development Nov 8, 2021
@chapulina chapulina moved this from In progress to To do in Core development Dec 20, 2021
@chapulina chapulina moved this from To do to In progress in Core development Dec 20, 2021
@j-rivero j-rivero removed this from In progress in Core development May 6, 2022
@chapulina chapulina changed the base branch from main to ign-sensors6 June 2, 2022 03:51
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina added this to Inbox in Core development via automation Jun 6, 2022
@chapulina chapulina moved this from Inbox to In review in Core development Jun 6, 2022
@chapulina chapulina mentioned this pull request Jun 15, 2022
7 tasks
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Jun 15, 2022
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #136 (ec8117f) into ign-sensors6 (efe3151) will decrease coverage by 7.83%.
The diff coverage is 58.49%.

❗ Current head ec8117f differs from pull request most recent head 79dd5b3. Consider uploading reports for the commit 79dd5b3 to get more accurate results

@@               Coverage Diff                @@
##           ign-sensors6     #136      +/-   ##
================================================
- Coverage         80.00%   72.16%   -7.84%     
================================================
  Files                 1       34      +33     
  Lines                15     3470    +3455     
================================================
+ Hits                 12     2504    +2492     
- Misses                3      966     +963     
Impacted Files Coverage Δ
src/BoundingBoxCameraSensor.cc 58.49% <58.49%> (ø)
test_config.h
include/ignition/sensors/Manager.hh 81.81% <0.00%> (ø)
src/ImageNoise.cc 43.47% <0.00%> (ø)
src/DepthCameraSensor.cc 74.39% <0.00%> (ø)
src/Lidar.cc 72.91% <0.00%> (ø)
src/ImuSensor.cc 85.71% <0.00%> (ø)
src/LogicalCameraSensor.cc 90.14% <0.00%> (ø)
src/SegmentationCameraSensor.cc 53.90% <0.00%> (ø)
src/PointCloudUtil.cc 80.66% <0.00%> (ø)
... and 25 more

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 efe3151...79dd5b3. Read the comment docs.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina merged commit ee48612 into gazebosim:ign-sensors6 Jun 17, 2022
Core development automation moved this from In review to Done Jun 17, 2022
@chapulina chapulina moved this from Done to Highlights in Core development Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants