Navigation events #489

zugaldia · 2017-06-21T21:14:07Z

Expands the telemetry module with new navigation events.

ericrwolfe · 2017-06-26T21:09:59Z

...roid-telemetry/src/main/java/com/mapbox/services/android/telemetry/utils/TelemetryUtils.java

+   */
+  public static int getVolumeLevel(Context context) {
+    AudioManager audioManager = (AudioManager) context.getSystemService(Context.AUDIO_SERVICE);
+    return audioManager.getStreamVolume(AudioManager.STREAM_SYSTEM);


Based on what @danesfeder has mentioned, Android has multiple streams for different types of audio. @danesfeder can you verify that this is the volume of the stream we'll be outputting voice on?

Also, let's make sure to normalize this as an int from 0-100 (it looks like there's a getStreamMaxVolume(int) to normalize against)

@danesfeder can you verify that this is the volume of the stream we'll be outputting voice on?

@danesfeder Anything to keep in mind here?

@ericrwolfe @zugaldia Yeah I'm streaming the voice audio through AudioManager.STREAM_MUSIC and AudioManager.STREAM_NORMAL - we are going to need to ticket it this to find a better way to get the volume.

ericrwolfe · 2017-06-26T21:15:34Z

...emetry/src/main/java/com/mapbox/services/android/telemetry/constants/TelemetryConstants.java

@@ -10,7 +10,8 @@

 public class TelemetryConstants {

-  public static final String OPERATING_SYSTEM = "Android - " + Build.VERSION.RELEASE;
+  public static final String PLATFORM = "Android";
+  public static final String OPERATING_SYSTEM = PLATFORM + " - " + Build.VERSION.RELEASE;


The current iOS Maps SDK omits the - (e.g. iOS 10.3.1); @mick specified the - so @boundsj is iOS 10.3.1 (no dash) an iOS telem bug?

FWIW, @mick also specified the platforms as lowercased without spaces, e.g. (android-x.x.x and ios-x.x.x). We should circle back on this with @mick for consistency.

@ericrwolfe This has been the value the library has been setting all along (this PR doesn't change that). Happy to change the formatting upon @mick confirmation to make sure the backend pipeline doesn't get affected.

we havent been relying on attribute, 👍 for making it consistent with the spec

ericrwolfe · 2017-06-26T21:17:51Z

...ry/src/main/java/com/mapbox/services/android/telemetry/navigation/MapboxNavigationEvent.java

+    event.put(KEY_DURATION_REMAINING, durationRemaining);
+    event.put(KEY_NEW_DISTANCE_REMAINING, newDistanceRemaining);
+    event.put(KEY_NEW_DURATION_REMAINING, newDurationRemaining);
+    event.put(KEY_SECONDS_SINCE_LAST_REROUTE, secondsSinceLastReroute);


newDistanceRemaining, newDurationRemaining, and secondsSinceLastReroute are only required when the feedbackType=="reroute", they should be empty or null otherwise.

ericrwolfe · 2017-07-13T18:28:08Z

...ry/src/main/java/com/mapbox/services/android/telemetry/navigation/MapboxNavigationEvent.java

+
+  // Event keys
+  public static final String KEY_EVENT = "event";
+  public static final String KEY_PLATFORM = "platform";


We've removed platform from the spec since it can be derived from the operating system/SDK identifier.

ericrwolfe · 2017-07-13T18:32:12Z

...ry/src/main/java/com/mapbox/services/android/telemetry/navigation/MapboxNavigationEvent.java

+  public static final String KEY_DISTANCE_COMPLETED = "distanceCompleted";
+  public static final String KEY_DISTANCE_REMAINING = "distanceRemaining";
+  public static final String KEY_DURATION_REMAINING = "durationRemaining";
+  public static final String KEY_SECONDS_SINCE_LAST_REROUTE = "secondsSinceLastReroute";


We've added a few additional keys to look out for in the spec:

General:

simulation

originalRequestIdentifier

requestIdentifier

originalGeometry

originalEstimatedDistance

originalEstimatedDuration

audioOutput

Feedback:

description

userId

feedbackId

screenshot

feedbackType (added more enum values)

step (see below)

Reroute:

feedbackId

newDistanceRemaining

newDurationRemaining

newGeometry

screenshot

step (see below)

Step (Metadata about the current + upcoming step as a JSON object):

upcomingInstruction (string)

upcomingType (string)

upcomingModifier (string)

upcomingName (Name of the street)

previousInstruction (string)

previousType (string)

previousModifier (string)

previousName (Name of the street)

distance (int, meters)

duration (int, seconds)

distanceRemaining (int, meters)

durationRemaining (int, seconds)

Cancel:

arrivalTimestamp

Event version has been increased to 5 along with the above changes.

ericrwolfe · 2017-07-13T18:32:56Z

@zugaldia added a few notes on some of the relevant changes we've made to the spec

zugaldia · 2017-07-14T19:59:34Z

I've made a few changes to address the PR comments but we still need to sync the fields for each event. Like #489 (comment) points out, we need to make sure we're up to date with the latest spec.

zugaldia · 2017-07-27T16:12:25Z

Per chat with @cammace , he's gonna give it a pass to make sure we've included all items from the latest version of the spec. If so, we're good to 🚢 . Next would be mapbox/mapbox-navigation-android#113.

cammace · 2017-08-01T01:28:54Z

…PluggedIn, batteryLevel, and connectivity

…LastReroute for a reroute type

zugaldia · 2017-08-01T13:25:26Z

Should the encoding be done in this library or in the nav sdk? I would prefer the former.

Agreed - let's put this logic on the telemetry library, other clients could benefit from this functionality. Related: mapbox/mapbox-gl-native#8040.

I'm also not completely sure whether the step metadata is being added correctly, @zugaldia could you check this?

Sure thing - could you point me at the code you want me to look at?

zugaldia · 2017-08-01T14:27:43Z

I see it now, thanks (

mapbox-java/mapbox/libandroid-telemetry/src/main/java/com/mapbox/services/android/telemetry/navigation/MapboxNavigationEvent.java

Line 132 in 2145616

    
           event.put(KEY_STEP, getStepMetadata(upcomingInstruction, upcomingType, upcomingModifier, upcomingName,

) - could you add test that checks that all the data gets serialized? Basically, the test will build the JSON object like we do here and then we'd check the content of the resulting string.

cammace · 2017-08-01T19:34:26Z

I've made a few fixes to missing metadata values, this PR should be good to merge now @zugaldia @ericrwolfe

ericrwolfe

Looks like you've added everything @cammace 👍.

Are the locations/keys for locationsBefore and locationsAfter being serialized in the Navigation SDK?

I'll defer to @zugaldia for the final ✅.

cammace · 2017-08-09T01:45:46Z

Bumping this since we need it in order to begin nav metric work.

zugaldia added libandroid-telemetry ⚠️ DO NOT MERGE labels Jun 21, 2017

ericrwolfe reviewed Jun 26, 2017

View reviewed changes

ericrwolfe mentioned this pull request Jun 30, 2017

Integrate metrics mapbox/mapbox-navigation-android#113

Closed

ericrwolfe reviewed Jul 13, 2017

View reviewed changes

danesfeder mentioned this pull request Jul 14, 2017

Find / use correct audio stream for Telem event #510

Closed

zugaldia removed the ⚠️ DO NOT MERGE label Jul 27, 2017

zugaldia added 17 commits July 31, 2017 22:01

enable custom turnstile event and new events

beafa41

separate platform and os constants

3cef4e2

make date formatting a bit more flexible

2ccf558

include static generators for all new nav types

2d05850

move UUID logic to utils

f235364

document buildUUID in the context of navigation

88fa260

add javadoc to events

8d7af05

sdkName is now sdKIdentifier

5149cca

schemaVersion is now eventVersion

11c01c2

sessionUUID is now sessionIdentifier

c8e80b9

add lat and lng keys

e9dd7ca

add a method to compute screen brightness

7ee0c5c

add method to detect volume level

d81fa2a

add keys for volumeLevel, screenBrightness, applicationState, battery…

153c5b3

…PluggedIn, batteryLevel, and connectivity

remove completedDuration

e2b5486

completedDistance is now distanceCompleted

2f6de48

add distanceRemaining and durationRemaining

4d98058

zugaldia and others added 10 commits July 31, 2017 22:01

changes to feedback event

9b3b162

normalize getVolumeLevel

a4a584b

only set newDistanceRemaining, newDurationRemaining, and secondsSince…

6394031

…LastReroute for a reroute type

remove platform key

43cd1ca

add reroute event

1e4a9ff

fix checkstyle

06d19c7

added some missing metadata

b55499e

added step metadata to reroute

f20a258

seperated step key to make seperate json object

4dd4fb0

added step metadata to feedback

2145616

cammace force-pushed the nav-telem branch from ece84aa to 2145616 Compare August 1, 2017 02:04

fixed a few missing metadata

28d798f

ericrwolfe reviewed Aug 1, 2017

View reviewed changes

ericrwolfe approved these changes Aug 11, 2017

View reviewed changes

cammace merged commit 7e1a87b into master Aug 14, 2017

cammace deleted the nav-telem branch August 14, 2017 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Navigation events #489

Navigation events #489

zugaldia commented Jun 21, 2017

ericrwolfe Jun 26, 2017

ericrwolfe Jun 26, 2017

zugaldia Jul 14, 2017

danesfeder Jul 14, 2017

danesfeder Jul 14, 2017

ericrwolfe Jun 26, 2017 •

edited

Loading

ericrwolfe Jun 26, 2017

zugaldia Jul 14, 2017

mick Jul 15, 2017

ericrwolfe Jun 26, 2017

ericrwolfe Jul 13, 2017

ericrwolfe Jul 13, 2017 •

edited

Loading

ericrwolfe commented Jul 13, 2017

zugaldia commented Jul 14, 2017 •

edited

Loading

zugaldia commented Jul 27, 2017

cammace commented Aug 1, 2017 •

edited

Loading

zugaldia commented Aug 1, 2017

zugaldia commented Aug 1, 2017

cammace commented Aug 1, 2017

ericrwolfe left a comment

cammace commented Aug 9, 2017

Navigation events #489

Navigation events #489

Conversation

zugaldia commented Jun 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericrwolfe Jun 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericrwolfe Jul 13, 2017 • edited Loading

Choose a reason for hiding this comment

ericrwolfe commented Jul 13, 2017

zugaldia commented Jul 14, 2017 • edited Loading

zugaldia commented Jul 27, 2017

cammace commented Aug 1, 2017 • edited Loading

zugaldia commented Aug 1, 2017

zugaldia commented Aug 1, 2017

cammace commented Aug 1, 2017

ericrwolfe left a comment

Choose a reason for hiding this comment

cammace commented Aug 9, 2017

ericrwolfe Jun 26, 2017 •

edited

Loading

ericrwolfe Jul 13, 2017 •

edited

Loading

zugaldia commented Jul 14, 2017 •

edited

Loading

cammace commented Aug 1, 2017 •

edited

Loading