From ec0c171735a2a8064589134e39a8249c72747967 Mon Sep 17 00:00:00 2001 From: Hugh Chen Date: Fri, 29 Apr 2022 08:40:21 +0000 Subject: [PATCH] Fix can't select the [USB Tethering] item in [Developer Options] - This CL is using 'USB_CONFIGURED' to check whether the USB enumeration is completed from the intent. If 'USB_CONFIGURED' is true then update the UI. Bug: 229200265 Test: make -j65 RunSettingsRoboTests Change-Id: Icab05e37ae3fcc9f1bf404a610fc97c368c453f5 --- .../usb/ConnectedUsbDeviceUpdater.java | 2 +- .../usb/UsbConnectionBroadcastReceiver.java | 7 +- .../usb/UsbDefaultFragment.java | 11 +-- .../usb/UsbDetailsFragment.java | 2 +- .../usb/ConnectedUsbDeviceUpdaterTest.java | 6 +- .../UsbConnectionBroadcastReceiverTest.java | 30 ++++++-- .../usb/UsbDefaultFragmentTest.java | 71 +++++++++++++------ 7 files changed, 92 insertions(+), 37 deletions(-) diff --git a/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdater.java b/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdater.java index dc73ee0ef17..54d039212e4 100644 --- a/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdater.java +++ b/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdater.java @@ -55,7 +55,7 @@ public class ConnectedUsbDeviceUpdater { @VisibleForTesting UsbConnectionBroadcastReceiver.UsbConnectionListener mUsbConnectionListener = - (connected, functions, powerRole, dataRole) -> { + (connected, functions, powerRole, dataRole, isUsbConfigured) -> { if (connected) { mUsbPreference.setSummary(getSummary(dataRole == DATA_ROLE_DEVICE ? functions : UsbManager.FUNCTION_NONE, powerRole)); diff --git a/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiver.java b/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiver.java index c73c957b93b..1a1f8ba4142 100644 --- a/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiver.java +++ b/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiver.java @@ -61,6 +61,8 @@ public class UsbConnectionBroadcastReceiver extends BroadcastReceiver implements if (DEBUG) { Log.d(TAG, "onReceive() action : " + intent.getAction()); } + boolean isUsbConfigured = intent.getExtras() != null + ? intent.getExtras().getBoolean(UsbManager.USB_CONFIGURED) : false; if (UsbManager.ACTION_USB_STATE.equals(intent.getAction())) { mConnected = intent.getExtras().getBoolean(UsbManager.USB_CONNECTED) || intent.getExtras().getBoolean(UsbManager.USB_HOST_CONNECTED); @@ -98,7 +100,7 @@ public class UsbConnectionBroadcastReceiver extends BroadcastReceiver implements } if (mUsbConnectionListener != null) { mUsbConnectionListener.onUsbConnectionChanged(mConnected, mFunctions, mPowerRole, - mDataRole); + mDataRole, isUsbConfigured); } } @@ -142,6 +144,7 @@ public class UsbConnectionBroadcastReceiver extends BroadcastReceiver implements * Interface definition for a callback to be invoked when usb connection is changed. */ interface UsbConnectionListener { - void onUsbConnectionChanged(boolean connected, long functions, int powerRole, int dataRole); + void onUsbConnectionChanged(boolean connected, long functions, int powerRole, int dataRole, + boolean isUsbConfigured); } } diff --git a/src/com/android/settings/connecteddevice/usb/UsbDefaultFragment.java b/src/com/android/settings/connecteddevice/usb/UsbDefaultFragment.java index d58e97b3cee..b5a9e5a9317 100644 --- a/src/com/android/settings/connecteddevice/usb/UsbDefaultFragment.java +++ b/src/com/android/settings/connecteddevice/usb/UsbDefaultFragment.java @@ -69,22 +69,23 @@ public class UsbDefaultFragment extends RadioButtonPickerFragment { @VisibleForTesting UsbConnectionBroadcastReceiver.UsbConnectionListener mUsbConnectionListener = - (connected, functions, powerRole, dataRole) -> { + (connected, functions, powerRole, dataRole, isUsbConfigured) -> { final long defaultFunctions = mUsbBackend.getDefaultUsbFunctions(); Log.d(TAG, "UsbConnectionListener() connected : " + connected + ", functions : " + functions + ", defaultFunctions : " + defaultFunctions - + ", mIsStartTethering : " + mIsStartTethering); - if (connected && !mIsConnected && (defaultFunctions == UsbManager.FUNCTION_RNDIS + + ", mIsStartTethering : " + mIsStartTethering + + ", isUsbConfigured : " + isUsbConfigured); + if (connected && !mIsConnected && ((defaultFunctions == UsbManager.FUNCTION_RNDIS || defaultFunctions == UsbManager.FUNCTION_NCM) + && defaultFunctions == functions) && !mIsStartTethering) { mCurrentFunctions = defaultFunctions; startTethering(); } - if (mIsStartTethering && connected) { + if ((mIsStartTethering || isUsbConfigured) && connected) { mCurrentFunctions = functions; refresh(functions); - mUsbBackend.setDefaultUsbFunctions(functions); mIsStartTethering = false; } mIsConnected = connected; diff --git a/src/com/android/settings/connecteddevice/usb/UsbDetailsFragment.java b/src/com/android/settings/connecteddevice/usb/UsbDetailsFragment.java index 8850acd4b35..0c94d192f61 100644 --- a/src/com/android/settings/connecteddevice/usb/UsbDetailsFragment.java +++ b/src/com/android/settings/connecteddevice/usb/UsbDetailsFragment.java @@ -50,7 +50,7 @@ public class UsbDetailsFragment extends DashboardFragment { UsbConnectionBroadcastReceiver mUsbReceiver; private UsbConnectionBroadcastReceiver.UsbConnectionListener mUsbConnectionListener = - (connected, functions, powerRole, dataRole) -> { + (connected, functions, powerRole, dataRole, isUsbFigured) -> { for (UsbDetailsController controller : mControllers) { controller.refresh(connected, functions, powerRole, dataRole); } diff --git a/tests/robotests/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdaterTest.java b/tests/robotests/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdaterTest.java index 5c66db7266c..1b2ef9ac6e8 100644 --- a/tests/robotests/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdaterTest.java +++ b/tests/robotests/src/com/android/settings/connecteddevice/usb/ConnectedUsbDeviceUpdaterTest.java @@ -97,7 +97,8 @@ public class ConnectedUsbDeviceUpdaterTest { public void initUsbPreference_usbConnected_preferenceAdded() { mDeviceUpdater.initUsbPreference(mContext); mDeviceUpdater.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_NONE, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_NONE, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); verify(mDevicePreferenceCallback).onDeviceAdded(mDeviceUpdater.mUsbPreference); } @@ -106,7 +107,8 @@ public class ConnectedUsbDeviceUpdaterTest { public void initUsbPreference_usbDisconnected_preferenceRemoved() { mDeviceUpdater.initUsbPreference(mContext); mDeviceUpdater.mUsbConnectionListener.onUsbConnectionChanged(false /* connected */, - UsbManager.FUNCTION_NONE, POWER_ROLE_NONE, DATA_ROLE_NONE); + UsbManager.FUNCTION_NONE, POWER_ROLE_NONE, DATA_ROLE_NONE, + /* isUsbConfigured= */ true); verify(mDevicePreferenceCallback).onDeviceRemoved(mDeviceUpdater.mUsbPreference); } diff --git a/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiverTest.java b/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiverTest.java index 21ec48e0f06..7cd7dcce099 100644 --- a/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiverTest.java +++ b/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbConnectionBroadcastReceiverTest.java @@ -65,11 +65,12 @@ public class UsbConnectionBroadcastReceiverTest { final Intent intent = new Intent(); intent.setAction(UsbManager.ACTION_USB_STATE); intent.putExtra(UsbManager.USB_CONNECTED, true); + intent.putExtra(UsbManager.USB_CONFIGURED, true); mReceiver.onReceive(mContext, intent); verify(mListener).onUsbConnectionChanged(true /* connected */, UsbManager.FUNCTION_NONE, - POWER_ROLE_NONE, DATA_ROLE_NONE); + POWER_ROLE_NONE, DATA_ROLE_NONE, /* isUsbConfigured= */ true); } @Test @@ -77,11 +78,12 @@ public class UsbConnectionBroadcastReceiverTest { final Intent intent = new Intent(); intent.setAction(UsbManager.ACTION_USB_STATE); intent.putExtra(UsbManager.USB_CONNECTED, false); + intent.putExtra(UsbManager.USB_CONFIGURED, true); mReceiver.onReceive(mContext, intent); verify(mListener).onUsbConnectionChanged(false /* connected */, UsbManager.FUNCTION_NONE, - POWER_ROLE_NONE, DATA_ROLE_NONE); + POWER_ROLE_NONE, DATA_ROLE_NONE, /* isUsbConfigured= */ true); } @Test @@ -91,11 +93,12 @@ public class UsbConnectionBroadcastReceiverTest { intent.putExtra(UsbManager.USB_CONNECTED, true); intent.putExtra(UsbManager.USB_FUNCTION_MTP, true); intent.putExtra(UsbManager.USB_DATA_UNLOCKED, true); + intent.putExtra(UsbManager.USB_CONFIGURED, true); mReceiver.onReceive(mContext, intent); verify(mListener).onUsbConnectionChanged(true /* connected */, UsbManager.FUNCTION_MTP, - POWER_ROLE_NONE, DATA_ROLE_NONE); + POWER_ROLE_NONE, DATA_ROLE_NONE, /* isUsbConfigured= */ true); } @Test @@ -105,26 +108,41 @@ public class UsbConnectionBroadcastReceiverTest { intent.putExtra(UsbManager.USB_CONNECTED, true); intent.putExtra(UsbManager.USB_FUNCTION_NCM, true); intent.putExtra(UsbManager.USB_DATA_UNLOCKED, true); + intent.putExtra(UsbManager.USB_CONFIGURED, true); mReceiver.onReceive(mContext, intent); verify(mListener).onUsbConnectionChanged(/* connected */ true, UsbManager.FUNCTION_NCM, - POWER_ROLE_NONE, DATA_ROLE_NONE); + POWER_ROLE_NONE, DATA_ROLE_NONE, /* isUsbConfigured= */ true); } @Test - public void onReceive_usbPortStatus_invokeCallback() { + public void onReceive_usbPortStatus_invokesCallback() { final Intent intent = new Intent(); intent.setAction(UsbManager.ACTION_USB_PORT_CHANGED); final UsbPortStatus status = new UsbPortStatus(0, POWER_ROLE_SINK, DATA_ROLE_DEVICE, 0, CONTAMINANT_PROTECTION_NONE, CONTAMINANT_DETECTION_NOT_SUPPORTED); intent.putExtra(UsbManager.EXTRA_PORT_STATUS, status); + intent.putExtra(UsbManager.USB_CONFIGURED, true); mReceiver.onReceive(mContext, intent); verify(mListener).onUsbConnectionChanged(false /* connected */, UsbManager.FUNCTION_NONE, - POWER_ROLE_SINK, DATA_ROLE_DEVICE); + POWER_ROLE_SINK, DATA_ROLE_DEVICE, /* isUsbConfigured= */ true); + } + + @Test + public void onReceive_usbConfiguredIsFalse_invokesCallback() { + final Intent intent = new Intent(); + intent.setAction(UsbManager.ACTION_USB_STATE); + intent.putExtra(UsbManager.USB_CONNECTED, true); + intent.putExtra(UsbManager.USB_CONFIGURED, false); + + mReceiver.onReceive(mContext, intent); + + verify(mListener).onUsbConnectionChanged(true /* connected */, UsbManager.FUNCTION_NONE, + POWER_ROLE_NONE, DATA_ROLE_NONE, /* isUsbConfigured= */ false); } @Test diff --git a/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbDefaultFragmentTest.java b/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbDefaultFragmentTest.java index 9afc6778c92..71bbff24db6 100644 --- a/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbDefaultFragmentTest.java +++ b/tests/robotests/src/com/android/settings/connecteddevice/usb/UsbDefaultFragmentTest.java @@ -27,7 +27,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -204,12 +203,13 @@ public class UsbDefaultFragmentTest { public void onPause_receivedRndis_shouldSetRndis() { mFragment.mIsStartTethering = true; mFragment.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); when(mUsbBackend.getCurrentFunctions()).thenReturn(UsbManager.FUNCTION_RNDIS); mFragment.onPause(); - verify(mUsbBackend, times(2)).setDefaultUsbFunctions(UsbManager.FUNCTION_RNDIS); + verify(mUsbBackend).setDefaultUsbFunctions(UsbManager.FUNCTION_RNDIS); assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_RNDIS); } @@ -217,11 +217,12 @@ public class UsbDefaultFragmentTest { public void onPause_receivedNone_shouldSetNone() { mFragment.mIsStartTethering = true; mFragment.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_NONE, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_NONE, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); mFragment.onPause(); - verify(mUsbBackend, times(2)).setDefaultUsbFunctions(UsbManager.FUNCTION_NONE); + verify(mUsbBackend).setDefaultUsbFunctions(UsbManager.FUNCTION_NONE); assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_NONE); } @@ -229,12 +230,13 @@ public class UsbDefaultFragmentTest { public void onPause_receivedMtp_shouldSetMtp() { mFragment.mIsStartTethering = true; mFragment.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_MTP, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_MTP, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); when(mUsbBackend.getCurrentFunctions()).thenReturn(UsbManager.FUNCTION_MTP); mFragment.onPause(); - verify(mUsbBackend, times(2)).setDefaultUsbFunctions(UsbManager.FUNCTION_MTP); + verify(mUsbBackend).setDefaultUsbFunctions(UsbManager.FUNCTION_MTP); assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_MTP); } @@ -242,12 +244,13 @@ public class UsbDefaultFragmentTest { public void onPause_receivedPtp_shouldSetPtp() { mFragment.mIsStartTethering = true; mFragment.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_PTP, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_PTP, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); when(mUsbBackend.getCurrentFunctions()).thenReturn(UsbManager.FUNCTION_PTP); mFragment.onPause(); - verify(mUsbBackend, times(2)).setDefaultUsbFunctions(UsbManager.FUNCTION_PTP); + verify(mUsbBackend).setDefaultUsbFunctions(UsbManager.FUNCTION_PTP); assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_PTP); } @@ -255,12 +258,13 @@ public class UsbDefaultFragmentTest { public void onPause_receivedMidi_shouldSetMidi() { mFragment.mIsStartTethering = true; mFragment.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_MIDI, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_MIDI, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); when(mUsbBackend.getCurrentFunctions()).thenReturn(UsbManager.FUNCTION_MIDI); mFragment.onPause(); - verify(mUsbBackend, times(2)).setDefaultUsbFunctions(UsbManager.FUNCTION_MIDI); + verify(mUsbBackend).setDefaultUsbFunctions(UsbManager.FUNCTION_MIDI); assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_MIDI); } @@ -268,12 +272,13 @@ public class UsbDefaultFragmentTest { public void onPause_receivedNcm_setsNcm() { mFragment.mIsStartTethering = true; mFragment.mUsbConnectionListener.onUsbConnectionChanged(/* connected */ true, - UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); when(mUsbBackend.getCurrentFunctions()).thenReturn(UsbManager.FUNCTION_NCM); mFragment.onPause(); - verify(mUsbBackend, times(2)).setDefaultUsbFunctions(UsbManager.FUNCTION_NCM); + verify(mUsbBackend).setDefaultUsbFunctions(UsbManager.FUNCTION_NCM); assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_NCM); } @@ -282,9 +287,11 @@ public class UsbDefaultFragmentTest { when(mUsbBackend.getDefaultUsbFunctions()).thenReturn(UsbManager.FUNCTION_RNDIS); mFragment.mUsbConnectionListener.onUsbConnectionChanged(false /* connected */, - UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); mFragment.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); verify(mTetheringManager).startTethering(eq(TetheringManager.TETHERING_USB), any(), @@ -296,9 +303,11 @@ public class UsbDefaultFragmentTest { when(mUsbBackend.getDefaultUsbFunctions()).thenReturn(UsbManager.FUNCTION_NCM); mFragment.mUsbConnectionListener.onUsbConnectionChanged(/* connected */ false, - UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); mFragment.mUsbConnectionListener.onUsbConnectionChanged(/* connected */ true, - UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); verify(mTetheringManager).startTethering(eq(TetheringManager.TETHERING_USB), any(), @@ -310,7 +319,8 @@ public class UsbDefaultFragmentTest { when(mUsbBackend.getDefaultUsbFunctions()).thenReturn(UsbManager.FUNCTION_RNDIS); mFragment.mUsbConnectionListener.onUsbConnectionChanged(false /* connected */, - UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); verify(mTetheringManager, never()).startTethering(eq(TetheringManager.TETHERING_USB), any(), @@ -323,15 +333,36 @@ public class UsbDefaultFragmentTest { when(mUsbBackend.getDefaultUsbFunctions()).thenReturn(UsbManager.FUNCTION_RNDIS); mFragment.mUsbConnectionListener.onUsbConnectionChanged(false /* connected */, - UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); mFragment.mUsbConnectionListener.onUsbConnectionChanged(true /* connected */, - UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE); + UsbManager.FUNCTION_RNDIS, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); verify(mTetheringManager, never()).startTethering(eq(TetheringManager.TETHERING_USB), any(), eq(mFragment.mOnStartTetheringCallback)); } + @Test + public void onUsbConnectionChanged_usbConfiguredIsTrue_updatesCurrentFunctions() { + mFragment.mCurrentFunctions = UsbManager.FUNCTION_NONE; + mFragment.mUsbConnectionListener.onUsbConnectionChanged(/* connected= */ true, + UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ true); + assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_NCM); + } + + @Test + public void onUsbConnectionChanged_usbConfiguredIsFalse_doesNotUpdateCurrentFunctions() { + mFragment.mCurrentFunctions = UsbManager.FUNCTION_NONE; + mFragment.mUsbConnectionListener.onUsbConnectionChanged(/* connected= */ true, + UsbManager.FUNCTION_NCM, POWER_ROLE_SINK, DATA_ROLE_DEVICE, + /* isUsbConfigured= */ false); + assertThat(mFragment.mCurrentFunctions).isEqualTo(UsbManager.FUNCTION_NONE); + } + + public static class TestFragment extends UsbDefaultFragment { public final PreferenceScreen mScreen;