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

Adds support for polyline6 #287

Merged
merged 3 commits into from
Feb 9, 2017
Merged

Adds support for polyline6 #287

merged 3 commits into from
Feb 9, 2017

Conversation

cammace
Copy link
Contributor

@cammace cammace commented Jan 24, 2017

Closes #207
Closes #288

Bring over the work done in #228 and completes it.

@mention-bot
Copy link

@cammace, thanks for your PR! By analyzing the history of the files in this pull request, we identified @zugaldia and @ahuang13 to be potential reviewers.

@cammace cammace added this to the v2.0.0 milestone Jan 24, 2017
@cammace cammace changed the title Route Utils refactor Route Utils refactor and adds support for polyline6 Jan 25, 2017
@zugaldia
Copy link
Member

@cammace Should we close #228 then?

@cammace cammace changed the title Route Utils refactor and adds support for polyline6 Adds support for polyline6 Feb 8, 2017
@cammace
Copy link
Contributor Author

cammace commented Feb 8, 2017

I've fixed up this PR to only add support for precision 6 in the directions api. The route utils refactor will come laster down the road but I don't expect it to break the API.

Copy link
Member

@zugaldia zugaldia left a comment

Choose a reason for hiding this comment

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

A couple of small changes.

@@ -26,6 +26,7 @@
public static final int GOOGLE_PRECISION = 5;
public static final int OSRM_PRECISION_V4 = 6;
public static final int OSRM_PRECISION_V5 = 5;
public static final int OSRM_PRECISION_6_V5 = 6;
Copy link
Member

Choose a reason for hiding this comment

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

@cammace Let's not do this, let's use PRECISION_6 from #312 instead.

// We only support polyline encoded geometries to reduce the size of the response.
// If we need the corresponding LineString object, this SDK can do the decoding with
// LineString.fromPolyline(String polyline, int precision).
// by defauly the geometry is polyline with precision 5.
this.geometries = DirectionsCriteria.GEOMETRY_POLYLINE;
Copy link
Member

Choose a reason for hiding this comment

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

@cammace Let's default to precision 6. That's what I do on MapMatching and the payload isn't much bigger but there's a gain in precision.

@cammace cammace merged commit 083b5c0 into master Feb 9, 2017
@cammace cammace deleted the cam-routeutils-refactor branch February 9, 2017 14:24
@zugaldia zugaldia mentioned this pull request Feb 10, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Feb 22, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Mar 9, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Mar 17, 2017
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants