Allow proper usage of namespaces for openni's nodes and nodelets

[bit-pirate]

This is a slightly different patch for issue #2.
The core changes are:

• Nodelet manager runs on the same level as the nodelets, thereby removing the need for an absolute manager name as nodelet parameter. This allows pushing down all openni node's and nodelets in extra namespaces, e.g. for different robots, applications etc.
• Sorts the various nodelet topics (depth, ir, rgb, ...) by remapping the nodelets' i/o topics

Additional changes:

• The worker threads parameter has been made accessible.
• Assumed duplicated include of depth nodelets for depth_registered has been removed. This also makes the service registration error go away.

[bit-pirate]

Can we get this into Hydro? We would need this for TurtleBot and our other own robots.

[jonbinney]

Thanks for fixing this! I'm going to do a little testing before i merge it. The key for hydro will be to make sure that it is backwards compatible with most of the ways people used openni_launch in groovy (making sure the defaults for the args result in the old behavior). But other than that, I don't see a problem getting this into hydro.

[jonbinney]

@piyushk - does freenect_launch allow namespaces in this way? If we can keep openni_launch and freenect_launch somewhat similar it should make life easier for users.

[bit-pirate]

@jonbinney I took special care to not break to old structure. Let me know in case you find anything which breaks the structure and I'll fix it.

[bit-pirate]

@jonbinney I just added a few more parameters to allow (de)activating each processing module.

We added this feature to our custom launcher for TurtleBot, since the TB netbooks don't have a lot of horsepower.

[piyushk]

@jonbinney There are minor differences, though I am not sure if they appear on the user side. I'll spend a bit of time testing this evening.

Ideally, I would like to reduce code redundancy by creating a rgbd_launch package with all these files and have freenect_launch and openni_launch depend on it. What do you all think?

[jonbinney]

@piyushk an rgbd_launch package sounds like a great idea. So then openni.launch would just be a thin wrapper around the files in rgbd_launch, setting the correct device nodelet?

[jonbinney]

Tried this and it seems to preserve the old functionality, so i'm merging it.

[piyushk]

@jonbinney sounds good. your description of rgbd_launch is exactly what I meant. Can you create an rgbd_launch and rgbd_launch-release repository and give me rw permissions? I will play around with it this weekend and see if I can merge the launch files.

[jonbinney]

@piyushk I've created rgbd_launch here: https://github.com/ros-drivers/rgbd_launch and the release repo here: https://github.com/ros-gbp/rgbd_launch-release. For the source repo i gave the freenect_maintainers team access, and for the release repo i made a new team to give you push/pull access.

[bit-pirate]

@jonbinney Please let me know, when you have re-released openni_launch. Then we can re-release the required changes in the turtlebot packages.

[jonbinney]

@bit-pirate i've done the release, so now the pull request just has to be accepted to rosdistro and the build farm needs to process it: ros/rosdistro#1572

[piyushk]

@jonbinney I have ported this new release to rgbd_launch and have ported openni_launch against it (this was pretty easy). I am in the process of porting freenect_launch against it right now. I will break the freenect_launch API to conform with this one starting with Hydro, as Marcus's solution is much cleaner than mine.

@bit-pirate Marcus, when you use openni_launch on multi-robot systems, do you overwrite the rgb_frame_id and depth_frame_id arguments to attach a robot specific tf_prefix?

[piyushk]

@bit-pirate (adding to previous question) Using your patch, I was unable to find a way to set a robot specific tf prefix to the static publishers in kinect_frames (which are still getting incorrectly resolved). I don't use kinect_frames.launch in my own code, but I'll try and think of a way to fix it if necessary.

[bit-pirate]

@jonbinney Thanks for getting this out there!

@piyushk Honestly, I haven't thought about the tf publishers at all and left it untouched. We don't use it here either and I also believe tf and even tf2 is not suited for a multi-robot (multi-machine) setup with limited network bandwidth. However, allowing users to use the tf prefix seems like a good idea to me.

I had a quick look and saw that even though one can set the tf names, they are not passed onto the tf publishers (all frames are set to <camera_name>_some_tf_suffix). I guess, the two necessary changes are:

• adding args use_tf_prefix and tf_prefix
• passing both the tf_prefix and the frame names to the processing and tf publisher launchers.

Let me know, when you have a new version and I'll give it a try!

[piyushk]

@bit-pirate Thanks for the info. I updated the code accordingly. I am facing 1 last problem - I can't seem to produce a registered xyzrgb cloud from openni.launch anymore - If you know if off the top of your head then let me know. I will try and figure out the combination later tonight.

[bit-pirate]

Just found out that we have built in a tricky corner case. If someone turns on depth_registration, but has depth_registered_processing turned off, he ends up having no point cloud at all, even if he has depth_processing turned on.

I believe, we can leave things as they are now. Turning on depth_registered_processing when turning on depth_registration is quite logical and if people miss that, I believe they can figure it out quickly. But we should keep this in mind, when designing rgbd_launch.