From 16fddb444eba17d04da0aaa422e51f4503cba123 Mon Sep 17 00:00:00 2001 From: Brian Stack Date: Wed, 3 Feb 2021 13:54:27 -0800 Subject: [PATCH] Pass SessionHandle to UWB Service when opening a session Instead of having the UWB Service generate a SessionHandle for each call to IUwbAdapter#openRanging, now the framework must select and pass in a SessionHandle. This helps to improve consistency across the API surface and simplifies the implementation of IUwbAdapter#openRanging. Bug: 179273688 Test: atest UwbManagerTests Change-Id: Ie08216638c8a5bf7260fff8752a46aad06f53fcb --- core/java/android/uwb/IUwbAdapter.aidl | 13 ++- core/java/android/uwb/RangingManager.java | 22 ++-- .../src/android/uwb/RangingManagerTest.java | 110 ++++++++---------- 3 files changed, 66 insertions(+), 79 deletions(-) diff --git a/core/java/android/uwb/IUwbAdapter.aidl b/core/java/android/uwb/IUwbAdapter.aidl index 468a69c7bddb2..4036892fb9e7d 100644 --- a/core/java/android/uwb/IUwbAdapter.aidl +++ b/core/java/android/uwb/IUwbAdapter.aidl @@ -62,9 +62,6 @@ interface IUwbAdapter { /** * Request to open a new ranging session * - * This function must return before calling any functions in - * IUwbAdapterCallbacks. - * * This function does not start the ranging session, but all necessary * components must be initialized and ready to start a new ranging * session prior to calling IUwbAdapterCallback#onRangingOpened. @@ -77,12 +74,16 @@ interface IUwbAdapter { * RANGING_SESSION_OPEN_THRESHOLD_MS milliseconds of #openRanging being called * if the ranging session fails to be opened. * + * If the provided sessionHandle is already open for the calling client, then + * #onRangingOpenFailed must be called and the new session must not be opened. + * + * @param sessionHandle the session handle to open ranging for * @param rangingCallbacks the callbacks used to deliver ranging information * @param parameters the configuration to use for ranging - * @return a SessionHandle used to identify this ranging request */ - SessionHandle openRanging(in IUwbRangingCallbacks rangingCallbacks, - in PersistableBundle parameters); + void openRanging(in SessionHandle sessionHandle, + in IUwbRangingCallbacks rangingCallbacks, + in PersistableBundle parameters); /** * Request to start ranging diff --git a/core/java/android/uwb/RangingManager.java b/core/java/android/uwb/RangingManager.java index 5e75a629ff39a..85f2c1ccc1801 100644 --- a/core/java/android/uwb/RangingManager.java +++ b/core/java/android/uwb/RangingManager.java @@ -33,6 +33,7 @@ public class RangingManager extends android.uwb.IUwbRangingCallbacks.Stub { private final IUwbAdapter mAdapter; private final Hashtable mRangingSessionTable = new Hashtable<>(); + private int mNextSessionId = 1; public RangingManager(IUwbAdapter adapter) { mAdapter = adapter; @@ -51,24 +52,17 @@ public class RangingManager extends android.uwb.IUwbRangingCallbacks.Stub { public CancellationSignal openSession(@NonNull PersistableBundle params, @NonNull Executor executor, @NonNull RangingSession.Callback callbacks) { - SessionHandle sessionHandle; - try { - sessionHandle = mAdapter.openRanging(this, params); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } - synchronized (this) { - if (hasSession(sessionHandle)) { - Log.w(TAG, "Newly created session unexpectedly reuses an active SessionHandle"); - executor.execute(() -> callbacks.onClosed( - RangingSession.Callback.REASON_GENERIC_ERROR, - new PersistableBundle())); - } - + SessionHandle sessionHandle = new SessionHandle(mNextSessionId++); RangingSession session = new RangingSession(executor, callbacks, mAdapter, sessionHandle); mRangingSessionTable.put(sessionHandle, session); + try { + mAdapter.openRanging(sessionHandle, this, params); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + CancellationSignal cancellationSignal = new CancellationSignal(); cancellationSignal.setOnCancelListener(() -> session.close()); return cancellationSignal; diff --git a/core/tests/uwbtests/src/android/uwb/RangingManagerTest.java b/core/tests/uwbtests/src/android/uwb/RangingManagerTest.java index c01bb75c32aaf..e41805dd3a59b 100644 --- a/core/tests/uwbtests/src/android/uwb/RangingManagerTest.java +++ b/core/tests/uwbtests/src/android/uwb/RangingManagerTest.java @@ -22,7 +22,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import android.os.PersistableBundle; import android.os.RemoteException; @@ -32,6 +31,7 @@ import androidx.test.filters.SmallTest; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import java.util.concurrent.Executor; @@ -42,51 +42,23 @@ import java.util.concurrent.Executor; @RunWith(AndroidJUnit4.class) public class RangingManagerTest { - private static final IUwbAdapter ADAPTER = mock(IUwbAdapter.class); private static final Executor EXECUTOR = UwbTestUtils.getExecutor(); private static final PersistableBundle PARAMS = new PersistableBundle(); private static final @RangingChangeReason int REASON = RangingChangeReason.UNKNOWN; @Test public void testOpenSession_OpenRangingInvoked() throws RemoteException { - RangingManager rangingManager = new RangingManager(ADAPTER); + IUwbAdapter adapter = mock(IUwbAdapter.class); + RangingManager rangingManager = new RangingManager(adapter); RangingSession.Callback callback = mock(RangingSession.Callback.class); rangingManager.openSession(PARAMS, EXECUTOR, callback); - verify(ADAPTER, times(1)).openRanging(eq(rangingManager), eq(PARAMS)); - } - - @Test - public void testOpenSession_ErrorIfSameSessionHandleReturned() throws RemoteException { - RangingManager rangingManager = new RangingManager(ADAPTER); - RangingSession.Callback callback = mock(RangingSession.Callback.class); - SessionHandle handle = new SessionHandle(1); - when(ADAPTER.openRanging(any(), any())).thenReturn(handle); - - rangingManager.openSession(PARAMS, EXECUTOR, callback); - - // Calling openSession will cause the same session handle to be returned. The onClosed - // callback should be invoked - RangingSession.Callback callback2 = mock(RangingSession.Callback.class); - rangingManager.openSession(PARAMS, EXECUTOR, callback2); - verify(callback, times(0)).onClosed(anyInt(), any()); - verify(callback2, times(1)).onClosed(anyInt(), any()); - } - - @Test - public void testOnRangingOpened_ValidSessionHandle() throws RemoteException { - RangingManager rangingManager = new RangingManager(ADAPTER); - RangingSession.Callback callback = mock(RangingSession.Callback.class); - SessionHandle handle = new SessionHandle(1); - when(ADAPTER.openRanging(any(), any())).thenReturn(handle); - - rangingManager.openSession(PARAMS, EXECUTOR, callback); - rangingManager.onRangingOpened(handle); - verify(callback, times(1)).onOpened(any()); + verify(adapter, times(1)).openRanging(any(), eq(rangingManager), eq(PARAMS)); } @Test public void testOnRangingOpened_InvalidSessionHandle() throws RemoteException { - RangingManager rangingManager = new RangingManager(ADAPTER); + IUwbAdapter adapter = mock(IUwbAdapter.class); + RangingManager rangingManager = new RangingManager(adapter); RangingSession.Callback callback = mock(RangingSession.Callback.class); rangingManager.onRangingOpened(new SessionHandle(2)); @@ -95,18 +67,20 @@ public class RangingManagerTest { @Test public void testOnRangingOpened_MultipleSessionsRegistered() throws RemoteException { - SessionHandle sessionHandle1 = new SessionHandle(1); - SessionHandle sessionHandle2 = new SessionHandle(2); + IUwbAdapter adapter = mock(IUwbAdapter.class); RangingSession.Callback callback1 = mock(RangingSession.Callback.class); RangingSession.Callback callback2 = mock(RangingSession.Callback.class); + ArgumentCaptor sessionHandleCaptor = + ArgumentCaptor.forClass(SessionHandle.class); - when(ADAPTER.openRanging(any(), any())) - .thenReturn(sessionHandle1) - .thenReturn(sessionHandle2); - - RangingManager rangingManager = new RangingManager(ADAPTER); + RangingManager rangingManager = new RangingManager(adapter); rangingManager.openSession(PARAMS, EXECUTOR, callback1); + verify(adapter, times(1)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle sessionHandle1 = sessionHandleCaptor.getValue(); + rangingManager.openSession(PARAMS, EXECUTOR, callback2); + verify(adapter, times(2)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle sessionHandle2 = sessionHandleCaptor.getValue(); rangingManager.onRangingOpened(sessionHandle1); verify(callback1, times(1)).onOpened(any()); @@ -119,12 +93,17 @@ public class RangingManagerTest { @Test public void testCorrectCallbackInvoked() throws RemoteException { - RangingManager rangingManager = new RangingManager(ADAPTER); + IUwbAdapter adapter = mock(IUwbAdapter.class); + RangingManager rangingManager = new RangingManager(adapter); RangingSession.Callback callback = mock(RangingSession.Callback.class); - SessionHandle handle = new SessionHandle(1); - when(ADAPTER.openRanging(any(), any())).thenReturn(handle); + + ArgumentCaptor sessionHandleCaptor = + ArgumentCaptor.forClass(SessionHandle.class); rangingManager.openSession(PARAMS, EXECUTOR, callback); + verify(adapter, times(1)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle handle = sessionHandleCaptor.getValue(); + rangingManager.onRangingOpened(handle); verify(callback, times(1)).onOpened(any()); @@ -156,20 +135,23 @@ public class RangingManagerTest { @Test public void testOnRangingClosed_MultipleSessionsRegistered() throws RemoteException { + IUwbAdapter adapter = mock(IUwbAdapter.class); // Verify that if multiple sessions are registered, only the session that is // requested to close receives the associated callbacks - SessionHandle sessionHandle1 = new SessionHandle(1); - SessionHandle sessionHandle2 = new SessionHandle(2); RangingSession.Callback callback1 = mock(RangingSession.Callback.class); RangingSession.Callback callback2 = mock(RangingSession.Callback.class); - when(ADAPTER.openRanging(any(), any())) - .thenReturn(sessionHandle1) - .thenReturn(sessionHandle2); + RangingManager rangingManager = new RangingManager(adapter); + ArgumentCaptor sessionHandleCaptor = + ArgumentCaptor.forClass(SessionHandle.class); - RangingManager rangingManager = new RangingManager(ADAPTER); rangingManager.openSession(PARAMS, EXECUTOR, callback1); + verify(adapter, times(1)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle sessionHandle1 = sessionHandleCaptor.getValue(); + rangingManager.openSession(PARAMS, EXECUTOR, callback2); + verify(adapter, times(2)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle sessionHandle2 = sessionHandleCaptor.getValue(); rangingManager.onRangingClosed(sessionHandle1, REASON, PARAMS); verify(callback1, times(1)).onClosed(anyInt(), any()); @@ -182,19 +164,22 @@ public class RangingManagerTest { @Test public void testOnRangingReport_MultipleSessionsRegistered() throws RemoteException { - SessionHandle sessionHandle1 = new SessionHandle(1); - SessionHandle sessionHandle2 = new SessionHandle(2); + IUwbAdapter adapter = mock(IUwbAdapter.class); RangingSession.Callback callback1 = mock(RangingSession.Callback.class); RangingSession.Callback callback2 = mock(RangingSession.Callback.class); - when(ADAPTER.openRanging(any(), any())) - .thenReturn(sessionHandle1) - .thenReturn(sessionHandle2); + ArgumentCaptor sessionHandleCaptor = + ArgumentCaptor.forClass(SessionHandle.class); - RangingManager rangingManager = new RangingManager(ADAPTER); + RangingManager rangingManager = new RangingManager(adapter); rangingManager.openSession(PARAMS, EXECUTOR, callback1); + verify(adapter, times(1)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle sessionHandle1 = sessionHandleCaptor.getValue(); + rangingManager.onRangingStarted(sessionHandle1, PARAMS); rangingManager.openSession(PARAMS, EXECUTOR, callback2); + verify(adapter, times(2)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle sessionHandle2 = sessionHandleCaptor.getValue(); rangingManager.onRangingStarted(sessionHandle2, PARAMS); rangingManager.onRangingResult(sessionHandle1, UwbTestUtils.getRangingReports(1)); @@ -232,17 +217,24 @@ public class RangingManagerTest { private void runReason(@RangingChangeReason int reasonIn, @RangingSession.Callback.Reason int reasonOut) throws RemoteException { - RangingManager rangingManager = new RangingManager(ADAPTER); + IUwbAdapter adapter = mock(IUwbAdapter.class); + RangingManager rangingManager = new RangingManager(adapter); RangingSession.Callback callback = mock(RangingSession.Callback.class); - SessionHandle handle = new SessionHandle(1); - when(ADAPTER.openRanging(any(), any())).thenReturn(handle); + + ArgumentCaptor sessionHandleCaptor = + ArgumentCaptor.forClass(SessionHandle.class); + rangingManager.openSession(PARAMS, EXECUTOR, callback); + verify(adapter, times(1)).openRanging(sessionHandleCaptor.capture(), any(), any()); + SessionHandle handle = sessionHandleCaptor.getValue(); rangingManager.onRangingOpenFailed(handle, reasonIn, PARAMS); verify(callback, times(1)).onOpenFailed(eq(reasonOut), eq(PARAMS)); // Open a new session rangingManager.openSession(PARAMS, EXECUTOR, callback); + verify(adapter, times(2)).openRanging(sessionHandleCaptor.capture(), any(), any()); + handle = sessionHandleCaptor.getValue(); rangingManager.onRangingOpened(handle); rangingManager.onRangingStartFailed(handle, reasonIn, PARAMS);