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

Integrate PT #1824

Merged
merged 26 commits into from
Dec 12, 2019
Merged

Integrate PT #1824

merged 26 commits into from
Dec 12, 2019

Conversation

michaz
Copy link
Member

@michaz michaz commented Dec 12, 2019

Goal: Functionally integrate PT with rest of GraphHopper, meaning I can put PT into a regular instance that also has other modes, CH etc.

Secondary goal: Also integrate the code a bit.

I still use my own web resources. I just forward requests with ?vehicle=pt to my own resource. (Invisibly. No HTTP redirect, just give Jersey a hint where the request must go.)

GraphHopperGtfs is now analogous to GraphHopperOSM (and inherits from it, for better or worse).

I needed two extra entry points in GraphHopper, and I still need to trick GraphHopper in two places: First, I may want to create a Graph completely without an OSM network, so I need to bypass a validation. Second, I need to reset the LocationIndex because I need it after importing OSM, but I can only finish it after importing GTFS.

In /info, I need to add PT as an extra mode if we have it, because it doesn't correspond to a FlagEncoder.

Copy link
Member

@karussell karussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

And a good timing for 1.0 and #1776 :)

@@ -521,6 +521,7 @@ public GraphHopper init(CmdArgs args) {
if (!flagEncodersStr.isEmpty() || !encodedValueStr.isEmpty()) {
if (!encodedValueStr.isEmpty())
emBuilder.addAll(tagParserFactory, encodedValueStr);
registerCustomEncodedValues(emBuilder);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is a bit unfortunate that this is necessary but we would need yet another factory (EncodedValueFactory) plus a method in EncodingManager to support this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, talking about factories, extension points and such is a new issue, or epic. But I don't think it's that bad in this case. "Builder" sounds like something that should be safe to pass around and add stuff to.

@michaz michaz merged commit 7a6eca5 into master Dec 12, 2019
@michaz michaz deleted the pt-integration branch April 22, 2020 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants