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

Add versioning for Lanelet2 primitives (#1) #323

Conversation

aslastnikova
Copy link

Hello, I'm currently working on algorythm to merge changes, made on base of one map, but done by different people. For this reason, I need to determine the last version of changed primitives,luckily, the osm format has "version" field, but in Lanelet2 library the version field was hardcodly set to "1". So, here some updates I've done:

  • Optional parameter "version" in primitives' constructors. Default value 0.
  • Read primitives' versions from file.
  • Write primitives' versions into file.
  • Possibility to increment version for single primitive.
  • Possibility to increment versions for all primitives in layer.
  • Possibility to increment all primitives' versions on writing to file by bool parameter (minimum version will be written "1").
  • Possibility to set version for single primitive.
  • Possibility to set version for all primitives in layer.
  • Updated Python API with versioning functionality.

Versioning for Lanelet2 primitives:
* Optional parameter "version" in primitives' constructors. Default value 0.   
* Read primitives' versions from file.
* Write primitives' versions into file.
* Possibility to increment version for single primitive.
* Possibility to increment versions for all primitives in layer.
* Possibility to increment all primitives' versions on writing to file by bool parameter (minimum version will be written "1").
* Possibility to set version for single primitive.
* Possibility to set version for all primitives in layer.
* Updated Python API with versioning functionality.
---------

Co-authored-by: Anna Slastnikova <[email protected]>
@aslastnikova
Copy link
Author

aslastnikova commented Nov 20, 2023

Hello, @poggenhans, @immel-f, any updates on my pull request?

@poggenhans
Copy link
Contributor

Hi, and thank you for your MR. I appreciate the effort you must have put into this, but to be honest, it would have been simpler had you raised an issue to discuss potential solutions before implementing one.

I think it is a good idea to add versioning support to lanelet2, but I think your changes touch the interface a bit too much in terms of memory layout and ABI compability for something that probably only a few people will actually use and change.

Here is what I would propose:

  • Make the version a "special" kind of attribute instead of a permanent member of the primitive class
  • The osm parser will store the version into the AttributeMap of a primitive if it is different from 1.
  • The osm writer will similarly use the attribute if it exists, otherwise assume it is 1.
  • The ConstPrimitive and Primitive classes get getVersion and setVersion respectively that will just return or set the value of that attribute or use 1 as default (I wouldn't offer incrementing because that might be too narrow for some use cases)
  • We add using Version = std::uint64_t for better semantics (similarly as we have it with ID). If it is an optional attribute anyways, it makes sense to use the full 64 bits.
  • I wouldn't offer any increment_version options for lanelet maps or for reading/writing. If some wants to increment the version, he can easily implement that for himself. And there are simply too many ways in which one might increment the version for this to be a useful feature. Only for the modified primitives, only for the modified ones and their parents, everything, nothing, ...
  • I also wouldn't add this to the constructors, if one wants to have a version, he can pass it via the AttributeMap.

That way we can still offer this feature while still keeping the interface and the ABI stable.

Copy link

This PR is stale because it has been open for 90 days with no activity. Is this PR still work in progress? If yes, mark it as "WIP" or comment, otherwise it will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 19, 2024
Copy link

This PR was closed because it has been stalled for 30 days with no activity.

@github-actions github-actions bot closed this Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants