API review
Proposer: Eitan Marder-Eppstein
For API details see the following page: http://pr.willowgarage.com/pr-docs/ros-packages/costmap_2d/html/index.html
Present at review:
- List reviewers
Question / concerns / comments
Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.
- Josh
What are the Costmap2D::lock()/unlock() functions for? Gone now
cellSizeX()/cellSizeY()/metersSizeX()/metersSizeY() are confusing -- cellSize sounds like it should the size of a cell, not the number of cells in the map. names changed to getSizeInCellsX/Y and getSizeInMetersX/Y
Getting the char array back right now is messy. The user needs to allocate something. Possibly a higher level structure... like a vector. Create a charMap structure that has height, width, resolution and remove the getCharMap call from the Costmap2DROS api, moving it solely to the Costmap2D api. The user is responsible for making the copy themselves now, they can get a const pointer to the underlying char structure but must do the memcpy themselves
Mark add observation buffer as experimental, and also add book-keeping so that the Costmap2DROS object is not responsible for deletion.
TODO: We need to have an overall discussion about whether or not to use get in all the accessors we have
Add get to all the costmap accessors
Remove the lock() and unlock() calls for the costmap
laserScanCallback and PointCloudCallback should be private
Get rid of resume and just make start intelligent and add documentation
enqueue should be private
sizeInCellsX vs sizeInMetersX
getRobotFrameID, getBaseFrameID
get rid of frame_from_message
observation_sources instead of observation_topics and add a topic field in the source namespace
raytrace_range should be per-source as well
see search param
If static map is true the following valus will be overridden
document default values for parameters
nicer formatting on param docs?
- Roland
documentation: add diagram explaining the meaning of cell costs (move from http://pr.willowgarage.com/pr-docs/ros-packages/mpglue/html/index.html))--
--(documentation: does Costmap2D::circumscribedCell() return true only when the cell is strictly in the ring outward from inscribed, or does it encompass the lethal and inscribed regions? True only when the cell is in the ring that lies between the inscribed and circumscribed regions, changed docs to be more clear on this
It would be nice to have a direct way to add and remove lethal obstacles from the costmap, without going through an Observation object. The costmap would automagically do the right inflation, but we would not be forced to provide "origins" for the "observations". You can construct an Observation without an origin and use it for marking obstacles, for clearing, however, you need to specify an origin since raytracing is the only supported clearing operation
Meeting agenda
To be filled out by proposer based on comments gathered during API review period
Conclusion
Package status change mark change manifest)
Action items that need to be taken.
Major issues that need to be resolved
api conditionally cleared