Send bus notifications if first leg is bus #264
Conversation
|
@binh-dam-ibigroup the test that is failing also fails under dev. I think it is a time/locale issue. I bet it will work when I re-run it tomorrow morning! Failed test has this comment: |
| * 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) |
There was a problem hiding this comment.
Extract a variable for the leg to pass to isBusLeg and isWithinOperationalNotifyWindow depending on isStartOfTrip, and remove line 180.
| @@ -136,10 +136,7 @@ public static TripInstruction alignTravelerToTrip( | |||
| Locale locale = travelerPosition.locale; | |||
|
|
|||
| if (isApproachingEndOfLeg(travelerPosition)) { | |||
There was a problem hiding this comment.
If this is at the start of a trip where the first leg is transit, there is no approaching the end of the leg.
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
@binh-dam-ibigroup this has grown a bit from my initial take on it! I think this covers the what is needed.
|
@br648 One of the E2E tests is failing and outputting an in incorrect instruction... |
src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java
Outdated
Show resolved
Hide resolved
…eTripTracking.java Co-authored-by: Binh Dam <[email protected]>
@binh-dam-ibigroup The same test is failing under dev and is not specific to this PR. The CI incorrect instruction is different to the one I get locally. Will see if I can fix. |
| BusOperatorActions | ||
| .getDefault() | ||
| .handleSendNotificationAction(tripStatus, travelerPosition); | ||
| if (sendBusNotification(travelerPosition, isStartOfTrip)) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@binh-dam-ibigroup I have updated, I hope it meets expectations.
| if (tripInstruction != null) { | ||
| return tripInstruction.build(); | ||
| } | ||
| } | ||
|
|
||
| if (tripStatus.equals(TripStatus.DEVIATED)) { | ||
| TripInstruction tripInstruction = getBackOnTrack(travelerPosition, isStartOfTrip, tripStatus); | ||
| TripInstruction tripInstruction = getBackOnTrack(travelerPosition, isStartOfTrip); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@binh-dam-ibigroup Update here: eeea569
There was a problem hiding this comment.
@binh-dam-ibigroup I will hold off merging until you have "approved" this addition.
Checklist
devbefore they can be merged tomaster)Description
If the trip starts with a bus leg notify bus op.