search

jhu-lcsr-forks / rtt_ros_integration

Orocos-ROS integration libraries and tools
1 stars 2 forks source link

rtt_roscomm: Unified naming scheme for rtt_rosservice and use a singleton for the ROSServiceRegistryService #55

Closed meyerj closed 10 years ago

meyerj commented 10 years ago

The first commit just renames a few files that had ros_service in their name so that all files and libraries have the rtt_rosservice_ prefix. This is just a cleanup to avoid confusion with the rtt_ros_service located in package rtt_ros.

The second commit adds a singleton pattern to the ROSServiceRegistryService class and updated all classes that use that service to call the static ROSServiceRegistryService::Instance() method instead of relying that the service has already been loaded in the global service. This fixes an error if typekit packages that contain service proxies are imported without using the ros.import() operation. They will now load the rosservice_registry service automatically.

jbohren commented 10 years ago

Cool. This sounds good to merge, I just need to fix the test harness.

jbohren commented 10 years ago

I'll merge this locally.

meyerj commented 10 years ago

Rebased onto hydro-devel.

jbohren commented 10 years ago

It looks like this depends on https://github.com/orocos-toolchain/rtt/pull/18

meyerj commented 10 years ago

Hmm, I wrote these patches two weeks before I added the UNBUFFERED type to the ConnPolicy class and none of the commits modifies the rtt_rostopic_service.cpp file.

I am sorry. The error was introduced with my merge with master in 5a54286ca2b0508a32fcb24fe17578dd3574cdd5, which I also pushed to your hydro-devel branch in jhu-lcsr-forks in order to rebase the pull requests because they would have required non-trivial merges in the meantime. I guess we can merge orocos-toolchain/rtt#18 quickly as this was already discussed in orocos-toolchain/rtt_ros_integration#3...

jbohren commented 10 years ago

So we just need to merge in the UNBUFFERED connection implementation into the https://github.com/jhu-lcsr-forks/rtt fork, right?

meyerj commented 10 years ago

This pr has been merged in upstream by @smits as part of https://github.com/orocos-toolchain/rtt_ros_integration/pull/5 earlier today.