Add more occupancy update functions to VoxelGridShape #1083
Conversation
Codecov Report
@@ Coverage Diff @@
## release-6.6 #1083 +/- ##
============================================
Coverage 56.52% 56.52%
============================================
Files 316 316
Lines 24398 24398
============================================
Hits 13791 13791
Misses 10607 10607 |
dart/dynamics/VoxelGridShape.cpp
Outdated
| { | ||
| if (inCoordinatesOf == Frame::World()) | ||
| { | ||
| mOctree->insertPointCloud(pointCloud, toVector3(sensorOrigin)); |
There was a problem hiding this comment.
Just to be clear, will this also clear out the previous occupancy? To me, the function name insertPointCloud implies that it will add these points to the already existing occupancy grid, but the name updateOccupancy implies that we will replace the existing occupancy grid with the new one.
Maybe we could add a comment here to clarify that the existing occupancy will be cleared (or else change the name of updateOccupancy).
There was a problem hiding this comment.
Just to be clear, will this also clear out the previous occupancy? To me, the function name insertPointCloud implies that it will add these points to the already existing occupancy grid, ...
You're correct. More precisely, the function updates the occupancy probabilities. The probability at the end point will be increased, and the probabilities at the points that the rays pass through will be decreased. I used "update" term in that sense. I just couldn't come up with better name for that. Any suggestions? 😄
There was a problem hiding this comment.
I think I follow. Does this function have hysteresis, then? Where the previous state will have an effect on the current state? If so, it might be good to mention that in the documentation of updateOccupancy.
I think the word "update" is reasonable, or at least I don't have a better suggestion off the top of my head. But I think sometimes "update" implies that the previous state will be wiped out, so that might be worth clarifying in documentation somewhere.
There was a problem hiding this comment.
Does this function have hysteresis, then? Where the previous state will have an effect on the current state? If so, it might be good to mention that in the documentation of updateOccupancy.
Hm, not sure if I understand your question correctly. The probability updates are done by Octomap (e.g., by insertPointCloud). Let me try to update documentation more correctly.
But I think sometimes "update" implies that the previous state will be wiped out, so that might be worth clarifying in documentation somewhere.
I agree. The meaning of "update" term often cases too broad. I might use "set" in the case of wiping out the original value.
dart/dynamics/VoxelGridShape.hpp
Outdated
| #include "dart/dynamics/Shape.hpp" | ||
|
|
||
| namespace dart { | ||
| namespace dynamics { | ||
|
|
||
| class Frame; |
There was a problem hiding this comment.
Since we're including the Frame.hpp header, we won't need this forward declaration.
dart/dynamics/VoxelGridShape.hpp
Outdated
| void updateOccupancy( | ||
| const octomap::Pointcloud& pointCloud, | ||
| const octomap::point3d& sensorOrigin); | ||
| const Eigen::Vector3d& sensorOrigin, | ||
| const Eigen::Isometry3d& inCoordinatesOf); |
There was a problem hiding this comment.
I think this parameter should be named relativeTo to match our established terminology. inCoordinatesOf has been used to imply only a rotation of coordinate frame, without accounting for any difference in position.
dart/dynamics/VoxelGridShape.hpp
Outdated
| /// and the new sensor measurement. | ||
| /// | ||
| /// The voxels of the end points of rays will increase the probability because | ||
| /// that's where the ray hit an object. On the other hand, the voxles that the |
dart/dynamics/VoxelGridShape.hpp
Outdated
| /// and the new sensor measurement. | ||
| /// | ||
| /// The voxels of the end points of rays will increase the probability because | ||
| /// that's where the ray hit an object. On the other hand, the voxles that the |
jslee02
changed the title
This PR adds more occupancy update functions that can specify reference frame of the sensor origin and the point cloud.
Before creating a pull request
clang-formatBefore merging a pull request
CHANGELOG.md