Consider availability of rental vehicles in routing #6697

tobsesHub · 2025-06-12T18:56:34Z

Summary

consider vehicles[].available_until field vehicle_status.json in direct routing

Issue

#6547

This is the implementation for two steps explained in this commend. #6547 (comment)

Support availability in direct routing by only boarding vehicles witch are available in a fixed time-window by using the departure time(when the rental of the vehicle starts)
Add time-window(as Duration) for availability to the GTFS-GraphQL API and configuration(routing defaults). I named the field rentalDuration. rentalDuraton is zero by default. If it is negative or zero, it will not be considered. Therefore it should not change the previous behavior, if the new parameter was not used.

Unit tests

added tests for routerequestMapper for the new rentalDuration field(gtfs graphql api)
added tests for the availability in VehicleRentalEdgeTest. For that use case updated TestFreeFloatingRentalVehicleBuilder and StreetModelForTest
updated timestamp in rental-vehicle.json for the gtfs-graphql api integreation test

Documentation

documentation update for the new rentalDuration field in the configuration (generated RouteRequest.md)

optionsome · 2025-06-19T13:39:50Z

Perhaps only use this for car rental as it's probably the only thing you are interested in? Could you join a developer meeting at some point to discuss this again?

tobsesHub · 2025-06-23T07:54:34Z

Perhaps only use this for car rental as it's probably the only thing you are interested in? Could you join a developer meeting at some point to discuss this again?

Yes I actually just interested in car rental, so I could do it just for the car rental.
Yes, I will be in the developer meeting on Thursday the 26. Than we can discuss about this pr. Sorry I could not attend the meetings before.

tobsesHub · 2025-07-03T11:16:47Z

First of all draft. I continue with this pr later.

codecov · 2025-07-09T13:27:21Z

Codecov Report

❌ Patch coverage is 88.57143% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.24%. Comparing base (e9e4dff) to head (0fec972).
⚠️ Report is 82 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
...ervice/vehiclerental/street/VehicleRentalEdge.java	75.00%	1 Missing and 2 partials ⚠️
...ntripplanner/apis/gtfs/generated/GraphQLTypes.java	50.00%	2 Missing ⚠️
...fs/mapping/routerequest/ModePreferencesMapper.java	85.71%	0 Missing and 2 partials ⚠️
...ner/routing/api/request/request/StreetRequest.java	91.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6697      +/-   ##
=============================================
+ Coverage      72.18%   72.24%   +0.05%     
- Complexity     19840    19933      +93     
=============================================
  Files           2155     2163       +8     
  Lines          80051    80284     +233     
  Branches        8082     8113      +31     
=============================================
+ Hits           57786    57998     +212     
- Misses         19419    19422       +3     
- Partials        2846     2864      +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

t2gran

I think access/egress time-shifting support (as discussed in dev meeting) should be added in a follow up PR. Until then the API request parameter should be documented as experimental.

Implementing the Access/Egress opening hours can be done extending the DefaultAccessEgress and implementing: earliestDepartureTime(int requestedDepartureTime), latestArrivalTime(int requestedArrivalTime), numberOfRides()and hasOpeningHours()

Should the rentalDuration be relative to the requestTime or the pick-up-time, I am leaning towards requestTime using pick-up time may lead to very strange results (same problem as checking the time constraints during routing).

...c/main/java/org/opentripplanner/routing/api/request/preference/VehicleRentalPreferences.java

t2gran · 2025-08-15T09:04:50Z

...c/main/java/org/opentripplanner/routing/api/request/preference/VehicleRentalPreferences.java

    this.pickupCost = builder.pickupCost;
    this.dropOffTime = Duration.ofSeconds(Units.duration(builder.dropOffTime));
    this.dropOffCost = builder.dropOffCost;
+    this.rentalDuration = Duration.ofSeconds(Units.duration(builder.rentalDuration));


You should validate the duration here by calling DurationUtils#requireNonNegative.

added this validatio to ModePreferencesMapper

t2gran · 2025-08-15T09:40:56Z

...cation/src/main/java/org/opentripplanner/service/vehiclerental/street/VehicleRentalEdge.java

+              Instant departureTime = s0.getTime().plus(preferences.pickupTime());
+              OffsetDateTime rentalEndTime = OffsetDateTime.ofInstant(
+                departureTime,
+                ZoneId.systemDefault()
+              ).plus(preferences.rentalDuration());


I think the rental available period should be relative to the request time, not relative to the pickup-time. For depart after [request.dateTime, +rentalDuration] and for arrive by [-rentalDuration, request.dateTime]. This should be calculated and set in the StreetSearchRequest and checked here.

Unsure about the time-zone, using ZoneId.systemDefault() seams wrong. Also, I think the time calculations should use the internal AStar model time format.

@leonardehrenfried & @optionsome What happens if we traverse the graph in reverse - do we ever do that?

What should we do if the journey can not be completed within the rentalDuration? I guess this should end up in a empty result. So I think there need to be a check that we have not exceeded the rentalDuration limit her as well.

hmm I don't get what's the problem by using the pickup-time.
The pickup time seems to be more more precise to me as it really just considers the rental duration.
But if there is any experience with similar cases, I can change it.

The problem using the pick-up time is that if you have two paths to walk to the rental A, and B. Then A takes 5 min and B 6 min. Then the rental is dropped for path A but accepted for path B. Then the returned result is not optimal. It is possible that path B i dropped in the "from" vertex state - if so this would be ok. Another problem is how to explain this to the end user. Let say there are 2 cars available from 12:00 to 15:00, you start walking 11:50. It takes 9 minutes to the first and 30 minutes to the second. rentalDuration is 1h. In this case the best option is to walk to the first car and WAIT 1 minute. Can you explain why we are not checking the "available from time"?

We do not really have the "available from time" right now. The GBFS standard support currently available vehicles and the availableUntil fiel. So we do not have the informatin what is the start time, on which a vehicle is available.

Thank you for the clarification. This was discussed shortly at todays developer meeting, and we decided:

to use departure-time + rentalDuration for depart-after-search. For arrival search the vehicle need to be available at when the search start. An the search should be stopped when the time is less than the arrival-time - rentalDuration.

Any rentalDuration value needs to be cleared before we do access/egress search since time-shifting is not supported. This can be implemented later - if someone needs it.

The following text should be added to the rentalDuration doc (domain model JavaDoc and API doc):

The rentalDuration only apply to direct search, support for transit search is not implemented.

I changed the departure by using the start time of the StreetSearchRequest.
I also updated the behavior of arrival requests. I used the start time (arrival time) of the StreetSearchRequest to check, if the vehicle is available at this arrival time.

But I am not sure about "An the search should be stopped when the time is less than the arrival-time - rentalDuration" What exactly do you mean with "time". Maybe we can clarify that in the dev meeting.

I added the describing sentence to the GreaphQL parameter(schema) and as Java doc to the rentalDuration property in StreetRequest and StreetSearchRequest.

I modelled the startTime and endTime of the rentalduration as RentalPeriod and get build in the StreetSearchRequestMapper.

t2gran · 2025-08-21T09:16:30Z

I am unsure what we need to do to make this work with access/egress and regular transit. There are a few things that need investigation:

How to do time-shifting of access/egress. The business rules need to be described.
There need to be a walk only option for all stops, witch can be used when time-shifting the rental option is not available.

If this feature is not supported for access/egress then we need to make sure it is disabled in the access/egress search. Either of these two options needs to be implemented in this PR.

t2gran · 2025-08-21T21:48:19Z

...c/main/java/org/opentripplanner/routing/api/request/preference/VehicleRentalPreferences.java

+   * An assumed duration of the rental trip, to make sure the vehicle is available during this time
+   */
+  public Duration rentalDuration() {


Suggested change

* An assumed duration of the rental trip, to make sure the vehicle is available during this time

*/

public Duration rentalDuration() {

* An assumed duration of the rental trip, to make sure the vehicle is available during this time

*/

@Nullable

public Duration rentalDuration() {

I added nullable annotation to the rentalDuration property of the streetRequest and streetSearchRequest

t2gran · 2025-08-21T22:14:14Z

...cation/src/main/java/org/opentripplanner/service/vehiclerental/street/VehicleRentalEdge.java

+              Instant departureTime = s0.getTime().plus(preferences.pickupTime());
+              OffsetDateTime rentalEndTime = OffsetDateTime.ofInstant(
+                departureTime,
+                ZoneId.systemDefault()
+              ).plus(preferences.rentalDuration());


Thank you for the clarification. This was discussed shortly at todays developer meeting, and we decided:

to use departure-time + rentalDuration for depart-after-search. For arrival search the vehicle need to be available at when the search start. An the search should be stopped when the time is less than the arrival-time - rentalDuration.

Any rentalDuration value needs to be cleared before we do access/egress search since time-shifting is not supported. This can be implemented later - if someone needs it.

The following text should be added to the rentalDuration doc (domain model JavaDoc and API doc):

The rentalDuration only apply to direct search, support for transit search is not implemented.

tobsesHub · 2025-09-02T15:52:15Z

Before merging this PR, I would like to squash the commits. There are now quite a few commits for just one feature.

t2gran · 2025-10-13T11:48:17Z

@tobsesHub Can you go over the pending review comments.

t2gran · 2025-08-27T19:40:13Z

application/src/main/java/org/opentripplanner/routing/api/request/request/StreetRequest.java

-    return ToStringBuilder.ofEmbeddedType().addEnum("mode", mode, DEFAULT.mode).toString();
+    return ToStringBuilder.ofEmbeddedType()
+      .addEnum("mode", mode, DEFAULT.mode)
+      .addObj("rentalDuration", rentalDuration, DEFAULT.rentalDuration)


Suggested change

.addObj("rentalDuration", rentalDuration, DEFAULT.rentalDuration)

.addDuration("rentalDuration", rentalDuration, DEFAULT.rentalDuration)

changed it.

t2gran · 2025-08-27T19:44:22Z

application/src/main/java/org/opentripplanner/routing/api/request/request/StreetRequest.java

    return mode;
  }

+  public Duration rentalDuration() {


Suggested change

public Duration rentalDuration() {

@Nullable

public Duration rentalDuration() {

changed it.

t2gran · 2025-08-27T19:46:47Z

application/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequest.java

added nullable to rentalPeriod()

t2gran · 2025-08-27T19:47:43Z

...tion/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequestBuilder.java

  boolean wheelchair;
  GenericLocation from;
  GenericLocation to;
+  Duration rentalDuration;


Suggested change

Duration rentalDuration;

@Nullable

Duration rentalDuration;

t2gran · 2025-08-27T19:58:40Z

.../src/main/java/org/opentripplanner/apis/gtfs/mapping/routerequest/ModePreferencesMapper.java

      }
      var streetModes = direct.stream().map(DirectModeMapper::map).toList();
-      journey.withDirect(new StreetRequest(getStreetModeForRouting(streetModes)));
+


There is a lot of repetition in the code below, consider:

if(a != null) { var b = a.b; if(b != null) { var c = b.c if(c != null) { ... } } }

Actually I do not like both, because this solution has lots of nesting instead, but I think it is less error prone, so it should be better. I outsourced it to a static method. I hope makes the code more readable.

t2gran · 2025-10-01T09:21:21Z

application/src/main/java/org/opentripplanner/routing/api/request/request/StreetRequest.java

    }
    StreetRequest that = (StreetRequest) o;
-    return mode == that.mode;
+    return (mode == that.mode && Objects.equals(rentalDuration, that.rentalDuration));


Suggested change

return (mode == that.mode && Objects.equals(rentalDuration, that.rentalDuration));

return (mode == that.mode) && Objects.equals(rentalDuration, that.rentalDuration);

changed it.

t2gran · 2025-10-01T09:22:51Z

application/src/main/java/org/opentripplanner/routing/api/request/request/StreetRequest.java

+  public StreetRequest(StreetMode mode, Duration rentalDuration) {
+    this.mode = mode;
+    this.rentalDuration = rentalDuration;
+  }


I think this is good enough, but the next time we add a new parameter to this class, we should make a builder.

t2gran · 2025-10-13T12:11:01Z

application/src/main/java/org/opentripplanner/street/search/request/RentalPeriod.java

+  @Nonnull
+  private final Instant rentalEndTime;
+
+  public RentalPeriod(RentalPeriodBuilder rentalPeriodBuilder) {


Consider: pass in rentalStartTime and rentalEndTime here, not the builder.

changed it.

t2gran · 2025-10-13T12:14:18Z

...ation/src/main/java/org/opentripplanner/street/search/request/StreetSearchRequestMapper.java

+      RentalPeriod rentalPeriod = new RentalPeriodBuilder()
+        .setRentalStartTime(rentalStart)
+        .setRentalEndTime(rentalEnd)
+        .build();


This logic can be moved inside the RentalPeriodBuilder - then it become easy to unit-test. Instead
of the regular builder pattern it is better to create two builder methods:

var builder = RentalPeriod.of(time, rentalDuration); var rentalPeriod = request.arriveBy() ? builder.buildFromLatestArrivalTime() : builder.buildFromEarliestDepartureTime();

good advice, thanks, I changed it that way.

I also was thinking, to add the arriveBy to the builder. Than the whole logic would be at one place at the RentalPeriodBuilder. But I was not sure, if it is a good idea.

t2gran · 2025-10-13T12:29:09Z

...cation/src/main/java/org/opentripplanner/service/vehiclerental/street/VehicleRentalEdge.java

+              if (availableUntil != null && availableUntil.isBefore(rentalEndTime)) {
+                return State.empty();
+              }
+            }


Why is there 2 identical code blocks here?

Good point, there is no good reason. I added a isRentalVehicleAvailableDuringRentalPeriod method, to prevent the duplicate code blocks.

…ping

tobsesHub requested a review from a team as a code owner June 12, 2025 18:56

tobsesHub closed this Jun 13, 2025

tobsesHub reopened this Jun 13, 2025

tobsesHub marked this pull request as draft July 3, 2025 11:15

tobsesHub marked this pull request as ready for review July 10, 2025 09:40

tobsesHub force-pushed the feat/rental-routing-avaibility-window branch from bbd6ddb to 3af25f0 Compare July 17, 2025 14:18

optionsome requested a review from t2gran August 5, 2025 08:52

t2gran added this to the 2.8 (next release) milestone Aug 11, 2025

tobsesHub force-pushed the feat/rental-routing-avaibility-window branch from 3af25f0 to f1b62aa Compare August 14, 2025 09:30

t2gran requested changes Aug 15, 2025

View reviewed changes

t2gran requested changes Aug 21, 2025

View reviewed changes

tobsesHub marked this pull request as draft August 28, 2025 11:32

tobsesHub force-pushed the feat/rental-routing-avaibility-window branch 2 times, most recently from 2f5f8e1 to f217651 Compare September 2, 2025 15:29

tobsesHub marked this pull request as ready for review September 2, 2025 15:49

t2gran modified the milestones: 2.8, 2.9 (next release) Sep 10, 2025

tobsesHub force-pushed the feat/rental-routing-avaibility-window branch from 95b841a to c0b9a69 Compare September 18, 2025 11:56

habrahamsson-skanetrafiken requested a review from optionsome October 9, 2025 12:54

t2gran requested changes Oct 13, 2025

View reviewed changes

tobsesHub marked this pull request as draft October 16, 2025 07:38

tobsesHub added 2 commits October 21, 2025 19:34

feat(vehicle rental): use availability time window

d8e36e0

test(vehicle rental): add test for availability field

3b9f106

tobsesHub added 17 commits October 21, 2025 19:34

feat(vehicle rental): make rental duration configurable

083cc33

test(vehicle rental): add tests for availability

eaa9860

feat(vehicle rental): use rental duration just in car rental routing

b440828

refactor(vehicle rental): chane rentalDuration as Duration in builder

13ecfc1

refactor(vehicle rental): make default duration null

5fbe570

refactor(vehicle rental): move rentalDuration to StreetRequest

9b5a71c

refactor(vehicle rental): add rentalDuration validation

78a467c

docs(vehicle rental): add rentalDuration doc

902f18f

feat(vehicle rental): use street request as start time of rental window

90a376f

feat(vehicle rental): consider rental duration on arrival request

d0df06c

fix(vehicle rental): replace getter because of rebase

186baea

refactor(vehicle rental): calculate rentEnd during street request map…

0d2da7c

…ping

refactor(vehicle rental): represent rental duration as RentalPeriod

442ae41

refactor(vehicle rental): refactor null checks to access rental duration

46f7f0f

refactor(vehicle rental): annotate streetRequest fields nullable

fb34070

refactor(vehicle rental): prevent duplicate code on VehicleRentalEdge

2e23e5d

refactor(vehicle rental): make rental period better testable

0fec972

tobsesHub force-pushed the feat/rental-routing-avaibility-window branch from c0b9a69 to 0fec972 Compare October 23, 2025 17:52

tobsesHub marked this pull request as ready for review October 23, 2025 18:06

	.addObj("rentalDuration", rentalDuration, DEFAULT.rentalDuration)
	.addDuration("rentalDuration", rentalDuration, DEFAULT.rentalDuration)

	public Duration rentalDuration() {
	@Nullable
	public Duration rentalDuration() {

	return (mode == that.mode && Objects.equals(rentalDuration, that.rentalDuration));
	return (mode == that.mode) && Objects.equals(rentalDuration, that.rentalDuration);

Consider availability of rental vehicles in routing #6697

Are you sure you want to change the base?

Consider availability of rental vehicles in routing #6697

Uh oh!

Conversation

tobsesHub commented Jun 12, 2025

Summary

Issue

Unit tests

Documentation

Uh oh!

optionsome commented Jun 19, 2025

Uh oh!

tobsesHub commented Jun 23, 2025

Uh oh!

tobsesHub commented Jul 3, 2025

Uh oh!

codecov bot commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

t2gran left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobsesHub Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t2gran Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobsesHub Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t2gran commented Aug 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobsesHub commented Sep 2, 2025

Uh oh!

t2gran commented Oct 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobsesHub Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 9, 2025 •

edited

Loading

t2gran left a comment •

edited

Loading

tobsesHub Aug 20, 2025 •

edited

Loading

t2gran Aug 21, 2025 •

edited

Loading

tobsesHub Sep 2, 2025 •

edited

Loading

tobsesHub Oct 23, 2025 •

edited

Loading

tobsesHub Oct 23, 2025 •

edited

Loading

tobsesHub Oct 23, 2025 •

edited

Loading