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

Merge OSM parsers #468

Closed
zod opened this issue Oct 18, 2022 · 6 comments · Fixed by #555
Closed

Merge OSM parsers #468

zod opened this issue Oct 18, 2022 · 6 comments · Fixed by #555

Comments

@zod
Copy link
Collaborator

zod commented Oct 18, 2022

Currently BRouter contains two parsers for OSM data

According to the README of PBF parser the XML Parser is just for testing and I found some issues. It doesn't use a XML parser library and therefore rejects some files which are valid XML (e.g. it doesn't parser JOSM exports correctly because of different quotes).

It seems like the PBF parser should be used but it's not integrated into our build because of the osmosis dependency. This causes more work for users. @mjaschen provides a nice Dockerfile which handles building BRouter with PBF parser.

We use gradle / maven to get our dependencies and osmosis is available via maven central.

Perhaps we should just remove the XML parser and add osmosis as a dependency for map-creator.

@polyscias
Copy link
Contributor

Yes, I think the XML parser should be removed, see Planet.osm:

PBF (Protocol Buffer Format) is a compact binary format that is smaller to download and much faster to process and should be used when possible.

The PBF parsing is done using osmosis and that does work nicely for me, I found a problem with extensions but with that patched I was able to generate all .rd5 file for a planet.pbf file.

@mjaschen
Copy link
Contributor

Hi,

did anyone by chance test the routing data creation after PR #555 was merged?

In my case the routing data files are created and routing is possible with them but elevation data seems to be no longer be merged into the rd5 files. This is the case for elevation data in .bef files as well as in SRTM ASCII files.

I wonder if there's something wrong in my setup or if the problem is elsewhere. I'd appreciate it if anyone can find 5 minutes to test this 🙏

@afischerdev
Copy link
Collaborator

@mjaschen
Yes, I've tested and I found a problem with hgt handling.
A change on this is part of PR #556 (please see)

@mjaschen
Copy link
Contributor

Ah, I see, thanks for updating me on this. Will follow the PR/discussion :-)

@zod
Copy link
Collaborator Author

zod commented May 22, 2023

It was caused by #548 and I've added a PR (#557) which also includes the fix from @afischerdev and uses elevation data in tests which should keep us from breaking this stuff again ;)

@mjaschen
Copy link
Contributor

@zod Thanks :-)

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 a pull request may close this issue.

4 participants