API review

Proposer: TullyFoote

Present at review:

  • List reviewers

Open tickets

https://prdev.willowgarage.com/trac/personalrobots/query?status=assigned&status=new&status=reopened&group=status&milestone=common_msgs-0.1

These are from the previous review robot_msgs/2009-05-05_API_Review

Question / concerns / comments

We need to review all messages in these packages. We are looking to make ~6 months of stability for these messages. If we can't get to that level they should find new homes.

http://pr.willowgarage.com/pr-docs/ros-packages/diagnostic_msgs/html/index.html  
Messages in [diagnostic_msgs]:

diagnostic_msgs/DiagnosticMessage
diagnostic_msgs/DiagnosticStatus
diagnostic_msgs/DiagnosticValue
diagnostic_msgs/DiagnosticString

Services in [diagnostic_msgs]:

diagnostic_msgs/SelfTest

http://pr.willowgarage.com/pr-docs/ros-packages/mapping_msgs/html/index.html
Messages in [mapping_msgs]:

mapping_msgs/PolygonalMap
mapping_msgs/OrientedBoundingBox
mapping_msgs/CollisionMap
mapping_msgs/Object
mapping_msgs/AttachedObject
mapping_msgs/ObjectInMap

http://pr.willowgarage.com/pr-docs/ros-packages/nav_msgs/html/index.html
Messages in [nav_msgs]:

nav_msgs/MapMetaData
nav_msgs/ParticleCloud
nav_msgs/OccGrid


http://pr.willowgarage.com/pr-docs/ros-packages/robot_msgs/html/index.html
Messages in [robot_msgs]:

robot_msgs/PoseDot
robot_msgs/Path
robot_msgs/JointCmd
robot_msgs/PoseWithCovariance
robot_msgs/Polygon3D
robot_msgs/Wrench
robot_msgs/AngularVelocity
robot_msgs/Vector3Stamped
robot_msgs/Vector3
robot_msgs/PointCloud
robot_msgs/Twist
robot_msgs/ControllerState
robot_msgs/Quaternion
robot_msgs/AngularAcceleration
robot_msgs/PoseStamped
robot_msgs/ChannelFloat32
robot_msgs/Point32
robot_msgs/Velocity
robot_msgs/PointStamped
robot_msgs/PoseDDot
robot_msgs/PoseWithRatesStamped
robot_msgs/Pose
robot_msgs/QuaternionStamped
robot_msgs/Point
robot_msgs/VOPose
robot_msgs/Acceleration
robot_msgs/BatteryState
robot_msgs/TransformStamped
robot_msgs/Transform


http://pr.willowgarage.com/pr-docs/ros-packages/robot_srvs/html/index.html
Services in [robot_srvs]:

robot_srvs/SetJointCmd
robot_srvs/GetVector
robot_srvs/MoveToPose
robot_srvs/SetVector
robot_srvs/GetQuaternion
robot_srvs/GetValue
robot_srvs/GetJointCmd
robot_srvs/SetPoseStamped

http://pr.willowgarage.com/pr-docs/ros-packages/sensor_msgs/html/index.html
Messages in [sensor_msgs]:

sensor_msgs/DisparityInfo
sensor_msgs/CompressedImage
sensor_msgs/StereoInfo
sensor_msgs/Image
sensor_msgs/CamInfo
sensor_msgs/RawStereo
sensor_msgs/LaserScan


Possibly including
http://pr.willowgarage.com/pr-docs/ros-packages/manipulation_msgs/html/index.html
Messages in [manipulation_msgs]:

manipulation_msgs/SplineTraj
manipulation_msgs/JointTraj
manipulation_msgs/TaskFrameFormalism
manipulation_msgs/JointTrajPoint
manipulation_msgs/SplineTrajSegment
manipulation_msgs/IKRequest


http://pr.willowgarage.com/pr-docs/ros-packages/manipulation_srvs/html/index.html
Services in [manipulation_srvs]:

manipulation_srvs/IKService
manipulation_srvs/IKQuery


http://pr.willowgarage.com/pr-docs/ros-packages/nav_srvs/html/index.html
Services in [nav_srvs]:

nav_srvs/StaticMap
nav_srvs/Plan

Other questions:

  • Jeremy: We should really change the Image format before we do this. Nobody is satisfied with the current format. The best proposal so far is to mirror the OpenCV 2d Matrix data representation. James should be putting together a proposal for this.

Patrick:

  • James' Image proposal, from discussion at BinaryMultiArray:

    To allow efficient transport of OpenCV CvMat http://opencv.willowgarage.com/documentation/basic_structures.html#CvMat objects.

    int32 type            # Type of elements, and channels.  Encoded as in OpenCV's CV_8UC1 .. CV_64FC4.
    int32 step            # Full row length in bytes
    uint8 data[]          # Pointers to the actual matrix data
    int32 rows            # Number of rows
    int32 cols            # Number of columns
    uint8 is_bigendian    # is this data bigendian
  • I also propose CompressedImage be modified slightly:

    Header header        # Header
    string label         # Label for the image
    string encoding      # Specifies the color encoding of the data
                         #   Acceptable values are:
                         #    1 channel types:
                         #     mono
                         #    3 channel types:
                         #     rgb, bgr
    string format        # Specifies the format of the data
                         #   Acceptable values:
                         #     jpeg, png
    # No depth field, 8 bit channels assumed
    
    int32 rows
    int32 cols
    uint8[] data  # previously UInt8MultiArray

Stu:

  • Either robot_msgs/PoseDot or robot_msgs/Twist should go. They represent exactly the same thing in exactly the same way.
  • I would still prefer to reduce the following down to Vector3: AngularVelocity, AngularAcceleration, Velocity, Point.

Meeting agenda

Step through all messages and verify that they are as we want for the next 6 months.

Conclusion

Package status change mark change manifest)

  • /!\ Action items that need to be taken.

  • {X} Major issues that need to be resolved

Notes

change DiagnosticValue to become KeyValue
string key
string value

Remove DiagnosticString

Rename DiagnosticMessage to DiagnosticArray

if possible remove mapping_msgs from stack

rename Occgrid to OccupancyGrid

add header to OccupancyGrid and ParticleCloud

ParticleCloud -> PoseArray

convert Image to cvMat copy with row major comment and Constants in message definiton, and a unit test in different package \
to assert that the opencv and message enums are the same.  Note no auto big endian support.  Add a header

nav_msgs/StaticMap -> GetMap

nav_msgs/Plan -> GetPlan

both nav_srvs into nav_msgs

manipulation_msgs and srvs aren't mature and should not be released in the stack for now, they can be added later.  move the\
m to a different stack/sandbox

sensor_msgs/CamInfo need comments ot describe matrix dimentions and row major
rename to CameraInfo  and link to descriptions of what they are

ditto for StereoInfo

review ROI interaction with height/width in Info Messages

consider postponing the release of imaging messages

compressedImage
look up if encoding in necessary outside the format spec, if not remove
drop label
switch to uint8[]


Wiki: common_msgs/Reviews/2009-07-20_API_Review (last edited 2009-08-14 20:52:27 by localhost)