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.

Sign up for GitHub

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

Jump to bottom

Save map builder options in options proto. Rename it to BuilderOptions. #1504

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Save map builder options in options proto. Rename it to BuilderOptions. #1504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01a3067
Save map builder options in options proto. Rename it to BuilderOptions.
ojura Jan 17, 2019
7f3b27d
Ran clang-format.
pifon2a Feb 12, 2019
7bdb980
Ran clang-format.
pifon2a Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions cartographer/cloud/client/map_builder_stub.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ int MapBuilderStub::AddTrajectoryBuilder(

int MapBuilderStub::AddTrajectoryForDeserialization(
const mapping::proto::TrajectoryBuilderOptionsWithSensorIds&
options_with_sensor_ids_proto) {
trajectory_builder_options_with_sensor_ids_proto) {
LOG(FATAL) << "Not implemented";
}

Expand Down Expand Up @@ -197,12 +197,11 @@ std::map<int, int> MapBuilderStub::LoadState(
request.set_load_frozen_state(load_frozen_state);
CHECK(client.Write(request));
}
// Request with an AllTrajectoryBuilderOptions should be third.
// Request with BuilderOptions should be third.
{
proto::LoadStateRequest request;
*request.mutable_serialized_data()
->mutable_all_trajectory_builder_options() =
deserializer.all_trajectory_builder_options();
*request.mutable_serialized_data()->mutable_builder_options() =
deserializer.builder_options();
request.set_load_frozen_state(load_frozen_state);
CHECK(client.Write(request));
}
Expand Down
2 changes: 1 addition & 1 deletion cartographer/cloud/client/map_builder_stub.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class MapBuilderStub : public mapping::MapBuilderInterface {
LocalSlamResultCallback local_slam_result_callback) override;
int AddTrajectoryForDeserialization(
const mapping::proto::TrajectoryBuilderOptionsWithSensorIds&
options_with_sensor_ids_proto) override;
trajectory_builder_options_with_sensor_ids_proto) override;
mapping::TrajectoryBuilderInterface* GetTrajectoryBuilder(
int trajectory_id) const override;
void FinishTrajectory(int trajectory_id) override;
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/client/pose_graph_stub.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/client/pose_graph_stub.h"

#include "async_grpc/client.h"
#include "cartographer/cloud/internal/handlers/delete_trajectory_handler.h"
#include "cartographer/cloud/internal/handlers/get_all_submap_poses.h"
Expand Down
49 changes: 24 additions & 25 deletions cartographer/cloud/internal/client_server_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ constexpr char kPoseGraphProtoString[] = R"(pose_graph {
submap: {}
}
})";
constexpr char kAllTrajectoryBuilderOptionsProtoString[] =
R"(all_trajectory_builder_options {
options_with_sensor_ids: {}
constexpr char kBuilderOptionsProtoString[] =
R"(builder_options {
trajectory_builder_options_with_sensor_ids: {}
map_builder_options: {}
})";
constexpr char kSubmapProtoString[] = "submap {}";
constexpr char kNodeProtoString[] = "node {}";
Expand Down Expand Up @@ -637,17 +638,16 @@ TEST_P(ClientServerTestByGridType, LoadStateAndDelete) {
InitializeStub();

// Load text proto into in_memory_reader.
auto reader =
ProtoReaderFromStrings(kSerializationHeaderProtoString,
{
kPoseGraphProtoString,
kAllTrajectoryBuilderOptionsProtoString,
kSubmapProtoString,
kNodeProtoString,
kImuDataProtoString,
kOdometryDataProtoString,
kLandmarkDataProtoString,
});
auto reader = ProtoReaderFromStrings(kSerializationHeaderProtoString,
{
kPoseGraphProtoString,
kBuilderOptionsProtoString,
kSubmapProtoString,
kNodeProtoString,
kImuDataProtoString,
kOdometryDataProtoString,
kLandmarkDataProtoString,
});

auto trajectory_remapping = stub_->LoadState(reader.get(), true);
int expected_trajectory_id = 0;
Expand Down Expand Up @@ -676,17 +676,16 @@ TEST_P(ClientServerTestByGridType, LoadUnfrozenStateAndDelete) {
InitializeStub();

// Load text proto into in_memory_reader.
auto reader =
ProtoReaderFromStrings(kSerializationHeaderProtoString,
{
kPoseGraphProtoString,
kAllTrajectoryBuilderOptionsProtoString,
kSubmapProtoString,
kNodeProtoString,
kImuDataProtoString,
kOdometryDataProtoString,
kLandmarkDataProtoString,
});
auto reader = ProtoReaderFromStrings(kSerializationHeaderProtoString,
{
kPoseGraphProtoString,
kBuilderOptionsProtoString,
kSubmapProtoString,
kNodeProtoString,
kImuDataProtoString,
kOdometryDataProtoString,
kLandmarkDataProtoString,
});

auto trajectory_remapping =
stub_->LoadState(reader.get(), false /* load_frozen_state */);
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/add_fixed_frame_pose_data_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/add_fixed_frame_pose_data_handler.h"

#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
#include "google/protobuf/text_format.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/add_imu_data_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/add_imu_data_handler.h"

#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
#include "google/protobuf/text_format.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/add_landmark_data_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/add_landmark_data_handler.h"

#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
#include "google/protobuf/text_format.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/add_odometry_data_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/add_odometry_data_handler.h"

#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
#include "google/protobuf/text_format.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/add_rangefinder_data_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/add_rangefinder_data_handler.h"

#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
#include "google/protobuf/text_format.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/add_trajectory_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/add_trajectory_handler.h"

#include "cartographer/cloud/internal/sensor/serialization.h"
#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/get_landmark_poses_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/get_landmark_poses_handler.h"

#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
#include "google/protobuf/text_format.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/cloud/internal/handlers/set_landmark_pose_handler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/cloud/internal/handlers/set_landmark_pose_handler.h"

#include "cartographer/cloud/internal/testing/handler_test.h"
#include "cartographer/cloud/internal/testing/test_helpers.h"
#include "cartographer/transform/rigid_transform_test_helpers.h"
Expand Down
2 changes: 0 additions & 2 deletions cartographer/cloud/internal/map_builder_context_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
* limitations under the License.
*/

#include "cartographer/cloud/internal/map_builder_server.h"

#include "cartographer/cloud/internal/map_builder_server.h"
#include "cartographer/mapping/internal/2d/local_slam_result_2d.h"
#include "cartographer/mapping/internal/3d/local_slam_result_3d.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/common/lockless_queue_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "cartographer/common/lockless_queue.h"

#include "gtest/gtest.h"

namespace cartographer {
Expand Down
7 changes: 3 additions & 4 deletions cartographer/common/port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,12 @@
#ifndef CARTOGRAPHER_COMMON_PORT_H_
#define CARTOGRAPHER_COMMON_PORT_H_

#include <cinttypes>
#include <cmath>
#include <string>

#include <boost/iostreams/device/back_inserter.hpp>
#include <boost/iostreams/filter/gzip.hpp>
#include <boost/iostreams/filtering_stream.hpp>
#include <cinttypes>
#include <cmath>
#include <string>

namespace cartographer {

Expand Down
1 change: 1 addition & 0 deletions cartographer/common/time.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "cartographer/common/time.h"

#include <time.h>

#include <cerrno>
#include <cstring>
#include <string>
Expand Down
3 changes: 2 additions & 1 deletion cartographer/io/fake_file_writer_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@
* limitations under the License.
*/

#include "cartographer/io/fake_file_writer.h"

#include <vector>

#include "cartographer/io/fake_file_writer.h"
#include "glog/logging.h"
#include "gtest/gtest.h"

Expand Down
2 changes: 1 addition & 1 deletion cartographer/io/internal/in_memory_proto_stream_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
*/

#include "cartographer/io/internal/in_memory_proto_stream.h"

#include "cartographer/mapping/proto/pose_graph.pb.h"
#include "cartographer/mapping/proto/serialization.pb.h"

#include "gtest/gtest.h"

namespace cartographer {
Expand Down
32 changes: 18 additions & 14 deletions cartographer/io/internal/mapping_state_serialization.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/io/internal/mapping_state_serialization.h"

#include "cartographer/mapping/proto/serialization.pb.h"
#include "cartographer/transform/transform.h"

Expand All @@ -28,17 +29,18 @@ using mapping::SubmapId;
using mapping::TrajectoryNode;
using mapping::proto::SerializedData;

mapping::proto::AllTrajectoryBuilderOptions
CreateAllTrajectoryBuilderOptionsProto(
mapping::proto::BuilderOptions CreateBuilderOptionsProto(
const std::vector<mapping::proto::TrajectoryBuilderOptionsWithSensorIds>&
all_options_with_sensor_ids,
all_trajectory_builder_options_with_sensor_ids,
const mapping::proto::MapBuilderOptions& map_builder_options,
const std::vector<int>& trajectory_ids_to_serialize) {
mapping::proto::AllTrajectoryBuilderOptions all_options_proto;
mapping::proto::BuilderOptions builder_options_proto;
for (auto id : trajectory_ids_to_serialize) {
*all_options_proto.add_options_with_sensor_ids() =
all_options_with_sensor_ids[id];
*builder_options_proto.add_trajectory_builder_options_with_sensor_ids() =
all_trajectory_builder_options_with_sensor_ids[id];
}
return all_options_proto;
*builder_options_proto.mutable_map_builder_options() = map_builder_options;
return builder_options_proto;
}

// Will return all trajectory ids, that have `state != DELETED`.
Expand Down Expand Up @@ -67,14 +69,15 @@ SerializedData SerializePoseGraph(const mapping::PoseGraph& pose_graph,
return proto;
}

SerializedData SerializeTrajectoryBuilderOptions(
SerializedData SerializeBuilderOptions(
const std::vector<mapping::proto::TrajectoryBuilderOptionsWithSensorIds>&
trajectory_builder_options,
const mapping::proto::MapBuilderOptions& map_builder_options,
const std::vector<int>& trajectory_ids_to_serialize) {
SerializedData proto;
*proto.mutable_all_trajectory_builder_options() =
CreateAllTrajectoryBuilderOptionsProto(trajectory_builder_options,
trajectory_ids_to_serialize);
*proto.mutable_builder_options() =
CreateBuilderOptionsProto(trajectory_builder_options, map_builder_options,
trajectory_ids_to_serialize);
return proto;
}

Expand Down Expand Up @@ -213,13 +216,14 @@ void SerializeLandmarkNodes(
void WritePbStream(
const mapping::PoseGraph& pose_graph,
const std::vector<mapping::proto::TrajectoryBuilderOptionsWithSensorIds>&
trajectory_builder_options,
all_trajectory_builder_options,
const mapping::proto::MapBuilderOptions& map_builder_options,
ProtoStreamWriterInterface* const writer, bool include_unfinished_submaps) {
writer->WriteProto(CreateHeader());
writer->WriteProto(
SerializePoseGraph(pose_graph, include_unfinished_submaps));
writer->WriteProto(SerializeTrajectoryBuilderOptions(
trajectory_builder_options,
writer->WriteProto(SerializeBuilderOptions(
all_trajectory_builder_options, map_builder_options,
GetValidTrajectoryIds(pose_graph.GetTrajectoryStates())));

SerializeSubmaps(pose_graph.GetAllSubmapData(), include_unfinished_submaps,
Expand Down
3 changes: 2 additions & 1 deletion cartographer/io/internal/mapping_state_serialization.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ static constexpr int kFormatVersionWithoutSubmapHistograms = 1;
void WritePbStream(
const mapping::PoseGraph& pose_graph,
const std::vector<mapping::proto::TrajectoryBuilderOptionsWithSensorIds>&
builder_options,
all_trajectory_builder_options,
const mapping::proto::MapBuilderOptions& map_builder_options,
ProtoStreamWriterInterface* const writer, bool include_unfinished_submaps);

} // namespace io
Expand Down
8 changes: 6 additions & 2 deletions cartographer/io/internal/pbstream_info.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,14 @@ void Run(const std::string& pbstream_filename, bool all_debug_strings) {
const auto header = deserializer.header();
LOG(INFO) << "Header: " << header.DebugString();
for (const mapping::proto::TrajectoryBuilderOptionsWithSensorIds&
trajectory_options : deserializer.all_trajectory_builder_options()
.options_with_sensor_ids()) {
trajectory_options :
deserializer.builder_options()
.trajectory_builder_options_with_sensor_ids()) {
LOG(INFO) << "Trajectory options: " << trajectory_options.DebugString();
}
LOG(INFO)
<< "Map builder options: "
<< deserializer.builder_options().map_builder_options().DebugString();
const mapping::proto::PoseGraph pose_graph = deserializer.pose_graph();
for (const mapping::proto::Trajectory& trajectory : pose_graph.trajectory()) {
LOG(INFO) << "Trajectory id: " << trajectory.trajectory_id()
Expand Down
1 change: 1 addition & 0 deletions cartographer/io/probability_grid_points_processor_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "cartographer/io/probability_grid_points_processor.h"

#include <string>

#include "cartographer/common/lua_parameter_dictionary.h"
#include "cartographer/common/lua_parameter_dictionary_test_helpers.h"
#include "cartographer/common/port.h"
Expand Down
1 change: 1 addition & 0 deletions cartographer/io/proto_stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "cartographer/io/proto_stream.h"

#include "glog/logging.h"

namespace cartographer {
Expand Down
14 changes: 7 additions & 7 deletions cartographer/io/proto_stream_deserializer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ ProtoStreamDeserializer::ProtoStreamDeserializer(
"`SerializationHeader`, but got field tag "
<< pose_graph_.data_case();

CHECK(ReadNextSerializedData(&all_trajectory_builder_options_))
<< "Serialized stream misses `AllTrajectoryBuilderOptions`.";
CHECK(all_trajectory_builder_options_.has_all_trajectory_builder_options())
CHECK(ReadNextSerializedData(&builder_options_))
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this break compatibility?

Copy link
Contributor Author

@ojura ojura Feb 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It is the same proto, just renamed to builder options (instead of all trajectory builder options), since it now contains the map builder options as well.

Also I mentioned above that I tested it with bags built by upstream Cartographer, it works okay (prints a blank line for nonexisting map builder options).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yes, true!

<< "Serialized stream misses `BuilderOptions`.";
CHECK(builder_options_.has_builder_options())
<< "Serialized stream order corrupt. Expecting "
"`AllTrajectoryBuilderOptions` after "
"`BuilderOptions` after "
"PoseGraph, got field tag "
<< all_trajectory_builder_options_.data_case();
<< builder_options_.data_case();

CHECK_EQ(pose_graph_.pose_graph().trajectory_size(),
all_trajectory_builder_options_.all_trajectory_builder_options()
.options_with_sensor_ids_size());
builder_options_.builder_options()
.trajectory_builder_options_with_sensor_ids_size());
}

bool ProtoStreamDeserializer::ReadNextSerializedData(
Expand Down
7 changes: 3 additions & 4 deletions cartographer/io/proto_stream_deserializer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ class ProtoStreamDeserializer {
return pose_graph_.pose_graph();
}

const mapping::proto::AllTrajectoryBuilderOptions&
all_trajectory_builder_options() {
return all_trajectory_builder_options_.all_trajectory_builder_options();
const mapping::proto::BuilderOptions& builder_options() {
return builder_options_.builder_options();
}

// Reads the next `SerializedData` message of the ProtoStream into `data`.
Expand All @@ -63,7 +62,7 @@ class ProtoStreamDeserializer {

mapping::proto::SerializationHeader header_;
mapping::proto::SerializedData pose_graph_;
mapping::proto::SerializedData all_trajectory_builder_options_;
mapping::proto::SerializedData builder_options_;
};

} // namespace io
Expand Down
Loading