Update RouteRetrievalEvent #1731
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1731 +/- ##
============================================
- Coverage 30.26% 29.84% -0.42%
Complexity 860 860
============================================
Files 225 226 +1
Lines 8212 8233 +21
Branches 636 636
============================================
- Hits 2485 2457 -28
- Misses 5493 5542 +49
Partials 234 234 |
|
This fixes #1732. |
Guardiola31337
left a comment
Guardiola31337
left a comment
There was a problem hiding this comment.
Left some minor comments. Other than that this looks awesome 🚀
Let me know if you need help with
- Verify new data in mode
Thanks @danesfeder
| RouteRetrievalEvent(double elapsedTime, String routeUuid, String sessionId) { | ||
| super(sessionId); | ||
|
|
||
| addCounter(new LongCounter(ELAPSED_TIME_NAME, elapsedTime)); |
There was a problem hiding this comment.
Class 'LongCounter' is never used
Could we remove it altogether?
|
|
||
| public class ElapsedTimeTest { | ||
|
|
||
| private static final double DELTA = 1E-10; |
There was a problem hiding this comment.
We don't need such big DELTA especially if we're rounding to 2 point decimals and we're testing that the rounding is correct.
...tion/src/test/java/com/mapbox/services/android/navigation/v5/navigation/ElapsedTimeTest.java
Show resolved
Hide resolved
| long elapsedTimeInNanoseconds = end - start; | ||
| double elapsedTimeInSeconds = elapsedTimeInNanoseconds / 1e+9; | ||
| double roundedTime = Math.round(elapsedTimeInSeconds * 100d) / 100d; | ||
| assertEquals(elapsedTime.getElapsedTime(), roundedTime, DELTA); |
There was a problem hiding this comment.
NIT the "expected" and the "actual" values are reversed.
...tion/src/test/java/com/mapbox/services/android/navigation/v5/navigation/ElapsedTimeTest.java
Show resolved
Hide resolved
|
Thanks for opening this @danesfeder . This is a must-have for this Wednesday's release, so let's please get it wrapped up and merged. Thank you! |
danesfeder
force-pushed
the
dan-retrieval-event
branch
from
7ee7767 to
a748bfe
Compare
|
@Guardiola31337 this is updated, thanks for the feedback. Let's get that job going so we can see the data in mode 🚀 |
danesfeder
force-pushed
the
dan-retrieval-event
branch
2 times, most recently
from
bc08c26 to
affbc28
Compare
danesfeder
changed the title
danesfeder
force-pushed
the
dan-retrieval-event
branch
3 times, most recently
from
951a809 to
1b4a25c
Compare
devotaaabel
left a comment
devotaaabel
left a comment
There was a problem hiding this comment.
Looks good, thanks for running with this!
danesfeder
force-pushed
the
dan-retrieval-event
branch
from
1b4a25c to
0fbcf06
Compare
6 participants
Fixes #1732
This PR updates the route retrieval event to send seconds (with
.00precision) instead of nanoseconds. It also uses anEventListenerfrom OkHttp for monitoring the start and end time of the network request.TODO:
4.4.0release of Java library