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

Send bus notifications if first leg is bus #264

Merged
merged 9 commits into from
Nov 6, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

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;
import org.opentripplanner.middleware.triptracker.interactions.busnotifiers.BusOperatorActions;
Expand All @@ -10,6 +12,10 @@
import spark.Request;

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;
import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt;

public class ManageTripTracking {
Expand Down Expand Up @@ -145,19 +151,40 @@ private static EndTrackingResponse completeJourney(TripTrackingData tripData, bo
tripData.trip.journeyState.matchingItinerary,
Persistence.otpUsers.getById(tripData.trip.userId)
);
BusOperatorActions
.getDefault()
.handleCancelNotificationAction(travelerPosition);
cancelBusNotification(travelerPosition, tripData.trip.journeyState.matchingItinerary);
TrackedJourney trackedJourney = travelerPosition.trackedJourney;
trackedJourney.end(isForciblyEnded);
Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_TIME_FIELD_NAME, trackedJourney.endTime);
Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_CONDITION_FIELD_NAME, trackedJourney.endCondition);

// Provide response.
return new EndTrackingResponse(
TripInstruction.NO_INSTRUCTION,
TripStatus.ENDED.name()
);
}

/**
* Cancel bus notifications which will not be fulfilled.
br648 marked this conversation as resolved.
Show resolved Hide resolved
*/
private static void cancelBusNotification(TravelerPosition travelerPosition, Itinerary itinerary) {
Leg firstLegOfTrip = getFirstLeg(itinerary);
Leg busLeg = getLegToCancel(travelerPosition, firstLegOfTrip);
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.
*/
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,20 @@ public static String getInstruction(
) {
if (hasRequiredWalkLeg(travelerPosition)) {
if (hasRequiredTripStatus(tripStatus)) {
TripInstruction tripInstruction = alignTravelerToTrip(travelerPosition, isStartOfTrip, tripStatus);
TripInstruction tripInstruction = alignTravelerToTrip(travelerPosition, isStartOfTrip);
if (tripInstruction != null) {
return tripInstruction.build();
}
}

if (tripStatus.equals(TripStatus.DEVIATED)) {
TripInstruction tripInstruction = getBackOnTrack(travelerPosition, isStartOfTrip, tripStatus);
TripInstruction tripInstruction = getBackOnTrack(travelerPosition, isStartOfTrip);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bonus points question: I didn't see an equivalent to "Head to " on an itinerary where the first leg is bus and tracking is started at a location away from the bus stop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@binh-dam-ibigroup I will hold off merging until you have "approved" this addition.

if (tripInstruction != null) {
return tripInstruction.build();
}
}
} else if (hasRequiredTransitLeg(travelerPosition) && hasRequiredTripStatus(tripStatus)) {
TripInstruction tripInstruction = alignTravelerToTransitTrip(travelerPosition);
TripInstruction tripInstruction = alignTravelerToTransitTrip(travelerPosition, isStartOfTrip);
if (tripInstruction != null) {
return tripInstruction.build();
}
Expand Down Expand Up @@ -111,10 +111,9 @@ private static boolean hasRequiredTripStatus(TripStatus tripStatus) {
@Nullable
private static TripInstruction getBackOnTrack(
TravelerPosition travelerPosition,
boolean isStartOfTrip,
TripStatus tripStatus
boolean isStartOfTrip
) {
TripInstruction instruction = alignTravelerToTrip(travelerPosition, isStartOfTrip, tripStatus);
TripInstruction instruction = alignTravelerToTrip(travelerPosition, isStartOfTrip);
if (instruction != null && instruction.hasInstruction()) {
return instruction;
}
Expand All @@ -130,13 +129,12 @@ private static TripInstruction getBackOnTrack(
@Nullable
public static TripInstruction alignTravelerToTrip(
TravelerPosition travelerPosition,
boolean isStartOfTrip,
TripStatus tripStatus
boolean isStartOfTrip
) {
Locale locale = travelerPosition.locale;

if (isApproachingEndOfLeg(travelerPosition)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is at the start of a trip where the first leg is transit, there is no approaching the end of the leg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@binh-dam-ibigroup If possible can you provide a trip which starts with a transit leg? I'm going to edit an existing trip (walk-to-bus-transition.json) to test with, but would prefer a real-world exampl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@binh-dam-ibigroup this has grown a bit from my initial take on it! I think this covers the what is needed.

if (sendBusNotification(travelerPosition, isStartOfTrip, tripStatus)) {
if (sendBusNotification(travelerPosition, isStartOfTrip)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might have misinterpreted isStartOfTrip when using with sendBusNotification, and that is what is causing the failing trip tracking E2E test. I think true means that this is the first tracking location that is sent (the user just activated live tracking). I think you meant to pass to sendBusNotification a boolean indicating whether a transit leg is the first leg of the trip.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@binh-dam-ibigroup I have updated, I hope it meets expectations.

// Regardless of whether the notification is sent or qualifies, provide a 'wait for bus' instruction.
return new WaitForTransitInstruction(travelerPosition.nextLeg, travelerPosition.currentTime, locale);
}
Expand All @@ -159,13 +157,13 @@ public static TripInstruction alignTravelerToTrip(
*/
public static boolean sendBusNotification(
TravelerPosition travelerPosition,
boolean isStartOfTrip,
TripStatus tripStatus
boolean isStartOfTrip
) {
if (shouldNotifyBusOperator(travelerPosition, isStartOfTrip)) {
Leg busLeg = (isStartOfTrip) ? travelerPosition.expectedLeg : travelerPosition.nextLeg;
if (shouldNotifyBusOperator(travelerPosition, busLeg)) {
BusOperatorActions
.getDefault()
.handleSendNotificationAction(tripStatus, travelerPosition);
.handleSendNotificationAction(travelerPosition, busLeg);
return true;
}
return false;
Expand All @@ -174,21 +172,37 @@ 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, boolean isStartOfTrip) {
return (isStartOfTrip)
? isBusLeg(travelerPosition.expectedLeg) && isWithinOperationalNotifyWindow(travelerPosition.currentTime, travelerPosition.expectedLeg)
: isBusLeg(travelerPosition.nextLeg) && isWithinOperationalNotifyWindow(travelerPosition);
public static boolean shouldNotifyBusOperator(TravelerPosition travelerPosition, Leg busLeg) {
return isBusLeg(busLeg) && isWithinOperationalNotifyWindow(travelerPosition.currentTime, busLeg);
}

/**
* A trip which starts with a transit leg.
*/
private static boolean tripStartsWithTransitLeg(TravelerPosition travelerPosition, boolean isStartOfTrip) {
return isStartOfTrip && travelerPosition.expectedLeg.transitLeg;
}

/**
* Align the traveler's position to the nearest transit stop or destination.
*/
@Nullable
public static TripInstruction alignTravelerToTransitTrip(TravelerPosition travelerPosition) {
public static TripInstruction alignTravelerToTransitTrip(
TravelerPosition travelerPosition,
boolean isStartOfTrip
) {
Locale locale = travelerPosition.locale;
Leg expectedLeg = travelerPosition.expectedLeg;
String finalStop = expectedLeg.to.name;

if (
tripStartsWithTransitLeg(travelerPosition, isStartOfTrip) &&
sendBusNotification(travelerPosition, isStartOfTrip)
) {
// Regardless of whether the notification is sent or qualifies, provide a 'wait for bus' instruction.
return new WaitForTransitInstruction(expectedLeg, travelerPosition.currentTime, locale);
}

if (isApproachingEndOfLeg(travelerPosition)) {
return new GetOffHereTransitInstruction(finalStop, locale);
}
Expand Down Expand Up @@ -241,10 +255,6 @@ public static boolean isAtEndOfLeg(TravelerPosition travelerPosition) {
return getDistanceToEndOfLeg(travelerPosition) <= TRIP_INSTRUCTION_IMMEDIATE_RADIUS;
}

public static boolean isWithinOperationalNotifyWindow(TravelerPosition travelerPosition) {
return isWithinOperationalNotifyWindow(travelerPosition.currentTime, travelerPosition.nextLeg);
}

/**
* Make sure the traveler is on schedule or ahead of schedule (but not too far) to be within an operational window
* for the bus service.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,13 @@ public TravelerPosition(Leg expectedLeg, Leg nextLeg, Instant currentTime) {
this.currentTime = currentTime;
}

/** Used for unit testing. */
public TravelerPosition(Leg expectedLeg, Leg nextLeg, TrackedJourney trackedJourney) {
this.expectedLeg = expectedLeg;
this.nextLeg = nextLeg;
this.trackedJourney = trackedJourney;
}

/** Computes the current deviation in meters from the expected itinerary. */
public double getDeviationMeters() {
return getDistanceFromLine(legSegmentFromPosition.start, legSegmentFromPosition.end, currentPosition);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package org.opentripplanner.middleware.triptracker.interactions.busnotifiers;

import com.fasterxml.jackson.databind.JsonNode;
import org.opentripplanner.middleware.otp.response.Leg;
import org.opentripplanner.middleware.triptracker.TravelerPosition;
import org.opentripplanner.middleware.triptracker.TripStatus;
import org.opentripplanner.middleware.utils.JsonUtils;
import org.opentripplanner.middleware.utils.YamlUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -48,8 +48,8 @@ public BusOperatorActions(List<AgencyAction> agencyActions) {
/**
* Get the action that matches the given agency id.
*/
public AgencyAction getAgencyAction(TravelerPosition travelerPosition) {
String agencyId = removeAgencyPrefix(getAgencyGtfsIdFromLeg(travelerPosition.nextLeg));
public AgencyAction getAgencyAction(Leg busLeg) {
String agencyId = removeAgencyPrefix(getAgencyGtfsIdFromLeg(busLeg));
if (agencyId != null) {
for (AgencyAction agencyAction : agencyActions) {
if (agencyAction.agencyId.equalsIgnoreCase(agencyId)) {
Expand All @@ -63,12 +63,12 @@ public AgencyAction getAgencyAction(TravelerPosition travelerPosition) {
/**
* Get the correct action for agency and send notification.
*/
public void handleSendNotificationAction(TripStatus tripStatus, TravelerPosition travelerPosition) {
AgencyAction action = getAgencyAction(travelerPosition);
public void handleSendNotificationAction(TravelerPosition travelerPosition, Leg busLeg) {
AgencyAction action = getAgencyAction(busLeg);
if (action != null) {
BusOperatorInteraction interaction = getBusOperatorInteraction(action);
try {
interaction.sendNotification(tripStatus, travelerPosition);
interaction.sendNotification(travelerPosition, busLeg);
} catch (Exception e) {
LOG.error("Could not trigger class {} for agency {}", action.trigger, action.agencyId, e);
throw new RuntimeException(e);
Expand All @@ -81,12 +81,12 @@ public void handleSendNotificationAction(TripStatus tripStatus, TravelerPosition
/**
* Get the correct action for agency and cancel notification.
*/
public void handleCancelNotificationAction(TravelerPosition travelerPosition) {
AgencyAction action = getAgencyAction(travelerPosition);
public void handleCancelNotificationAction(TravelerPosition travelerPosition, Leg busLeg) {
AgencyAction action = getAgencyAction(busLeg);
if (action != null) {
BusOperatorInteraction interaction = getBusOperatorInteraction(action);
try {
interaction.cancelNotification(travelerPosition);
interaction.cancelNotification(travelerPosition, busLeg);
} catch (Exception e) {
LOG.error("Could not trigger class {} for agency {}", action.trigger, action.agencyId, e);
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package org.opentripplanner.middleware.triptracker.interactions.busnotifiers;

import org.opentripplanner.middleware.otp.response.Leg;
import org.opentripplanner.middleware.triptracker.TravelerPosition;
import org.opentripplanner.middleware.triptracker.TripStatus;

public interface BusOperatorInteraction {

void sendNotification(TripStatus tripStatus, TravelerPosition travelerPosition);
void sendNotification(TravelerPosition travelerPosition, Leg busLeg);

void cancelNotification(TravelerPosition travelerPosition);
void cancelNotification(TravelerPosition travelerPosition, Leg busLeg);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.opentripplanner.middleware.models.TrackedJourney;
import org.opentripplanner.middleware.otp.response.Leg;
import org.opentripplanner.middleware.triptracker.TravelerPosition;
import org.opentripplanner.middleware.triptracker.TripStatus;
import org.opentripplanner.middleware.utils.HttpUtils;
import org.opentripplanner.middleware.utils.JsonUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -79,8 +79,8 @@ private static List<String> getBusOperatorNotifierQualifyingRoutes() {
/**
* Stage notification to bus operator by making sure all required conditions are met.
*/
public void sendNotification(TripStatus tripStatus, TravelerPosition travelerPosition) {
var routeId = getRouteGtfsIdFromLeg(travelerPosition.nextLeg);
public void sendNotification(TravelerPosition travelerPosition, Leg busLeg) {
var routeId = getRouteGtfsIdFromLeg(busLeg);
try {
if (
hasNotSentNotificationForRoute(travelerPosition.trackedJourney, routeId) &&
Expand All @@ -98,13 +98,13 @@ public void sendNotification(TripStatus tripStatus, TravelerPosition travelerPos
}

/**
* Cancel a previously sent notification for the next bus leg.
* Cancel a previously sent notification for the expected or next leg.
*/
public void cancelNotification(TravelerPosition travelerPosition) {
var routeId = getRouteGtfsIdFromLeg(travelerPosition.nextLeg);
public void cancelNotification(TravelerPosition travelerPosition, Leg busLeg) {
var routeId = getRouteGtfsIdFromLeg(busLeg);
try {
if (
isBusLeg(travelerPosition.nextLeg) && routeId != null &&
isBusLeg(busLeg) && routeId != null &&
hasNotCanceledNotificationForRoute(travelerPosition.trackedJourney, routeId)
) {
Map<String, String> busNotificationRequests = travelerPosition.trackedJourney.busNotificationMessages;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ private static boolean isAfterServiceStart(ZonedDateTime time) {
/**
* Check whether a new leg of an itinerary matches the previous itinerary leg for the purposes of trip monitoring.
*/
private static boolean legsMatch(Leg referenceItineraryLeg, Leg candidateItineraryLeg) {
public static boolean legsMatch(Leg referenceItineraryLeg, Leg candidateItineraryLeg) {
// for now don't analyze non-transit legs
if (!referenceItineraryLeg.transitLeg) return true;

Expand Down Expand Up @@ -360,4 +360,15 @@ public static String getStopGtfsIdFromPlace(Place place) {
public static String getRouteShortNameFromLeg(Leg leg) {
return (leg != null && leg.route != null) ? leg.route.shortName : null;
}

/**
* Get the first leg in an itinerary.
*/
public static Leg getFirstLeg(Itinerary itinerary) {
if (itinerary != null && itinerary.legs != null && !itinerary.legs.isEmpty()) {
return itinerary.legs.get(0);
}
return null;
}

}
Loading
Loading