Add executor to NavigationManager.setListener.
Also cleaned up naming of functions as per API guidelines and
added param tags to methods.
Relnote: Add Executor to NavigationManager.setCallback.
Test: ./gradlew :car:app:app:connectedDebugAndroidTest
Bug: 173625207
Change-Id: I650387e3d79774021d0d0782ac27a784188fd664
diff --git a/car/app/app/api/current.txt b/car/app/app/api/current.txt
index 118a4bf..87c8771 100644
--- a/car/app/app/api/current.txt
+++ b/car/app/app/api/current.txt
@@ -771,15 +771,17 @@
package androidx.car.app.navigation {
public class NavigationManager {
+ method @MainThread public void clearNavigationManagerListener();
method @MainThread public void navigationEnded();
method @MainThread public void navigationStarted();
- method @MainThread public void setListener(androidx.car.app.navigation.NavigationManagerListener?);
+ method @MainThread public void setNavigationManagerListener(androidx.car.app.navigation.NavigationManagerListener);
+ method @MainThread public void setNavigationManagerListener(java.util.concurrent.Executor, androidx.car.app.navigation.NavigationManagerListener);
method @MainThread public void updateTrip(androidx.car.app.navigation.model.Trip);
}
public interface NavigationManagerListener {
method public void onAutoDriveEnabled();
- method public void stopNavigation();
+ method public void onStopNavigation();
}
}
diff --git a/car/app/app/api/public_plus_experimental_current.txt b/car/app/app/api/public_plus_experimental_current.txt
index 118a4bf..87c8771 100644
--- a/car/app/app/api/public_plus_experimental_current.txt
+++ b/car/app/app/api/public_plus_experimental_current.txt
@@ -771,15 +771,17 @@
package androidx.car.app.navigation {
public class NavigationManager {
+ method @MainThread public void clearNavigationManagerListener();
method @MainThread public void navigationEnded();
method @MainThread public void navigationStarted();
- method @MainThread public void setListener(androidx.car.app.navigation.NavigationManagerListener?);
+ method @MainThread public void setNavigationManagerListener(androidx.car.app.navigation.NavigationManagerListener);
+ method @MainThread public void setNavigationManagerListener(java.util.concurrent.Executor, androidx.car.app.navigation.NavigationManagerListener);
method @MainThread public void updateTrip(androidx.car.app.navigation.model.Trip);
}
public interface NavigationManagerListener {
method public void onAutoDriveEnabled();
- method public void stopNavigation();
+ method public void onStopNavigation();
}
}
diff --git a/car/app/app/api/restricted_current.txt b/car/app/app/api/restricted_current.txt
index 118a4bf..87c8771 100644
--- a/car/app/app/api/restricted_current.txt
+++ b/car/app/app/api/restricted_current.txt
@@ -771,15 +771,17 @@
package androidx.car.app.navigation {
public class NavigationManager {
+ method @MainThread public void clearNavigationManagerListener();
method @MainThread public void navigationEnded();
method @MainThread public void navigationStarted();
- method @MainThread public void setListener(androidx.car.app.navigation.NavigationManagerListener?);
+ method @MainThread public void setNavigationManagerListener(androidx.car.app.navigation.NavigationManagerListener);
+ method @MainThread public void setNavigationManagerListener(java.util.concurrent.Executor, androidx.car.app.navigation.NavigationManagerListener);
method @MainThread public void updateTrip(androidx.car.app.navigation.model.Trip);
}
public interface NavigationManagerListener {
method public void onAutoDriveEnabled();
- method public void stopNavigation();
+ method public void onStopNavigation();
}
}
diff --git a/car/app/app/src/main/aidl/androidx/car/app/navigation/INavigationManager.aidl b/car/app/app/src/main/aidl/androidx/car/app/navigation/INavigationManager.aidl
index 4e16e7e..7a3a01d 100644
--- a/car/app/app/src/main/aidl/androidx/car/app/navigation/INavigationManager.aidl
+++ b/car/app/app/src/main/aidl/androidx/car/app/navigation/INavigationManager.aidl
@@ -28,5 +28,5 @@
* <p>The app should stop any audio guidance, routing notifications tagged for
* the car, and metadata state updates.
*/
- void stopNavigation(IOnDoneCallback callback) = 1;
+ void onStopNavigation(IOnDoneCallback callback) = 1;
}
diff --git a/car/app/app/src/main/java/androidx/car/app/CarAppService.java b/car/app/app/src/main/java/androidx/car/app/CarAppService.java
index 7c3c854..69ced55 100644
--- a/car/app/app/src/main/java/androidx/car/app/CarAppService.java
+++ b/car/app/app/src/main/java/androidx/car/app/CarAppService.java
@@ -129,7 +129,7 @@
mRegistry.handleLifecycleEvent(Event.ON_STOP);
// Stop any active navigation
- mCarContext.getCarService(NavigationManager.class).stopNavigation();
+ mCarContext.getCarService(NavigationManager.class).onStopNavigation();
// Destroy all screens in the stack
mCarContext.getCarService(ScreenManager.class).destroyAndClearScreenStack();
diff --git a/car/app/app/src/main/java/androidx/car/app/CarContext.java b/car/app/app/src/main/java/androidx/car/app/CarContext.java
index 11837d2..cbff699 100644
--- a/car/app/app/src/main/java/androidx/car/app/CarContext.java
+++ b/car/app/app/src/main/java/androidx/car/app/CarContext.java
@@ -442,7 +442,7 @@
this.mHostDispatcher = hostDispatcher;
mAppManager = AppManager.create(this, hostDispatcher);
- mNavigationManager = NavigationManager.create(hostDispatcher);
+ mNavigationManager = NavigationManager.create(this, hostDispatcher);
mScreenManager = ScreenManager.create(this, lifecycle);
mOnBackPressedDispatcher =
new OnBackPressedDispatcher(() -> getCarService(ScreenManager.class).pop());
diff --git a/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java b/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java
index 787fa0e..356c4e5 100644
--- a/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java
+++ b/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManager.java
@@ -23,6 +23,7 @@
import static java.util.Objects.requireNonNull;
import android.annotation.SuppressLint;
+import android.os.Looper;
import android.util.Log;
import androidx.annotation.MainThread;
@@ -38,6 +39,9 @@
import androidx.car.app.serialization.Bundleable;
import androidx.car.app.serialization.BundlerException;
import androidx.car.app.utils.RemoteUtils;
+import androidx.core.content.ContextCompat;
+
+import java.util.concurrent.Executor;
/**
* Manager for communicating navigation related events with the host.
@@ -51,17 +55,20 @@
* called.
*
* <p>Navigation apps must also register a {@link NavigationManagerListener} to handle callbacks to
- * {@link NavigationManagerListener#stopNavigation()} issued by the host.
+ * {@link NavigationManagerListener#onStopNavigation()} issued by the host.
*/
public class NavigationManager {
private static final String TAG = "NavigationManager";
- private final INavigationManager.Stub mNavigationmanager;
+ private final CarContext mCarContext;
+ private final INavigationManager.Stub mNavigationManager;
private final HostDispatcher mHostDispatcher;
// Guarded by main thread access.
@Nullable
- private NavigationManagerListener mListener;
+ private NavigationManagerListener mNavigationManagerListener;
+ @Nullable
+ private Executor mNavigationManagerListenerExecutor;
private boolean mIsNavigating;
private boolean mIsAutoDriveEnabled;
@@ -74,7 +81,7 @@
* <p>This method should only be invoked once the navigation app has called {@link
* #navigationStarted()}, or else the updates will be dropped by the host. Once the app has
* called {@link #navigationEnded()} or received
- * {@link NavigationManagerListener#stopNavigation()} it should stop sending updates.
+ * {@link NavigationManagerListener#onStopNavigation()} it should stop sending updates.
*
* <p>As the location changes, and in accordance with speed and rounded distance changes, the
* {@link TravelEstimate}s in the provided {@link Trip} should be rebuilt and this method called
@@ -86,13 +93,15 @@
* androidx.car.app.navigation.model.Maneuver}s of unknown type may be skipped while on other
* displays the associated icon may be shown.
*
+ * @param trip destination, steps, and trip estimates to be sent to the host
+ *
* @throws HostException if the call is invoked by an app that is not declared as
- * a navigation app in the manifest.
+ * a navigation app in the manifest
* @throws IllegalStateException if the call occurs when navigation is not started. See
- * {@link #navigationStarted()} for more info.
+ * {@link #navigationStarted()} for more info
* @throws IllegalArgumentException if any of the destinations, steps, or trip position is
- * not well formed.
- * @throws IllegalStateException if the current thread is not the main thread.
+ * not well formed
+ * @throws IllegalStateException if the current thread is not the main thread
*/
@MainThread
public void updateTrip(@NonNull Trip trip) {
@@ -118,25 +127,59 @@
}
/**
- * Sets a listener to start receiving navigation manager events, or {@code null} to clear the
- * listener.
+ * Sets a listener to start receiving navigation manager events.
*
- * @throws IllegalStateException if {@code null} is passed in while navigation is started. See
+ * Note that the listener will be executed on the main thread using
+ * {@link Looper#getMainLooper()}. To specify the execution thread, use
+ * {@link #setNavigationManagerListener(Executor, NavigationManagerListener)}.
+ *
+ * @param listener the {@link NavigationManagerListener} to use
+ *
+ * @throws IllegalStateException if the current thread is not the main thread
+ */
+ @SuppressLint("ExecutorRegistration")
+ @MainThread
+ public void setNavigationManagerListener(@NonNull NavigationManagerListener listener) {
+ checkMainThread();
+ Executor executor = ContextCompat.getMainExecutor(mCarContext);
+ setNavigationManagerListener(executor, listener);
+ }
+
+ /**
+ * Sets a listener to start receiving navigation manager events.
+ *
+ * @param executor the executor which will be used for invoking the listener
+ * @param listener the {@link NavigationManagerListener} to use
+ *
+ * @throws IllegalStateException if the current thread is not the main thread.
+ */
+ @MainThread
+ public void setNavigationManagerListener(@NonNull /* @CallbackExecutor */ Executor executor,
+ @NonNull NavigationManagerListener listener) {
+ checkMainThread();
+
+ mNavigationManagerListenerExecutor = executor;
+ mNavigationManagerListener = listener;
+ if (mIsAutoDriveEnabled) {
+ onAutoDriveEnabled();
+ }
+ }
+
+ /**
+ * Clears the listener for receiving navigation manager events.
+ *
+ * @throws IllegalStateException if navigation is started. See
* {@link #navigationStarted()} for more info.
* @throws IllegalStateException if the current thread is not the main thread.
*/
- // TODO(rampara): Add Executor parameter.
- @SuppressLint("ExecutorRegistration")
@MainThread
- public void setListener(@Nullable NavigationManagerListener listener) {
+ public void clearNavigationManagerListener() {
checkMainThread();
- if (mIsNavigating && listener == null) {
+ if (mIsNavigating) {
throw new IllegalStateException("Removing listener while navigating");
}
- this.mListener = listener;
- if (mIsAutoDriveEnabled && listener != null) {
- listener.onAutoDriveEnabled();
- }
+ mNavigationManagerListenerExecutor = null;
+ mNavigationManagerListener = null;
}
/**
@@ -146,10 +189,11 @@
* the host. The app must call this method to inform the system that it has started
* navigation in response to user action.
*
- * <p>This function can only called if {@link #setListener(NavigationManagerListener)} has been
+ * <p>This function can only called if
+ * {@link #setNavigationManagerListener(NavigationManagerListener)} has been
* called with a non-{@code null} value. The listener is required so that a signal to stop
* navigation from the host can be handled using
- * {@link NavigationManagerListener#stopNavigation()}.
+ * {@link NavigationManagerListener#onStopNavigation()}.
*
* <p>This method is idempotent.
*
@@ -162,7 +206,7 @@
if (mIsNavigating) {
return;
}
- if (mListener == null) {
+ if (mNavigationManagerListener == null) {
throw new IllegalStateException("No listener has been set");
}
mIsNavigating = true;
@@ -209,8 +253,9 @@
*/
@RestrictTo(LIBRARY)
@NonNull
- public static NavigationManager create(@NonNull HostDispatcher hostDispatcher) {
- return new NavigationManager(hostDispatcher);
+ public static NavigationManager create(@NonNull CarContext carContext,
+ @NonNull HostDispatcher hostDispatcher) {
+ return new NavigationManager(carContext, hostDispatcher);
}
/**
@@ -221,7 +266,7 @@
@RestrictTo(LIBRARY)
@NonNull
public INavigationManager.Stub getIInterface() {
- return mNavigationmanager;
+ return mNavigationManager;
}
/**
@@ -231,13 +276,15 @@
*/
@RestrictTo(LIBRARY)
@MainThread
- public void stopNavigation() {
+ public void onStopNavigation() {
checkMainThread();
if (!mIsNavigating) {
return;
}
mIsNavigating = false;
- requireNonNull(mListener).stopNavigation();
+ requireNonNull(mNavigationManagerListenerExecutor).execute(() -> {
+ requireNonNull(mNavigationManagerListener).onStopNavigation();
+ });
}
/**
@@ -255,9 +302,11 @@
public void onAutoDriveEnabled() {
checkMainThread();
mIsAutoDriveEnabled = true;
- if (mListener != null) {
+ if (mNavigationManagerListener != null) {
Log.d(TAG, "Executing onAutoDriveEnabled");
- mListener.onAutoDriveEnabled();
+ requireNonNull(mNavigationManagerListenerExecutor).execute(() -> {
+ mNavigationManagerListener.onAutoDriveEnabled();
+ });
} else {
Log.w(TAG, "NavigationManagerListener not set, skipping onAutoDriveEnabled");
}
@@ -266,14 +315,17 @@
/** @hide */
@RestrictTo(LIBRARY_GROUP) // Restrict to testing library
@SuppressWarnings({"methodref.receiver.bound.invalid"})
- protected NavigationManager(@NonNull HostDispatcher hostDispatcher) {
- this.mHostDispatcher = requireNonNull(hostDispatcher);
- mNavigationmanager =
+ protected NavigationManager(@NonNull CarContext carContext,
+ @NonNull HostDispatcher hostDispatcher) {
+ mCarContext = carContext;
+ mHostDispatcher = requireNonNull(hostDispatcher);
+ mNavigationManager =
new INavigationManager.Stub() {
@Override
- public void stopNavigation(IOnDoneCallback callback) {
+ public void onStopNavigation(IOnDoneCallback callback) {
RemoteUtils.dispatchHostCall(
- NavigationManager.this::stopNavigation, callback, "stopNavigation");
+ NavigationManager.this::onStopNavigation, callback,
+ "onStopNavigation");
}
};
}
diff --git a/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManagerListener.java b/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManagerListener.java
index ee4c29f..8f0cd2a 100644
--- a/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManagerListener.java
+++ b/car/app/app/src/main/java/androidx/car/app/navigation/NavigationManagerListener.java
@@ -16,12 +16,10 @@
package androidx.car.app.navigation;
-import android.annotation.SuppressLint;
-
import androidx.car.app.navigation.model.Trip;
/**
- * Listener of events from the {@link NavigationManager}.
+ * Listener for events from the {@link NavigationManager}.
*
* @see NavigationManager
*/
@@ -34,11 +32,7 @@
* guidance, routing-related notifications, and updating trip information via {@link
* NavigationManager#updateTrip(Trip)}.
*/
-
- // TODO(rampara): Listener method names must follow the on<Something> style. Consider
- // onShouldStopNavigation.
- @SuppressLint("CallbackMethodName")
- void stopNavigation();
+ void onStopNavigation();
/**
* Notifies the app that, from this point onwards, when the user chooses to navigate to a
diff --git a/car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java b/car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java
index ad05559..6605c9b 100644
--- a/car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java
+++ b/car/app/app/src/test/java/androidx/car/app/navigation/NavigationManagerTest.java
@@ -37,6 +37,8 @@
import androidx.car.app.navigation.model.TravelEstimate;
import androidx.car.app.navigation.model.Trip;
import androidx.car.app.serialization.Bundleable;
+import androidx.car.app.testing.TestCarContext;
+import androidx.test.core.app.ApplicationProvider;
import org.junit.Before;
import org.junit.Test;
@@ -47,6 +49,7 @@
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.internal.DoNotInstrument;
+import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
/** Tests for {@link NavigationManager}. */
@@ -90,6 +93,9 @@
public void setUp() throws RemoteException {
MockitoAnnotations.initMocks(this);
+ TestCarContext testCarContext =
+ TestCarContext.createCarContext(ApplicationProvider.getApplicationContext());
+
INavigationHost navHostStub =
new INavigationHost.Stub() {
@Override
@@ -111,14 +117,14 @@
mHostDispatcher.setCarHost(mMockCarHost);
- mNavigationManager = NavigationManager.create(mHostDispatcher);
+ mNavigationManager = NavigationManager.create(testCarContext, mHostDispatcher);
}
@Test
public void navigationStarted_sendState_navigationEnded() throws RemoteException {
InOrder inOrder = inOrder(mMockNavHost);
- mNavigationManager.setListener(mNavigationListener);
+ mNavigationManager.setNavigationManagerListener(mNavigationListener);
mNavigationManager.navigationStarted();
inOrder.verify(mMockNavHost).navigationStarted();
@@ -137,7 +143,7 @@
@Test
public void navigationStarted_multiple() throws RemoteException {
- mNavigationManager.setListener(mNavigationListener);
+ mNavigationManager.setNavigationManagerListener(mNavigationListener);
mNavigationManager.navigationStarted();
mNavigationManager.navigationStarted();
@@ -158,22 +164,24 @@
}
@Test
- public void stopNavigation_notNavigating() throws RemoteException {
- mNavigationManager.setListener(mNavigationListener);
- mNavigationManager.getIInterface().stopNavigation(mock(IOnDoneCallback.class));
- verify(mNavigationListener, never()).stopNavigation();
+ public void onStopNavigation_notNavigating() throws RemoteException {
+ mNavigationManager.setNavigationManagerListener(mNavigationListener);
+ mNavigationManager.getIInterface().onStopNavigation(mock(IOnDoneCallback.class));
+ verify(mNavigationListener, never()).onStopNavigation();
}
@Test
- public void stopNavigation_navigating_restart() throws RemoteException {
+ public void onStopNavigation_navigating_restart() throws RemoteException {
InOrder inOrder = inOrder(mMockNavHost, mNavigationListener);
- mNavigationManager.setListener(mNavigationListener);
+ mNavigationManager.setNavigationManagerListener(new SynchronousExecutor(),
+ mNavigationListener);
mNavigationManager.navigationStarted();
inOrder.verify(mMockNavHost).navigationStarted();
- mNavigationManager.getIInterface().stopNavigation(mock(IOnDoneCallback.class));
- inOrder.verify(mNavigationListener).stopNavigation();
+ mNavigationManager.getIInterface().onStopNavigation(mock(IOnDoneCallback.class));
+
+ inOrder.verify(mNavigationListener).onStopNavigation();
mNavigationManager.navigationStarted();
inOrder.verify(mMockNavHost).navigationStarted();
@@ -181,7 +189,8 @@
@Test
public void onAutoDriveEnabled_callsListener() {
- mNavigationManager.setListener(mNavigationListener);
+ mNavigationManager.setNavigationManagerListener(new SynchronousExecutor(),
+ mNavigationListener);
mNavigationManager.onAutoDriveEnabled();
verify(mNavigationListener).onAutoDriveEnabled();
@@ -190,8 +199,17 @@
@Test
public void onAutoDriveEnabledBeforeRegisteringListener_callsListener() {
mNavigationManager.onAutoDriveEnabled();
- mNavigationManager.setListener(mNavigationListener);
+ mNavigationManager.setNavigationManagerListener(new SynchronousExecutor(),
+ mNavigationListener);
verify(mNavigationListener).onAutoDriveEnabled();
}
+
+ static class SynchronousExecutor implements Executor {
+ @Override
+ public void execute(Runnable r) {
+ r.run();
+ }
+ }
+
}
diff --git a/car/app/app/src/test/java/androidx/car/app/testing/TestCarContext.java b/car/app/app/src/test/java/androidx/car/app/testing/TestCarContext.java
index 4f09fbd..07cc5e9 100644
--- a/car/app/app/src/test/java/androidx/car/app/testing/TestCarContext.java
+++ b/car/app/app/src/test/java/androidx/car/app/testing/TestCarContext.java
@@ -226,7 +226,7 @@
this.mFakeHost = new FakeHost(this);
this.mTestLifecycleOwner = testLifecycleOwner;
this.mTestAppManager = new TestAppManager(this, hostDispatcher);
- this.mTestNavigationManager = new TestNavigationManager(hostDispatcher);
+ this.mTestNavigationManager = new TestNavigationManager(this, hostDispatcher);
this.mTestScreenManager = new TestScreenManager(this);
}
}
diff --git a/car/app/app/src/test/java/androidx/car/app/testing/navigation/TestNavigationManager.java b/car/app/app/src/test/java/androidx/car/app/testing/navigation/TestNavigationManager.java
index db99643..3892e58 100644
--- a/car/app/app/src/test/java/androidx/car/app/testing/navigation/TestNavigationManager.java
+++ b/car/app/app/src/test/java/androidx/car/app/testing/navigation/TestNavigationManager.java
@@ -24,6 +24,7 @@
import androidx.car.app.navigation.NavigationManager;
import androidx.car.app.navigation.NavigationManagerListener;
import androidx.car.app.navigation.model.Trip;
+import androidx.car.app.testing.TestCarContext;
import androidx.car.app.testing.navigation.model.TripController;
import java.util.ArrayList;
@@ -37,7 +38,8 @@
*
* <ul>
* <li>All the {@link Trip}s sent via {@link NavigationManager#updateTrip}.
- * <li>All the {@link NavigationManagerListener}s set via {@link NavigationManager#setListener}.
+ * <li>All the {@link NavigationManagerListener}s set via
+ * {@link NavigationManager#setNavigationManagerListener}.
* <li>Count of times that the navigation was started via {@link
* NavigationManager#navigationStarted()}.
* <li>Count of times that the navigation was ended via {@link NavigationManager#navigationEnded}.
@@ -71,14 +73,14 @@
/**
* Retrieves all the {@link NavigationManagerListener}s added via {@link
- * NavigationManager#setListener(NavigationManagerListener)}.
+ * NavigationManager#setNavigationManagerListener(NavigationManagerListener)}.
*
* <p>The listeners are stored in order of calls.
*
* <p>The listeners will be stored until {@link #reset} is called.
*/
@NonNull
- public List<NavigationManagerListener> getNavigationManagerListenersSet() {
+ public List<NavigationManagerListener> getNavigationManagerCallbacksSet() {
return mListenersSet;
}
@@ -105,9 +107,9 @@
}
@Override
- public void setListener(@Nullable NavigationManagerListener listener) {
+ public void setNavigationManagerListener(@Nullable NavigationManagerListener listener) {
mListenersSet.add(listener);
- super.setListener(listener);
+ super.setNavigationManagerListener(listener);
}
@Override
@@ -122,7 +124,7 @@
super.navigationEnded();
}
- public TestNavigationManager(HostDispatcher hostDispatcher) {
- super(hostDispatcher);
+ public TestNavigationManager(TestCarContext testCarContext, HostDispatcher hostDispatcher) {
+ super(testCarContext, hostDispatcher);
}
}