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

OTP-1458 Provide traveler with continue instruction #268

Merged

Conversation

br648
Copy link
Contributor

@br648 br648 commented Nov 13, 2024

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

Provide a "continue on street" reassurance instruction when the traveler is on track, but no immediate instruction is available.

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Can you address the wrong street name, and put back literals in the tests, please.


return Stream.of(
Arguments.of(
monitoredTrip,
createPoint(firstStepCoords, 1, NORTH_EAST_BEARING),
"IMMEDIATE: Head WEST on Adair Avenue Northeast",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we'll need the plain text to keep track of the kinds of instructions expected. Or, we can further break down the instructions, this one could be a "directional" instruction for instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be cleaner to further break down the instruction. Also, this way eliminates time zone issues. But I think that is out of scope. Let me know.


@Override
public String build() {
if (isNotEmpty(legStep) && isNotEmpty(legStep.streetName)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For objects, can we use != null and for strings, Strings.isBlank (or what is the rationale for pulling in a new library)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated: 6a1a94a

Locale locale
) {
if (!travelerPosition.expectedLeg.transitLeg && nextStep != null) {
Step currentStep = isPositionPastStep(travelerPosition, nextStep)
Copy link
Collaborator

Choose a reason for hiding this comment

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

At times, this piece produces the incorrect street name. I'll send an example where the behavior is not what you/d expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reworked this. Seems to work ok now with the example you provided.

@br648 br648 assigned binh-dam-ibigroup and unassigned br648 and JymDyerIBI Nov 20, 2024
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

@br648 br648 changed the title Provide traveler with continue instruction OTP-1458 Provide traveler with continue instruction Dec 5, 2024
Copy link
Contributor

@JymDyerIBI JymDyerIBI left a comment

Choose a reason for hiding this comment

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

This looks good.

@binh-dam-ibigroup binh-dam-ibigroup merged commit 6c42bd4 into dev Dec 9, 2024
2 checks passed
@binh-dam-ibigroup binh-dam-ibigroup deleted the feature/OTP-1458-replace-no-instruction-on-walk-leg branch December 9, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants