From 3528778b383496ac71a5e1a332ee6f11782c51c2 Mon Sep 17 00:00:00 2001 From: Ryan Friedman Date: Sat, 20 Jan 2024 11:00:31 -0700 Subject: [PATCH] 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 * Add badges for all jobs but point them to the default ros2 branch rather than any branch * Fix style check not running on ros2 branch Signed-off-by: Ryan Friedman --- .github/workflows/build_test.yml | 9 ++- .github/workflows/check_style.yml | 2 +- CMakeLists.txt | 14 ++--- README.md | 4 +- include/grid_map_geo/grid_map_geo.hpp | 9 +-- include/grid_map_geo/transform.hpp | 48 +--------------- src/grid_map_geo.cpp | 2 +- src/test_tif_loader.cpp | 16 +++--- src/transform.cpp | 79 +++++++++++++++++++++++++++ test/export/CMakeLists.txt | 7 +++ test/export/main.cpp | 10 ++++ test/{ => unit}/CMakeLists.txt | 0 test/{ => unit}/main.cpp | 0 test/{ => unit}/test_grid_map_geo.cpp | 0 14 files changed, 133 insertions(+), 67 deletions(-) create mode 100644 src/transform.cpp create mode 100644 test/export/CMakeLists.txt create mode 100644 test/export/main.cpp rename test/{ => unit}/CMakeLists.txt (100%) rename test/{ => unit}/main.cpp (100%) rename test/{ => unit}/test_grid_map_geo.cpp (100%) diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index b87c819..173351b 100644 --- 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 src/grid_map_geo/test/export + colcon build + ./build/grid_map_export_test/main + shell: bash # - name: unit_tests # working-directory: # run: | diff --git a/.github/workflows/check_style.yml b/.github/workflows/check_style.yml index fa01ca6..ad8af78 100644 --- a/.github/workflows/check_style.yml +++ b/.github/workflows/check_style.yml @@ -3,7 +3,7 @@ name: Style Checks on: push: branches: - - master + - ros2 pull_request: branches: - '*' diff --git a/CMakeLists.txt b/CMakeLists.txt index 53f6666..c180be9 100644 --- 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 - "$" - "$" -) + "$" + "$") 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/README.md b/README.md index b10d6fa..da2b88a 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ # grid_map_geo -[![Build Test](https://github.com/ethz-asl/grid_map_geo/actions/workflows/build_test.yml/badge.svg)](https://github.com/ethz-asl/grid_map_geo/actions/workflows/build_test.yml) +[![Build Test](https://github.com/ethz-asl/grid_map_geo/actions/workflows/build_test.yml/badge.svg?branch=ros2)](https://github.com/ethz-asl/grid_map_geo/actions/workflows/build_test.yml) +[![Doxygen Build](https://github.com/ethz-asl/grid_map_geo/actions/workflows/doxygen_build.yml/badge.svg?branch=ros2)](https://github.com/ethz-asl/grid_map_geo/actions/workflows/doxygen_build.yml) +[![Style Checks](https://github.com/ethz-asl/grid_map_geo/actions/workflows/check_style.yml/badge.svg?branch=ros2)](https://github.com/ethz-asl/grid_map_geo/actions/workflows/check_style.yml) This package provides a georeferenced extension to the elevation map [grid_map](https://github.com/ANYbotics/grid_map) using [GDAL](https://gdal.org/), library for raster and vector geospatial data formats diff --git a/include/grid_map_geo/grid_map_geo.hpp b/include/grid_map_geo/grid_map_geo.hpp index 9b54d8e..d71c8e2 100644 --- a/include/grid_map_geo/grid_map_geo.hpp +++ b/include/grid_map_geo/grid_map_geo.hpp @@ -34,12 +34,13 @@ #ifndef GRID_MAP_GEO_H #define GRID_MAP_GEO_H +#include + #include #include #include -#include "grid_map_geo/transform.hpp" -#include "tf2_ros/transform_broadcaster.h" +#include "transform.hpp" struct Location { ESPG espg{ESPG::WGS84}; Eigen::Vector3d position{Eigen::Vector3d::Zero()}; @@ -95,7 +96,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 +185,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 index d03006e..1518d6a 100644 --- 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 -#include -#include -#include -#else -#include -#include -#include -#include -#endif - #include 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(src_coord)); - target.importFromEPSG(static_cast(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,6 @@ 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(wkt.c_str()); - source.importFromEPSG(static_cast(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 index 7d08716..45ecd06 100644 --- 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 */ diff --git a/src/test_tif_loader.cpp b/src/test_tif_loader.cpp index ddb2ba6..3b84210 100644 --- a/src/test_tif_loader.cpp +++ b/src/test_tif_loader.cpp @@ -31,19 +31,21 @@ * ****************************************************************************/ /** - * @brief Node to test planner in the view utiltiy map + * @brief Node to test planner in the view utility map * * * @author Jaeyoung Lim */ -#include "geometry_msgs/msg/transform_stamped.hpp" +#include +#include + +#include +#include +#include +#include + #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" using namespace std::chrono_literals; diff --git a/src/transform.cpp b/src/transform.cpp new file mode 100644 index 0000000..885ed31 --- /dev/null +++ b/src/transform.cpp @@ -0,0 +1,79 @@ +/**************************************************************************** + * + * 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 +#include +#include +#include +#else +#include +#include +#include +#include +#endif + +Eigen::Vector3d transformCoordinates(ESPG src_coord, ESPG tgt_coord, const Eigen::Vector3d source_coordinates) { + OGRSpatialReference source, target; + source.importFromEPSG(static_cast(src_coord)); + target.importFromEPSG(static_cast(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(wkt.c_str()); + source.importFromEPSG(static_cast(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 new file mode 100644 index 0000000..a712b81 --- /dev/null +++ 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 new file mode 100644 index 0000000..668a86a --- /dev/null +++ b/test/export/main.cpp @@ -0,0 +1,10 @@ +#include + +#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 b/test/unit/CMakeLists.txt similarity index 100% rename from test/CMakeLists.txt rename to test/unit/CMakeLists.txt diff --git a/test/main.cpp b/test/unit/main.cpp similarity index 100% rename from test/main.cpp rename to test/unit/main.cpp diff --git a/test/test_grid_map_geo.cpp b/test/unit/test_grid_map_geo.cpp similarity index 100% rename from test/test_grid_map_geo.cpp rename to test/unit/test_grid_map_geo.cpp