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.

costmap_2d documentation

  • 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.

  • {X} Major issues that need to be resolved

api conditionally cleared


Wiki: costmap_2d/Reviews/2009-07-13_API_Review (last edited 2009-08-14 20:53:41 by localhost)