-
Notifications
You must be signed in to change notification settings - Fork 319
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
[Android Auto] Add support for Junction Views #6849
[Android Auto] Add support for Junction Views #6849
Conversation
ChangelogFeatures
Bug fixes and improvements
Known issues
|
Codecov Report
@@ Coverage Diff @@
## main #6849 +/- ##
=========================================
Coverage 72.63% 72.63%
Complexity 5568 5568
=========================================
Files 780 780
Lines 30104 30104
Branches 3556 3556
=========================================
Hits 21867 21867
Misses 6809 6809
Partials 1428 1428 |
...androidauto/src/test/java/com/mapbox/androidauto/navigation/CarNavigationInfoProviderTest.kt
Show resolved
Hide resolved
...i-androidauto/src/test/java/com/mapbox/androidauto/navigation/CarNavigationInfoMapperTest.kt
Show resolved
Hide resolved
<item>JCT</item> | ||
<item>SAPA</item> | ||
<item>CITYREAL</item> | ||
<item>TOLLBRANCH</item> | ||
<item>AFTERTOLL</item> | ||
<item>EXPRESSWAY_ENTRANCE</item> | ||
<item>EXPRESSWAY_EXIT</item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i can see these are copied over from the MapboxJunctionActivity
so could be considered out of scope for this change.
but i think we should have human readable descriptions for these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What human-readable description? These values are TestRoutes
enum names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.google.com/search?q=SAPA+mapbox+junction+view
I don't know what SAPA means, do you? If you do I'm only suggesting that we add words
android-auto-app/src/main/java/com/mapbox/navigation/examples/androidauto/app/MainActivity.kt
Outdated
Show resolved
Hide resolved
/** | ||
* Simple component for detecting and rendering Junction Views. | ||
*/ | ||
private class Junctions( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that junction views will eventually be part of drop-in-ui and this is an android-auto
example. I think we should remove the app code so we don't have to maintain it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll remove it once Drop-In UI adds Junction View support.
...i-androidauto/src/main/java/com/mapbox/androidauto/navigation/lanes/CarLanesImageRenderer.kt
Show resolved
Hide resolved
@kmadsen Thanks for the review. I've applied your suggestions. Please let me know if there is anything else that needs to change. |
|
||
assertNotNull(img1) | ||
assertSame(img1, img2) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not testing the actual equality since it's the same object.
It may be better to add build the data objects, also test the cache miss case
@OptIn(ExperimentalMapboxNavigationAPI::class)
@Test
fun cache_hit_test() {
val lane1 = LaneFactory.buildLane(
allLanes = listOf(LaneIndicator.Builder()
.drivingSide("right")
.activeDirection("uturn")
.isActive(true)
.directions(listOf("uturn"))
.build())
)
val lane2 = LaneFactory.buildLane(
allLanes = listOf(LaneIndicator.Builder()
.drivingSide("right")
.activeDirection("uturn")
.isActive(true)
.directions(listOf("uturn"))
.build())
)
val img1 = carLanesImageGenerator.renderLanesImage(lane1)
val img2 = carLanesImageGenerator.renderLanesImage(lane2)
assertNotNull(img1)
assertSame(img1, img2)
}
@OptIn(ExperimentalMapboxNavigationAPI::class)
@Test
fun cache_miss_test() {
val lane1 = LaneFactory.buildLane(
allLanes = listOf(LaneIndicator.Builder()
.drivingSide("right")
.activeDirection("uturn")
.isActive(true)
.directions(listOf("uturn"))
.build())
)
val lane2 = LaneFactory.buildLane(
allLanes = listOf(LaneIndicator.Builder()
.drivingSide("right")
.activeDirection("uturn")
.isActive(true)
.directions(listOf("straight"))
.build())
)
val img1 = carLanesImageGenerator.renderLanesImage(lane1)
val img2 = carLanesImageGenerator.renderLanesImage(lane2)
assertNotNull(img1)
assertNotSame(img1, img2)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the cache miss case is covered by the other tests, but this will fail even though the values are the same
@Test
fun cache_test() {
val lane1 = mockk<Lane> {
every { allLanes } returns listOf(
mockk {
every { drivingSide } returns "right"
every { activeDirection } returns "uturn"
every { isActive } returns true
every { directions } returns listOf("uturn")
}
)
}
val lane2 = mockk<Lane> {
every { allLanes } returns listOf(
mockk {
every { drivingSide } returns "right"
every { activeDirection } returns "uturn"
every { isActive } returns true
every { directions } returns listOf("uturn")
}
)
}
val img1 = carLanesImageGenerator.renderLanesImage(lane1)
val img2 = carLanesImageGenerator.renderLanesImage(lane2)
assertNotNull(img1)
assertSame(img1, img2)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea. I've updated CarLanesImageRendererTest
to include both tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for addressing feedback!
@SevaZhukov it seems the changelog verification fails when android-auto changelogs are added
Closes NAVAND-980
Description
Android Auto
CarNavigationInfoServices.junctionApi(...)
factory methodCarNavigationInfoProvider
to detect and load Junction Bitmap on banner instructions change, and inject it intoNavigationTemplate.NavigationInfo
viaCarNavigationInfoMapper
CarLanesImageRenderer
by caching the last generated image in theLruCache
of size 1Android Auto App
MainActivity
and introducedDrawerActivity
for the debug menu - borrowed from the qa-test-appTestRoutes
coordinates - borrowed from theexamples//MapboxJunctionActivity
Screenshots or Gifs
Screen recording of the android-auto-app running on Google Pixel 6 and Android Auto running on Desktop Head Unit
(playback speed up to 2x)
output.mp4