Hide GDAL from public interface, fix export

* Enforce export in CI * Hide implementation details of GDAL in implementation * Switch to build-scope includes * Install headers per latest ament recommendations to allow package: https://docs.ros.org/en/humble/How-To-Guides/Ament-CMake-Documentation.html#adding-targets * This is crucial before realeasing as a library * Remove unused yaml dependency * Add all missing libraries in ament_export_dependencies * Fix a few typos in comments Signed-off-by: Ryan Friedman <[email protected]>
ethz-asl · Jan 20, 2024 · 5316213 · 5316213
1 parent 92dbada
commit 5316213
Show file tree

Hide file tree

Showing 12 changed files with 133 additions and 65 deletions.
diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml
@@ -28,11 +28,18 @@ jobs:
         rosdep install --from-paths src --ignore-src -y
       shell: bash
     - name: Colcon Build (Release)
-      working-directory: 
       run: |
         source /opt/ros/${{matrix.config.rosdistro}}/setup.bash
         colcon build --cmake-args -DCMAKE_BUILD_TYPE=Release --packages-up-to grid_map_geo
       shell: bash
+    - name: Verify Export
+      run: |
+        source /opt/ros/${{matrix.config.rosdistro}}/setup.bash
+        source install/setup.bash
+        cd test/export
+        colcon build
+        ./build/grid_map_export_test/main
+      shell: bash
     # - name: unit_tests
     #   working-directory:
     #   run: |

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -25,8 +25,6 @@ find_package(rclcpp REQUIRED)
 find_package(Eigen3 REQUIRED)
 find_package(GDAL 3 REQUIRED)
 
-find_package(yaml_cpp_vendor REQUIRED)
-link_directories(${yaml_cpp_vendor_LIBRARY_DIRS})
 
 # Reverse compatability for GDAL<3.5
 # https://gdal.org/development/cmake.html
@@ -37,13 +35,13 @@ endif()
 # Build
 add_library(${PROJECT_NAME}
   src/grid_map_geo.cpp
+  src/transform.cpp
 )
 
 target_include_directories(${PROJECT_NAME}
   PUBLIC
-  "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
-  "$<INSTALL_INTERFACE:include/${PROJECT_NAME}>"
-)
+    "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>"
+    "$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")
 
 target_link_libraries(${PROJECT_NAME} PUBLIC Eigen3::Eigen PRIVATE GDAL::GDAL)
 
@@ -77,7 +75,9 @@ install(
 )
 
 ament_export_targets(export_${PROJECT_NAME} HAS_LIBRARY_TARGET)
-ament_export_dependencies(GDAL)
+
+# Everything in find_package goes here too, ordered alphabetically.
+ament_export_dependencies(Eigen3 GDAL grid_map_core grid_map_msgs grid_map_ros tf2_ros rclcpp)
 
 install(
   TARGETS
@@ -103,7 +103,7 @@ install(DIRECTORY
 # Test
 include(CTest)
 if(BUILD_TESTING)
-  add_subdirectory(test)
+  add_subdirectory(test/unit)
 endif()
 
 ament_package()
diff --git a/include/grid_map_geo/grid_map_geo.hpp b/include/grid_map_geo/grid_map_geo.hpp
@@ -38,8 +38,8 @@
 #include <grid_map_core/iterators/GridMapIterator.hpp>
 #include <iostream>
 
-#include "grid_map_geo/transform.hpp"
-#include "tf2_ros/transform_broadcaster.h"
+#include "transform.hpp"
+#include <tf2_ros/transform_broadcaster.h>
 struct Location {
   ESPG espg{ESPG::WGS84};
   Eigen::Vector3d position{Eigen::Vector3d::Zero()};
@@ -95,7 +95,7 @@ class GridMapGeo {
    *
    * @param map_path Path to dsm path (Supported formats are *.tif)
    */
-  bool Load(const std::string& map_path) { Load(map_path, ""); }
+  bool Load(const std::string& map_path) { return Load(map_path, ""); }
 
   /**
    * @brief Helper function for loading terrain from path
@@ -184,4 +184,4 @@ class GridMapGeo {
   std::string frame_id_{""};
   std::string coordinate_name_{""};
 };
-#endif
+#endif // GRID_MAP_GEO_H
diff --git a/include/grid_map_geo/transform.hpp b/include/grid_map_geo/transform.hpp
@@ -34,18 +34,6 @@
 #ifndef GRID_MAP_GEO_TRANSFORM_H
 #define GRID_MAP_GEO_TRANSFORM_H
 
-#if __APPLE__
-#include <gdal.h>
-#include <gdal_priv.h>
-#include <ogr_p.h>
-#include <ogr_spatialref.h>
-#else
-#include <gdal/gdal.h>
-#include <gdal/gdal_priv.h>
-#include <gdal/ogr_p.h>
-#include <gdal/ogr_spatialref.h>
-#endif
-
 #include <Eigen/Dense>
 
 enum class ESPG { ECEF = 4978, WGS84 = 4326, WGS84_32N = 32632, CH1903_LV03 = 21781 };
@@ -58,21 +46,7 @@ enum class ESPG { ECEF = 4978, WGS84 = 4326, WGS84_32N = 32632, CH1903_LV03 = 21
  * @param source_coordinates
  * @return Eigen::Vector3d
  */
-inline Eigen::Vector3d transformCoordinates(ESPG src_coord, ESPG tgt_coord, const Eigen::Vector3d source_coordinates) {
-  OGRSpatialReference source, target;
-  source.importFromEPSG(static_cast<int>(src_coord));
-  target.importFromEPSG(static_cast<int>(tgt_coord));
-
-  OGRPoint p;
-  p.setX(source_coordinates(0));
-  p.setY(source_coordinates(1));
-  p.setZ(source_coordinates(2));
-  p.assignSpatialReference(&source);
-
-  p.transformTo(&target);
-  Eigen::Vector3d target_coordinates(p.getX(), p.getY(), p.getZ());
-  return target_coordinates;
-}
+Eigen::Vector3d transformCoordinates(ESPG src_coord, ESPG tgt_coord, const Eigen::Vector3d source_coordinates);
 
 /**
  * @brief Helper function for transforming using gdal
@@ -82,22 +56,7 @@ inline Eigen::Vector3d transformCoordinates(ESPG src_coord, ESPG tgt_coord, cons
  * @param source_coordinates
  * @return Eigen::Vector3d
  */
-inline Eigen::Vector3d transformCoordinates(ESPG src_coord, const std::string wkt,
-                                            const Eigen::Vector3d source_coordinates) {
-  OGRSpatialReference source, target;
-  char* wkt_string = const_cast<char*>(wkt.c_str());
-  source.importFromEPSG(static_cast<int>(src_coord));
-  target.importFromWkt(&wkt_string);
-
-  OGRPoint p;
-  p.setX(source_coordinates(0));
-  p.setY(source_coordinates(1));
-  p.setZ(source_coordinates(2));
-  p.assignSpatialReference(&source);
-
-  p.transformTo(&target);
-  Eigen::Vector3d target_coordinates(p.getX(), p.getY(), p.getZ());
-  return target_coordinates;
-}
+Eigen::Vector3d transformCoordinates(ESPG src_coord, const std::string wkt,
+                                            const Eigen::Vector3d source_coordinates);
 
-#endif
+#endif // GRID_MAP_GEO_TRANSFORM_H
diff --git a/src/grid_map_geo.cpp b/src/grid_map_geo.cpp
@@ -32,7 +32,7 @@
  ****************************************************************************/
 
 /**
- * @brief Terain map representation
+ * @brief Terrain map representation
  *
  * @author Jaeyoung Lim <[email protected]>
  */

diff --git a/src/test_tif_loader.cpp b/src/test_tif_loader.cpp
@@ -31,19 +31,19 @@
  *
  ****************************************************************************/
 /**
- * @brief Node to test planner in the view utiltiy map
+ * @brief Node to test planner in the view utility map
  *
  *
  * @author Jaeyoung Lim <[email protected]>
  */
 
-#include "geometry_msgs/msg/transform_stamped.hpp"
+#include <geometry_msgs/msg/transform_stamped.hpp>
 #include "grid_map_geo/grid_map_geo.hpp"
-#include "grid_map_msgs/msg/grid_map.h"
-#include "grid_map_ros/GridMapRosConverter.hpp"
-#include "rclcpp/rclcpp.hpp"
-#include "std_msgs/msg/string.hpp"
-#include "tf2_ros/static_transform_broadcaster.h"
+#include <grid_map_msgs/msg/grid_map.h>
+#include <grid_map_ros/GridMapRosConverter.hpp>
+#include <rclcpp/rclcpp.hpp>
+#include <std_msgs/msg/string.hpp>
+#include <tf2_ros/static_transform_broadcaster.h>
 
 using namespace std::chrono_literals;
 

diff --git a/src/transform.cpp b/src/transform.cpp
@@ -0,0 +1,82 @@
+/****************************************************************************
+ *
+ *   Copyright (c) 2022 Jaeyoung Lim, ASL, ETH Zurich, Switzerland
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name terrain-navigation nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+#include "grid_map_geo/transform.hpp"
+
+#if __APPLE__
+#include <gdal.h>
+#include <gdal_priv.h>
+#include <ogr_p.h>
+#include <ogr_spatialref.h>
+#else
+#include <gdal/gdal.h>
+#include <gdal/gdal_priv.h>
+#include <gdal/ogr_p.h>
+#include <gdal/ogr_spatialref.h>
+#endif
+
+Eigen::Vector3d transformCoordinates(ESPG src_coord, ESPG tgt_coord, const Eigen::Vector3d source_coordinates) {
+  OGRSpatialReference source, target;
+  source.importFromEPSG(static_cast<int>(src_coord));
+  target.importFromEPSG(static_cast<int>(tgt_coord));
+
+  OGRPoint p;
+  p.setX(source_coordinates(0));
+  p.setY(source_coordinates(1));
+  p.setZ(source_coordinates(2));
+  p.assignSpatialReference(&source);
+
+  p.transformTo(&target);
+  Eigen::Vector3d target_coordinates(p.getX(), p.getY(), p.getZ());
+  return target_coordinates;
+}
+
+Eigen::Vector3d transformCoordinates(ESPG src_coord, const std::string wkt,
+                                            const Eigen::Vector3d source_coordinates) {
+  OGRSpatialReference source, target;
+  char* wkt_string = const_cast<char*>(wkt.c_str());
+  source.importFromEPSG(static_cast<int>(src_coord));
+  target.importFromWkt(&wkt_string);
+
+  OGRPoint p;
+  p.setX(source_coordinates(0));
+  p.setY(source_coordinates(1));
+  p.setZ(source_coordinates(2));
+  p.assignSpatialReference(&source);
+
+  p.transformTo(&target);
+  Eigen::Vector3d target_coordinates(p.getX(), p.getY(), p.getZ());
+  return target_coordinates;
+}
+
+
diff --git a/test/export/CMakeLists.txt b/test/export/CMakeLists.txt
@@ -0,0 +1,7 @@
+# Add gtest based cpp test target and link libraries
+cmake_minimum_required(VERSION 3.14)
+project(grid_map_export_test)
+find_package(grid_map_geo REQUIRED)
+add_executable(main main.cpp)
+target_link_libraries(main PRIVATE grid_map_geo::grid_map_geo)
+
diff --git a/test/export/main.cpp b/test/export/main.cpp
@@ -0,0 +1,13 @@
+#include <iostream>
+
+#include "grid_map_geo/grid_map_geo.hpp"
+
+
+
+int main() {
+    // Instantiate the class and call it so this doesn't get optimized out.
+    auto gmg = GridMapGeo();
+    std::cout << gmg.getCoordinateName() << std::endl;
+    return 0;
+}
+
diff --git a/test/CMakeLists.txt → test/unit/CMakeLists.txt b/test/CMakeLists.txt → test/unit/CMakeLists.txt
diff --git a/test/main.cpp → test/unit/main.cpp b/test/main.cpp → test/unit/main.cpp
diff --git a/test/test_grid_map_geo.cpp → test/unit/test_grid_map_geo.cpp b/test/test_grid_map_geo.cpp → test/unit/test_grid_map_geo.cpp