-
Notifications
You must be signed in to change notification settings - Fork 58
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
Segmentation Sensor #133
Segmentation Sensor #133
Conversation
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: AmrElsersy <[email protected]>
void BuildScene(rendering::ScenePtr scene) | ||
{ | ||
math::Vector3d leftPosition(3, -1.5, 0); | ||
math::Vector3d rightPosition(3, 1.5, 0); | ||
math::Vector3d middlePosition(3, 0, 0); | ||
math::Vector3d hiddenPosition(8, 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add one more box that is partially hidden? Perhaps it can be poking out of the right of the left-most box. It'd be nice to add test coverage to make sure that for partially hidden objects, we only see the part that is visible in the segmented image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is already tested in the rendering::SegmentationCamera
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it seems like there is some overlap/redundancy in testing between ign-sensors
and ign-rendering
, but I'm not sure what kind of tests should go in which repository for a given sensor and its rendering capabilities... @iche033 would you mind chiming in to help clarify which tests belong to which repository?
Signed-off-by: AmrElsersy <[email protected]>
void BuildScene(rendering::ScenePtr scene) | ||
{ | ||
math::Vector3d leftPosition(3, -1.5, 0); | ||
math::Vector3d rightPosition(3, 1.5, 0); | ||
math::Vector3d middlePosition(3, 0, 0); | ||
math::Vector3d hiddenPosition(8, 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it seems like there is some overlap/redundancy in testing between ign-sensors
and ign-rendering
, but I'm not sure what kind of tests should go in which repository for a given sensor and its rendering capabilities... @iche033 would you mind chiming in to help clarify which tests belong to which repository?
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
…lization Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: AmrElsersy <[email protected]>
I'm unable to build this PR in a colcon workspace. I believe I am including all of the dependent PRs (gazebosim/sdformat#592, gazebosim/gz-rendering#329, gazebosim/gz-msgs#165), but I see the following error output when building:
I am also seeing similar issues in #136. It looks like it's only an issue with the integration tests, which makes me think that this behavior started in ed5c136, after merging in the changes from #90. I was able to go back to commit e2c5b52 (the commit before the merge mentioned above) and build this PR just fine. |
How about switching to use DirIter from ign-common? Example usage here |
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: AmrElsersy <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! We'll need to wait for gazebosim/gz-rendering#419 to be merged before merging this, since the label map behavior and integration test fails without it
I think we should leave the tutorials without numbers on their filenames. That doesn't scale well |
So, instead of |
Yup 👍 I see some other files being moved too. That can be reverted |
Signed-off-by: Ashton Larkin <[email protected]>
Okay, all numbers have been removed from the tutorial files in 8642b60. We will probably need to make a release of |
Codecov Report
@@ Coverage Diff @@
## main #133 +/- ##
==========================================
- Coverage 77.90% 75.52% -2.38%
==========================================
Files 24 25 +1
Lines 2394 2644 +250
==========================================
+ Hits 1865 1997 +132
- Misses 529 647 +118
Continue to review full report at Codecov.
|
🎉 New feature
related to #134
Summary
Segmentation Camera Sensor that allow users to use it easily in SDF, and It publishes the sensor data image to ign-transport
Depends on
SDF #592
Rendering #329
Demo
Semantic
Instance
Test it
Will add a tutorial later.
Format
It requires visuals annotating in the sdf via Gazeb #853
Checklist
codecheck
passed (See contributing)