Update Maps SDK with MAS 3.0 #697

danesfeder · 2018-02-12T22:02:35Z

Closes #891 Closes #880 Closes #837 Closes #746

Currently not compiling due to a dex error with something related to offline

cc @osana @cammace (for 👀 on the NavigationMapRoute code)

danesfeder · 2018-02-12T22:04:41Z

@tobrun any ideas here?

Error:com.android.builder.dexing.DexArchiveBuilderException: Failed to process /Users/danesfeder/.gradle/caches/transforms-1/files-1.1/mapbox-android-sdk-6.0.0-SNAPSHOT.aar/0db8873ff2b10dc56820a1208701e197/jars/classes.jar

Error:com.android.builder.dexing.DexArchiveBuilderException: Error while dexing com/mapbox/mapboxsdk/offline/OfflineRegion$2.class

Error:com.android.dx.cf.code.SimException: invalid opcode ba (invokedynamic requires --min-sdk-version >= 26)```

danesfeder · 2018-02-12T22:43:46Z

Found the issue:

  compileOptions {
    sourceCompatibility JavaVersion.VERSION_1_8
    targetCompatibility JavaVersion.VERSION_1_8
  }

Needed to be added to the test app.

tobrun · 2018-03-13T07:10:21Z

...-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

+        PropertyFactory.iconImage(match(
+          get("waypoint"),
+          literal("originMarker"), literal("stop"), literal("originMarker"),
+          literal("destination"), literal("destinationMarker")


This conversion doesn't seem correct:

match(
get("waypoint"),
literal("origin"), literal("originMarker"),
literal("destination"), literal("destinationMarker"),
literal("defaultOutputIfAboveDoesntMatch")
)

danesfeder

Looks good so far - after talking with @Guardiola31337, we need to keep this PR up to date with master, while we wait for the libraries that libandroid-navigation-ui depend on to be released. Currently, this PR is using specific SNAPSHOTS in order to build correctly. Biggest piece left here, imo, is removing all of the telem hardcoded values with the proper methods from the telem library.

danesfeder · 2018-03-13T18:10:05Z

app/src/main/java/com/mapbox/services/android/navigation/testapp/MainActivity.java

 public class MainActivity extends AppCompatActivity implements PermissionsListener {

+  // TODO Check and remove if not necessary
  private static final String[] PERMISSIONS = {Manifest.permission.ACCESS_FINE_LOCATION,


Confirmed: can be removed 👍

danesfeder · 2018-03-13T18:20:10Z

...ain/java/com/mapbox/services/android/navigation/testapp/activity/MockNavigationActivity.java

      mapboxMap.moveCamera(CameraUpdateFactory.newLatLngZoom(latLng, 12));
-      mapboxMap.setMyLocationEnabled(true);
-      mapboxMap.getTrackingSettings().setMyLocationTrackingMode(MyLocationTracking.TRACKING_FOLLOW);
+      // TODO Check and remove if not necessary


Confirmed deprecated code: can be removed 👍

danesfeder · 2018-03-13T18:22:47Z

...ain/java/com/mapbox/services/android/navigation/testapp/activity/MockNavigationActivity.java

  private void calculateRoute() {
-    Location userLocation = mapboxMap.getMyLocation();
+    // TODO Check
+    Location userLocation = locationEngine.getLastLocation();


Confirmed this is good to replace 👍

danesfeder · 2018-03-13T18:27:11Z

...ain/java/com/mapbox/services/android/navigation/v5/navigation/NavigationEventDispatcher.java

    navigationEventListeners = new ArrayList<>();
    milestoneEventListeners = new ArrayList<>();
-    progressChangeListeners = new ArrayList<>();
+    progressChangeListeners = new CopyOnWriteArrayList<>();


Let's do the same for the rest of the lists, great catch here 💯

danesfeder · 2018-03-13T18:38:52Z

...roid-navigation/src/main/java/com/mapbox/services/android/navigation/v5/utils/MathUtils.java

+package com.mapbox.services.android.navigation.v5.utils;
+
+
+// TODO Check and remove if not necessary


If we are using these methods in the nav SDK and they are no longer provided by the libraries we are using, let's add them to our utils

danesfeder · 2018-03-13T22:29:24Z

@Guardiola31337 taking a look at the map route changes and found that the start/end points are missing now in this branch:

tobrun · 2018-03-13T23:26:26Z

...-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

-            stop("destination", PropertyFactory.iconImage("destinationMarker"))
+        PropertyFactory.iconImage(match(
+          get("waypoint"),
+          literal("originMarker"), literal("originMarker"),


the first originMarker should be "origin"

tobrun · 2018-03-13T23:27:11Z

...-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

+          get("waypoint"),
+          literal("originMarker"), literal("originMarker"),
+          literal("destination"), literal("destinationMarker"),
+          literal("defaultOutputIfAboveDoesntMatch")


it's better to actually assign a real icon id value for above, eg. originMarker

Guardiola31337 · 2018-04-11T17:58:01Z

While testing the integration noticed that routes are not drawn properly 👇

This is the MockNavigationActivity from the test app (tapping the map twice). Note that black color in the route after placing the second waypoint.

Comparing with master (Maps SDK 5.5.0 at the moment of writing) 👇

it seems that we're messing up with Expressions related code. Going to dig deeper. BTW, it'd be great @tobrun if you could take a look at it as well and let us know if you 👀 something missing or wrong. Thanks 🙏

tobrun · 2018-04-12T06:26:17Z

it seems that we're messing up with Expressions related code. Going to dig deeper. BTW, it'd be great @tobrun if you could take a look at it as well and let us know if you 👀 something missing or wrong. Thanks

A quick glance over the code found here didn't flag anything to me.
Could you specify/link which layer configuration is not working correctly?

Guardiola31337 · 2018-04-12T14:22:31Z

@tobrun

Could you specify/link which layer configuration is not working correctly?

We're adding the route layer to the map either using the custom style values or the default in NavigationMapRoute#addRouteLayer and the code in which we "paint" the traffic colors is

mapbox-navigation-android/libandroid-navigation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/route/NavigationMapRoute.java

Lines 423 to 429 in 55139e0

    
           PropertyFactory.lineColor(match( 
        
             get(CONGESTION_KEY), 
        
             literal("moderate"), color(index == primaryRouteIndex ? routeModerateColor : alternativeRouteModerateColor), 
        
             literal("heavy"), color(index == primaryRouteIndex ? routeSevereColor : alternativeRouteSevereColor), 
        
             literal("severe"), color(index == primaryRouteIndex ? routeSevereColor : alternativeRouteSevereColor), 
        
             color(index == primaryRouteIndex ? routeDefaultColor : alternativeRouteDefaultColor)) 
        
           )

We don't draw black on the route line at any time. Leading me to believe this is an issue with the Maps SDK or how we are using it.

Route using Maps SDK 5.5.0 vs 6.0.0 👀

Any hints? Couldn't find anything wrong either 😞

Guardiola31337 · 2018-04-16T10:56:14Z

Noting here that when testing mapbox/mapbox-plugins-android#424 (comment) noticed some building conflicts 👀

* What went wrong:
Execution failed for task ':app:preDebugBuild'.
> Android dependency 'android.arch.lifecycle:runtime' has different version for the compile (1.0.3) and runtime (1.1.1) classpath. You should manually set the same version via DependencyResolution

We aren't using 1.1.1 (LLP included it in mapbox/mapbox-plugins-android#419) so in order to avoid them we needed to include implementation "android.arch.lifecycle:extensions:1.1.1" in the app's build.gradle file. We should fix it in the Nav side though (bumping the version). Ticket to follow.

tobrun · 2018-04-16T11:38:14Z

Note that with final release of LLP this shouldn't be an issue anymore, with mapbox/mapbox-plugins-android#419 we limited the scope of the included dependency.

Guardiola31337 · 2018-04-16T13:06:24Z

@tobrun

Note that with final release of LLP this shouldn't be an issue anymore, with mapbox/mapbox-plugins-android#419 we limited the scope of the included dependency.

What do you mean with we limited the scope of the included dependency?

I think this will be an issue in the Nav side (when including LLP final release dependency) until #864 gets addressed.

danesfeder · 2018-04-16T15:51:46Z

@Guardiola31337 I'd look at NavigationMapRoute#generateFeatureCollectionList. I'm wondering if the issue with the route line has to do with how we generate the features a second time, or with waypoints.

Guardiola31337 · 2018-04-28T00:11:01Z

Green 👉 New implementation
Yellow 👉 Old implementation

In terms of numbers we're getting 99 events with the old implementation whereas 95 events with the new implementation. Above the green points can't be seen because they're right under the yellow ones (i.e. same exact location). As you can see below the differences only happened at the beginning right before starting the navigation session.

Here we're only 🕵️ the location events, it'd be great to do the same with the Nav-related events.

BTW, based on what we're 👀 with the location events seems legit to go ahead and merge the PR. We can revisit/continue analyzing the navigation events later.

cc @danesfeder @zugaldia

danesfeder added the navigation-ui label Feb 12, 2018

danesfeder self-assigned this Feb 12, 2018

danesfeder requested review from Guardiola31337, cammace and devotaaabel February 12, 2018 22:02

danesfeder mentioned this pull request Feb 16, 2018

bumped maps sdk to 6.0.0-beta.2 #695

Closed

brsbl-zz assigned Guardiola31337 and unassigned danesfeder Feb 27, 2018

zugaldia mentioned this pull request Mar 8, 2018

Usage of mapboxServices vs mapboxSdkServices #746

Closed

Guardiola31337 force-pushed the dan-maps-3.0 branch 3 times, most recently from 5cae2e8 to 5b0e5ed Compare March 12, 2018 15:32

tobrun approved these changes Mar 12, 2018

View reviewed changes

tobrun reviewed Mar 13, 2018

View reviewed changes

Guardiola31337 force-pushed the dan-maps-3.0 branch from 29de7cc to c26a82c Compare March 13, 2018 10:24

danesfeder commented Mar 13, 2018

View reviewed changes

danesfeder mentioned this pull request Mar 13, 2018

Outdated location layer plugin code in MockNavigationActivity #673

Closed

Guardiola31337 force-pushed the dan-maps-3.0 branch 2 times, most recently from a656430 to 282bd1a Compare March 13, 2018 19:26

tobrun reviewed Mar 13, 2018

View reviewed changes

Guardiola31337 mentioned this pull request Mar 21, 2018

LocationEngine: services.android.telemetry vs core.location #790

Closed

Guardiola31337 force-pushed the dan-maps-3.0 branch 2 times, most recently from 61f8251 to 2572841 Compare March 29, 2018 16:07

danesfeder mentioned this pull request Apr 2, 2018

LocationLayer 0.5.0 snapshot with navigation issue mapbox/mapbox-plugins-android#396

Closed

Guardiola31337 force-pushed the dan-maps-3.0 branch 3 times, most recently from 477baaa to 882c4ca Compare April 6, 2018 18:32

Guardiola31337 force-pushed the dan-maps-3.0 branch from c1ca5f2 to 55139e0 Compare April 11, 2018 17:44

Guardiola31337 force-pushed the dan-maps-3.0 branch from 55139e0 to 334f60a Compare April 12, 2018 13:21

Guardiola31337 force-pushed the dan-maps-3.0 branch from 334f60a to a19952e Compare April 12, 2018 16:18

Guardiola31337 mentioned this pull request Apr 16, 2018

Bump Architecture Components version to avoid building conflict issues #864

Closed

Guardiola31337 force-pushed the dan-maps-3.0 branch from a19952e to b6ca9ff Compare April 16, 2018 16:50

danesfeder modified the milestones: v0.13.0, 0.14.0 Apr 18, 2018

Guardiola31337 force-pushed the dan-maps-3.0 branch from b6ca9ff to baecdd1 Compare April 23, 2018 09:17

LukasPaczos mentioned this pull request Apr 24, 2018

Mapbox zoom LatLngBound sometimes wrong #880

Closed

Guardiola31337 force-pushed the dan-maps-3.0 branch from baecdd1 to 0b2d256 Compare April 27, 2018 03:01

Guardiola31337 force-pushed the master branch from 96cd589 to 279c7b2 Compare April 27, 2018 18:34

danesfeder added 3 commits April 27, 2018 13:28

Update Maps SDK with MAS 3.0

484264e

Add java 8 to test app

8b1d172

Update Maps SDK with MAS 3.0

ff18a94

Guardiola31337 force-pushed the dan-maps-3.0 branch from 17242ce to 418b401 Compare April 27, 2018 20:28

bump dependencies to the latest versions and adapt code to them

251ea82

Guardiola31337 force-pushed the dan-maps-3.0 branch from 418b401 to 251ea82 Compare April 28, 2018 00:15

Guardiola31337 merged commit d339f62 into master Apr 28, 2018

Guardiola31337 deleted the dan-maps-3.0 branch April 28, 2018 00:34

Guardiola31337 mentioned this pull request Apr 30, 2018

Crash due to class naming collision in 0.12.0 #897

Closed

Guardiola31337 mentioned this pull request Sep 3, 2018

Runtime crashing on Map #1254

Closed

Guardiola31337 mentioned this pull request Sep 18, 2018

Add debug logging support back #1298

Merged

		package com.mapbox.services.android.navigation.v5.utils;


		// TODO Check and remove if not necessary

Update Maps SDK with MAS 3.0 #697

Update Maps SDK with MAS 3.0 #697

Uh oh!

Conversation

danesfeder commented Feb 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danesfeder commented Feb 12, 2018

Uh oh!

danesfeder commented Feb 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danesfeder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danesfeder commented Mar 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Guardiola31337 commented Apr 11, 2018

Uh oh!

tobrun commented Apr 12, 2018

Uh oh!

Guardiola31337 commented Apr 12, 2018

Uh oh!

Guardiola31337 commented Apr 16, 2018

Uh oh!

tobrun commented Apr 16, 2018

Uh oh!

Guardiola31337 commented Apr 16, 2018

Uh oh!

danesfeder commented Apr 16, 2018

Uh oh!

Guardiola31337 commented Apr 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danesfeder commented Feb 12, 2018 •

edited

Loading