API review

Reviewer:

Ethan

Question / concerns / comments

As with image_proc I would like to see an easy way to perform the stereo computations without spawning a separate node.
If processing is on demand then what happens when a topic is subscribed to that does not make sense given the current inputs.
At some point we will want to transition the point cloud type. Can we add an optional parameter to make this easier? Something along the lines of "point_cloud_type" perhaps. By default it would publish the old point cloud type. When the new point cloud type is available you would be able to select it by setting the parameter to what you want. Eventually we'll tic-toc so that you are warned if you are using the old point cloud type, then after that the default becomes the new point cloud type (and any code that has not been updated is broken). At this point we can either provide an option to still get the old point cloud type by explicitly requesting it with this parameter or decide we want to force people to make the transition.
Why does image_proc subscribe on demand to the camera and camera_info topics, while stereo_image_proc is always subscribed?
Does the current architecture always calculate / publish the image form of the disparity? It appears to do so. Can a parameter be added to disable this, since it is really only useful for debugging / visualization, and probably consumes a non-trivial amount of processing resources.
Can the colormap be put somewhere else? It seems like this is something that would be potentially generally useful, and also it clutters up stereo_image_proc.cpp.

Conclusion

$/!\$ Create functional form of stereo processing.
$/!\$ Add parameter to ease point cloud message transition.
$/!\$ Add parameter to prevent computation / publishing of image form of disparity.
Determine behavior when camera type and subscription type are incompatible. Prevent advertisement of topics that are incompatible with chosen camera if possible.