Skip to content

Commit eec3168

Browse files
optionsomeoptionsome
authored
Merge pull request opentripplanner#6599 from HSLdevcom/fix-scooter-prefs
Don't use bicycle as street routing mode when car or scooter rental is requested
2 parents 6f06c1b + 915e487 commit eec3168

5 files changed

Lines changed: 51 additions & 22 deletions

File tree

‎application/src/main/java/org/opentripplanner/routing/algorithm/mapping/StreetModeToRentalTraverseModeMapper.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package org.opentripplanner.routing.algorithm.mapping;
2+
3+
import org.opentripplanner.routing.api.request.StreetMode;
4+
import org.opentripplanner.street.search.TraverseMode;
5+
6+
public class StreetModeToRentalTraverseModeMapper {
7+
8+
/**
9+
* Maps street mode to rental traverse mode (i.e. CAR_RENTAL -> CAR).
10+
*/
11+
public static TraverseMode map(StreetMode mode) {
12+
return switch (mode) {
13+
case BIKE_RENTAL -> TraverseMode.BICYCLE;
14+
case SCOOTER_RENTAL -> TraverseMode.SCOOTER;
15+
case CAR_RENTAL -> TraverseMode.CAR;
16+
case NOT_SET,
17+
WALK,
18+
BIKE,
19+
BIKE_TO_PARK,
20+
CAR,
21+
CAR_TO_PARK,
22+
CAR_PICKUP,
23+
CAR_HAILING,
24+
FLEXIBLE -> throw new IllegalArgumentException("%s is not a rental mode.".formatted(mode));
25+
};
26+
}
27+
}

‎application/src/main/java/org/opentripplanner/street/search/state/StateData.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.ArrayList;
99
import java.util.List;
1010
import java.util.Set;
11+
import org.opentripplanner.routing.algorithm.mapping.StreetModeToRentalTraverseModeMapper;
1112
import org.opentripplanner.routing.api.request.StreetMode;
1213
import org.opentripplanner.street.model.RentalFormFactor;
1314
import org.opentripplanner.street.search.TraverseMode;
@@ -157,17 +158,18 @@ private static List<StateData> getInitialStateDatas(
157158
// - BEFORE_RENTING
158159
else if (requestMode.includesRenting()) {
159160
if (arriveBy) {
161+
var vehicleMode = StreetModeToRentalTraverseModeMapper.map(requestMode);
160162
if (allowArrivingInRentedVehicleAtDestination) {
161163
var keptVehicleStateData = proto.clone();
162164
keptVehicleStateData.vehicleRentalState = RENTING_FROM_STATION;
163-
keptVehicleStateData.currentMode = TraverseMode.BICYCLE;
165+
keptVehicleStateData.currentMode = vehicleMode;
164166
keptVehicleStateData.mayKeepRentedVehicleAtDestination = true;
165167
res.add(keptVehicleStateData);
166168
}
167169
var floatingRentalStateData = proto.clone();
168170
floatingRentalStateData.vehicleRentalState = RENTING_FLOATING;
169171
floatingRentalStateData.rentalVehicleFormFactor = toFormFactor(requestMode);
170-
floatingRentalStateData.currentMode = TraverseMode.BICYCLE;
172+
floatingRentalStateData.currentMode = vehicleMode;
171173
res.add(floatingRentalStateData);
172174
var stationReturnedStateData = proto.clone();
173175
stationReturnedStateData.vehicleRentalState = HAVE_RENTED;

‎application/src/main/java/org/opentripplanner/street/search/state/StateEditor.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.util.Set;
44
import javax.annotation.Nullable;
5+
import org.opentripplanner.routing.algorithm.mapping.StreetModeToRentalTraverseModeMapper;
56
import org.opentripplanner.street.model.RentalFormFactor;
67
import org.opentripplanner.street.model.edge.Edge;
78
import org.opentripplanner.street.model.vertex.Vertex;
@@ -324,7 +325,7 @@ public void dropFloatingVehicle(RentalFormFactor formFactor, String network, boo
324325
child.stateData.vehicleRentalState = VehicleRentalState.RENTING_FLOATING;
325326
child.stateData.currentMode = formFactor != null
326327
? formFactor.traverseMode
327-
: TraverseMode.BICYCLE;
328+
: StreetModeToRentalTraverseModeMapper.map(child.getRequest().mode());
328329
child.stateData.vehicleRentalNetwork = network;
329330
child.stateData.rentalVehicleFormFactor = formFactor;
330331
} else {

‎application/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeGeofencingTest.java‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import static org.junit.jupiter.api.Assertions.assertTrue;
88
import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex;
99
import static org.opentripplanner.street.model._data.StreetModelForTest.streetEdge;
10-
import static org.opentripplanner.street.search.TraverseMode.BICYCLE;
1110
import static org.opentripplanner.street.search.TraverseMode.SCOOTER;
1211
import static org.opentripplanner.street.search.TraverseMode.WALK;
1312
import static org.opentripplanner.street.search.state.VehicleRentalState.HAVE_RENTED;
@@ -224,7 +223,7 @@ public void pickupFloatingVehicleWhenLeavingAZone() {
224223
// we want to pick up a vehicle
225224
final State rentalState = states[1];
226225
assertEquals(RENTING_FLOATING, rentalState.getVehicleRentalState());
227-
assertEquals(BICYCLE, rentalState.currentMode());
226+
assertEquals(SCOOTER, rentalState.currentMode());
228227

229228
// but also keep on walking in case we don't find an edge where to leave the vehicle
230229
var walkingState = states[0];
@@ -257,7 +256,7 @@ public void pickupFloatingVehiclesWhenStartedInNoDropOffZone() {
257256
// then the speculative renting case for unknown rental network
258257
final State speculativeRenting = states[2];
259258
assertEquals(RENTING_FLOATING, speculativeRenting.getVehicleRentalState());
260-
assertEquals(BICYCLE, speculativeRenting.currentMode());
259+
assertEquals(SCOOTER, speculativeRenting.currentMode());
261260
// null means that the vehicle has been rented speculatively and the rest of the backwards search
262261
// needs to check if we really find a vehicle to pick up
263262
assertNull(speculativeRenting.getVehicleRentalNetwork());
@@ -269,7 +268,7 @@ public void pickupFloatingVehiclesWhenStartedInNoDropOffZone() {
269268
// then the speculative renting cases for specific rental networks
270269
final State tierState = states[1];
271270
assertEquals(RENTING_FLOATING, tierState.getVehicleRentalState());
272-
assertEquals(BICYCLE, tierState.currentMode());
271+
assertEquals(SCOOTER, tierState.currentMode());
273272
assertEquals(NETWORK_TIER, tierState.getVehicleRentalNetwork());
274273
assertEquals(Set.of(), tierState.stateData.noRentalDropOffZonesAtStartOfReverseSearch);
275274
}
@@ -286,22 +285,22 @@ public void pickupFloatingVehiclesWhenStartedInNoDropOffZoneAllNetworksAllowedBy
286285

287286
final State unknownNetworkState = states[3];
288287
assertEquals(RENTING_FLOATING, unknownNetworkState.getVehicleRentalState());
289-
assertEquals(BICYCLE, unknownNetworkState.currentMode());
288+
assertEquals(SCOOTER, unknownNetworkState.currentMode());
290289
assertNull(unknownNetworkState.getVehicleRentalNetwork());
291290

292291
final State tierState = Arrays.stream(states)
293292
.filter(s -> NETWORK_TIER.equals(s.getVehicleRentalNetwork()))
294293
.findFirst()
295294
.get();
296295
assertEquals(RENTING_FLOATING, tierState.getVehicleRentalState());
297-
assertEquals(BICYCLE, tierState.currentMode());
296+
assertEquals(SCOOTER, tierState.currentMode());
298297

299298
final State birdState = Arrays.stream(states)
300299
.filter(s -> NETWORK_BIRD.equals(s.getVehicleRentalNetwork()))
301300
.findFirst()
302301
.get();
303302
assertEquals(RENTING_FLOATING, birdState.getVehicleRentalState());
304-
assertEquals(BICYCLE, birdState.currentMode());
303+
assertEquals(SCOOTER, birdState.currentMode());
305304
}
306305

307306
@Test
@@ -316,22 +315,22 @@ public void pickupFloatingVehiclesWhenStartedInNoDropOffZoneAllNetworksAllowed()
316315

317316
final State unknownNetworkState = states[3];
318317
assertEquals(RENTING_FLOATING, unknownNetworkState.getVehicleRentalState());
319-
assertEquals(BICYCLE, unknownNetworkState.currentMode());
318+
assertEquals(SCOOTER, unknownNetworkState.currentMode());
320319
assertNull(unknownNetworkState.getVehicleRentalNetwork());
321320

322321
final State tierState = Arrays.stream(states)
323322
.filter(s -> NETWORK_TIER.equals(s.getVehicleRentalNetwork()))
324323
.findFirst()
325324
.get();
326325
assertEquals(RENTING_FLOATING, tierState.getVehicleRentalState());
327-
assertEquals(BICYCLE, tierState.currentMode());
326+
assertEquals(SCOOTER, tierState.currentMode());
328327

329328
final State birdState = Arrays.stream(states)
330329
.filter(s -> NETWORK_BIRD.equals(s.getVehicleRentalNetwork()))
331330
.findFirst()
332331
.get();
333332
assertEquals(RENTING_FLOATING, birdState.getVehicleRentalState());
334-
assertEquals(BICYCLE, birdState.currentMode());
333+
assertEquals(SCOOTER, birdState.currentMode());
335334
}
336335

337336
@Test
@@ -346,12 +345,12 @@ public void pickupFloatingVehiclesWhenStartedInNoDropOffZoneSomeNetworksAllowed(
346345

347346
final State unknownNetworkState = states[2];
348347
assertEquals(RENTING_FLOATING, unknownNetworkState.getVehicleRentalState());
349-
assertEquals(BICYCLE, unknownNetworkState.currentMode());
348+
assertEquals(SCOOTER, unknownNetworkState.currentMode());
350349
assertNull(unknownNetworkState.getVehicleRentalNetwork());
351350

352351
final State tierState = states[1];
353352
assertEquals(RENTING_FLOATING, tierState.getVehicleRentalState());
354-
assertEquals(BICYCLE, tierState.currentMode());
353+
assertEquals(SCOOTER, tierState.currentMode());
355354
assertEquals(NETWORK_TIER, tierState.getVehicleRentalNetwork());
356355
}
357356

@@ -380,12 +379,12 @@ public void pickupFloatingVehiclesWhenStartedInNoDropOffZoneSomeNetworksBanned()
380379

381380
final State unknownNetworkState = states[2];
382381
assertEquals(RENTING_FLOATING, unknownNetworkState.getVehicleRentalState());
383-
assertEquals(BICYCLE, unknownNetworkState.currentMode());
382+
assertEquals(SCOOTER, unknownNetworkState.currentMode());
384383
assertNull(unknownNetworkState.getVehicleRentalNetwork());
385384

386385
final State tierState = states[1];
387386
assertEquals(RENTING_FLOATING, tierState.getVehicleRentalState());
388-
assertEquals(BICYCLE, tierState.currentMode());
387+
assertEquals(SCOOTER, tierState.currentMode());
389388
assertEquals(NETWORK_TIER, tierState.getVehicleRentalNetwork());
390389
}
391390

@@ -416,7 +415,7 @@ private static State makeHaveRentedState(Vertex vertex, StreetSearchRequest req)
416415
private static StreetSearchRequest defaultArriveByRequest() {
417416
return StreetSearchRequest.of()
418417
.withPreferences(p ->
419-
p.withBike(b -> b.withRental(r -> r.withAllowedNetworks(Set.of(NETWORK_TIER))))
418+
p.withScooter(b -> b.withRental(r -> r.withAllowedNetworks(Set.of(NETWORK_TIER))))
420419
)
421420
.withMode(StreetMode.SCOOTER_RENTAL)
422421
.withArriveBy(true)
@@ -429,7 +428,7 @@ private static StreetSearchRequest makeArriveByRequest(
429428
) {
430429
return StreetSearchRequest.of()
431430
.withPreferences(p ->
432-
p.withBike(b ->
431+
p.withScooter(b ->
433432
b.withRental(r ->
434433
r.withAllowedNetworks(allowedNetworks).withBannedNetworks(bannedNetworks)
435434
)

‎application/src/test/java/org/opentripplanner/street/search/state/StateTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex;
1717
import static org.opentripplanner.street.search.TraverseMode.BICYCLE;
1818
import static org.opentripplanner.street.search.TraverseMode.CAR;
19+
import static org.opentripplanner.street.search.TraverseMode.SCOOTER;
1920
import static org.opentripplanner.street.search.TraverseMode.WALK;
2021
import static org.opentripplanner.street.search.state.TestStateBuilder.ofCarRental;
2122
import static org.opentripplanner.street.search.state.TestStateBuilder.ofDriving;
@@ -51,12 +52,11 @@ class StateTest {
5152
static Stream<Arguments> testCases() {
5253
return Stream.of(
5354
of(SCOOTER_RENTAL, false, Set.of(BEFORE_RENTING), Set.of(WALK)),
54-
//FIXME: it's strange that the arriveBy rental searches all start on a bicycle
55-
of(SCOOTER_RENTAL, true, Set.of(HAVE_RENTED, RENTING_FLOATING), Set.of(WALK, BICYCLE)),
55+
of(SCOOTER_RENTAL, true, Set.of(HAVE_RENTED, RENTING_FLOATING), Set.of(WALK, SCOOTER)),
5656
of(BIKE_RENTAL, false, Set.of(BEFORE_RENTING), Set.of(WALK)),
5757
of(BIKE_RENTAL, true, Set.of(HAVE_RENTED, RENTING_FLOATING), Set.of(WALK, BICYCLE)),
5858
of(CAR_RENTAL, false, Set.of(BEFORE_RENTING), Set.of(WALK)),
59-
of(CAR_RENTAL, true, Set.of(HAVE_RENTED, RENTING_FLOATING), Set.of(WALK, BICYCLE)),
59+
of(CAR_RENTAL, true, Set.of(HAVE_RENTED, RENTING_FLOATING), Set.of(WALK, CAR)),
6060
of(StreetMode.CAR, false, NULL_RENTAL_STATES, Set.of(CAR)),
6161
of(BIKE, false, NULL_RENTAL_STATES, Set.of(BICYCLE)),
6262
of(StreetMode.WALK, false, NULL_RENTAL_STATES, Set.of(TraverseMode.WALK)),

0 commit comments

Comments
 (0)