search

ros-perception / camera_info_manager_py

Subset of the C++ camera_info_manager ROS package for Python camera drivers
http://wiki.ros.org/camera_info_manager_py
7 stars 25 forks source link

Make saveCalibrationFile yamls compatible with C++ CameraInfoManager #8

Closed lucasw closed 8 years ago

lucasw commented 8 years ago

For #7

My C++ camera info generator could be made into an actual test, right now it is a matter of running that manually, and running the python equivalent manually.

jack-oquin commented 8 years ago

Thanks for the contribution!

jack-oquin commented 8 years ago

Note to self: after this is merged, convert the package.xml to format 2, and convert the <build_depend> for camera_info_manger to a <test_depend>, making the corresponding CMakeLists.txt changes.

jack-oquin commented 8 years ago

I wonder if this is safe to release in Indigo?

We need a test to ensure that existing old-format YAML files still work. I don't want to break anyone's currently-working system.

lucasw commented 8 years ago

There ought to be a C++ to python test, python to C++ test, and python to python test (already exists?), and save an old format file into the test repository and load it, check values (then save and reload in new format and check values again?).

I have some of the C++ to python started in the last commit, though the cmake isn't hooked up properly- can add_rostest() have a dependency that triggers a cmake custom command? Or another route would be to turn the C++ camera_info generator into a ros node that the python test case can talk to. (Would it make sense for camera_info_manager to have a ros node wrapper for general use?)

jack-oquin commented 8 years ago

There ought to be a C++ to python test, python to C++ test, and python to python test (already exists?), and save an old format file into the test repository and load it, check values (then save and reload in new format and check values again?).

I agree, and though it's not that much work, it's not something I am likely to get to any time soon.

I have some of the C++ to python started in the last commit, though the cmake isn't hooked up properly- can add_rostest() have a dependency that triggers a cmake custom command?

I suppose so. I do want to be careful with the test dependencies, so they don't end up being install dependencies of the actual package.

Or another route would be to turn the C++ camera_info generator into a ros node that the python test case can talk to. (Would it make sense for camera_info_manager to have a ros node wrapper for general use?)

Probably not much general use. The camera_info_manager is really just a small part of a camera driver. But, your generator node could definitely serve as test scaffolding in place of a real camera calibration node. That's a good approach.

lucasw commented 8 years ago

I can add the tests.

The cmake camera_info file generation route doesn't work for calling rostest from the command line, so the generator node is now in the .test file and the python calls the C++ service to save the camera info to disk, then reloads and compares. (and add_rostest DEPENDENCIES didn't seem to trigger a cmake add_custom_command)

The next one will be the python to C++ test.

jack-oquin commented 8 years ago

That looks great. Much appreciated!