Skip to content

Conversation

@danesfeder
Copy link
Contributor

@danesfeder danesfeder commented Nov 5, 2018

Closes #1501 @navdeepg to confirm

This is an example of a last step from the Directions API (containing the maneuver type "arrive"):

       {
        "intersections": [{
          "in": 0,
          "entry": [true],
          "bearings": [252],
          "location": [-71.068719, 42.352396]
        }],
        "driving_side": "right",
        "geometry": "wo~woA|aupfC",
        "mode": "driving",
        "maneuver": {
          "bearing_after": 0,
          "bearing_before": 72,
          "location": [-71.068719, 42.352396],
          "type": "arrive",
          "instruction": "You have arrived at your destination"
        },
        "ref": "MA 2",
        "weight": 0,
        "duration": 0,
        "name": "Boylston Street (MA 2)",
        "distance": 0,
        "voiceInstructions": [],
        "bannerInstructions": []
      }

As you can see, the step has a single point that represents the geometry. Navigator ignores the step, making it impossible for a user to be "on the step" and have maneuver type == "arrive".

This PR adds a RouteProgressState that not only gives information about when a user has arrived, but also a lot of other information given to us via the Navigator RouteState. Even if we did allow navigation on this step, it would be very hard to get a location update that was snapped to this geometry.

Example usage to detect when a user has arrived at the end of the given RouteLeg:

  @Override
  public void onProgressChange(Location location, RouteProgress routeProgress) {
    Integer currentState = routeProgress.currentState();
    if (currentState != null && currentState == RouteProgressState.ROUTE_ARRIVED) {
      // Arrived at the end of the leg!
    }
  }

This is marked SEMVER because it breaks RouteUtils.

cc @kevinkreiser

@navdeepg
Copy link

navdeepg commented Nov 5, 2018

Hey Dan,
This is exactly what i needed. Let me know when you guys are going to merge this, so that i can start testing this.

Thanks,
Navdeep

// Class should not be initialized
}

@IntDef({ROUTE_INVALID, ROUTE_INITIALIZED, ROUTE_ARRIVED, LOCATION_TRACKING, LOCATION_STALE})
Copy link
Contributor

Choose a reason for hiding this comment

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

should we have OFF ROUTE as well? technically if you dont do a re-route you will stay off route until you get back on

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that's not needed because of

put(RouteState.OFFROUTE, null); // Ignore off-route (info already provided via listener)
but I'll defer to @danesfeder to confirm

@navdeepg
Copy link

navdeepg commented Nov 6, 2018

currentState() in RoutrProgress.java is not available in current version 0.22.0 of navigation sdk. Can you please let me know when you are going to publish it?

@danesfeder danesfeder force-pushed the dan-arrival-callback branch from 6922783 to 7caaa06 Compare November 7, 2018 14:53
@danesfeder danesfeder force-pushed the dan-arrival-callback branch from 7caaa06 to 854a053 Compare November 7, 2018 14:57
@danesfeder
Copy link
Contributor Author

@Guardiola31337 I converted RouteProgressState to an enum and this is ready for another 👀

Copy link
Contributor

@Guardiola31337 Guardiola31337 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 addressing the feedback @danesfeder 🙇

🚢

@danesfeder danesfeder merged commit b562551 into master Nov 7, 2018
@danesfeder danesfeder deleted the dan-arrival-callback branch November 7, 2018 15:15
@danesfeder
Copy link
Contributor Author

@navdeepg now that this is merged to master, you can test with 0.23.0-SNAPSHOT see here. Stay tuned for the 0.23.0 release incoming soon and thanks for your patience.

@danesfeder danesfeder mentioned this pull request Nov 7, 2018
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards incompatible Requires a SEMVER major version change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants