-
Notifications
You must be signed in to change notification settings - Fork 59
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 support for 8 bit thermal camera image format #92
Conversation
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-sensors4 #92 +/- ##
================================================
+ Coverage 76.00% 76.13% +0.12%
================================================
Files 23 23
Lines 2359 2388 +29
================================================
+ Hits 1793 1818 +25
- Misses 566 570 +4
Continue to review full report at Codecov.
|
Signed-off-by: Ian Chen <[email protected]>
There are a few test failures on macOS, but I don't think they're related to this PR. If that's true, then everything else looks good to me, assuming that defaulting to 16 bit is okay. |
yes that also preserves existing behavior |
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.
Something is broken on the macOS github actions. It's related with OGRE2
[Err] [Ogre2RenderEngine.cc:436] Unable to find Ogre Plugin[/usr/local/opt/ogre2.1/lib/OGRE-2.1/RenderSystem_GL3Plus]. Rendering will not be possible.Make sure you have installed OGRE properly.
13 - INTEGRATION_camera_plugin (SEGFAULT)
15 - INTEGRATION_depth_camera_plugin (SEGFAULT)
17 - INTEGRATION_gpu_lidar_sensor_plugin (SEGFAULT)
19 - INTEGRATION_rgbd_camera_plugin (SEGFAULT)
Otherwise LGTM
Signed-off-by: Nate Koenig <[email protected]>
When running the integration test for an invalid configuration in gazebosim/gz-sim#614, I notice the following output:
It's confusing to me to see two different ranges. I am assuming that cc @iche033 |
the |
There are now two thermal integration tests: |
The warning messages look correct to me. There are multiple problems with the |
The thing that seems confusing to me is that the first warning indicates that the camera's temperature range is between |
Maybe the warning messages could be reworded. The third warning says the that min and max ranges of the camera are between [-inf and inf]. These are ranges that the camera can The first warning message is about the resolution of the output. My understanding is that there is a difference between what the camera can |
That makes sense. Okay, what if I change the first warning to this (notice the addition of the word
I think the other warnings are fine as-is. |
Signed-off-by: Ashton Larkin <[email protected]>
Made the change in c6b39cc, I think we can merge this. |
a little late but this sums it up:
e.g. user can limit the max temp produced by the camera by specifying something like [0, 200]. If the user specifies something like [0, 800], the max reading will be the min(800, 255*3), which is 765. |
Summary
Depends on
gazebosim/gz-rendering#235Adds the necessary hooks to set ign-rendering thermal camera sensor to 8 bit image format and publish the generated image data.
Note that ign-rendering still returns
uint16_t
in the image callback but it actually contains 8 bit data. This was done to avoid breaking ABI. However, the image that's published by ign-sensors will have the correct format in a 8 bit unsigned char arrayChecklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge