Skip to content
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] Start declaring apis with java interop #6292

Merged
merged 1 commit into from
Sep 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone.
#### Features
#### Bug fixes and improvements
- Optimized calls to modify route arrow layer visibility. [#6308](https://github.com/mapbox/mapbox-navigation-android/pull/6308)
- Provide better java support for `MapboxNavigationApp`. [#6292](https://github.com/mapbox/mapbox-navigation-android/pull/6292)
Copy link
Contributor

@RingerJK RingerJK Sep 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we automate checks if any of public objects' functions or public static functions don't have java interrupt @kmadsen ?

Copy link
Contributor Author

@kmadsen kmadsen Sep 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I don't know I just lost some confidence in the @JvmStatic https://mapbox.atlassian.net/browse/NAVSDK-544

https://issuetracker.google.com/issues/158393309

java.lang.IncompatibleClassChangeError: The method 'com.mapbox.navigation.core.lifecycle.MapboxNavigationApp com.mapbox.navigation.core.lifecycle.MapboxNavigationApp.registerObserver(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver)' was expected to be of type virtual but instead was found to be of type static (declaration of 'com.mapbox.androidauto.MapboxCarApp' appears in /data/app/~~wWRVVkoksDjAWyl1zcYreA==/com.mapbox.navigation.qa_test_app-grYQDy8r9n-vE5Z1qLuR0Q==/base.apk!classes5.dex)

It changes the method signature so any changes will risk SEMVER

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.8 is the first stable version for MapboxNavigationApp so this could be fine. But for other functions that have had previous stable versions - it will change the method signature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crash looks like it should be fixed by #6316.


## Mapbox Navigation SDK 2.8.0-beta.3 - 09 September, 2022
### Changelog
Expand Down Expand Up @@ -40,7 +41,6 @@ This release depends on, and has been tested with, the following Mapbox dependen
- Mapbox Android Core `v5.0.2` ([release notes](https://github.com/mapbox/mapbox-events-android/releases/tag/core-5.0.2))
- Mapbox Android Telemetry `v8.1.5` ([release notes](https://github.com/mapbox/mapbox-events-android/releases/tag/telem-8.1.5-core-5.0.2))


## Mapbox Navigation SDK 2.8.0-beta.2 - 01 September, 2022
### Changelog
[Changes between v2.8.0-beta.1 and v2.8.0-beta.2](https://github.com/mapbox/mapbox-navigation-android/compare/v2.8.0-beta.1...v2.8.0-beta.2)
Expand Down
24 changes: 13 additions & 11 deletions libnavigation-core/api/current.txt
Original file line number Diff line number Diff line change
Expand Up @@ -311,19 +311,21 @@ package com.mapbox.navigation.core.history.model {
package com.mapbox.navigation.core.lifecycle {

public final class MapboxNavigationApp {
method @UiThread public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp attach(androidx.lifecycle.LifecycleOwner lifecycleOwner);
method @UiThread public static com.mapbox.navigation.core.lifecycle.MapboxNavigationApp attach(androidx.lifecycle.LifecycleOwner lifecycleOwner);
method public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp attachAllActivities(android.app.Application application);
method public com.mapbox.navigation.core.MapboxNavigation? current();
method @UiThread public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp detach(androidx.lifecycle.LifecycleOwner lifecycleOwner);
method @UiThread public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp disable();
method public static com.mapbox.navigation.core.MapboxNavigation? current();
method @UiThread public static com.mapbox.navigation.core.lifecycle.MapboxNavigationApp detach(androidx.lifecycle.LifecycleOwner lifecycleOwner);
method @UiThread public static com.mapbox.navigation.core.lifecycle.MapboxNavigationApp disable();
method public androidx.lifecycle.LifecycleOwner getLifecycleOwner();
method public <T extends com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver> T getObserver(kotlin.reflect.KClass<T> kClass);
method public <T extends com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver> java.util.List<T> getObservers(kotlin.reflect.KClass<T> kClass);
method @UiThread public boolean isSetup();
method @UiThread public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp registerObserver(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver mapboxNavigationObserver);
method @UiThread public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp setup(com.mapbox.navigation.base.options.NavigationOptions navigationOptions);
method @UiThread public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp setup(com.mapbox.navigation.core.lifecycle.NavigationOptionsProvider navigationOptionsProvider);
method @UiThread public com.mapbox.navigation.core.lifecycle.MapboxNavigationApp unregisterObserver(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver mapboxNavigationObserver);
method public static <T extends com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver> T getObserver(kotlin.reflect.KClass<T> kClass);
method public static <T extends com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver> T getObserver(Class<T> clazz);
method public static <T extends com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver> java.util.List<T> getObservers(kotlin.reflect.KClass<T> kClass);
method public static <T extends com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver> java.util.List<T> getObservers(Class<T> clazz);
method @UiThread public static boolean isSetup();
method @UiThread public static com.mapbox.navigation.core.lifecycle.MapboxNavigationApp registerObserver(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver mapboxNavigationObserver);
method @UiThread public static com.mapbox.navigation.core.lifecycle.MapboxNavigationApp setup(com.mapbox.navigation.base.options.NavigationOptions navigationOptions);
method @UiThread public static com.mapbox.navigation.core.lifecycle.MapboxNavigationApp setup(com.mapbox.navigation.core.lifecycle.NavigationOptionsProvider navigationOptionsProvider);
method @UiThread public static com.mapbox.navigation.core.lifecycle.MapboxNavigationApp unregisterObserver(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver mapboxNavigationObserver);
property public final androidx.lifecycle.LifecycleOwner lifecycleOwner;
field public static final com.mapbox.navigation.core.lifecycle.MapboxNavigationApp INSTANCE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ object MapboxNavigationApp {
* Returns true after [setup] has been called and false after [disable] is called.
*/
@UiThread
@JvmStatic
fun isSetup(): Boolean = mapboxNavigationAppDelegate.isSetup

/**
Expand All @@ -81,6 +82,7 @@ object MapboxNavigationApp {
* [Activity.isChangingConfigurations].
*/
@UiThread
@JvmStatic
fun setup(navigationOptions: NavigationOptions) = apply {
mapboxNavigationAppDelegate.setup { navigationOptions }
}
Expand All @@ -93,6 +95,7 @@ object MapboxNavigationApp {
* [Activity.isChangingConfigurations].
*/
@UiThread
@JvmStatic
fun setup(navigationOptionsProvider: NavigationOptionsProvider) = apply {
mapboxNavigationAppDelegate.setup(navigationOptionsProvider)
}
Expand All @@ -110,6 +113,7 @@ object MapboxNavigationApp {
* You can re-enable [MapboxNavigation] by calling [MapboxNavigationApp.setup].
*/
@UiThread
@JvmStatic
fun disable() = apply {
mapboxNavigationAppDelegate.disable()
}
Expand All @@ -124,6 +128,7 @@ object MapboxNavigationApp {
* it will be removed from the observers automatically.
*/
@UiThread
@JvmStatic
fun attach(lifecycleOwner: LifecycleOwner) = apply {
mapboxNavigationAppDelegate.attach(lifecycleOwner)
}
Expand All @@ -138,6 +143,7 @@ object MapboxNavigationApp {
* and will potentially cause [MapboxNavigation] to never be created.
*/
@UiThread
@JvmStatic
fun detach(lifecycleOwner: LifecycleOwner) = apply {
mapboxNavigationAppDelegate.detach(lifecycleOwner)
}
Expand All @@ -146,6 +152,7 @@ object MapboxNavigationApp {
* Register an observer to receive the [MapboxNavigation] instance.
*/
@UiThread
@JvmStatic
fun registerObserver(mapboxNavigationObserver: MapboxNavigationObserver) = apply {
mapboxNavigationAppDelegate.registerObserver(mapboxNavigationObserver)
}
Expand All @@ -154,6 +161,7 @@ object MapboxNavigationApp {
* Unregister the observer that was registered through [registerObserver].
*/
@UiThread
@JvmStatic
fun unregisterObserver(mapboxNavigationObserver: MapboxNavigationObserver) = apply {
mapboxNavigationAppDelegate.unregisterObserver(mapboxNavigationObserver)
}
Expand All @@ -164,6 +172,7 @@ object MapboxNavigationApp {
*
* @throws IllegalStateException when the class has not been registered.
*/
@JvmStatic
fun <T : MapboxNavigationObserver> getObserver(kClass: KClass<T>): T {
return mapboxNavigationAppDelegate.getObserver(kClass)
}
Expand All @@ -172,16 +181,38 @@ object MapboxNavigationApp {
* Provides access to any registered observer instance. If no observers have been registered
* with this class type, an empty list is returned.
*/
@JvmStatic
fun <T : MapboxNavigationObserver> getObservers(kClass: KClass<T>): List<T> {
return mapboxNavigationAppDelegate.getObservers(kClass)
}

/**
* Java interoperability. Provides access to any registered observer instance. If multiple
* instances of the same class have been registered, the first observer will be returned.
*
* @throws IllegalStateException when the class has not been registered.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to add @Throws annotation? It provides better java interoperability. Although in this case it doesn't matter since IllegalStateException is unchecked. However, I think it's a good pattern.

*/
@JvmStatic
fun <T : MapboxNavigationObserver> getObserver(clazz: Class<T>): T {
return mapboxNavigationAppDelegate.getObserver(clazz)
}

/**
* Java interoperability. Provides access to any registered observer instance. If no observers
* have been registered with this class type, an empty list is returned.
*/
@JvmStatic
fun <T : MapboxNavigationObserver> getObservers(clazz: Class<T>): List<T> {
return mapboxNavigationAppDelegate.getObservers(clazz)
}

/**
* [MapboxNavigation] has functions that do not require observation streams. This function
* allows you to get the current instance to call those functions.
*
* For example, you do not need to [registerObserver] in order to call
* [MapboxNavigation.postUserFeedback] or [MapboxNavigation.setRoutes].
*/
@JvmStatic
fun current(): MapboxNavigation? = mapboxNavigationAppDelegate.current()
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.mapbox.navigation.core.MapboxNavigation
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.unmockkAll
import io.mockk.verify
import io.mockk.verifyOrder
Expand Down Expand Up @@ -50,6 +51,7 @@ class RequireMapboxNavigationTest {
@Before
fun setup() {
mockkObject(MapboxNavigationApp)
mockkStatic(MapboxNavigationApp::class)
}

@After
Expand Down
1 change: 1 addition & 0 deletions libnavui-androidauto/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Mapbox welcomes participation and contributions from everyone.
## Unreleased
#### Features
#### Bug fixes and improvements
- Remove experimental from `MapboxCarNavigationManager` and showcase java. [#6292](https://github.com/mapbox/mapbox-navigation-android/pull/6292)

## androidauto-v0.10.0 - Sep 9, 2022
### Changelog
Expand Down
2 changes: 1 addition & 1 deletion libnavui-androidauto/api/current.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ package com.mapbox.androidauto {
field public static final com.mapbox.androidauto.MapboxCarApp INSTANCE;
}

@com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public final class MapboxCarNavigationManager implements com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver {
public final class MapboxCarNavigationManager implements com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver {
ctor public MapboxCarNavigationManager(androidx.car.app.CarContext carContext);
method public kotlinx.coroutines.flow.StateFlow<java.lang.Boolean> getAutoDriveEnabledFlow();
method public void onAttached(com.mapbox.navigation.core.MapboxNavigation mapboxNavigation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import com.mapbox.androidauto.car.navigation.CarDistanceFormatter
import com.mapbox.androidauto.car.navigation.maneuver.CarManeuverMapper
import com.mapbox.androidauto.car.telemetry.MapboxCarTelemetry
import com.mapbox.androidauto.internal.logAndroidAuto
import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
import com.mapbox.navigation.core.MapboxNavigation
import com.mapbox.navigation.core.formatter.MapboxDistanceFormatter
import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp
Expand All @@ -24,7 +23,6 @@ import kotlinx.coroutines.flow.StateFlow
* registered, the trip status of [MapboxNavigation] will be sent to the [NavigationManager].
* This is needed to keep the vehicle cluster display updated.
*/
@ExperimentalPreviewMapboxNavigationAPI
class MapboxCarNavigationManager(
carContext: CarContext
) : MapboxNavigationObserver {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver
import io.mockk.every
import io.mockk.just
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.runs
import io.mockk.unmockkAll
import org.junit.rules.TestWatcher
Expand All @@ -26,6 +27,7 @@ class CarAppTestRule : TestWatcher() {
every { AndroidAutoLog.logAndroidAutoFailure(any(), any()) } just runs

mockkObject(MapboxNavigationApp)
mockkStatic(MapboxNavigationApp::class)
every {
MapboxNavigationApp.registerObserver(any())
} answers {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver
import com.mapbox.navigation.ui.base.lifecycle.UIComponent
import io.mockk.mockk
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.slot
import io.mockk.unmockkAll
import io.mockk.verify
Expand All @@ -28,6 +29,7 @@ class ComponentInstallerTest {
@Before
fun setUp() {
mockkObject(MapboxNavigationApp)
mockkStatic(MapboxNavigationApp::class)
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import com.google.android.material.bottomsheet.BottomSheetBehavior
import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
import com.mapbox.navigation.core.MapboxNavigation
import com.mapbox.navigation.dropin.NavigationViewContext
import com.mapbox.navigation.dropin.NavigationViewModel
import com.mapbox.navigation.dropin.databinding.MapboxNavigationViewLayoutBinding
import com.mapbox.navigation.dropin.util.TestStore
import com.mapbox.navigation.ui.app.internal.navigation.NavigationState
Expand Down Expand Up @@ -54,7 +53,7 @@ class InfoPanelCoordinatorTest {
viewContext = NavigationViewContext(
context = ctx,
lifecycleOwner = TestLifecycleOwner(),
viewModel = NavigationViewModel(),
viewModel = mockk(),
storeProvider = { testStore }
)
mapboxNavigation = mockk(relaxed = true)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package com.mapbox.navigation.qa_test_app.testing;

import androidx.annotation.NonNull;
import androidx.car.app.CarContext;
import androidx.lifecycle.LifecycleOwner;

import com.mapbox.androidauto.MapboxCarNavigationManager;
import com.mapbox.androidauto.internal.car.search.CarPlaceSearch;
import com.mapbox.maps.extension.androidauto.MapboxCarMap;
import com.mapbox.navigation.base.options.NavigationOptions;
import com.mapbox.navigation.core.MapboxNavigation;
import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp;
import com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver;

import java.util.List;

import kotlinx.coroutines.CoroutineScope;

class CarJavaInterfaceChecker {

void MapboxNavigationApp(
LifecycleOwner lifecycleOwner,
NavigationOptions navigationOptions,
MapboxNavigationObserver observer
) {
// Set up now
MapboxNavigationApp.setup(navigationOptions);

// Set up provider
MapboxNavigationApp.setup(() -> navigationOptions);

// Control lifecycles
MapboxNavigationApp.attach(lifecycleOwner);
MapboxNavigationApp.disable();
MapboxNavigationApp.detach(lifecycleOwner);

// Get current instance
MapboxNavigation mapboxNavigation = MapboxNavigationApp.current();

// Register and unregister observer
MapboxNavigationApp.registerObserver(observer);
MapboxNavigationApp.unregisterObserver(observer);

MapboxNavigationObserver otherObserver = MapboxNavigationApp.getObserver(CarPlaceSearch.class);
List<CarPlaceSearch> otherObservers = MapboxNavigationApp.getObservers(CarPlaceSearch.class);
}

void MapboxNavigationObserver() {
MapboxNavigationObserver observer = new MapboxNavigationObserver() {
@Override
public void onAttached(@NonNull MapboxNavigation mapboxNavigation) {

}

@Override
public void onDetached(@NonNull MapboxNavigation mapboxNavigation) {

}
};
}

void MapboxCarNavigationManager(
CarContext carContext,
LifecycleOwner lifecycleOwner,
MapboxCarMap mapboxCarMap
) {
// Constructor
MapboxCarNavigationManager sut = new MapboxCarNavigationManager(carContext);

// Observing auto drive
CoroutineScope scope = JavaFlow.lifecycleScope(lifecycleOwner);
JavaFlow.collect(sut.getAutoDriveEnabledFlow(), scope, (enabled) -> {
// check enabled
});

// Get auto drive value
boolean isEnabled = sut.getAutoDriveEnabledFlow().getValue();

// Register onto MapboxNavigationAPp
MapboxNavigationApp.registerObserver(sut);
MapboxNavigationApp.unregisterObserver(sut);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.mapbox.navigation.qa_test_app.testing

import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.lifecycleScope
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.launch

/**
* This is not production tested. It is used to showcase java interoperability.
*/
object JavaFlow {

@JvmStatic
fun lifecycleScope(owner: LifecycleOwner): CoroutineScope = owner.lifecycleScope

@JvmStatic
fun <T> collect(flow: Flow<T>, scope: CoroutineScope, consumer: Consumer<T>) {
scope.launch {
flow.collect { value -> consumer.accept(value) }
}
}
}

interface Consumer<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is java.util.function.Consumer, why do you need this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't use it because it requires api 24

Screen Shot 2022-09-12 at 12 52 13 PM

fun accept(value: T)
}