Fix Eigen Plugins compilation when linking to grid_map_core
Purpose
Here's a proposed patch for #382. Because the original PR for adding the plugins through CMake and ament is not well documented, I really don't know how this will affect others. Please try this PR out. It will stay open for a while until I get a reasonable number of approvals it doesn't break anything.
Test instructions
- Check this branch out
-
colcon build --packages-up-to issue382
And try building your own packages that link to grid_map_core.
Just adding my $0.02 that this PR does fix the Eigen linker issues when using grid_map_core for me, thank you! I can't speak to its potential impact on other plugins, though.
Just adding my $0.02 that this PR does fix the Eigen linker issues when using grid_map_core for me, thank you! I can't speak to its potential impact on other plugins, though.
Thanks for sharing!
My $0.02: This is in line with what I proposed. I would only move the code from the extas file to the main cmakelists and remove the extras file. If it is not installed, there's no reason to make it an extra file, and with the naming, it could be confusing.
Generally, setting these Eigen compile time variables in the public interface of a library is IMHO bad style as it limits or at least makes it a lot more complicated for people to add their own extensions and makes it incompatible with any other library that does the same (of which I hope there are none but still).
If it's only used in the cpp files, you could set them as PRIVATE and retain the functionality without leaking into downstream packages.
Thanks for this! I am not sure about the proper solution but this is what I have been end up using to be able to build and use properly grid_map_ros in external packages (in Jazzy):
https://github.com/botsandus/grid_map/commit/3ce1c30b199718e1d8e5a74014c530c8ed438e15
Thanks for this! I am not sure about the proper solution but this is what I have been end up using to be able to build and use properly
grid_map_rosin external packages (in Jazzy):
THanks for the report. If you take this branch, and try it, does it fix the problem? I think we should merge this, no one said it made things worse.
Hi @Ryanf55 , sorry for my late answer. My branch fixes the problem: botsandus@3ce1c30
Not the one from this PR
Linking to my proposed fix #519.
cmake extras should not be needed IMO.