From 06d81efbdf448b9b2203395c26d5f35b3cbf9f03 Mon Sep 17 00:00:00 2001 From: Vania Januar Date: Thu, 2 Mar 2023 11:04:24 +0000 Subject: [PATCH] Combine StylusCallback and StylusBatteryCallback. These were initially separate to allow listening to battery events separately from InputDevice events. However, in practice, these events are always listened to together in StylusManager. Separating the two does not provide any performance improvement, and has in fact caused bugs (ag/21382305). Bug: 269088577 Test: StylusManagerTest, StylusUsiPowerStartableTest, StylusBluetoothPowerStartableTest Change-Id: I2c83d8e7db93601ec9d32fc6b868396d073910f8 --- .../android/systemui/stylus/StylusManager.kt | 25 ++------------- .../stylus/StylusUsiPowerStartable.kt | 3 +- .../systemui/stylus/StylusManagerTest.kt | 31 +++---------------- .../stylus/StylusUsiPowerStartableTest.kt | 1 - 4 files changed, 8 insertions(+), 52 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/stylus/StylusManager.kt b/packages/SystemUI/src/com/android/systemui/stylus/StylusManager.kt index 030c54fbd87d9..9952cfd4e85bb 100644 --- a/packages/SystemUI/src/com/android/systemui/stylus/StylusManager.kt +++ b/packages/SystemUI/src/com/android/systemui/stylus/StylusManager.kt @@ -62,8 +62,6 @@ constructor( BluetoothAdapter.OnMetadataChangedListener { private val stylusCallbacks: CopyOnWriteArrayList = CopyOnWriteArrayList() - private val stylusBatteryCallbacks: CopyOnWriteArrayList = - CopyOnWriteArrayList() // This map should only be accessed on the handler private val inputDeviceAddressMap: MutableMap = ArrayMap() @@ -106,14 +104,6 @@ constructor( stylusCallbacks.remove(callback) } - fun registerBatteryCallback(callback: StylusBatteryCallback) { - stylusBatteryCallbacks.add(callback) - } - - fun unregisterBatteryCallback(callback: StylusBatteryCallback) { - stylusBatteryCallbacks.remove(callback) - } - override fun onInputDeviceAdded(deviceId: Int) { if (!hasStarted) return @@ -195,7 +185,7 @@ constructor( "${device.address}: $isCharging" } - executeStylusBatteryCallbacks { cb -> + executeStylusCallbacks { cb -> cb.onStylusBluetoothChargingStateChanged(inputDeviceId, device, isCharging) } } @@ -221,7 +211,7 @@ constructor( onStylusUsed() } - executeStylusBatteryCallbacks { cb -> + executeStylusCallbacks { cb -> cb.onStylusUsiBatteryStateChanged(deviceId, eventTimeMillis, batteryState) } } @@ -329,10 +319,6 @@ constructor( stylusCallbacks.forEach(run) } - private fun executeStylusBatteryCallbacks(run: (cb: StylusBatteryCallback) -> Unit) { - stylusBatteryCallbacks.forEach(run) - } - private fun registerBatteryListener(deviceId: Int) { try { inputManager.addInputDeviceBatteryListener(deviceId, executor, this) @@ -378,13 +364,6 @@ constructor( fun onStylusBluetoothConnected(deviceId: Int, btAddress: String) {} fun onStylusBluetoothDisconnected(deviceId: Int, btAddress: String) {} fun onStylusFirstUsed() {} - } - - /** - * Callback interface to receive stylus battery events from the StylusManager. All callbacks are - * runs on the same background handler. - */ - interface StylusBatteryCallback { fun onStylusBluetoothChargingStateChanged( inputDeviceId: Int, btDevice: BluetoothDevice, diff --git a/packages/SystemUI/src/com/android/systemui/stylus/StylusUsiPowerStartable.kt b/packages/SystemUI/src/com/android/systemui/stylus/StylusUsiPowerStartable.kt index 27cafb10c07df..3667392b515e0 100644 --- a/packages/SystemUI/src/com/android/systemui/stylus/StylusUsiPowerStartable.kt +++ b/packages/SystemUI/src/com/android/systemui/stylus/StylusUsiPowerStartable.kt @@ -37,7 +37,7 @@ constructor( private val inputManager: InputManager, private val stylusUsiPowerUi: StylusUsiPowerUI, private val featureFlags: FeatureFlags, -) : CoreStartable, StylusManager.StylusCallback, StylusManager.StylusBatteryCallback { +) : CoreStartable, StylusManager.StylusCallback { override fun onStylusAdded(deviceId: Int) { // On some devices, the addition of a new internal stylus indicates the use of a @@ -74,7 +74,6 @@ constructor( stylusUsiPowerUi.init() stylusManager.registerCallback(this) - stylusManager.registerBatteryCallback(this) stylusManager.startListener() } diff --git a/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusManagerTest.kt index f8bf4b91e11aa..4525ad27b7492 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusManagerTest.kt @@ -65,8 +65,6 @@ class StylusManagerTest : SysuiTestCase() { @Mock lateinit var uiEventLogger: UiEventLogger @Mock lateinit var stylusCallback: StylusManager.StylusCallback @Mock lateinit var otherStylusCallback: StylusManager.StylusCallback - @Mock lateinit var stylusBatteryCallback: StylusManager.StylusBatteryCallback - @Mock lateinit var otherStylusBatteryCallback: StylusManager.StylusBatteryCallback private lateinit var mockitoSession: StaticMockitoSession private lateinit var stylusManager: StylusManager @@ -123,7 +121,6 @@ class StylusManagerTest : SysuiTestCase() { stylusManager.startListener() stylusManager.registerCallback(stylusCallback) - stylusManager.registerBatteryCallback(stylusBatteryCallback) clearInvocations(inputManager) } @@ -433,23 +430,6 @@ class StylusManagerTest : SysuiTestCase() { .logWithInstanceId(StylusUiEvent.BLUETOOTH_STYLUS_DISCONNECTED, 0, null, instanceId) } - @Test - fun onMetadataChanged_multipleRegisteredBatteryCallbacks_executesAll() { - stylusManager.onInputDeviceAdded(BT_STYLUS_DEVICE_ID) - stylusManager.registerBatteryCallback(otherStylusBatteryCallback) - - stylusManager.onMetadataChanged( - bluetoothDevice, - BluetoothDevice.METADATA_MAIN_CHARGING, - "true".toByteArray() - ) - - verify(stylusBatteryCallback, times(1)) - .onStylusBluetoothChargingStateChanged(BT_STYLUS_DEVICE_ID, bluetoothDevice, true) - verify(otherStylusBatteryCallback, times(1)) - .onStylusBluetoothChargingStateChanged(BT_STYLUS_DEVICE_ID, bluetoothDevice, true) - } - @Test fun onMetadataChanged_chargingStateTrue_executesBatteryCallbacks() { stylusManager.onInputDeviceAdded(BT_STYLUS_DEVICE_ID) @@ -460,7 +440,7 @@ class StylusManagerTest : SysuiTestCase() { "true".toByteArray() ) - verify(stylusBatteryCallback, times(1)) + verify(stylusCallback, times(1)) .onStylusBluetoothChargingStateChanged(BT_STYLUS_DEVICE_ID, bluetoothDevice, true) } @@ -474,7 +454,7 @@ class StylusManagerTest : SysuiTestCase() { "false".toByteArray() ) - verify(stylusBatteryCallback, times(1)) + verify(stylusCallback, times(1)) .onStylusBluetoothChargingStateChanged(BT_STYLUS_DEVICE_ID, bluetoothDevice, false) } @@ -486,7 +466,7 @@ class StylusManagerTest : SysuiTestCase() { "true".toByteArray() ) - verifyNoMoreInteractions(stylusBatteryCallback) + verifyNoMoreInteractions(stylusCallback) } @Test @@ -499,8 +479,7 @@ class StylusManagerTest : SysuiTestCase() { "true".toByteArray() ) - verify(stylusBatteryCallback, never()) - .onStylusBluetoothChargingStateChanged(any(), any(), any()) + verify(stylusCallback, never()).onStylusBluetoothChargingStateChanged(any(), any(), any()) } @Test @@ -614,7 +593,7 @@ class StylusManagerTest : SysuiTestCase() { fun onBatteryStateChanged_executesBatteryCallbacks() { stylusManager.onBatteryStateChanged(STYLUS_DEVICE_ID, 1, batteryState) - verify(stylusBatteryCallback, times(1)) + verify(stylusCallback, times(1)) .onStylusUsiBatteryStateChanged(STYLUS_DEVICE_ID, 1, batteryState) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusUsiPowerStartableTest.kt b/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusUsiPowerStartableTest.kt index 82b80f53d6b08..3db0ecc4e8df7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusUsiPowerStartableTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/stylus/StylusUsiPowerStartableTest.kt @@ -96,7 +96,6 @@ class StylusUsiPowerStartableTest : SysuiTestCase() { startable.start() verify(stylusManager, times(1)).registerCallback(startable) - verify(stylusManager, times(1)).registerBatteryCallback(startable) } @Test