diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java index 94845c4a..f7259396 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java @@ -2,7 +2,6 @@ import org.eclipse.jetty.http.HttpStatus; import org.opentripplanner.middleware.models.TrackedJourney; -import org.opentripplanner.middleware.otp.response.Itinerary; import org.opentripplanner.middleware.otp.response.Leg; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.triptracker.instruction.TripInstruction; @@ -11,8 +10,8 @@ import org.opentripplanner.middleware.triptracker.response.TrackingResponse; import spark.Request; +import static org.opentripplanner.middleware.triptracker.TravelerLocator.isAtStartOfLeg; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsInt; -import static org.opentripplanner.middleware.utils.ItineraryUtils.getFirstLeg; import static org.opentripplanner.middleware.utils.ItineraryUtils.getRouteGtfsIdFromLeg; import static org.opentripplanner.middleware.utils.ItineraryUtils.isBusLeg; import static org.opentripplanner.middleware.utils.ItineraryUtils.legsMatch; @@ -151,7 +150,7 @@ private static EndTrackingResponse completeJourney(TripTrackingData tripData, bo tripData.trip.journeyState.matchingItinerary, Persistence.otpUsers.getById(tripData.trip.userId) ); - cancelBusNotification(travelerPosition, tripData.trip.journeyState.matchingItinerary); + cancelBusNotification(travelerPosition); TrackedJourney trackedJourney = travelerPosition.trackedJourney; trackedJourney.end(isForciblyEnded); Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_TIME_FIELD_NAME, trackedJourney.endTime); @@ -166,25 +165,38 @@ private static EndTrackingResponse completeJourney(TripTrackingData tripData, bo /** * Cancel bus notifications which are no longer needed/relevant. */ - private static void cancelBusNotification(TravelerPosition travelerPosition, Itinerary itinerary) { - Leg firstLegOfTrip = getFirstLeg(itinerary); - Leg busLeg = getLegToCancel(travelerPosition, firstLegOfTrip); + private static void cancelBusNotification(TravelerPosition travelerPosition) { + Leg busLeg = travelerPosition.nextLeg; + if (shouldCancelBusNotificationForStartOfTrip(travelerPosition)) { + busLeg = travelerPosition.expectedLeg; + } BusOperatorActions .getDefault() .handleCancelNotificationAction(travelerPosition, busLeg); } /** - * If the traveler is still on the first leg of their trip and bus notification has been sent, cancel notification - * related to this first leg. If the traveler is passed the first leg, cancel notification related to the next leg. + * Traveler is still waiting to board the bus at the start of a trip and notification has been sent. */ - public static Leg getLegToCancel(TravelerPosition travelerPosition, Leg firstLegOfTrip) { - if (legsMatch(travelerPosition.expectedLeg, firstLegOfTrip) && isBusLeg(travelerPosition.expectedLeg)) { - var routeId = getRouteGtfsIdFromLeg(travelerPosition.expectedLeg); - if (routeId != null && travelerPosition.trackedJourney.busNotificationMessages.containsKey(routeId)) { - return firstLegOfTrip; - } - } - return travelerPosition.nextLeg; + public static boolean shouldCancelBusNotificationForStartOfTrip(TravelerPosition travelerPosition) { + return hasSentBusNotificationForStartOfTrip(travelerPosition) && isWaitingForBusAtStartOfTrip(travelerPosition); + } + + /** + * Bus notification has been sent for the start of the trip. + */ + private static boolean hasSentBusNotificationForStartOfTrip(TravelerPosition travelerPosition) { + var routeId = getRouteGtfsIdFromLeg(travelerPosition.expectedLeg); + return routeId != null && travelerPosition.trackedJourney.busNotificationMessages.containsKey(routeId); + } + + /** + * Traveler is waiting for a bus at the start of a trip. + */ + private static boolean isWaitingForBusAtStartOfTrip(TravelerPosition travelerPosition) { + return + legsMatch(travelerPosition.expectedLeg, travelerPosition.firstLegOfTrip) && + isBusLeg(travelerPosition.expectedLeg) && + isAtStartOfLeg(travelerPosition); } -} +} \ No newline at end of file diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java index a4d511a2..46dfa7c1 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java @@ -33,6 +33,7 @@ import static org.opentripplanner.middleware.utils.GeometryUtils.getDistance; import static org.opentripplanner.middleware.utils.GeometryUtils.isPointBetween; import static org.opentripplanner.middleware.utils.ItineraryUtils.isBusLeg; +import static org.opentripplanner.middleware.utils.ItineraryUtils.legsMatch; /** * Locate the traveler in relation to the nearest step or destination and provide the appropriate instructions. @@ -70,7 +71,7 @@ public static String getInstruction( } } } else if (hasRequiredTransitLeg(travelerPosition) && hasRequiredTripStatus(tripStatus)) { - TripInstruction tripInstruction = alignTravelerToTransitTrip(travelerPosition, isStartOfTrip); + TripInstruction tripInstruction = alignTravelerToTransitTrip(travelerPosition); if (tripInstruction != null) { return tripInstruction.build(); } @@ -134,7 +135,7 @@ public static TripInstruction alignTravelerToTrip( Locale locale = travelerPosition.locale; if (isApproachingEndOfLeg(travelerPosition)) { - if (sendBusNotification(travelerPosition, isStartOfTrip)) { + if (sendBusNotification(travelerPosition)) { // Regardless of whether the notification is sent or qualifies, provide a 'wait for bus' instruction. return new WaitForTransitInstruction(travelerPosition.nextLeg, travelerPosition.currentTime, locale); } @@ -155,12 +156,9 @@ public static TripInstruction alignTravelerToTrip( /** * Send bus notification if the first leg is a bus leg or approaching a bus leg and within the notify window. */ - public static boolean sendBusNotification( - TravelerPosition travelerPosition, - boolean isStartOfTrip - ) { - Leg busLeg = (isStartOfTrip) ? travelerPosition.expectedLeg : travelerPosition.nextLeg; - if (shouldNotifyBusOperator(travelerPosition, busLeg)) { + public static boolean sendBusNotification(TravelerPosition travelerPosition) { + Leg busLeg = atStartOfTransitTrip(travelerPosition) ? travelerPosition.expectedLeg : travelerPosition.nextLeg; + if (isBusLeg(busLeg) && isWithinOperationalNotifyWindow(travelerPosition.currentTime, busLeg)) { BusOperatorActions .getDefault() .handleSendNotificationAction(travelerPosition, busLeg); @@ -170,35 +168,26 @@ public static boolean sendBusNotification( } /** - * Given the traveler's position and leg type, check if bus notification should be sent. - */ - public static boolean shouldNotifyBusOperator(TravelerPosition travelerPosition, Leg busLeg) { - return isBusLeg(busLeg) && isWithinOperationalNotifyWindow(travelerPosition.currentTime, busLeg); - } - - /** - * A trip which starts with a transit leg. + * A trip which starts with a transit leg and the traveler is on that leg. */ - private static boolean tripStartsWithTransitLeg(TravelerPosition travelerPosition, boolean isStartOfTrip) { - return isStartOfTrip && travelerPosition.expectedLeg.transitLeg; + private static boolean atStartOfTransitTrip(TravelerPosition travelerPosition) { + return + travelerPosition.expectedLeg != null && + travelerPosition.firstLegOfTrip != null && + travelerPosition.firstLegOfTrip.transitLeg && + legsMatch(travelerPosition.expectedLeg, travelerPosition.firstLegOfTrip); } /** * Align the traveler's position to the nearest transit stop or destination. */ @Nullable - public static TripInstruction alignTravelerToTransitTrip( - TravelerPosition travelerPosition, - boolean isStartOfTrip - ) { + public static TripInstruction alignTravelerToTransitTrip(TravelerPosition travelerPosition) { Locale locale = travelerPosition.locale; Leg expectedLeg = travelerPosition.expectedLeg; String finalStop = expectedLeg.to.name; - if ( - tripStartsWithTransitLeg(travelerPosition, isStartOfTrip) && - sendBusNotification(travelerPosition, isStartOfTrip) - ) { + if (sendBusNotification(travelerPosition)) { // Regardless of whether the notification is sent or qualifies, provide a 'wait for bus' instruction. return new WaitForTransitInstruction(expectedLeg, travelerPosition.currentTime, locale); } @@ -248,6 +237,14 @@ private static boolean isApproachingEndOfLeg(TravelerPosition travelerPosition) return getDistanceToEndOfLeg(travelerPosition) <= TRIP_INSTRUCTION_UPCOMING_RADIUS; } + /** + * Is the traveler at the start of a leg. + */ + public static boolean isAtStartOfLeg(TravelerPosition travelerPosition) { + Coordinates legDestination = new Coordinates(travelerPosition.expectedLeg.from); + return getDistance(travelerPosition.currentPosition, legDestination) <= TRIP_INSTRUCTION_UPCOMING_RADIUS; + } + /** * Is the traveler at the leg destination. */ diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerPosition.java b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerPosition.java index 9230d31c..0328ae84 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerPosition.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerPosition.java @@ -14,6 +14,7 @@ import static org.opentripplanner.middleware.triptracker.ManageLegTraversal.getNextLeg; import static org.opentripplanner.middleware.triptracker.ManageLegTraversal.getSegmentFromPosition; import static org.opentripplanner.middleware.utils.GeometryUtils.getDistanceFromLine; +import static org.opentripplanner.middleware.utils.ItineraryUtils.getFirstLeg; public class TravelerPosition { @@ -44,6 +45,9 @@ public class TravelerPosition { /** The traveler's locale. */ public Locale locale; + /** The first leg of the trip. **/ + public Leg firstLegOfTrip; + public TravelerPosition(TrackedJourney trackedJourney, Itinerary itinerary, OtpUser otpUser) { TrackingLocation lastLocation = trackedJourney.locations.get(trackedJourney.locations.size() - 1); currentTime = lastLocation.timestamp.toInstant(); @@ -61,6 +65,7 @@ public TravelerPosition(TrackedJourney trackedJourney, Itinerary itinerary, OtpU } this.locale = I18nUtils.getOtpUserLocale(otpUser); } + firstLegOfTrip = getFirstLeg(itinerary); } /** Used for unit testing. */ @@ -90,10 +95,11 @@ public TravelerPosition(Leg expectedLeg, Leg nextLeg, Instant currentTime) { } /** Used for unit testing. */ - public TravelerPosition(Leg expectedLeg, Leg nextLeg, TrackedJourney trackedJourney) { + public TravelerPosition(Leg expectedLeg, TrackedJourney trackedJourney, Leg first, Coordinates currentPosition) { this.expectedLeg = expectedLeg; - this.nextLeg = nextLeg; this.trackedJourney = trackedJourney; + this.firstLegOfTrip = first; + this.currentPosition = currentPosition; } /** Computes the current deviation in meters from the expected itinerary. */ diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java index 311df819..ec81deb6 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java @@ -37,7 +37,6 @@ import static org.opentripplanner.middleware.triptracker.TravelerLocator.ACCEPTABLE_AHEAD_OF_SCHEDULE_IN_MINUTES; import static org.opentripplanner.middleware.triptracker.TravelerLocator.getBusDepartureTime; import static org.opentripplanner.middleware.triptracker.interactions.busnotifiers.UsRideGwinnettNotifyBusOperator.getNotificationMessage; -import static org.opentripplanner.middleware.utils.ItineraryUtils.legsMatch; class NotifyBusOperatorTest extends OtpMiddlewareTestEnvironment { @@ -109,26 +108,31 @@ private static Stream creatNotifyBusOperatorForScheduledDepartureTrac } @ParameterizedTest - @MethodSource("creatGetCorrectLegToCancelNotificationTrace") - void canGetCorrectLegToCancelNotification(Leg expected, Leg next, String message) { + @MethodSource("shouldCancelBusNotificationForStartOfTripTrace") + void shouldCancelBusNotificationForStartOfTrip(boolean expected, Leg expectedLeg, Coordinates currentPosition, String message) { Leg first = firstLegBusTransit.legs.get(0); TrackedJourney journey = new TrackedJourney(); journey.busNotificationMessages.put(routeId, "{\"msg_type\": 1}"); - TravelerPosition travelerPosition = new TravelerPosition(expected, next, journey); - assertTrue(legsMatch(expected, ManageTripTracking.getLegToCancel(travelerPosition, first)), message); + TravelerPosition travelerPosition = new TravelerPosition(expectedLeg, journey, first, currentPosition); + assertEquals(expected, ManageTripTracking.shouldCancelBusNotificationForStartOfTrip(travelerPosition), message); } - private static Stream creatGetCorrectLegToCancelNotificationTrace() { + private static Stream shouldCancelBusNotificationForStartOfTripTrace() { + Leg first = firstLegBusTransit.legs.get(0); + Coordinates atStartOfBusJourney = new Coordinates(first.from); + Coordinates atEndOfBusJourney = new Coordinates(first.to); return Stream.of( Arguments.of( + true, firstLegBusTransit.legs.get(0), - firstLegBusTransit.legs.get(1), - "Should cancel notification for first leg." + atStartOfBusJourney, + "Still waiting for bus, should cancel notification." ), Arguments.of( + false, firstLegBusTransit.legs.get(1), - firstLegBusTransit.legs.get(1), - "Traveler has passed the first leg, no need to cancel notification for first leg." + atEndOfBusJourney, + "Already on the bus, no need to cancel notification." ) ); } @@ -243,39 +247,24 @@ private static Stream createWithinOperationalNotifyWindowTrace() { } @ParameterizedTest - @MethodSource("createShouldNotifyBusOperatorTrace") - void shouldNotifyBusOperator(boolean expected, TravelerPosition travelerPosition, Leg currentLeg, String message) { - assertEquals(expected, TravelerLocator.shouldNotifyBusOperator(travelerPosition, currentLeg), message); + @MethodSource("shouldSendBusNotificationAtStartOfTripTrace") + void shouldSendBusNotificationAtStartOfTrip(boolean expected, TravelerPosition travelerPosition, String message) { + assertEquals(expected, TravelerLocator.sendBusNotification(travelerPosition), message); } - private static Stream createShouldNotifyBusOperatorTrace() { + private static Stream shouldSendBusNotificationAtStartOfTripTrace() { + var busLeg = firstLegBusTransit.legs.get(0); var walkLeg = walkToBusTransition.legs.get(0); - var busLeg = walkToBusTransition.legs.get(1); - var busDepartureTime = getBusDepartureTime(busLeg); return Stream.of( Arguments.of( true, - new TravelerPosition(busLeg, busDepartureTime), - busLeg, - "Traveler approaching a bus leg, should notify." - ), - Arguments.of( - false, - new TravelerPosition(walkLeg, busDepartureTime), - walkLeg, - "Traveler approaching a walk leg, should not notify." - ), - Arguments.of( - true, - new TravelerPosition(busLeg, null, busDepartureTime), - busLeg, + new TravelerPosition(busLeg, getBusDepartureTime(busLeg)), "Traveler at the start of a trip which starts with a bus leg, should notify." ), Arguments.of( false, - new TravelerPosition(walkLeg, null, busDepartureTime), - walkLeg, + new TravelerPosition(walkLeg, getBusDepartureTime(walkLeg)), "Traveler at the start of a trip which starts with a walk leg, should not notify." ) );