Jmeyer1292 / robot_cal_tools

A suite of tools focused on calibration of sensors for robotic workcell development
Apache License 2.0
140 stars 40 forks source link

Add explicit Boost dependency #86

Closed schornakj closed 4 years ago

schornakj commented 4 years ago

Depends on #85

ROS2 no longer depends on Boost, since many of the features that ROS1 was using like boost::shared_ptr have been integrated into the C++ standard library. Our rct_image_tools package uses some Boost features, specifically boost::optional, so it fails to build in ROS2.

A quick fix is to explicitly depend on, find, and link against Boost in rct_image_tools.

A longer-term fix would be to minimize our use of Boost in general. There is now a std::optional in C++17, but it might be better to redesign the functions that return optionals to throw exceptions instead.

jdlangs commented 4 years ago

:+1: to not using optionals for error-handling purposes. But we might as well do the switch to std::optional and C++17 since it's supported by the default gcc even in 18.04. If that's all that's needed to avoid a Boost dependency, that is.

schornakj commented 4 years ago

+1 to not using optionals for error-handling purposes. But we might as well do the switch to std::optional and C++17 since it's supported by the default gcc even in 18.04. If that's all that's needed to avoid a Boost dependency, that is.

I remembered that rct_optimizations uses Boost accumulators (for example, here), which as far as I know can't be directly replaced with a standard-library equivalent. I think I'll need to add my dependency modification to rct_optimizations as well.

marip8 commented 4 years ago

I think we should eventually just remove the functions that use boost::optional and prefer the ones that throw exceptions.

schornakj commented 4 years ago

I added explicit Boost dependencies to rct_optimizations as well for now. @marip8, this is ready for review once CI passes.

I think we should eventually just remove the functions that use boost::optional and prefer the ones that throw exceptions.

I'll make this modification in an upcoming PR.