Target Loading Update

[marip8]

This PR revises the functions for loading a target definition from YAML file or string path in order to support loading different types of targets (specifically ChArUco and ArUco grid targets).

In order to have the functions with the same parameters (i.e. ROS node handles, strings, etc.) but return a different datatype (i.e. modified circle grid target, ChArUco grid target), I had to create a template struct that is specializable based on the target type.

@schornakj can you review?

[schornakj]

@marip8 I'll take a look!

[schornakj]

@marip8 I noticed that this functionality in rct_ros_tools didn't have any tests previously. Do you think this is a good time to add a test that exercises loading a target from parameters?

[marip8]

@schornakj I'm getting the following build error on Xenial from the ROS unit test that I added to rct_ros_tools.

target_loader_utest.cpp:(.text._ZN7testing8internal21TypeParameterizedTestI16TargetLoaderTestNS0_11TemplateSelI26TargetLoaderTest_test_TestEENS0_6Types1IN15rct_image_tools24ModifiedCircleGridTargetEEEE8RegisterEPKcRKNS0_12CodeLocationESC_SC_iRKSt6vectorINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESaISM_EE[_ZN7testing8internal21TypeParameterizedTestI16TargetLoaderTestNS0_11TemplateSelI26TargetLoaderTest_test_TestEENS0_6Types1IN15rct_image_tools24ModifiedCircleGridTargetEEEE8RegisterEPKcRKNS0_12CodeLocationESC_SC_iRKSt6vectorINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESaISM_EE]+0x1fd): undefined reference to `testing::internal::MakeAndRegisterTestInfo(char const, char const, char const, char const, testing::internal::CodeLocation, void const, void ()(), void ()(), testing::internal::TestFactoryBase)'

I think this might be a result of us downloading GTest for rct_optimizations but trying to link against a version of GTest provided by rostest in the unit test added by this PR. Any thoughts about how to resolve this? I don't think I can use GTest directly because this unit test requires ROS to be launched with parameters in order to successfully execute. Maybe we change which version of GTest we download to match the one included by rostest?

[schornakj]

@marip8 That's rather annoying. I agree that the way around this is probably to make sure that we use the same version of GTest as the one rostest is using.

We might be able to resolve this by changing how we install GTest. Right now we build it from source in the rct_common package, but during my brief research I noticed that there's a rosdep key for GTest, which for Ubuntu points to libgtest-dev. Maybe we can try installing that and see if it'll work with the pure-GTest unit tests in the other packages.

[marip8]

Per @Levi-Armstrong this issue is fixed by calling find_package(GTest REQUIRED) and targeting against GTest::GTest and GTest::Main before ${catkin_LIBRARIES}, which seems to use the downloaded version of GTest before finding the catkin-provided version.

[jdlangs]

Just curious, what was there reason the templated struct had to be added versus just templating and specializing the functions themselves?

[marip8]

In order to have the functions with the same parameters (i.e. ROS node handles, strings, etc.) but return a different datatype (i.e. modified circle grid target, ChArUco grid target), I had to create a template struct that is specializable based on the target type.

I ran into build errors initially trying to do what you suggested, so I implemented this solution instead. I didn't look too much into the issue. Ultimately I would like to remove the non-throwing loaders, so maybe we can revisit this in the future.