MapboxNavigationOptions object added #22
Conversation
zugaldia
left a comment
zugaldia
left a comment
There was a problem hiding this comment.
Let's also add tests for the new class.
| public MapboxNavigationOptions setMaxTurnCompletionOffset( | ||
| @FloatRange(from = 0, to = 359) double maxTurnCompletionOffset) { | ||
| this.maxTurnCompletionOffset = maxTurnCompletionOffset; | ||
| return this; |
There was a problem hiding this comment.
This follows the builder pattern, but there's no Builder.
There was a problem hiding this comment.
discussed internally, since no logic is happening inside MapboxNavigationOptions it is a builder in itself and has no need for the same builder pattern inside Mapbox Java.
|
|
||
| private double maximumDistanceOffRoute = NavigationConstants.MAXIMUM_DISTANCE_BEFORE_OFF_ROUTE; | ||
| private double deadReckoningTimeInterval = NavigationConstants.DEAD_RECKONING_TIME_INTERVAL; | ||
| private double maxManipulatedCourseAngle = NavigationConstants.MAX_MANIPULATED_COURSE_ANGLE; |
There was a problem hiding this comment.
How about we do this in the constructor.
cammace
added
ready for review
and removed
⚠️ DO NOT MERGE
|
All finished up here @zugaldia, can I get another review? :)
^ reason why i'm not doing this now is because of mapbox/mapbox-java#450 |
| @Experimental public static final int HIGH_ALERT_LEVEL = 4; | ||
| @Experimental public static final int ARRIVE_ALERT_LEVEL = 5; | ||
| @Experimental public static final int METERS_TO_INTERSECTION = 50; | ||
| @Experimental public static final int DEFAULT_ANGLE_TOLERANCE = 45; |
There was a problem hiding this comment.
Why are these two constants being removed?
There was a problem hiding this comment.
They were part of previous logic I implemented that I removed in favor of what iOS has, these constants just never got removed and noticed while I was working on this PR.
| isBound = false; | ||
| navigationService = null; | ||
| snapToRoute = true; | ||
| profile = DirectionsCriteria.PROFILE_DRIVING_TRAFFIC; |
There was a problem hiding this comment.
Why isn't the current profile an option too? Because options can't change once you instantiated MapboxNavigation but profile can?
There was a problem hiding this comment.
Yeah, eventually we will support the usage of different profiles which means until the user actual starts the navigation session, they should have the option to change the profile (they'll also need to get a new DirectionsResponse if the profiles changed).
|
@cammace I think we're almost there. Just a couple of questions. |
2 participants
Closes #18
android-docsto document this.