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

Create own parking preferences for bike and car in the internal model #5521

Merged
Merged
Show file tree
Hide file tree
Changes from 7 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
6 changes: 3 additions & 3 deletions docs/RouteRequest.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe
| arriveBy | `boolean` | Whether the trip should depart or arrive at the specified date and time. | *Optional* | `false` | 2.0 |
| [bikeBoardCost](#rd_bikeBoardCost) | `integer` | Prevents unnecessary transfers by adding a cost for boarding a vehicle. | *Optional* | `600` | 2.0 |
| bikeParkCost | `integer` | Cost to park a bike. | *Optional* | `120` | 2.0 |
| bikeParkTime | `integer` | Time to park a bike. | *Optional* | `60` | 2.0 |
| bikeParkTime | `duration` | Time to park a bike. | *Optional* | `"PT1M"` | 2.0 |
| bikeReluctance | `double` | A multiplier for how bad biking is, compared to being in transit for equal lengths of time. | *Optional* | `2.0` | 2.0 |
| bikeSpeed | `double` | Max bike speed along streets, in meters per second | *Optional* | `5.0` | 2.0 |
| bikeStairsReluctance | `double` | How bad is it to walk the bicycle up/down a flight of stairs compared to taking a detour. | *Optional* | `10.0` | 2.3 |
Expand All @@ -35,7 +35,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe
| carDecelerationSpeed | `double` | The deceleration speed of an automobile, in meters per second per second. | *Optional* | `2.9` | 2.0 |
| carDropoffTime | `integer` | Time to park a car in a park and ride, w/o taking into account driving and walking cost. | *Optional* | `120` | 2.0 |
| carParkCost | `integer` | Cost of parking a car. | *Optional* | `120` | 2.1 |
| carParkTime | `integer` | Time to park a car | *Optional* | `60` | 2.1 |
| carParkTime | `duration` | Time to park a car | *Optional* | `"PT1M"` | 2.1 |
| carPickupCost | `integer` | Add a cost for car pickup changes when a pickup or drop off takes place | *Optional* | `120` | 2.1 |
| carPickupTime | `integer` | Add a time for car pickup changes when a pickup or drop off takes place | *Optional* | `60` | 2.1 |
| carReluctance | `double` | A multiplier for how bad driving is, compared to being in transit for equal lengths of time. | *Optional* | `2.0` | 2.0 |
Expand Down Expand Up @@ -912,7 +912,7 @@ include stairs as a last result.
"dropOffTime" : 30,
"dropOffCost" : 30
},
"bikeParkTime" : 60,
"bikeParkTime" : "1m",
"bikeParkCost" : 120,
"carDropoffTime" : 120,
"waitReluctance" : 1.0,
Expand Down
2 changes: 1 addition & 1 deletion docs/RouterConfiguration.md
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ Used to group requests when monitoring OTP.
"dropOffTime" : 30,
"dropOffCost" : 30
},
"bikeParkTime" : 60,
"bikeParkTime" : "1m",
"bikeParkCost" : 120,
"carDropoffTime" : 120,
"waitReluctance" : 1.0,
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/entur/router-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"dropOffTime": 30,
"dropOffCost": 30
},
"bikeParkTime": 60,
"bikeParkTime": "1m",
"bikeParkCost": 120,
"carDropoffTime": 120,
"waitReluctance": 1.0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ private GraphQLObjectType createGraphQLType() {
.name("bikeParkTime")
.description("Time to park a bike.")
.type(Scalars.GraphQLInt)
.dataFetcher(env -> preferences.bike().parkTime())
.dataFetcher(env -> (int) preferences.bike().parking().parkTime().toSeconds())
.build()
)
.field(
Expand All @@ -229,7 +229,7 @@ private GraphQLObjectType createGraphQLType() {
.name("bikeParkCost")
.description("Cost to park a bike.")
.type(Scalars.GraphQLInt)
.dataFetcher(env -> preferences.bike().parkCost())
.dataFetcher(env -> preferences.bike().parking().parkCost().toSeconds())
.build()
)
.field(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.opentripplanner.routing.api.request.framework.CostLinearFunction;
import org.opentripplanner.routing.api.request.preference.ItineraryFilterPreferences;
import org.opentripplanner.routing.api.request.preference.RoutingPreferences;
import org.opentripplanner.routing.api.request.preference.VehicleParkingPreferences;
import org.opentripplanner.routing.core.BicycleOptimizeType;

class RequestToPreferencesMapper {
Expand Down Expand Up @@ -42,8 +43,11 @@ void map() {
private void mapCar() {
preferences.withCar(car -> {
setIfNotNull(req.carReluctance, car::withReluctance);
setIfNotNull(req.carParkCost, car::withParkCost);
setIfNotNull(req.carParkTime, car::withParkTime);
car.withParking(parking -> {
mapParking(parking);
setIfNotNull(req.carParkCost, parking::withParkCost);
setIfNotNull(req.carParkTime, parking::withParkTime);
});
});
}

Expand All @@ -63,8 +67,6 @@ private void mapBike() {
setIfNotNull(req.bikeBoardCost, bike::withBoardCost);
setIfNotNull(req.bikeWalkingSpeed, bike::withWalkingSpeed);
setIfNotNull(req.bikeWalkingReluctance, bike::withWalkingReluctance);
setIfNotNull(req.bikeParkCost, bike::withParkCost);
setIfNotNull(req.bikeParkTime, bike::withParkTime);
setIfNotNull(req.bikeSwitchTime, bike::withSwitchTime);
setIfNotNull(req.bikeSwitchCost, bike::withSwitchCost);
setIfNotNull(req.bikeOptimizeType, bike::withOptimizeType);
Expand All @@ -76,6 +78,12 @@ private void mapBike() {
setIfNotNull(req.triangleSafetyFactor, triangle::withSafety);
});
}

bike.withParking(parking -> {
mapParking(parking);
setIfNotNull(req.bikeParkCost, parking::withParkCost);
setIfNotNull(req.bikeParkTime, parking::withParkTime);
});
});
}

Expand Down Expand Up @@ -159,6 +167,16 @@ private TransitGeneralizedCostFilterParams mapTransitGeneralizedCostFilterParams
return new TransitGeneralizedCostFilterParams(costLimitFunction, intervalRelaxFactor);
}

private void mapParking(VehicleParkingPreferences.Builder builder) {
setIfNotNull(
req.useVehicleParkingAvailabilityInformation,
builder::withUseAvailabilityInformation
);

builder.withRequiredVehicleParkingTags(req.requiredVehicleParkingTags);
builder.withBannedVehicleParkingTags(req.bannedVehicleParkingTags);
}

private void mapSystem() {
preferences.withSystem(system -> {
setIfNotNull(req.geoidElevation, system::withGeoidElevation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
import org.opentripplanner.routing.api.request.preference.ItineraryFilterDebugProfile;
import org.opentripplanner.routing.api.request.request.filter.SelectRequest;
import org.opentripplanner.routing.api.request.request.filter.TransitFilterRequest;
import org.opentripplanner.routing.api.request.request.filter.VehicleParkingFilter.TagsFilter;
import org.opentripplanner.routing.api.request.request.filter.VehicleParkingFilterRequest;
import org.opentripplanner.routing.core.BicycleOptimizeType;
import org.opentripplanner.standalone.api.OtpServerRequestContext;
import org.opentripplanner.standalone.config.framework.file.ConfigFileLoader;
Expand Down Expand Up @@ -749,20 +747,6 @@ protected RouteRequest buildRequest(MultivaluedMap<String, String> queryParamete
setIfNotNull(allowedVehicleRentalNetworks, rental::setAllowedNetworks);
setIfNotNull(bannedVehicleRentalNetworks, rental::setBannedNetworks);
}
{
var parking = journey.parking();
setIfNotNull(
useVehicleParkingAvailabilityInformation,
parking::setUseAvailabilityInformation
);

parking.setFilter(
new VehicleParkingFilterRequest(
new TagsFilter(bannedVehicleParkingTags),
new TagsFilter(requiredVehicleParkingTags)
)
);
}
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved

setIfNotNull(arriveBy, request::setArriveBy);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@
import org.opentripplanner.routing.api.request.RouteRequest;
import org.opentripplanner.routing.api.request.framework.CostLinearFunction;
import org.opentripplanner.routing.api.request.preference.ItineraryFilterDebugProfile;
import org.opentripplanner.routing.api.request.preference.VehicleParkingPreferences;
import org.opentripplanner.routing.api.request.request.filter.SelectRequest;
import org.opentripplanner.routing.api.request.request.filter.TransitFilterRequest;
import org.opentripplanner.routing.api.request.request.filter.VehicleParkingFilter;
import org.opentripplanner.routing.api.request.request.filter.VehicleParkingFilter.TagsFilter;
import org.opentripplanner.routing.api.request.request.filter.VehicleParkingFilterRequest;
import org.opentripplanner.routing.core.BicycleOptimizeType;
import org.opentripplanner.transit.model.basic.MainAndSubMode;
import org.opentripplanner.transit.model.basic.TransitMode;
Expand Down Expand Up @@ -84,9 +82,14 @@ public static RouteRequest toRouteRequest(
callWith.argument("triangle.safetyFactor", triangle::withSafety);
});
}

bike.withParking(parking -> setParkingPreferences(callWith, parking));
});

preferences.withCar(car -> callWith.argument("carReluctance", car::withReluctance));
preferences.withCar(car -> {
callWith.argument("carReluctance", car::withReluctance);
car.withParking(parking -> setParkingPreferences(callWith, parking));
});

preferences.withWalk(b -> {
callWith.argument("walkReluctance", b::withReluctance);
Expand Down Expand Up @@ -244,37 +247,23 @@ public static RouteRequest toRouteRequest(
(Collection<String> v) -> vehicleRental.setBannedNetworks(new HashSet<>(v))
);

var parking = request.journey().parking();
callWith.argument("parking.unpreferredCost", parking::setUnpreferredCost);

callWith.argument(
"parking.filters",
(Collection<Map<String, Object>> filters) -> parking.setFilter(parseFilters(filters))
);

callWith.argument(
"parking.preferred",
(Collection<Map<String, Object>> filters) -> parking.setPreferred(parseFilters(filters))
);

callWith.argument(
"locale",
(String v) -> request.setLocale(GraphQLUtils.getLocale(environment, v))
);
return request;
}

private static VehicleParkingFilterRequest parseFilters(Collection<Map<String, Object>> filters) {
var not = parseFilters(filters, "not");
var select = parseFilters(filters, "select");
return new VehicleParkingFilterRequest(not, select);
private static Set<String> parseNotFilters(Collection<Map<String, Object>> filters) {
return parseFilters(filters, "not");
}

private static Set<String> parseSelectFilters(Collection<Map<String, Object>> filters) {
return parseFilters(filters, "select");
}

@Nonnull
private static Set<VehicleParkingFilter> parseFilters(
Collection<Map<String, Object>> filters,
String key
) {
private static Set<String> parseFilters(Collection<Map<String, Object>> filters, String key) {
return filters
.stream()
.flatMap(f ->
Expand All @@ -283,14 +272,12 @@ private static Set<VehicleParkingFilter> parseFilters(
.collect(Collectors.toSet());
}

private static Stream<VehicleParkingFilter> parseOperation(
Collection<Map<String, Collection<String>>> map
) {
private static Stream<String> parseOperation(Collection<Map<String, Collection<String>>> map) {
return map
.stream()
.map(f -> {
.flatMap(f -> {
var tags = f.getOrDefault("tags", List.of());
return new TagsFilter(Set.copyOf(tags));
return tags.stream();
});
}

Expand All @@ -314,6 +301,29 @@ private static GenericLocation toGenericLocation(Map<String, Object> m) {
return new GenericLocation(lat, lng);
}

private static void setParkingPreferences(
CallerWithEnvironment callWith,
VehicleParkingPreferences.Builder parking
) {
callWith.argument("parking.unpreferredCost", parking::withUnpreferredVehicleParkingTagCost);

callWith.argument(
"parking.filters",
(Collection<Map<String, Object>> filters) -> {
parking.withRequiredVehicleParkingTags(parseSelectFilters(filters));
parking.withBannedVehicleParkingTags(parseNotFilters(filters));
}
);

callWith.argument(
"parking.preferred",
(Collection<Map<String, Object>> preferred) -> {
parking.withPreferredVehicleParkingTags(parseSelectFilters(preferred));
parking.withNotPreferredVehicleParkingTags(parseNotFilters(preferred));
}
);
}

private static class CallerWithEnvironment {

private final DataFetchingEnvironment environment;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ public ToStringBuilder addBool(String name, Boolean value) {
return addIfNotNull(name, value);
}

public ToStringBuilder addBool(String name, Boolean value, Boolean ignoreValue) {
return addIfNotIgnored(name, value, ignoreValue, Object::toString);
}

Copy link
Member

Choose a reason for hiding this comment

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

I prefer the addBoolIfTrue over this. For example useAvailabilityInformation=false is not as readable as skipAvailabilityInfo. We try to keep toStrings short and readable. You would need to repeat the default value in the toString inside the class to do this, but I prefer that for the extra readability.

Consider adding JavaDoc recommending the addBoolIfTrue or converting the usage of this. Note! That the addBoolIfTrue fits well with things that are off by default, and not so good with things that are on. Note also that the consequences of flipping the value in the toString is very small - it is just confusing.

Copy link
Member

Choose a reason for hiding this comment

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

This is not important!

Copy link
Member Author

Choose a reason for hiding this comment

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

I ended up renaming the parameter ignoreRealtimeAvailability. I was considering using the excludeRealtimeUpdates naming we used the plan pr but this paramete doesn't really exclude all realtime updates as the parks itself are added through realtime updates, it just ignores the realtime availability. I only did the renaming in the preferences, the REST API parameter stays as is.

public ToStringBuilder addBoolIfTrue(String name, Boolean value) {
if (TRUE.equals(value)) {
addLabel(name);
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/opentripplanner/model/plan/Place.java
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,12 @@ public static Place forVehicleParkingEntrance(VehicleParkingEntranceVertex verte
traverseMode = TraverseMode.BICYCLE;
}

var parkingPreferences = traverseMode == TraverseMode.CAR
? request.preferences().car().parking()
: request.preferences().bike().parking();

boolean realTime =
request.parking().useAvailabilityInformation() &&
parkingPreferences.useAvailabilityInformation() &&
vertex.getVehicleParking().hasRealTimeDataForMode(traverseMode, request.wheelchair());
return new Place(
vertex.getName(),
Expand Down
Loading
Loading