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.

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

Save user data in proto stream #38

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

Conversation

ojura
Copy link
Contributor

@ojura ojura commented Jan 17, 2019

@ojura
Copy link
Contributor Author

ojura commented Jan 17, 2019

Conclusions from the open house:

  • MapBuilderOptions will be saved to the AllTrajectoryOptions proto, to be renamed as AllOptions or BuilderOptions (open to suggestions for the naming)

  • the pbstream info tool can be used to recover them afterwards; no support for directly loading the embedded node options in cartographer_ros is planned (sad face :) )

  • options for saving other user data:

    • users save their own protos at the beginning of the proto stream using the Writer API, then hand off the Writer to Cartographer for serialization

      • pros: users do not have to go through the entire stream to get to their data
      • cons: the produced streams will not be able to be used with upstream Cartographer, since it does not know how to skip until it hits the Header proto
    • users can expand the SerializedData proto with their own type and write out their own SerializedData protos using the Writer API after Cartographer is finished with serializing

      • pros: upstream Cartographer can ignore these in its huge SerializedData proto-type switch, no upstream changes necessary
      • cons: without interventions into Cartographer, there is no possibility to write the user data at the beginning of the proto stream, only at the end, which can be slow for large streams
    • similarly to the description in the RFC, SerializedData gets an Any field, and users hand off a vector of Any protos to Cartographer so it can write them out at the beginning (after the header)
      - pros: user data is at the beginning so can be accessed fast
      - cons: upstream Cartographer needs to be taught to do this, API change required

I am of course rooting for option 3 :)

@ojura
Copy link
Contributor Author

ojura commented Jan 17, 2019

Okay, the first part (options proto rename + storing map builder options) is in cartographer-project/cartographer#1504. I'll wait for your feedback for the other one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant