-
Notifications
You must be signed in to change notification settings - Fork 51
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 bounding box sensor API stubs #420
Conversation
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.
I'd like some feedback on an API decision I made before this gets merged. See my comment below.
Codecov Report
@@ Coverage Diff @@
## main #420 +/- ##
==========================================
- Coverage 55.18% 55.12% -0.07%
==========================================
Files 191 192 +1
Lines 19348 19381 +33
==========================================
+ Hits 10678 10683 +5
- Misses 8670 8698 +28
Continue to review full report at Codecov.
|
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.
Let's also add the dependency on ign-math
eigen. It's best not to add dependencies to stable versions.
Signed-off-by: Ashton Larkin <[email protected]>
Added in 98a7bf1. I'm assuming that the ogre2 component can be updated in #334 since that's a plugin, right? Here's what I am referring to in #334: https://github.com/ignitionrobotics/ign-rendering/blob/4162fd7b95969691d6ef4918d795172a66e3053a/ogre2/src/CMakeLists.txt#L38 |
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[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.
I hope that's all we need 🤞
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Summary
I've added the public API for a bounding box sensor. Most of this is copy/pasted from #334, with a few changes:
rendering::BoundingBox
struct, which takes a type and label argumentrendering::BoundingBoxCamera::DrawBoundingBox
method takes the box color as an argument instead of defaulting to greenThis PR just covers the breaking changes that are needed for the bounding box sensor so that we can add #334 to Fortress in a minor release. Some of the things I added here may not be breaking, but I wasn't sure, so I added whatever I thought could be ABI breaking. See #334 for an ogre2 implementation.
Test it
No tests have been added here because tests are written for this in #334
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge