grid_map icon indicating copy to clipboard operation
grid_map copied to clipboard

Fix Eigen Plugins compilation when linking to grid_map_core

Open Ryanf55 opened this issue 1 year ago • 2 comments

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.

Ryanf55 avatar Jul 31 '24 05:07 Ryanf55

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.

nhewitt99 avatar Aug 22 '24 16:08 nhewitt99

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!

Ryanf55 avatar Aug 22 '24 17:08 Ryanf55

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.

StefanFabian avatar Dec 01 '24 12:12 StefanFabian

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

doisyg avatar Jan 09 '25 13:01 doisyg

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):

botsandus@3ce1c30

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.

Ryanf55 avatar Jan 18 '25 21:01 Ryanf55

Hi @Ryanf55 , sorry for my late answer. My branch fixes the problem: botsandus@3ce1c30

Not the one from this PR

doisyg avatar Feb 17 '25 18:02 doisyg

Linking to my proposed fix #519.

cmake extras should not be needed IMO.

msplr avatar Apr 30 '25 18:04 msplr