From b1ce24d3f8e0b2a372467d65dc92d6c52329bb90 Mon Sep 17 00:00:00 2001 From: Wolfgang Hess Date: Fri, 6 Nov 2020 11:31:49 +0100 Subject: [PATCH] Clean up use of MapBuilderInterface. (#1776) Moves options next to the interface like we do for other interfaces. Adds a factory function to remove the need for direct use of MapBuilder. Signed-off-by: Wolfgang Hess --- .../cloud/internal/client_server_test.cc | 9 ++-- .../cloud/internal/map_builder_server.h | 1 - cartographer/cloud/map_builder_server_main.cc | 2 +- .../cloud/map_builder_server_options.cc | 2 +- .../common/configuration_files_test.cc | 2 +- .../io/serialization_format_migration.cc | 1 - cartographer/mapping/map_builder.cc | 23 +++------- cartographer/mapping/map_builder.h | 6 +-- cartographer/mapping/map_builder_interface.cc | 43 +++++++++++++++++++ cartographer/mapping/map_builder_interface.h | 4 ++ cartographer/mapping/map_builder_test.cc | 2 +- 11 files changed, 64 insertions(+), 31 deletions(-) create mode 100644 cartographer/mapping/map_builder_interface.cc diff --git a/cartographer/cloud/internal/client_server_test.cc b/cartographer/cloud/internal/client_server_test.cc index 9d8bc38..1eaea52 100644 --- a/cartographer/cloud/internal/client_server_test.cc +++ b/cartographer/cloud/internal/client_server_test.cc @@ -29,12 +29,13 @@ #include "cartographer/mapping/internal/testing/mock_trajectory_builder.h" #include "cartographer/mapping/internal/testing/test_helpers.h" #include "cartographer/mapping/local_slam_result_data.h" +#include "cartographer/mapping/map_builder.h" #include "glog/logging.h" #include "gmock/gmock.h" #include "gtest/gtest.h" using ::cartographer::io::testing::ProtoReaderFromStrings; -using ::cartographer::mapping::MapBuilder; +using ::cartographer::mapping::CreateMapBuilder; using ::cartographer::mapping::MapBuilderInterface; using ::cartographer::mapping::PoseGraphInterface; using ::cartographer::mapping::TrajectoryBuilderInterface; @@ -139,15 +140,15 @@ class ClientServerTestBase : public T { } void InitializeRealServer() { - auto map_builder = absl::make_unique( - map_builder_server_options_.map_builder_options()); + auto map_builder = + CreateMapBuilder(map_builder_server_options_.map_builder_options()); server_ = absl::make_unique(map_builder_server_options_, std::move(map_builder)); EXPECT_TRUE(server_ != nullptr); } void InitializeRealUploadingServer() { - auto map_builder = absl::make_unique( + auto map_builder = CreateMapBuilder( uploading_map_builder_server_options_.map_builder_options()); uploading_server_ = absl::make_unique( uploading_map_builder_server_options_, std::move(map_builder)); diff --git a/cartographer/cloud/internal/map_builder_server.h b/cartographer/cloud/internal/map_builder_server.h index 279dd2f..ed75943 100644 --- a/cartographer/cloud/internal/map_builder_server.h +++ b/cartographer/cloud/internal/map_builder_server.h @@ -29,7 +29,6 @@ #include "cartographer/mapping/3d/submap_3d.h" #include "cartographer/mapping/internal/submap_controller.h" #include "cartographer/mapping/local_slam_result_data.h" -#include "cartographer/mapping/map_builder.h" #include "cartographer/mapping/trajectory_builder_interface.h" #include "cartographer/metrics/family_factory.h" #include "cartographer/sensor/internal/dispatchable.h" diff --git a/cartographer/cloud/map_builder_server_main.cc b/cartographer/cloud/map_builder_server_main.cc index 67d4051..81bf87e 100644 --- a/cartographer/cloud/map_builder_server_main.cc +++ b/cartographer/cloud/map_builder_server_main.cc @@ -50,7 +50,7 @@ void Run(const std::string& configuration_directory, proto::MapBuilderServerOptions map_builder_server_options = LoadMapBuilderServerOptions(configuration_directory, configuration_basename); - auto map_builder = absl::make_unique( + auto map_builder = mapping::CreateMapBuilder( map_builder_server_options.map_builder_options()); std::unique_ptr map_builder_server = CreateMapBuilderServer(map_builder_server_options, diff --git a/cartographer/cloud/map_builder_server_options.cc b/cartographer/cloud/map_builder_server_options.cc index 25fcc87..84042f3 100644 --- a/cartographer/cloud/map_builder_server_options.cc +++ b/cartographer/cloud/map_builder_server_options.cc @@ -18,7 +18,7 @@ #include "absl/memory/memory.h" #include "cartographer/common/configuration_file_resolver.h" -#include "cartographer/mapping/map_builder.h" +#include "cartographer/mapping/map_builder_interface.h" namespace cartographer { namespace cloud { diff --git a/cartographer/common/configuration_files_test.cc b/cartographer/common/configuration_files_test.cc index 62db6d8..d3c451a 100644 --- a/cartographer/common/configuration_files_test.cc +++ b/cartographer/common/configuration_files_test.cc @@ -20,7 +20,7 @@ #include "cartographer/common/config.h" #include "cartographer/common/configuration_file_resolver.h" #include "cartographer/common/lua_parameter_dictionary.h" -#include "cartographer/mapping/map_builder.h" +#include "cartographer/mapping/map_builder_interface.h" #include "gtest/gtest.h" namespace cartographer { diff --git a/cartographer/io/serialization_format_migration.cc b/cartographer/io/serialization_format_migration.cc index fb3c222..7ed692e 100644 --- a/cartographer/io/serialization_format_migration.cc +++ b/cartographer/io/serialization_format_migration.cc @@ -28,7 +28,6 @@ #include "cartographer/mapping/id.h" #include "cartographer/mapping/internal/3d/pose_graph_3d.h" #include "cartographer/mapping/internal/3d/scan_matching/rotational_scan_matcher.h" -#include "cartographer/mapping/map_builder.h" #include "cartographer/mapping/map_builder_interface.h" #include "cartographer/mapping/pose_graph.h" #include "cartographer/mapping/probability_values.h" diff --git a/cartographer/mapping/map_builder.cc b/cartographer/mapping/map_builder.cc index 208d9e2..5621dbc 100644 --- a/cartographer/mapping/map_builder.cc +++ b/cartographer/mapping/map_builder.cc @@ -74,24 +74,6 @@ void MaybeAddPureLocalizationTrimmer( } // namespace -proto::MapBuilderOptions CreateMapBuilderOptions( - common::LuaParameterDictionary* const parameter_dictionary) { - proto::MapBuilderOptions options; - options.set_use_trajectory_builder_2d( - parameter_dictionary->GetBool("use_trajectory_builder_2d")); - options.set_use_trajectory_builder_3d( - parameter_dictionary->GetBool("use_trajectory_builder_3d")); - options.set_num_background_threads( - parameter_dictionary->GetNonNegativeInt("num_background_threads")); - options.set_collate_by_trajectory( - parameter_dictionary->GetBool("collate_by_trajectory")); - *options.mutable_pose_graph_options() = CreatePoseGraphOptions( - parameter_dictionary->GetDictionary("pose_graph").get()); - CHECK_NE(options.use_trajectory_builder_2d(), - options.use_trajectory_builder_3d()); - return options; -} - MapBuilder::MapBuilder(const proto::MapBuilderOptions& options) : options_(options), thread_pool_(options.num_background_threads()) { CHECK(options.use_trajectory_builder_2d() ^ @@ -414,5 +396,10 @@ std::map MapBuilder::LoadStateFromFile( return LoadState(&stream, load_frozen_state); } +std::unique_ptr CreateMapBuilder( + const proto::MapBuilderOptions& options) { + return absl::make_unique(options); +} + } // namespace mapping } // namespace cartographer diff --git a/cartographer/mapping/map_builder.h b/cartographer/mapping/map_builder.h index 8e6db5d..ee885a0 100644 --- a/cartographer/mapping/map_builder.h +++ b/cartographer/mapping/map_builder.h @@ -28,9 +28,6 @@ namespace cartographer { namespace mapping { -proto::MapBuilderOptions CreateMapBuilderOptions( - common::LuaParameterDictionary *const parameter_dictionary); - // Wires up the complete SLAM stack with TrajectoryBuilders (for local submaps) // and a PoseGraph for loop closure. class MapBuilder : public MapBuilderInterface { @@ -98,6 +95,9 @@ class MapBuilder : public MapBuilderInterface { all_trajectory_builder_options_; }; +std::unique_ptr CreateMapBuilder( + const proto::MapBuilderOptions& options); + } // namespace mapping } // namespace cartographer diff --git a/cartographer/mapping/map_builder_interface.cc b/cartographer/mapping/map_builder_interface.cc new file mode 100644 index 0000000..af75f6c --- /dev/null +++ b/cartographer/mapping/map_builder_interface.cc @@ -0,0 +1,43 @@ +/* + * Copyright 2016 The Cartographer Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "cartographer/mapping/map_builder_interface.h" + +#include "cartographer/mapping/pose_graph.h" + +namespace cartographer { +namespace mapping { + +proto::MapBuilderOptions CreateMapBuilderOptions( + common::LuaParameterDictionary* const parameter_dictionary) { + proto::MapBuilderOptions options; + options.set_use_trajectory_builder_2d( + parameter_dictionary->GetBool("use_trajectory_builder_2d")); + options.set_use_trajectory_builder_3d( + parameter_dictionary->GetBool("use_trajectory_builder_3d")); + options.set_num_background_threads( + parameter_dictionary->GetNonNegativeInt("num_background_threads")); + options.set_collate_by_trajectory( + parameter_dictionary->GetBool("collate_by_trajectory")); + *options.mutable_pose_graph_options() = CreatePoseGraphOptions( + parameter_dictionary->GetDictionary("pose_graph").get()); + CHECK_NE(options.use_trajectory_builder_2d(), + options.use_trajectory_builder_3d()); + return options; +} + +} // namespace mapping +} // namespace cartographer diff --git a/cartographer/mapping/map_builder_interface.h b/cartographer/mapping/map_builder_interface.h index 8c1cfb1..76062fb 100644 --- a/cartographer/mapping/map_builder_interface.h +++ b/cartographer/mapping/map_builder_interface.h @@ -27,6 +27,7 @@ #include "cartographer/io/proto_stream_interface.h" #include "cartographer/mapping/id.h" #include "cartographer/mapping/pose_graph_interface.h" +#include "cartographer/mapping/proto/map_builder_options.pb.h" #include "cartographer/mapping/proto/submap_visualization.pb.h" #include "cartographer/mapping/proto/trajectory_builder_options.pb.h" #include "cartographer/mapping/submaps.h" @@ -35,6 +36,9 @@ namespace cartographer { namespace mapping { +proto::MapBuilderOptions CreateMapBuilderOptions( + common::LuaParameterDictionary* const parameter_dictionary); + // This interface is used for both library and RPC implementations. // Implementations wire up the complete SLAM stack. class MapBuilderInterface { diff --git a/cartographer/mapping/map_builder_test.cc b/cartographer/mapping/map_builder_test.cc index c079775..4f205c1 100644 --- a/cartographer/mapping/map_builder_test.cc +++ b/cartographer/mapping/map_builder_test.cc @@ -64,7 +64,7 @@ class MapBuilderTestBase : public T { } void BuildMapBuilder() { - map_builder_ = absl::make_unique(map_builder_options_); + map_builder_ = CreateMapBuilder(map_builder_options_); } void SetOptionsTo3D() {