cartographer-project / cartographer

Cartographer is a system that provides real-time simultaneous localization and mapping (SLAM) in 2D and 3D across multiple platforms and sensor configurations.
Apache License 2.0
7.12k stars 2.25k forks source link

Localization in 3D crashes when using Visualization #1343

Open schwoere opened 6 years ago

schwoere commented 6 years ago

With PR #1286 unfinished submaps are serialized without the hybrid grid.

When running localization with the rviz, the call to Submap3D::ToResponseProto will crash, as the hybrid grid does not exist for the unfinished submaps.

gaschler commented 6 years ago

I think the bug is that we write/load unfinished submaps into pbstreams. Among other things, unfinished submaps will never be processed or finished after loading from a file. You have to draw a line how much of the runtime state you want to save/load in files. Unfinished submaps are pending in pose graph's work queue and there is no story how LoadStateFromFile could recover this runtime state. (I will file another bug that you cannot DeleteTrajectory a map loaded from pbstream because it has at least one unfinished submap that will never finish.)

The intention of #1286 is to support unfinished submaps over grpc. Perhaps we need to use different protos for grpc and pbstream in this case, the expected behavior is really different.

wohe commented 6 years ago

Two questions:

  1. Doesn't the proto already support both use cases? Or is WritePbStream() used for gRPC?
  2. If unfinished submaps are no longer serialized, it is no longer possible to visualize the final result of SLAM from the serialized data. Is that intended? Why?
cschuet commented 6 years ago

@schwoere @wohe

In the distributed SLAM setup we have the situation that all unfinished submaps do not have a probability grid. That lead to problems when calling serialize state on the cloud instance (since we would try to include the probability / hybrid grid for submaps that did not have one). Obviously I did not think all implications of that change through. Apologies for that.

How about we:

1) Revert SerializeSubmaps() to its original behavior to include hybrid grids also for unfinished submaps. 2) Change the LoadState routine to not load unfinished submaps from the stream. Because they are either useless since they can't receive scan matches anyways, or (b) we would wrongfully have to mark the unfinished submap as finished to make it a valid target for the scan matcher.

Would that work for you?

cschuet commented 6 years ago

@schwoere @wohe for now I will just revert #1286.

cschuet commented 6 years ago

https://github.com/googlecartographer/cartographer/pull/1346 contains the revert.