API review

Proposer: Blaise Gassend

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.

Vijay

  • Seems a little odd that the default value for ~skip is 1. Might make more sense for this to be 0. Thus, novice users won't lose half their data

    • Jeremy: The reason for this is that with intensity turned on and full field of view, this exceeds the hokuyo's bandwidth. Alternatives are to default to a narrower field of view or default to intensity turned off.

    • BG: Changed "~skip"'s default to 0, and intensity's default to false.
  • In order to match the rest of the system (and sicktoolbox_wrapper), parameter ~frameid should probably be ~frame_id

    • Docs should be explicit about positioning the frame:
      "This frame should be at the optical center of the laser, with the x-axis along the zero degree ray, and the y axis along the 90 degree ray"

      • Done.
    • Might be a little cleaner if the default value is something like NO_FRAME_SPECIFIED

      • BG: Default is now "laser".
  • Any thoughts for a better name than ~skip? This is definitely the most unclear name in this node's ROS API.

    • BG: After discussion, it seems that skip is satisfactory.

Jeremy

  • The ~model_04LX parameter is not necessarily going to generalize well into the future. This should probably be renamed to something like send_scip2_switch, but could potentially be removed all together if it's safe to just run all hokuyos in this mode.

    • BG: It has been removed. The driver now always tries to switch to SCIP2.0, and defaults to intensity=false.
  • FRAMEID_LASER doesn't feel like the right default frame. This should be chosen according to an appropriate convention. possibilities here are:
    • Completely unique: hokuyo_<id>

    • Driver unique: hokuyo

    • Device-type unique: laser

    • Non-unique: <empty> or NONE

  • BG: Changed default to "laser".

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


Wiki: hokuyo_node/Reviews/2009-11-24 API Review (last edited 2009-12-08 08:27:43 by BlaiseGassend)