Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

humble macos #1

Open
wants to merge 14 commits into
base: port-to-humble
Choose a base branch
from

Conversation

Ryanf55
Copy link
Owner

@Ryanf55 Ryanf55 commented Oct 28, 2023

From @srmainwaring , which gets this working on macos.

CMakeLists.txt Outdated
PUBLIC
"$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>"
${GDAL_INCLUDE_DIR}
Copy link
Owner Author

@Ryanf55 Ryanf55 Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed because the GDAL::GDAL target should already have all the properties. If it doesn't, let's add it up where the GDAL::GDAL alias is created. Modern cmake practices recommend carrying all properties through targets rather than variables.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in ethz-asl@766e75c

target_link_libraries(${PROJECT_NAME} Eigen3::Eigen GDAL::GDAL)

ament_target_dependencies(${PROJECT_NAME} SYSTEM
grid_map_core
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grid_map_core was already linked before through the namespaced target (preferred) compared to ament_target_dependencies. In conversations with the ROS maintainers, seems like the only thing ament_target_dependencies exists for these days is message packages because those can't use namespaced export targets yet.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If:

ament_target_dependencies(${PROJECT_NAME} SYSTEM
  grid_map_core
  grid_map_ros
)

is replaced with:

target_link_libraries(${PROJECT_NAME}
  Eigen3::Eigen
  GDAL::GDAL
  grid_map_core
  grid_map_ros
)

then there is an error:

In file included from /Users/rhys/Code/ros2/humble/ros2-aerial/src/grid_map_geo/src/grid_map_geo.cpp:40:
In file included from /Users/rhys/Code/ros2/humble/ros2-aerial/src/grid_map_geo/include/grid_map_geo/grid_map_geo.hpp:37:
In file included from /Users/rhys/Code/ros2/humble/ros2-aerial/src/grid_map_geo/include/grid_map_geo/transform.hpp:49:
In file included from /opt/homebrew/include/eigen3/Eigen/Dense:1:
In file included from /opt/homebrew/include/eigen3/Eigen/Core:256:
/opt/homebrew/include/eigen3/Eigen/src/Core/functors/StlFunctors.h:159:10: fatal error: 'grid_map_core/eigen_plugins/FunctorsPlugin.hpp' file not found
#include EIGEN_FUNCTORS_PLUGIN
         ^~~~~~~~~~~~~~~~~~~~~
<command line>:2:31: note: expanded from macro 'EIGEN_FUNCTORS_PLUGIN'
#define EIGEN_FUNCTORS_PLUGIN "grid_map_core/eigen_plugins/FunctorsPlugin.hpp"
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

The target grid_map_core::grid_map_core is not available. Something needing fixing upstream?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, try my patch here: ANYbotics/grid_map#404

Since grid_map seems largely unmaintained, we may have trouble getting changes in.

CMakeLists.txt Outdated

target_include_directories(test_tif_loader
PRIVATE
include
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary - we already did this in the install-portable way above, which is a direct copy of the ament_cmake tutorial; the generators above make it such that overrides are supported.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in ethz-asl@0143db6

CMakeLists.txt Outdated
"$<INSTALL_INTERFACE:include/${PROJECT_NAME}>"
)
# macOS
if(APPLE)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For cross-compilation purposes, have you considered using "${CMAK_SYSTEM_NAME} MATCHES Darwin as a condition?

Copy link

@srmainwaring srmainwaring Oct 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should use:

if(${CMAKE_SYSTEM_NAME} MATCHES Darwin)
  if(${CMAKE_SYSTEM_PROCESSOR} MATCHES arm64)
    link_directories(/opt/homebrew/opt/yaml-cpp/lib)
  endif()
  if(${CMAKE_SYSTEM_PROCESSOR} MATCHES x84_64)
    link_directories(/usr/local/opt/yaml-cpp/lib)
  endif()
endif()

None of this should be necessary, but something has gone wrong with the brew yaml-cpp or ROS 2 yaml-cpp-vendored package so it's dependencies are not propagated.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which version(s) do you have? After this patch I did, the exported namespace targets is available, and worked at least on Ubuntu. The 0.8 release has that included.

jbeder/yaml-cpp#1196

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srmainwaring Do you mind giving upstream yaml-cpp release 0.8 a try by adding it to your ROS workspace?

Ryanf55 and others added 13 commits October 30, 2023 02:17
* Port to ROS 2

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>

* Get launch file running aside from resources

* Update docs
* Remove ROS1 cruft

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>

---------

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* Update docs
* Remove ROS1 cruft

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
…irectory

Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Ryanf55 pushed a commit that referenced this pull request Jan 20, 2024
@srmainwaring srmainwaring deleted the wips/wip-humble-macos branch February 18, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants