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

rewrite node encoding for clarity and ease of extension #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

missinglink
Copy link
Member

@missinglink missinglink commented Jul 21, 2021

this PR rewrites some older code in a clearer way, which opens it up for extension.
in particular issue #96 could benefit from this refactor

some notes on what this part of the code does:

  • when 'node' elements from OSM are stored in leveldb they must be encoded/decoded from a slice of bytes []byte{}
  • we use a custom encoding scheme to save disk space
  • for the lat/lon values, we convert them to their IEEE 754 binary representation and truncate them to 6 bytes each, which is a nice tradeoff of precision vs. size (this consumes 12 bytes total)
  • we optionally use a 13th byte to store metadata about entrances and accessibility
  • there are still 4 bits reserved in that 13th byte for extension (such as storing information about the node role in a relation)

the subject of this PR is mainly to make the operations on the metadata byte more obvious and open them up for extension.

I didn't add new tests since https://github.com/pelias/pbf2json/blob/master/encoding_test.go seems to cover this sufficiently

Hey @paulmach if you get a second could you please do a quick review?

@missinglink missinglink force-pushed the refactor_node_encoding branch from ade3605 to dc26fe3 Compare July 21, 2021 17:27
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