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

Fix rental scooter access #5361

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,5 @@ src/ext/resources/reportapi/report.csv

streetGraph.obj
graph.obj
# IntelliJ creates these pid files when you attach the debugger to tests
.attach_pid*
32 changes: 32 additions & 0 deletions src/main/java/org/opentripplanner/astar/spi/AStarEdge.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

import javax.annotation.Nonnull;

/**
* Represents an edge in the street network. Most edges have a one-to-one mapping to real world
* things like street segments or stairs.
* However, there can be other edges that represent concepts that are invisible and only present
* in OTP's model. These are things that link entities like transit stops and rental vehicles to the
* street network. The links are necessary in order for A* to discover them and can contain logic for
* access permissions. For example, a car can not use an edge that links the
* street network with a transit stop to prevent a straight transfer from car to transit.
*/
public interface AStarEdge<
State extends AStarState<State, Edge, Vertex>,
Edge extends AStarEdge<State, Edge, Vertex>,
Expand All @@ -11,6 +20,29 @@ public interface AStarEdge<

Vertex getToVertex();

/**
* Traverse the edge from a given state and return the result of the traversal.
*
* @param u The 'current' state when arriving at the fromVertex.
Copy link
Member

Choose a reason for hiding this comment

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

Is u again some term from A* literature or java naming convention? Could we have a more descriptive name for this variable even if this is just an interface?

Copy link
Member Author

@leonardehrenfried leonardehrenfried Sep 22, 2023

Choose a reason for hiding this comment

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

I'm not sure. There must be a reason why Hannes called it u. In other places it's called s0.

The A* Wikipedia page mentions neither u nor s0.

Would s0 be a good name for you?

Copy link
Member

Choose a reason for hiding this comment

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

Yes that's fine

* @return The array of states that are the result of the state (the passenger) moving (traversing)
* through the edge.
* <p>
* In most cases this is a single state where, for example, the weight (cost) and time are
* increased according to the properties of the edge.
* <p>
* However, it is also possible that this edge is not traversable for the current state,
* for example if it's a walk-only edge but the state is arriving in a car. In such a
* case an empty array is returned (see {@link org.opentripplanner.street.search.state.State#empty()}).
* The Astar algorithm won't then explore this state any further as the destination is not
* reachable through this state/edge combination.
* <p>
* Lastly, there can also be cases where more than one state is returned: For example
* when a state is renting a free-floating vehicle but the edge is in a no-drop-off zone.
* In such a case two resulting states are possible: one where the state continues on the
* rental vehicle (as you may exit the no-drop-off zone later) and a second state where the
* vehicle is speculatively dropped off and the passenger continues on foot in case
* that the destination is inside the zone.
*/
@Nonnull
State[] traverse(State u);
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ public State[] traverse(State s0) {
}
}

switch (s0.currentMode()) {
case BICYCLE:
return switch (s0.currentMode()) {
case BICYCLE, SCOOTER -> {
// Forbid taking your own bike in the station if bike P+R activated.
if (s0.getRequest().mode().includesParking() && !s0.isVehicleParked()) {
return State.empty();
yield State.empty();
}
// Forbid taking a (station) rental vehicle in the station. This allows taking along
// floating rental vehicles.
Expand All @@ -101,32 +101,38 @@ else if (
s0.getRequest().rental().allowArrivingInRentedVehicleAtDestination()
)
) {
return State.empty();
yield State.empty();
}
// Allow taking an owned bike in the station
break;
case CAR:
yield buildState(s0, s1, pref);
}
// Allow taking an owned bike in the station
case CAR -> {
// Forbid taking your own car in the station if bike P+R activated.
if (s0.getRequest().mode().includesParking() && !s0.isVehicleParked()) {
return State.empty();
yield State.empty();
}
// For Kiss & Ride allow dropping of the passenger before entering the station
if (s0.getCarPickupState() != null) {
if (canDropOffAfterDriving(s0) && isLeavingStreetNetwork(s0.getRequest().arriveBy())) {
dropOffAfterDriving(s0, s1);
} else {
return State.empty();
yield State.empty();
}
}
// If Kiss & Ride (Taxi) mode is not enabled allow car traversal so that the Stop
// may be reached by car
break;
case WALK:
break;
default:
return State.empty();
}
if (s0.isRentingVehicleFromStation()) {
yield State.empty();
}
yield buildState(s0, s1, pref);
}
// If Kiss & Ride (Taxi) mode is not enabled allow car traversal so that the Stop
// may be reached by car
case WALK -> buildState(s0, s1, pref);
case FLEX -> State.empty();
};
}

@Nonnull
private State[] buildState(State s0, StateEditor s1, RoutingPreferences pref) {
if (
s0.isRentingVehicleFromStation() &&
s0.mayKeepRentedVehicleAtDestination() &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public boolean isRentingVehicle() {
);
}

public boolean vehicleRentalIsFinished() {
private boolean vehicleRentalIsFinished() {
return (
stateData.vehicleRentalState == VehicleRentalState.HAVE_RENTED ||
(
Expand All @@ -221,7 +221,7 @@ public boolean vehicleRentalIsFinished() {
);
}

public boolean vehicleRentalNotStarted() {
private boolean vehicleRentalNotStarted() {
return stateData.vehicleRentalState == VehicleRentalState.BEFORE_RENTING;
}

Expand Down Expand Up @@ -471,6 +471,12 @@ public String toString() {
.addNum("weight", weight)
.addObj("vertex", vertex)
.addBoolIfTrue("VEHICLE_RENT", isRentingVehicle())
.addEnum("formFactor", vehicleRentalFormFactor())
.addBoolIfTrue("RENTING_FROM_STATION", isRentingVehicleFromStation())
.addBoolIfTrue(
"RENTING_FREE_FLOATING",
isRentingFloatingVehicle() && !isRentingVehicleFromStation()
)
.addBoolIfTrue("VEHICLE_PARKED", isVehicleParked())
.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class AccessEgressPenaltyDecoratorTest {
static void verifyTestSetup() {
assertEquals("Walk 15m38s $238035 w/penalty(13m23s $1606) ~ 1", EXP_WALK_W_PENALTY.toString());
assertEquals(
"Walk 11m53s $237886 w/penalty(11m8s $1336) ~ 1",
"Walk 11m53s $237887 w/penalty(11m8s $1336) ~ 1",
EXP_CAR_RENTAL_W_PENALTY.toString()
);
}
Expand Down Expand Up @@ -114,7 +114,7 @@ void filterEgress() {}
private static DefaultAccessEgress ofCarRental(int duration) {
return ofAccessEgress(
duration,
TestStateBuilder.ofCarRental().streetEdge().pickUpCar().build()
TestStateBuilder.ofCarRental().streetEdge().pickUpCarFromStation().build()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void containsModeWalkOnly() {
var subject = new DefaultAccessEgress(0, stateWalk);
assertTrue(subject.isWalkOnly());

var carRentalState = TestStateBuilder.ofCarRental().streetEdge().pickUpCar().build();
var carRentalState = TestStateBuilder.ofCarRental().streetEdge().pickUpCarFromStation().build();
subject = new DefaultAccessEgress(0, carRentalState);
assertFalse(subject.isWalkOnly());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.opentripplanner.service.vehiclerental.model;

import javax.annotation.Nonnull;
import org.opentripplanner.framework.i18n.NonLocalizedString;
import org.opentripplanner.street.model.RentalFormFactor;
import org.opentripplanner.transit.model.framework.FeedScopedId;

public class TestFreeFloatingRentalVehicleBuilder {

public static final String NETWORK_1 = "Network-1";

private RentalVehicleType vehicleType = RentalVehicleType.getDefaultType(NETWORK_1);

public static TestFreeFloatingRentalVehicleBuilder of() {
return new TestFreeFloatingRentalVehicleBuilder();
}

public TestFreeFloatingRentalVehicleBuilder withVehicleScooter() {
return buildVehicleType(RentalFormFactor.SCOOTER);
}

public TestFreeFloatingRentalVehicleBuilder withVehicleBicycle() {
return buildVehicleType(RentalFormFactor.BICYCLE);
}

public TestFreeFloatingRentalVehicleBuilder withVehicleCar() {
return buildVehicleType(RentalFormFactor.CAR);
}

@Nonnull
private TestFreeFloatingRentalVehicleBuilder buildVehicleType(RentalFormFactor rentalFormFactor) {
this.vehicleType =
new RentalVehicleType(
new FeedScopedId(TestFreeFloatingRentalVehicleBuilder.NETWORK_1, rentalFormFactor.name()),
rentalFormFactor.name(),
rentalFormFactor,
RentalVehicleType.PropulsionType.ELECTRIC,
100000d
);
return this;
}

public VehicleRentalVehicle build() {
var vehicle = new VehicleRentalVehicle();
var stationName = "free-floating-" + vehicleType.formFactor.name().toLowerCase();
vehicle.id = new FeedScopedId(NETWORK_1, stationName);
vehicle.name = new NonLocalizedString(stationName);
vehicle.latitude = 47.510;
vehicle.longitude = 18.99;
vehicle.vehicleType = vehicleType;
return vehicle;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opentripplanner.service.vehiclerental.model;

import java.util.Map;
import javax.annotation.Nonnull;
import org.opentripplanner.framework.i18n.NonLocalizedString;
import org.opentripplanner.street.model.RentalFormFactor;
import org.opentripplanner.transit.model.framework.FeedScopedId;
Expand Down Expand Up @@ -39,12 +40,21 @@ public TestVehicleRentalStationBuilder withStationOn(boolean stationOn) {
return this;
}

public TestVehicleRentalStationBuilder withVehicleTypeBicycle() {
return buildVehicleType(RentalFormFactor.BICYCLE);
}

public TestVehicleRentalStationBuilder withVehicleTypeCar() {
return buildVehicleType(RentalFormFactor.CAR);
}

@Nonnull
private TestVehicleRentalStationBuilder buildVehicleType(RentalFormFactor rentalFormFactor) {
this.vehicleType =
new RentalVehicleType(
new FeedScopedId(TestVehicleRentalStationBuilder.NETWORK_1, "car"),
"car",
RentalFormFactor.CAR,
new FeedScopedId(TestVehicleRentalStationBuilder.NETWORK_1, rentalFormFactor.name()),
rentalFormFactor.name(),
rentalFormFactor,
RentalVehicleType.PropulsionType.ELECTRIC,
100000d
);
Expand Down
Loading