Skip to content

Create leg_time/distance entries for duplicate via-points #2924

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.

Sign up for GitHub

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

Jump to bottom
Merged
easbar merged 1 commit into from
Jan 11, 2024
Merged

Create leg_time/distance entries for duplicate via-points #2924

easbar merged 1 commit into from
Jan 11, 2024

Conversation

@easbar
Copy link
Member

@easbar easbar commented Jan 10, 2024 •
edited
Loading

Fixes #2915. This makes sure there are always N+1 leg_time/distance entries for requests with N via-points, even when some locations are duplicated. The interval of the path detail is empty for duplicate locations, because there are no edges inbetween. Here is the leg_time path detail list for the test mentioned in #2915:

image

@easbar easbar requested a review from karussell January 10, 2024 10:10
@karussell
Copy link
Member

karussell commented Jan 11, 2024

Nice! LGTM

@easbar easbar merged commit a12ee81 into master Jan 11, 2024
@easbar easbar deleted the empty_leg_path_detail branch January 11, 2024 18:22
@karussell
Copy link
Member

karussell commented Feb 22, 2024

Unfortunately I just found out there is still a problem if I add a duplicate point e.g. as the first point:

   addPoint(new GHPoint(42.509141, 1.546063)).
   addPoint(new GHPoint(42.509141, 1.546063)).
   ...

=>

! at java.base/java.util.ImmutableCollections$AbstractImmutableCollection.addAll(ImmutableCollections.java:148)
! at com.graphhopper.ResponsePath.addPathDetails(ResponsePath.java:271)
! at com.graphhopper.util.PathMerger.doWork(PathMerger.java:139)

The List.of needs to be replaced by a modifiable ArrayList (I think?)

@easbar
Copy link
Member Author

easbar commented Feb 22, 2024 •
edited
Loading

Yes, I think you are right. The list must be modifiable indeed, I fixed it in 99e9ba3

@karussell
Copy link
Member

karussell commented Feb 22, 2024

Nice, thank you!

@karussell karussell added this to the 9.0 milestone Mar 21, 2024
@easbar easbar mentioned this pull request May 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@karussell karussell karussell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

9.0

Development

Successfully merging this pull request may close these issues.

incorrect leg_distance entries if two stops are identical

3 participants

@easbar @karussell