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

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,7 @@ public static TripInstruction alignTravelerToTrip(
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 (isBusLeg(travelerPosition.nextLeg) && isWithinOperationalNotifyWindow(travelerPosition)) {
BusOperatorActions
.getDefault()
.handleSendNotificationAction(tripStatus, travelerPosition);
if (sendBusNotification(travelerPosition, isStartOfTrip, tripStatus)) {
// 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 @@ -157,6 +154,32 @@ public static TripInstruction alignTravelerToTrip(
return null;
}

/**
* 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,
TripStatus tripStatus
) {
if (shouldNotifyBusOperator(travelerPosition, isStartOfTrip)) {
BusOperatorActions
.getDefault()
.handleSendNotificationAction(tripStatus, travelerPosition);
return true;
}
return false;
}

/**
* 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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extract a variable for the leg to pass to isBusLeg and isWithinOperationalNotifyWindow depending on isStartOfTrip, and remove line 180.

? isBusLeg(travelerPosition.expectedLeg) && isWithinOperationalNotifyWindow(travelerPosition.currentTime, travelerPosition.expectedLeg)
: isBusLeg(travelerPosition.nextLeg) && isWithinOperationalNotifyWindow(travelerPosition);
}

/**
* Align the traveler's position to the nearest transit stop or destination.
*/
Expand Down Expand Up @@ -218,15 +241,19 @@ 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.
*/
public static boolean isWithinOperationalNotifyWindow(TravelerPosition travelerPosition) {
var busDepartureTime = getBusDepartureTime(travelerPosition.nextLeg);
public static boolean isWithinOperationalNotifyWindow(Instant currentTime, Leg busLeg) {
var busDepartureTime = getBusDepartureTime(busLeg);
return
(travelerPosition.currentTime.equals(busDepartureTime) || travelerPosition.currentTime.isBefore(busDepartureTime)) &&
ACCEPTABLE_AHEAD_OF_SCHEDULE_IN_MINUTES >= getMinutesAheadOfDeparture(travelerPosition.currentTime, busDepartureTime);
(currentTime.equals(busDepartureTime) || currentTime.isBefore(busDepartureTime)) &&
ACCEPTABLE_AHEAD_OF_SCHEDULE_IN_MINUTES >= getMinutesAheadOfDeparture(currentTime, busDepartureTime);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ public TravelerPosition(Leg expectedLeg, Coordinates currentPosition) {

/** Used for unit testing. */
public TravelerPosition(Leg nextLeg, Instant currentTime) {
this (null, nextLeg, currentTime);
}

/** Used for unit testing. */
public TravelerPosition(Leg expectedLeg, Leg nextLeg, Instant currentTime) {
this.expectedLeg = expectedLeg;
this.nextLeg = nextLeg;
this.currentTime = currentTime;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ void canNotifyBusOperatorOnlyOnce() throws InterruptedException, JsonProcessingE

@ParameterizedTest
@MethodSource("createWithinOperationalNotifyWindowTrace")
void isWithinOperationalNotifyWindow(boolean expected, TravelerPosition travelerPosition,String message) {
void isWithinOperationalNotifyWindow(boolean expected, TravelerPosition travelerPosition, String message) {
assertEquals(expected, TravelerLocator.isWithinOperationalNotifyWindow(travelerPosition), message);
}

Expand Down Expand Up @@ -188,6 +188,45 @@ private static Stream<Arguments> createWithinOperationalNotifyWindowTrace() {
);
}

@ParameterizedTest
@MethodSource("createShouldNotifyBusOperatorTrace")
void shouldNotifyBusOperator(boolean expected, TravelerPosition travelerPosition, boolean isStartOfTrip, String message) {
assertEquals(expected, TravelerLocator.shouldNotifyBusOperator(travelerPosition, isStartOfTrip), message);
}

private static Stream<Arguments> createShouldNotifyBusOperatorTrace() {
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),
false,
"Traveler approaching a bus leg, should notify."
),
Arguments.of(
false,
new TravelerPosition(walkLeg, busDepartureTime),
false,
"Traveler approaching a walk leg, should not notify."
),
Arguments.of(
true,
new TravelerPosition(busLeg, null, busDepartureTime),
true,
"Traveler at the start of a trip which starts with a bus leg, should notify."
),
Arguments.of(
false,
new TravelerPosition(walkLeg, null, busDepartureTime),
true,
"Traveler at the start of a trip which starts with a walk leg, should not notify."
)
);
}

private static OtpUser createOtpUser() {
MobilityProfile mobilityProfile = new MobilityProfile();
mobilityProfile.mobilityMode = "WChairE";
Expand Down
Loading