Skip to content

Set transit-group-priority for requests without via points #6253

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

Merged

Conversation

flaktack
Copy link
Contributor

Summary

Currently transitGroupPriority is turned off for non-via requests, due to hasPassThroughOnly() being true for a request without any via locations:

 request.getViaLocations().stream().allMatch(ViaLocation::isPassThroughLocation)

This updates hasPassThroughOnly() and hasViaLocationsOnly() to also check that the request is actually a isViaSearch().

Unit tests

New tests are added to verify that transitPriorityCalculator() is set for non-via and visit-via searches.

@t2gran
Copy link
Member

t2gran commented Nov 14, 2024

Thank you for reporting and fixing this. While reviewing I noticed that there was a regression for the relaxCostAtDestination witch should only be allowed if none of the other features is used. I made a fix and found a minor bug in addition. I added a new parameterized unit-test to test all combinations of via-visit, via-pass-through, transit-priority-group and relax-cost-at-destination.

If you can, cherry-pick or copy my commit into your PR(or I can create a new PR). I made my fix on top of your fix: 7cdce4a branch

@flaktack
Copy link
Contributor Author

I would prefer a separate PR to simplify the review process.

@t2gran
Copy link
Member

t2gran commented Nov 15, 2024

But there is a bug in the fix you made - it is minor so I can do it in a separate PR - but the relaxCostAtDestination does not work.

@t2gran
Copy link
Member

t2gran commented Nov 15, 2024

I will create a separate PR for it - it does not matter to me.

@flaktack flaktack merged commit c0abd8c into opentripplanner:dev-2.x Nov 15, 2024
5 checks passed
@flaktack flaktack deleted the feature/traffic-group-priority branch November 22, 2024 09:17
@t2gran t2gran added this to the 2.7 (next release) milestone Jan 29, 2025
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.

3 participants