Add currentFuelPercent and currentRangeMeters to RentalVehichle in the GTFS GraphQL API #6272
+351
−29
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package org.opentripplanner.service.vehiclerental.model; | ||
|
|
||
| import javax.annotation.Nullable; | ||
| import org.opentripplanner.transit.model.basic.Distance; | ||
| import org.opentripplanner.transit.model.basic.Ratio; | ||
|
|
||
| /** | ||
| * Contains information about the current battery or fuel status. | ||
| * See the <a href="https://github.com/MobilityData/gbfs/blob/v3.0/gbfs.md#vehicle_statusjson">GBFS | ||
| * vehicle_status specification</a> for more details. | ||
| */ | ||
| public class RentalVehicleFuel { | ||
|
|
||
| /** | ||
| * Current fuel percentage, expressed from 0 to 1. | ||
| */ | ||
| @Nullable | ||
| public final Ratio percent; | ||
|
|
||
| /** | ||
| * Current fuel percentage, expressed from 0 to 1. | ||
| */ | ||
| @Nullable | ||
| public final Distance range; | ||
|
|
||
| public RentalVehicleFuel(@Nullable Ratio fuelPercent, @Nullable Distance range) { | ||
| this.percent = fuelPercent; | ||
| this.range = range; | ||
| } | ||
|
|
||
| @Nullable | ||
| public Double getPercent() { | ||
| return this.percent != null ? this.percent.asDouble() : null; | ||
| } | ||
|
|
||
| @Nullable | ||
| public Integer getRange() { | ||
| return this.range != null ? this.range.toMeters() : null; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,44 @@ | ||||||
| package org.opentripplanner.transit.model.basic; | ||||||
|
|
||||||
| import java.util.Objects; | ||||||
|
|
||||||
| /** | ||||||
| * Represents a ratio within the range [0, 1]. | ||||||
| * The class ensures that the ratio value, represented as a double, | ||||||
| * falls withing the specified range. | ||||||
| */ | ||||||
| public class Ratio { | ||||||
|
|
||||||
| private final Double ratio; | ||||||
|
|
||||||
| public Ratio(Double ratio) { | ||||||
| if (ratio < 0d || ratio > 1d) { | ||||||
| throw new IllegalArgumentException("Ratio must be in range [0,1]"); | ||||||
| } | ||||||
|
|
||||||
| this.ratio = ratio; | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public boolean equals(Object other) { | ||||||
| if (other instanceof Ratio ratio) { | ||||||
|
There was a problem hiding this comment. Do not use 'instanceof' in equals methods, follw the same practise as other value-objects. |
||||||
| return ratio.ratio == this.ratio; | ||||||
| } else { | ||||||
| return false; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public int hashCode() { | ||||||
| return Objects.hash(this.ratio); | ||||||
|
There was a problem hiding this comment. You should use the Double not Objects hash function here. |
||||||
| } | ||||||
|
|
||||||
| @Override | ||||||
| public String toString() { | ||||||
| return this.ratio.toString(); | ||||||
|
There was a problem hiding this comment. Should the toString output the number as a percent (like There was a problem hiding this comment. We discussed this today in the dev meeting and decided that the to string should just return the double as string, not a percent. There was a problem hiding this comment. Ok, so it's fine as it is |
||||||
| } | ||||||
|
|
||||||
| public Double asDouble() { | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| return ratio; | ||||||
| } | ||||||
| } | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,14 +9,21 @@ | |
| import org.mobilitydata.gbfs.v2_3.free_bike_status.GBFSBike; | ||
| import org.mobilitydata.gbfs.v2_3.free_bike_status.GBFSRentalUris; | ||
| import org.opentripplanner.framework.i18n.NonLocalizedString; | ||
| import org.opentripplanner.service.vehiclerental.model.RentalVehicleFuel; | ||
| import org.opentripplanner.service.vehiclerental.model.RentalVehicleType; | ||
| import org.opentripplanner.service.vehiclerental.model.VehicleRentalStationUris; | ||
| import org.opentripplanner.service.vehiclerental.model.VehicleRentalSystem; | ||
| import org.opentripplanner.service.vehiclerental.model.VehicleRentalVehicle; | ||
| import org.opentripplanner.transit.model.basic.Distance; | ||
| import org.opentripplanner.transit.model.basic.Ratio; | ||
| import org.opentripplanner.transit.model.framework.FeedScopedId; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public class GbfsFreeVehicleStatusMapper { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(GbfsFreeVehicleStatusMapper.class); | ||
|
|
||
| private final VehicleRentalSystem system; | ||
|
|
||
| private final Map<String, RentalVehicleType> vehicleTypes; | ||
|
|
@@ -52,7 +59,42 @@ public VehicleRentalVehicle mapFreeVehicleStatus(GBFSBike vehicle) { | |
| vehicle.getLastReported() != null | ||
| ? Instant.ofEpochSecond((long) (double) vehicle.getLastReported()) | ||
| : null; | ||
| Ratio fuelPercent = null; | ||
| try { | ||
| if (vehicle.getCurrentFuelPercent() != null) { | ||
| fuelPercent = new Ratio(vehicle.getCurrentFuelPercent()); | ||
| } | ||
| } catch (IllegalArgumentException e) { | ||
| LOG.warn( | ||
| "Current fuel percent value not valid: {} - {}", | ||
| vehicle.getCurrentFuelPercent(), | ||
| e.getMessage() | ||
| ); | ||
| } | ||
| Distance rangeMeters = null; | ||
| try { | ||
| rangeMeters = | ||
| vehicle.getCurrentRangeMeters() != null | ||
| ? Distance.ofMeters(vehicle.getCurrentRangeMeters()) | ||
| : null; | ||
| } catch (IllegalArgumentException e) { | ||
| LOG.warn( | ||
| "Current range meter value not valid: {} - {}", | ||
| vehicle.getCurrentRangeMeters(), | ||
| e.getMessage() | ||
| ); | ||
| } | ||
| // if the propulsion type has an engine current_range_meters is required | ||
| if ( | ||
| vehicle.getVehicleTypeId() != null && | ||
| vehicleTypes.get(vehicle.getVehicleTypeId()) != null && | ||
|
There was a problem hiding this comment. I added this check, can you double check if it's right? There was a problem hiding this comment. I think this is correct, but this is in an area where a lot of feeds get things wrong so this validation might cause issues but I'm not sure if I'm against this or not. There was a problem hiding this comment. I spoke to @hbruch about this and he said that there are a number of feeds that don't include it where they should but he is in favour of enforcing the spec anyway. If we are not strict with data producers, they will never learn. |
||
| vehicleTypes.get(vehicle.getVehicleTypeId()).propulsionType != | ||
| RentalVehicleType.PropulsionType.HUMAN && | ||
| rangeMeters == null | ||
| ) { | ||
| return null; | ||
|
There was a problem hiding this comment. We should log a warning here since the whole vehicle is ignored. There was a problem hiding this comment. I added the check to be compliant to the GBFS standard and I think it's better this way in the future but I see that it could be an issue if feeds get this wrong. I can remove the check if you say it's for the best, I don't think it would break anything. |
||
| } | ||
| rentalVehicle.fuel = new RentalVehicleFuel(fuelPercent, rangeMeters); | ||
| rentalVehicle.pricingPlanId = vehicle.getPricingPlanId(); | ||
| GBFSRentalUris rentalUris = vehicle.getRentalUris(); | ||
| if (rentalUris != null) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.