map_saver: fix saved raw image being too dark #4712
Conversation
The map cell value comes from an OccupancyGrid and ranges from -1 to 100. So, it makes no sense to divide this value by 255 when computing the intensity ratio.
nav2_map_server/src/map_io.cpp
Outdated
| @@ -505,7 +505,7 @@ void tryWriteMapToFile( | |||
| if (map_cell < 0 || 100 < map_cell) { | |||
| q = MaxRGB; | |||
| } else { | |||
| q = map_cell / 255.0 * MaxRGB; | |||
| q = map_cell / 100.0 * MaxRGB; | |||
There was a problem hiding this comment.
Similar to the other PR, the ROS 1 version seems to use 255 (see negate) which has been in use for 10+ years. This seems OK on face value, but I'm a little weary
There was a problem hiding this comment.
Though, the MapMode::Scale seems to agree with you, so I'm leaning towards merging this. Lets continue to chat in the other PR you submitted and we'll handle these as a pair since they seems tied together
Thanks!
There was a problem hiding this comment.
Hi @SteveMacenski , thanks for your reviews. You are right, the other PR relies on this one. However, the opposite is not true.
In my understanding, this PR fixes a real bug. It could be merged even though discussions are still opened in the other one.
There was a problem hiding this comment.
I commented on the other thread. Can you update this PR to contain the updates that supersedes #4713? Lets just put both of these together, its a simple enough set of code changes to review and are related
This implementation is inspired from the Costmap2D implementation [1]. It remains valid even if the LETHAL_OBSTACLE, FREE_SPACE, OCC_GRID_OCCUPIED, and OCC_GRID_FREE constants are modified in the future. Unfortunately, the nav2_costmap_2d namespace cannot be accessed from this module, so the LETHAL_OBSTACLE and FREE_SPACE constants had to be hard-coded. By the same way, this commit fixes an issue in the output image where the areas of NO_INFORMATION and LETHAL_OBSTACLES cannot be distinguished as they both use the same value: 255. Now, the LETHAL_OBSTACLES pixels are restored to their rightful value: 254. [1] https://github.com/ros-navigation/navigation2/blob/main/nav2_costmap_2d/src/costmap_2d.cpp#L80
|
After investigation, I finally understood that there was no error in the initial implementation. The values stored in the .pgm are NOT meant to be equivalent to the ones stored in the Costmap2D internal grid, despite sharing the same range [0-255]. The .pgm file is just used as a uint8_t container to store the raw int8_t values of the OccupancyGrid ranging from 0 to 100. The only modified value is -1 (UNKNOWN) which is mapped to 255. So, it's expected to have a "dark" costmap stored in the .pgm file. And the UNKNOWN level is perfectly distinguishable from the others. My apologies for the time lost. |
|
No worries! |
The map cell value comes from an OccupancyGrid and ranges from 0 to
100. Dividing it by 255 makes the output ratio range from 0 to 0.39. This is clearly an error as the output should range from 0 to 1. The consequence is that the output raw image is darker than expected.
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Future work that may be required in bullet points
For Maintainers: