Skip to content

Fix waypoints APIs naming for map matching and directions services #962

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.

Sign up for GitHub

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

Merged
Guardiola31337 merged 1 commit into from
Feb 12, 2019
Merged

Fix waypoints APIs naming for map matching and directions services #962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 10 additions & 8 deletions services-directions/src/main/java/com/mapbox/api/directions/v5/DirectionsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,24 @@ public interface DirectionsService {
* @param steps define if you'd like the route steps
* @param bearings used to filter the road segment the waypoint will be placed on by
* direction and dictates the angle of approach
* @param continueStraight define whether the route should continue straight even if the route
* will be slower
* @param continueStraight define whether the route should continue straight even if the
* route will be slower
* @param annotations an annotations object that contains additional details about each
* line segment along the route geometry. Each entry in an annotations
* field corresponds to a coordinate along the route geometry
* line segment along the route geometry. Each entry in an
* annotations field corresponds to a coordinate along the route
* geometry
* @param language language of returned turn-by-turn text instructions
* @param roundaboutExits Add extra step when roundabouts occur with additional information for
* the user
* @param roundaboutExits Add extra step when roundabouts occur with additional information
* for the user
* @param voiceInstructions request that the response contain voice instruction information,
* useful for navigation
* @param bannerInstructions request that the response contain banner instruction information,
* useful for navigation
* @param voiceUnits voice units
* @param exclude exclude tolls, motorways or more along your route
* @param approaches which side of the road to approach a waypoint
* @param viaWayPoints which input coordinates should be treated as via waypoints
* @param waypointIndices which input coordinates should be treated as waypoints/separate legs.
* Note: coordinate indices not added here act as silent waypoints
* @param waypointNames custom names for waypoints used for the arrival instruction
* @param waypointTargets list of coordinate pairs for drop-off locations
* @return the {@link DirectionsResponse} in a Call wrapper
Expand Down Expand Up @@ -74,7 +76,7 @@ Call<DirectionsResponse> getCall(
@Query("voice_units") String voiceUnits,
@Query("exclude") String exclude,
@Query("approaches") String approaches,
@Query("waypoints") String viaWayPoints,
@Query("waypoints") String waypointIndices,
@Query("waypoint_names") String waypointNames,
@Query("waypoint_targets") String waypointTargets
);
Expand Down
42 changes: 20 additions & 22 deletions services-directions/src/main/java/com/mapbox/api/directions/v5/MapboxDirections.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ protected Call<DirectionsResponse> initializeCall() {
voiceUnits(),
exclude(),
approaches(),
viaWayPoints(),
waypointIndices(),
waypointNames(),
waypointTargets());
}
Expand Down Expand Up @@ -235,6 +235,7 @@ private List<DirectionsRoute> generateRouteOptions(Response<DirectionsResponse>
RouteOptions.builder()
.profile(profile())
.coordinates(coordinates())
.waypointIndices(waypointIndices())
.waypointNames(waypointNames())
.waypointTargets(waypointTargets())
.continueStraight(continueStraight())
Expand Down Expand Up @@ -362,7 +363,7 @@ private static String formatWaypointTargets(Point[] waypointTargets) {
abstract String approaches();

@Nullable
abstract String viaWayPoints();
abstract String waypointIndices();

@Nullable
abstract String waypointNames();
Expand Down Expand Up @@ -424,7 +425,7 @@ public abstract static class Builder {
private Point destination;
private Point origin;
private String[] approaches;
private Integer[] viaWayPoints;
private Integer[] waypointIndices;
private String[] waypointNames;
private Point[] waypointTargets;

Expand Down Expand Up @@ -794,24 +795,25 @@ public Builder addApproaches(String... approaches) {
abstract Builder approaches(@Nullable String approaches);

/**
* Optionally, set which input coordinates should be treated as via way points.
* Optionally, set which input coordinates should be treated as waypoints / separate legs.
* Note: coordinate indices not added here act as silent waypoints
* <p>
* Most useful in combination with steps=true and requests based on traces
Copy link
Contributor

@danesfeder danesfeder Feb 12, 2019

Choose a reason for hiding this comment

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

Does traces make sense here? Reading this javadoc, I'm still not entirely sure what separateLegs means.

set which input coordinates should be treated as separate legs.

By adding coordinates to a Directions API request, you are creating new legs. I think it may make sense here to say that separateLegs is directly influencing the guidance behavior when set.

Copy link
Contributor Author

@Guardiola31337 Guardiola31337 Feb 12, 2019

Choose a reason for hiding this comment

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

#962 (comment)

* with high sample rates. Can be an index corresponding to any of the input coordinates,
* but must contain the first ( 0 ) and last coordinates' index separated by ; .
* {@link #steps()}
* </p>
*
* @param viaWayPoints integer array of coordinate indices to be used as via way points
* @param waypointIndices integer array of coordinate indices to be used as waypoints
* @return this builder for chaining options together
* @since 4.4.0
*/
public Builder addViaWayPoints(@Nullable @IntRange(from = 0) Integer... viaWayPoints) {
this.viaWayPoints = viaWayPoints;
public Builder addWaypointIndices(@Nullable @IntRange(from = 0) Integer... waypointIndices) {
this.waypointIndices = waypointIndices;
return this;
}

abstract Builder viaWayPoints(@Nullable String viaWayPoints);
abstract Builder waypointIndices(@Nullable String waypointIndices);

/**
* Custom names for waypoints used for the arrival instruction,
Expand Down Expand Up @@ -871,30 +873,26 @@ public MapboxDirections build() {
+ " directions API request.");
}

if (viaWayPoints != null) {
if (viaWayPoints.length < 2) {
if (waypointIndices != null) {
if (waypointIndices.length < 2) {
throw new ServicesException(
"Via way points must be a list of at least two indexes separated by ';'");
"Waypoints must be a list of at least two indexes separated by ';'");
}
if (viaWayPoints[0] != 0
|| viaWayPoints[viaWayPoints.length - 1] != coordinates.size() - 1) {
if (waypointIndices[0] != 0 || waypointIndices[waypointIndices.length - 1]
!= coordinates.size() - 1) {
throw new ServicesException(
"Via way points must contain indices of the first and last coordinates"
"Waypoints must contain indices of the first and last coordinates"
);
}
for (int i = 1; i < viaWayPoints.length - 1; i++) {
if (viaWayPoints[i] < 0 || viaWayPoints[i] >= coordinates.size()) {
for (int i = 1; i < waypointIndices.length - 1; i++) {
if (waypointIndices[i] < 0 || waypointIndices[i] >= coordinates.size()) {
throw new ServicesException(
"Via way points index too large (no corresponding coordinate)");
"Waypoints index too large (no corresponding coordinate)");
}
}
}

if (waypointNames != null) {
if (waypointNames.length != coordinates.size()) {
throw new ServicesException("Number of waypoint names must match "
+ " the number of waypoints provided.");
}
final String waypointNamesStr = TextUtils.formatWaypointNames(waypointNames);
waypointNames(waypointNamesStr);
}
Expand Down Expand Up @@ -924,7 +922,7 @@ public MapboxDirections build() {
bearing(TextUtils.formatBearing(bearings));
annotation(TextUtils.join(",", annotations));
radius(TextUtils.formatRadiuses(radiuses));
viaWayPoints(TextUtils.join(";", viaWayPoints));
waypointIndices(TextUtils.join(";", waypointIndices));

MapboxDirections directions = autoBuild();

Expand Down
12 changes: 6 additions & 6 deletions services-directions/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,20 +268,20 @@ public static Builder builder() {
public abstract String approaches();

/**
* Indicates which input coordinates should be treated as via way points.
* Indicates which input coordinates should be treated as waypoints.
* <p>
* Most useful in combination with steps=true and requests based on traces
* with high sample rates. Can be an index corresponding to any of the input coordinates,
* but must contain the first ( 0 ) and last coordinates' index separated by ; .
* {@link #steps()}
* </p>
*
* @return a string representing indices to be used as via way points
* @return a string representing indices to be used as waypoints
* @since 4.4.0
*/
@SerializedName("waypoints")
@Nullable
public abstract String viaWayPoints();
public abstract String waypointIndices();

/**
* Custom names for waypoints used for the arrival instruction in banners and voice instructions,
Expand Down Expand Up @@ -558,15 +558,15 @@ public abstract Builder overview(
public abstract Builder approaches(String approaches);

/**
* The same via way points the user originally made when the request was made.
* The same waypoint indices the user originally made when the request was made.
*
* @param indices to be used as via way points
* @param indices to be used as waypoints
* @return this builder for chaining options together
* @since 4.4.0
*/

@Nullable
public abstract Builder viaWayPoints(@Nullable String indices);
public abstract Builder waypointIndices(@Nullable String indices);

/**
* The same waypoint names the user originally made when the request was made.
Expand Down
28 changes: 6 additions & 22 deletions services-directions/src/test/java/com/mapbox/api/directions/v5/MapboxDirectionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,12 @@ public void addWaypoint_doesGetFormattedInUrlCorrectly() throws Exception {
}

@Test
public void waypoints_doesGetFormattedInUrlCorrectly() throws Exception {
public void waypoints_doesGetFormattedInUrlCorrectly() {
MapboxDirections directions = MapboxDirections.builder()
.destination(Point.fromLngLat(13.4930, 9.958))
.addWaypoint(Point.fromLngLat(4.56, 7.89))
.origin(Point.fromLngLat(1.234, 2.345))
.addViaWayPoints(0, 2)
.addWaypointIndices(0, 2)
.accessToken(ACCESS_TOKEN)
.build();
String semicolon = "%3B";
Expand Down Expand Up @@ -673,7 +673,7 @@ public void build_exceptionThrownWhenLessThanTwoWaypointsProvided() {
MapboxDirections.builder()
.origin(Point.fromLngLat(2.0, 2.0))
.destination(Point.fromLngLat(4.0, 4.0))
.addViaWayPoints(0)
.addWaypointIndices(0)
.baseUrl("https://foobar.com")
.accessToken(ACCESS_TOKEN)
.build();
Expand All @@ -685,7 +685,7 @@ public void build_exceptionThrownWhenWaypointsDoNotStartWith0() {
.origin(Point.fromLngLat(2.0, 2.0))
.addWaypoint(Point.fromLngLat(3.0, 3.0))
.destination(Point.fromLngLat(4.0, 4.0))
.addViaWayPoints(1, 2)
.addWaypointIndices(1, 2)
.baseUrl("https://foobar.com")
.accessToken(ACCESS_TOKEN)
.build();
Expand All @@ -697,7 +697,7 @@ public void build_exceptionThrownWhenWaypointDoNotEndWithLast() {
.origin(Point.fromLngLat(2.0, 2.0))
.addWaypoint(Point.fromLngLat(3.0, 3.0))
.destination(Point.fromLngLat(4.0, 4.0))
.addViaWayPoints(0, 1)
.addWaypointIndices(0, 1)
.baseUrl("https://foobar.com")
.accessToken(ACCESS_TOKEN)
.build();
Expand All @@ -709,7 +709,7 @@ public void build_exceptionThrownWhenMiddleWaypointsAreWrong() {
.origin(Point.fromLngLat(2.0, 2.0))
.addWaypoint(Point.fromLngLat(3.0, 3.0))
.destination(Point.fromLngLat(4.0, 4.0))
.addViaWayPoints(0, 3, 2)
.addWaypointIndices(0, 3, 2)
.baseUrl("https://foobar.com")
.accessToken(ACCESS_TOKEN)
.build();
Expand All @@ -730,22 +730,6 @@ public void sanityWaypointNamesInstructions() throws Exception {
mapboxDirections.cloneCall().request().url().queryParameter("waypoint_names"));
}

@Test
public void build_exceptionThrownWhenWaypointNamesDoNotMatchCoordinates() throws Exception {
thrown.expect(ServicesException.class);
thrown.expectMessage(
startsWith("Number of waypoint names must match"));
MapboxDirections mapboxDirections = MapboxDirections.builder()
.origin(Point.fromLngLat(2.0, 2.0))
.addWaypoint(Point.fromLngLat(2.0, 2.0))
.addWaypoint(Point.fromLngLat(3.0, 3.0))
.destination(Point.fromLngLat(4.0, 4.0))
.addWaypointNames("Home", "Work")
.baseUrl("https://foobar.com")
.accessToken(ACCESS_TOKEN)
.build();
}

@Test
public void testWithWaypointNames() throws Exception {

Expand Down
14 changes: 8 additions & 6 deletions services-matching/src/main/java/com/mapbox/api/matching/v5/MapMatchingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ public interface MapMatchingService {
* @param voiceInstructions whether or not to return
* marked-up text for voice guidance along the route.
* @param voiceUnits voice units
* @param waypoints Which input coordinates should be treated as waypoints.
* @param waypointNames wustom names for waypoints used for the arrival instruction.
* @param waypointIndices which input coordinates should be treated as waypoints/separate legs.
* Note: coordinate indices not added here act as silent waypoints
* @param waypointNames custom names for waypoints used for the arrival instruction.
* @param approaches which side of the road to approach a waypoint.
* @return the MapMatchingResponse in a Call wrapper
* @since 2.0.0
Expand All @@ -84,7 +85,7 @@ Call<MapMatchingResponse> getCall(
@Query("banner_instructions") Boolean bannerInstructions,
@Query("voice_instructions") Boolean voiceInstructions,
@Query("voice_units") String voiceUnits,
@Query("waypoints") String waypoints,
@Query("waypoints") String waypointIndices,
@Query("waypoint_names") String waypointNames,
@Query("approaches") String approaches);

Expand Down Expand Up @@ -130,8 +131,9 @@ Call<MapMatchingResponse> getCall(
* @param voiceInstructions whether or not to return
* marked-up text for voice guidance along the route.
* @param voiceUnits voice units
* @param waypoints Which input coordinates should be treated as waypoints.
* @param waypointNames wustom names for waypoints used for the arrival instruction.
* @param waypointIndices which input coordinates should be treated as waypoints/separate legs.
* Note: coordinate indices not added here act as silent waypoints
* @param waypointNames custom names for waypoints used for the arrival instruction.
* @param approaches which side of the road to approach a waypoint.
* @return the MapMatchingResponse in a Call wrapper
* @since 4.4.0
Expand All @@ -156,7 +158,7 @@ Call<MapMatchingResponse> postCall(
@Field("banner_instructions") Boolean bannerInstructions,
@Field("voice_instructions") Boolean voiceInstructions,
@Field("voice_units") String voiceUnits,
@Field("waypoints") String waypoints,
@Field("waypoints") String waypointIndices,
@Field("waypoint_names") String waypointNames,
@Field("approaches") String approaches);
}
Loading