Use standard topics for camera drivers

[jonbinney]

Usually nodes publish using the public node handle, and use the private node handle only for looking up parameters. This creates problems when trying to use image_proc. The normal setup would be to start the camera driver (this package) as a nodelet in a namespace, and image_proc nodelets in the same namespaces. the image proc nodelets expect raw images on "image_raw, the camera driver nodelet publishes them on "image_raw", and you end up with a nice neat set of topics:

<camera_namespace>/image_raw # published by pylon_camera nodelet
<camera_namespace/camera_info # published by pylon_camera nodelet
<camera_namespace>/image_rect  # published by image_proc nodelet
<camera_namespace>/image_mono # published by debayering nodelet from image_proc 
<camera_namespace>/image_color # published by debayering nodelet from image_proc 

But because pylon_camera advertises image_raw using a private nodehandle, we end up with

<camera_namespace>/<node_name>/image_raw

It's probably possible to workaround this with remappings, but I think it would be better to use a public node handle.

[jonbinney]

The main downside would be that since this changes topics, it would break current setups that people have. For that reason, it would probably only make sense if a new branch is created for kinetic/lunar so that the indigo code could be left unchanged, or maybe some workaround where images are published on both new and old topics for now.

A couple of links:

The ROS driver for point grey cameras, which I think is one of the best designed ROS drivers: https://github.com/ros-drivers/pointgrey_camera_driver

Image proc on the ROS wiki, which has examples on how to do both debayering and rectification: http://wiki.ros.org/image_proc

[marc-up]

Thanks a lot for the explanation!

The reason why we decided to not do it the standard way is that the continuous publishing of images under a topic leads to problems if you have multiple nodes requesting images with different settings (e.g one node requests for bright images, and just afterwards another one asks for dark ones). So, to make sure, that every node gets only the images it wants, we implemented the action based acquisition, where after each grab all previous settings are restored. Because the image_proc pkg works only on normal topics, we made the rectification part of the pylon_camera pkg.

But I can see the point with the private namespace. I would propose to add a new optional parameter that is a string and defaulting to ~. With that we would be backwards compatible and we could get rid of the private ns. What do you think?

[jonbinney]

Ah, I understand. The action server is cool - definitely one of the better ways to handle cases where you want synchronous behavior. The asynchronous nature of pub/sub makes many things easier (introspection, logging, visualization) but definitely comes up short when ordering of operations and avoiding conflicts because multiple clients want to control a resource.

Ideally there should be some way to preserve the actionlib functionality you need while still providing an interface that meshes nicely with patterns used in other image processing ROS packages.

One possible idea: action server that sets the settings and returns timestamps in its feedback. Each timestamp in the feedback would correspond exactly to the timestamp of an image that has been taken with the settings asked for by that node, and published on the public image_raw topic.

Advantages

• other nodes could still use the images published if they don't care so much about the settings
• standard topics mean that image_proc could still be used. Any node (including the one who made sent the action goal) could use image_raw or image_rect, and even image_color (created by debayering nodelet). The timestamps for those images will be identical.
• putting the timestamps in the feedback means that the clients can use the images immediately if they want to, rather than doing batch processing at the end.

  Disadvantages

• a bit more work on the client side. Nodes will need to use the action server, and also get messages on the main message topic, plus match timestamps to figure out witch messages are theirs. This could be abstracted into a small C++ or python client class though, making things easy for users.

[jonbinney]

One other thought: dynamic_reconfigure is often useful for these types of things. It is synchronous, since it uses a service to update the settings on the server node (in this case the pylon_camera node). The thing it doesn't have, which actionlib does, is the ability to block other nodes from changing the camera settings for a while.

[130s]

Great thread. This sort of information should be put up somewhere common as the best practice for implementing ROS sensor driver.