diff --git a/services/core/java/com/android/server/input/BatteryController.java b/services/core/java/com/android/server/input/BatteryController.java index 324eefc809e82..06ee7c4d6c286 100644 --- a/services/core/java/com/android/server/input/BatteryController.java +++ b/services/core/java/com/android/server/input/BatteryController.java @@ -44,6 +44,8 @@ import java.io.PrintWriter; import java.util.Arrays; import java.util.Objects; import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Predicate; /** * A thread-safe component of {@link InputManagerService} responsible for managing the battery state @@ -98,8 +100,12 @@ final class BatteryController { } public void systemRunning() { - Objects.requireNonNull(mContext.getSystemService(InputManager.class)) - .registerInputDeviceListener(mInputDeviceListener, mHandler); + final InputManager inputManager = + Objects.requireNonNull(mContext.getSystemService(InputManager.class)); + inputManager.registerInputDeviceListener(mInputDeviceListener, mHandler); + for (int deviceId : inputManager.getInputDeviceIds()) { + mInputDeviceListener.onInputDeviceAdded(deviceId); + } } /** @@ -165,19 +171,20 @@ final class BatteryController { } } - @GuardedBy("mLock") - private void notifyAllListenersForDeviceLocked(State state) { - if (DEBUG) Slog.d(TAG, "Notifying all listeners of battery state: " + state); - mListenerRecords.forEach((pid, listenerRecord) -> { - if (listenerRecord.mMonitoredDevices.contains(state.deviceId)) { - notifyBatteryListener(listenerRecord, state); - } - }); + private void notifyAllListenersForDevice(State state) { + synchronized (mLock) { + if (DEBUG) Slog.d(TAG, "Notifying all listeners of battery state: " + state); + mListenerRecords.forEach((pid, listenerRecord) -> { + if (listenerRecord.mMonitoredDevices.contains(state.deviceId)) { + notifyBatteryListener(listenerRecord, state); + } + }); + } } @GuardedBy("mLock") private void updatePollingLocked(boolean delayStart) { - if (mDeviceMonitors.isEmpty() || !mIsInteractive) { + if (!mIsInteractive || !anyOf(mDeviceMonitors, DeviceMonitor::requiresPolling)) { // Stop polling. mIsPolling = false; mHandler.removeCallbacks(this::handlePollEvent); @@ -192,6 +199,13 @@ final class BatteryController { mHandler.postDelayed(this::handlePollEvent, delayStart ? POLLING_PERIOD_MILLIS : 0); } + private String getInputDeviceName(int deviceId) { + final InputDevice device = + Objects.requireNonNull(mContext.getSystemService(InputManager.class)) + .getInputDevice(deviceId); + return device != null ? device.getName() : ""; + } + private boolean hasBattery(int deviceId) { final InputDevice device = Objects.requireNonNull(mContext.getSystemService(InputManager.class)) @@ -199,6 +213,13 @@ final class BatteryController { return device != null && device.hasBattery(); } + private boolean isUsiDevice(int deviceId) { + final InputDevice device = + Objects.requireNonNull(mContext.getSystemService(InputManager.class)) + .getInputDevice(deviceId); + return device != null && device.supportsUsi(); + } + @GuardedBy("mLock") private DeviceMonitor getDeviceMonitorOrThrowLocked(int deviceId) { return Objects.requireNonNull(mDeviceMonitors.get(deviceId), @@ -252,8 +273,10 @@ final class BatteryController { if (!hasRegisteredListenerForDeviceLocked(deviceId)) { // There are no more listeners monitoring this device. final DeviceMonitor monitor = getDeviceMonitorOrThrowLocked(deviceId); - monitor.stopMonitoring(); - mDeviceMonitors.remove(deviceId); + if (!monitor.isPersistent()) { + monitor.onMonitorDestroy(); + mDeviceMonitors.remove(deviceId); + } } if (listenerRecord.mMonitoredDevices.isEmpty()) { @@ -298,9 +321,7 @@ final class BatteryController { if (monitor == null) { return; } - if (monitor.updateBatteryState(eventTime)) { - notifyAllListenersForDeviceLocked(monitor.getBatteryStateForReporting()); - } + monitor.onUEvent(eventTime); } } @@ -310,14 +331,7 @@ final class BatteryController { return; } final long eventTime = SystemClock.uptimeMillis(); - mDeviceMonitors.forEach((deviceId, monitor) -> { - // Re-acquire lock in the lambda to silence error-prone build warnings. - synchronized (mLock) { - if (monitor.updateBatteryState(eventTime)) { - notifyAllListenersForDeviceLocked(monitor.getBatteryStateForReporting()); - } - } - }); + mDeviceMonitors.forEach((deviceId, monitor) -> monitor.onPoll(eventTime)); mHandler.postDelayed(this::handlePollEvent, POLLING_PERIOD_MILLIS); } } @@ -329,15 +343,11 @@ final class BatteryController { final DeviceMonitor monitor = mDeviceMonitors.get(deviceId); if (monitor == null) { // The input device's battery is not being monitored by any listener. - return queryBatteryStateFromNative(deviceId, updateTime); + return queryBatteryStateFromNative(deviceId, updateTime, hasBattery(deviceId)); } // Force the battery state to update, and notify listeners if necessary. - final boolean stateChanged = monitor.updateBatteryState(updateTime); - final State state = monitor.getBatteryStateForReporting(); - if (stateChanged) { - notifyAllListenersForDeviceLocked(state); - } - return state; + monitor.onPoll(updateTime); + return monitor.getBatteryStateForReporting(); } } @@ -379,7 +389,14 @@ final class BatteryController { private final InputManager.InputDeviceListener mInputDeviceListener = new InputManager.InputDeviceListener() { @Override - public void onInputDeviceAdded(int deviceId) {} + public void onInputDeviceAdded(int deviceId) { + synchronized (mLock) { + if (isUsiDevice(deviceId) && !mDeviceMonitors.containsKey(deviceId)) { + // Start monitoring USI device immediately. + mDeviceMonitors.put(deviceId, new UsiDeviceMonitor(deviceId)); + } + } + } @Override public void onInputDeviceRemoved(int deviceId) {} @@ -392,9 +409,7 @@ final class BatteryController { return; } final long eventTime = SystemClock.uptimeMillis(); - if (monitor.updateBatteryState(eventTime)) { - notifyAllListenersForDeviceLocked(monitor.getBatteryStateForReporting()); - } + monitor.onConfiguration(eventTime); } } }; @@ -422,8 +437,7 @@ final class BatteryController { } // Queries the battery state of an input device from native code. - private State queryBatteryStateFromNative(int deviceId, long updateTime) { - final boolean isPresent = hasBattery(deviceId); + private State queryBatteryStateFromNative(int deviceId, long updateTime, boolean isPresent) { return new State( deviceId, updateTime, @@ -434,8 +448,9 @@ final class BatteryController { // Holds the state of an InputDevice for which battery changes are currently being monitored. private class DeviceMonitor { - @NonNull - private State mState; + private final State mState; + // Represents whether the input device has a sysfs battery node. + protected boolean mHasBattery = false; @Nullable private UEventBatteryListener mUEventBatteryListener; @@ -445,26 +460,32 @@ final class BatteryController { // Load the initial battery state and start monitoring. final long eventTime = SystemClock.uptimeMillis(); - updateBatteryState(eventTime); + configureDeviceMonitor(eventTime); } - // Returns true if the battery state changed since the last time it was updated. - public boolean updateBatteryState(long updateTime) { - mState.updateTime = updateTime; - - final State updatedState = queryBatteryStateFromNative(mState.deviceId, updateTime); - if (mState.equals(updatedState)) { - return false; + private void processChangesAndNotify(long eventTime, Consumer changes) { + final State oldState = getBatteryStateForReporting(); + changes.accept(eventTime); + final State newState = getBatteryStateForReporting(); + if (!oldState.equals(newState)) { + notifyAllListenersForDevice(newState); } - if (mState.isPresent != updatedState.isPresent) { - if (updatedState.isPresent) { + } + + public void onConfiguration(long eventTime) { + processChangesAndNotify(eventTime, this::configureDeviceMonitor); + } + + private void configureDeviceMonitor(long eventTime) { + if (mHasBattery != hasBattery(mState.deviceId)) { + mHasBattery = !mHasBattery; + if (mHasBattery) { startMonitoring(); } else { stopMonitoring(); } + updateBatteryStateFromNative(eventTime); } - mState = updatedState; - return true; } private void startMonitoring() { @@ -483,19 +504,44 @@ final class BatteryController { mUEventBatteryListener, "DEVPATH=" + formatDevPath(batteryPath)); } - private String formatDevPath(String path) { + private String formatDevPath(@NonNull String path) { // Remove the "/sys" prefix if it has one. return path.startsWith("/sys") ? path.substring(4) : path; } - // This must be called when the device is no longer being monitored. - public void stopMonitoring() { + private void stopMonitoring() { if (mUEventBatteryListener != null) { mUEventManager.removeListener(mUEventBatteryListener); mUEventBatteryListener = null; } } + // This must be called when the device is no longer being monitored. + public void onMonitorDestroy() { + stopMonitoring(); + } + + private void updateBatteryStateFromNative(long eventTime) { + mState.updateIfChanged( + queryBatteryStateFromNative(mState.deviceId, eventTime, mHasBattery)); + } + + public void onPoll(long eventTime) { + processChangesAndNotify(eventTime, this::updateBatteryStateFromNative); + } + + public void onUEvent(long eventTime) { + processChangesAndNotify(eventTime, this::updateBatteryStateFromNative); + } + + public boolean requiresPolling() { + return true; + } + + public boolean isPersistent() { + return false; + } + // Returns the current battery state that can be used to notify listeners BatteryController. public State getBatteryStateForReporting() { return new State(mState); @@ -503,8 +549,31 @@ final class BatteryController { @Override public String toString() { - return "state=" + mState - + ", uEventListener=" + (mUEventBatteryListener != null ? "added" : "none"); + return "DeviceId=" + mState.deviceId + + ", Name='" + getInputDeviceName(mState.deviceId) + "'" + + ", NativeBattery=" + mState + + ", UEventListener=" + (mUEventBatteryListener != null ? "added" : "none"); + } + } + + // Battery monitoring logic that is specific to stylus devices that support the + // Universal Stylus Initiative (USI) protocol. + private class UsiDeviceMonitor extends DeviceMonitor { + + UsiDeviceMonitor(int deviceId) { + super(deviceId); + } + + @Override + public boolean requiresPolling() { + // Do not poll the battery state for USI devices. + return false; + } + + @Override + public boolean isPersistent() { + // Do not remove the battery monitor for USI devices. + return true; } } @@ -548,18 +617,33 @@ final class BatteryController { private static class State extends IInputDeviceBatteryState { State(int deviceId) { - initialize(deviceId, 0 /*updateTime*/, false /*isPresent*/, BatteryState.STATUS_UNKNOWN, - Float.NaN /*capacity*/); + reset(deviceId); } State(IInputDeviceBatteryState s) { - initialize(s.deviceId, s.updateTime, s.isPresent, s.status, s.capacity); + copyFrom(s); } State(int deviceId, long updateTime, boolean isPresent, int status, float capacity) { initialize(deviceId, updateTime, isPresent, status, capacity); } + // Updates this from other if there is a difference between them, ignoring the updateTime. + public void updateIfChanged(IInputDeviceBatteryState other) { + if (!equalsIgnoringUpdateTime(other)) { + copyFrom(other); + } + } + + private void reset(int deviceId) { + initialize(deviceId, 0 /*updateTime*/, false /*isPresent*/, BatteryState.STATUS_UNKNOWN, + Float.NaN /*capacity*/); + } + + private void copyFrom(IInputDeviceBatteryState s) { + initialize(s.deviceId, s.updateTime, s.isPresent, s.status, s.capacity); + } + private void initialize(int deviceId, long updateTime, boolean isPresent, int status, float capacity) { this.deviceId = deviceId; @@ -569,11 +653,34 @@ final class BatteryController { this.capacity = capacity; } + private boolean equalsIgnoringUpdateTime(IInputDeviceBatteryState other) { + long updateTime = this.updateTime; + this.updateTime = other.updateTime; + boolean eq = this.equals(other); + this.updateTime = updateTime; + return eq; + } + @Override public String toString() { - return "BatteryState{deviceId=" + deviceId + ", updateTime=" + updateTime - + ", isPresent=" + isPresent + ", status=" + status + ", capacity=" + capacity - + " }"; + if (!isPresent) { + return "State{}"; + } + return "State{time=" + updateTime + + ", isPresent=" + isPresent + + ", status=" + status + + ", capacity=" + capacity + + "}"; } } + + // Check if any value in an ArrayMap matches the predicate in an optimized way. + private static boolean anyOf(ArrayMap arrayMap, Predicate test) { + for (int i = 0; i < arrayMap.size(); i++) { + if (test.test(arrayMap.valueAt(i))) { + return true; + } + } + return false; + } } diff --git a/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt b/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt index 65076a372b0b9..b095a506af211 100644 --- a/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt +++ b/services/tests/servicestests/src/com/android/server/input/BatteryControllerTests.kt @@ -32,6 +32,7 @@ import android.os.test.TestLooper import android.platform.test.annotations.Presubmit import android.view.InputDevice import androidx.test.InstrumentationRegistry +import com.android.server.input.BatteryController.POLLING_PERIOD_MILLIS import com.android.server.input.BatteryController.UEventManager import com.android.server.input.BatteryController.UEventManager.UEventBatteryListener import org.hamcrest.Description @@ -42,6 +43,8 @@ import org.hamcrest.TypeSafeMatcher import org.hamcrest.core.IsEqual.equalTo import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Assert.fail import org.junit.Before import org.junit.Rule @@ -63,14 +66,20 @@ import org.mockito.hamcrest.MockitoHamcrest import org.mockito.junit.MockitoJUnit import org.mockito.verification.VerificationMode -private fun createInputDevice(deviceId: Int, hasBattery: Boolean = true): InputDevice = +private fun createInputDevice( + deviceId: Int, + hasBattery: Boolean = true, + supportsUsi: Boolean = false, + generation: Int = -1, +): InputDevice = InputDevice.Builder() .setId(deviceId) .setName("Device $deviceId") .setDescriptor("descriptor $deviceId") .setExternal(true) .setHasBattery(hasBattery) - .setGeneration(0) + .setSupportsUsi(supportsUsi) + .setGeneration(generation) .build() // Returns a matcher that helps match member variables of a class. @@ -118,7 +127,10 @@ private fun matchesState( return Matchers.allOf(batteryStateMatchers) } -// Helper used to verify interactions with a mocked battery listener. +private fun isInvalidBatteryState(deviceId: Int): Matcher = + matchesState(deviceId, isPresent = false, status = STATUS_UNKNOWN, capacity = Float.NaN) + +// Helpers used to verify interactions with a mocked battery listener. private fun IInputDeviceBatteryListener.verifyNotified( deviceId: Int, mode: VerificationMode = times(1), @@ -127,8 +139,21 @@ private fun IInputDeviceBatteryListener.verifyNotified( capacity: Float? = null, eventTime: Long? = null ) { - verify(this, mode).onBatteryStateChanged( - MockitoHamcrest.argThat(matchesState(deviceId, isPresent, status, capacity, eventTime))) + verifyNotified(matchesState(deviceId, isPresent, status, capacity, eventTime), mode) +} + +private fun IInputDeviceBatteryListener.verifyNotified( + matcher: Matcher, + mode: VerificationMode = times(1) +) { + verify(this, mode).onBatteryStateChanged(MockitoHamcrest.argThat(matcher)) +} + +private fun createMockListener(): IInputDeviceBatteryListener { + val listener = mock(IInputDeviceBatteryListener::class.java) + val binder = mock(Binder::class.java) + `when`(listener.asBinder()).thenReturn(binder) + return listener } /** @@ -143,6 +168,8 @@ class BatteryControllerTests { const val PID = 42 const val DEVICE_ID = 13 const val SECOND_DEVICE_ID = 11 + const val USI_DEVICE_ID = 101 + const val SECOND_USI_DEVICE_ID = 102 const val TIMESTAMP = 123456789L } @@ -168,10 +195,11 @@ class BatteryControllerTests { testLooper = TestLooper() val inputManager = InputManager.resetInstance(iInputManager) `when`(context.getSystemService(eq(Context.INPUT_SERVICE))).thenReturn(inputManager) - `when`(iInputManager.inputDeviceIds).thenReturn(intArrayOf(DEVICE_ID, SECOND_DEVICE_ID)) - `when`(iInputManager.getInputDevice(DEVICE_ID)).thenReturn(createInputDevice(DEVICE_ID)) - `when`(iInputManager.getInputDevice(SECOND_DEVICE_ID)) - .thenReturn(createInputDevice(SECOND_DEVICE_ID)) + `when`(iInputManager.inputDeviceIds).then { + deviceGenerationMap.keys.toIntArray() + } + addInputDevice(DEVICE_ID) + addInputDevice(SECOND_DEVICE_ID) batteryController = BatteryController(context, native, testLooper.looper, uEventManager) batteryController.systemRunning() @@ -180,10 +208,30 @@ class BatteryControllerTests { devicesChangedListener = listenerCaptor.value } - private fun notifyDeviceChanged(deviceId: Int) { - deviceGenerationMap[deviceId] = deviceGenerationMap[deviceId]?.plus(1) ?: 1 + private fun notifyDeviceChanged( + deviceId: Int, + hasBattery: Boolean = true, + supportsUsi: Boolean = false + ) { + val generation = deviceGenerationMap[deviceId]?.plus(1) + ?: throw IllegalArgumentException("Device $deviceId was never added!") + deviceGenerationMap[deviceId] = generation + + `when`(iInputManager.getInputDevice(deviceId)) + .thenReturn(createInputDevice(deviceId, hasBattery, supportsUsi, generation)) val list = deviceGenerationMap.flatMap { listOf(it.key, it.value) } - devicesChangedListener.onInputDevicesChanged(list.toIntArray()) + if (::devicesChangedListener.isInitialized) { + devicesChangedListener.onInputDevicesChanged(list.toIntArray()) + } + } + + private fun addInputDevice( + deviceId: Int, + hasBattery: Boolean = true, + supportsUsi: Boolean = false + ) { + deviceGenerationMap[deviceId] = 0 + notifyDeviceChanged(deviceId, hasBattery, supportsUsi) } @After @@ -191,13 +239,6 @@ class BatteryControllerTests { InputManager.clearInstance() } - private fun createMockListener(): IInputDeviceBatteryListener { - val listener = mock(IInputDeviceBatteryListener::class.java) - val binder = mock(Binder::class.java) - `when`(listener.asBinder()).thenReturn(binder) - return listener - } - @Test fun testRegisterAndUnregisterBinderLifecycle() { val listener = createMockListener() @@ -303,19 +344,14 @@ class BatteryControllerTests { listener.verifyNotified(DEVICE_ID, status = STATUS_CHARGING, capacity = 0.78f) // If the battery presence for the InputDevice changes, the listener is notified. - `when`(iInputManager.getInputDevice(DEVICE_ID)) - .thenReturn(createInputDevice(DEVICE_ID, hasBattery = false)) - notifyDeviceChanged(DEVICE_ID) + notifyDeviceChanged(DEVICE_ID, hasBattery = false) testLooper.dispatchNext() - listener.verifyNotified(DEVICE_ID, isPresent = false, status = STATUS_UNKNOWN, - capacity = Float.NaN) + listener.verifyNotified(isInvalidBatteryState(DEVICE_ID)) // Since the battery is no longer present, the UEventListener should be removed. verify(uEventManager).removeListener(uEventListener.value) // If the battery becomes present again, the listener is notified. - `when`(iInputManager.getInputDevice(DEVICE_ID)) - .thenReturn(createInputDevice(DEVICE_ID, hasBattery = true)) - notifyDeviceChanged(DEVICE_ID) + notifyDeviceChanged(DEVICE_ID, hasBattery = true) testLooper.dispatchNext() listener.verifyNotified(DEVICE_ID, mode = times(2), status = STATUS_CHARGING, capacity = 0.78f) @@ -340,9 +376,17 @@ class BatteryControllerTests { // Move the time forward so that the polling period has elapsed. // The listener should be notified. - testLooper.moveTimeForward(BatteryController.POLLING_PERIOD_MILLIS - 1) + testLooper.moveTimeForward(POLLING_PERIOD_MILLIS - 1) + assertTrue("There should be a polling callbacks posted to the handler", testLooper.isIdle) testLooper.dispatchNext() listener.verifyNotified(DEVICE_ID, capacity = 0.80f) + + // Move the time forward so that another polling period has elapsed. + // The battery should still be polled, but there is no change so listeners are not notified. + testLooper.moveTimeForward(POLLING_PERIOD_MILLIS) + assertTrue("There should be a polling callbacks posted to the handler", testLooper.isIdle) + testLooper.dispatchNext() + listener.verifyNotified(DEVICE_ID, mode = times(1), capacity = 0.80f) } @Test @@ -357,7 +401,8 @@ class BatteryControllerTests { // The battery state changed, but we should not be polling for battery changes when the // device is not interactive. `when`(native.getBatteryCapacity(DEVICE_ID)).thenReturn(80) - testLooper.moveTimeForward(BatteryController.POLLING_PERIOD_MILLIS) + testLooper.moveTimeForward(POLLING_PERIOD_MILLIS) + assertFalse("There should be no polling callbacks posted to the handler", testLooper.isIdle) testLooper.dispatchAll() listener.verifyNotified(DEVICE_ID, mode = never(), capacity = 0.80f) @@ -368,7 +413,8 @@ class BatteryControllerTests { // Ensure that we continue to poll for battery changes. `when`(native.getBatteryCapacity(DEVICE_ID)).thenReturn(90) - testLooper.moveTimeForward(BatteryController.POLLING_PERIOD_MILLIS) + testLooper.moveTimeForward(POLLING_PERIOD_MILLIS) + assertTrue("There should be a polling callbacks posted to the handler", testLooper.isIdle) testLooper.dispatchNext() listener.verifyNotified(DEVICE_ID, capacity = 0.90f) } @@ -398,4 +444,44 @@ class BatteryControllerTests { matchesState(DEVICE_ID, status = STATUS_CHARGING, capacity = 0.80f)) listener.verifyNotified(DEVICE_ID, status = STATUS_CHARGING, capacity = 0.80f) } + + @Test + fun testUsiDeviceIsMonitoredPersistently() { + `when`(native.getBatteryDevicePath(USI_DEVICE_ID)).thenReturn("/sys/dev/usi_device") + addInputDevice(USI_DEVICE_ID, supportsUsi = true) + testLooper.dispatchNext() + + // Even though there is no listener added for this device, it is being monitored. + val uEventListener = ArgumentCaptor.forClass(UEventBatteryListener::class.java) + verify(uEventManager) + .addListener(uEventListener.capture(), eq("DEVPATH=/dev/usi_device")) + + // Add and remove a listener for the device. + val listener = createMockListener() + batteryController.registerBatteryListener(USI_DEVICE_ID, listener, PID) + batteryController.unregisterBatteryListener(USI_DEVICE_ID, listener, PID) + + // The device is still being monitored. + verify(uEventManager, never()).removeListener(uEventListener.value) + } + + @Test + fun testNoPollingWhenUsiDevicesAreMonitored() { + `when`(native.getBatteryDevicePath(USI_DEVICE_ID)).thenReturn("/sys/dev/usi_device") + addInputDevice(USI_DEVICE_ID, supportsUsi = true) + testLooper.dispatchNext() + `when`(native.getBatteryDevicePath(SECOND_USI_DEVICE_ID)).thenReturn("/sys/dev/usi_device2") + addInputDevice(SECOND_USI_DEVICE_ID, supportsUsi = true) + testLooper.dispatchNext() + + testLooper.moveTimeForward(POLLING_PERIOD_MILLIS) + assertFalse("There should be no polling callbacks posted to the handler", testLooper.isIdle) + + // Add a listener. + val listener = createMockListener() + batteryController.registerBatteryListener(USI_DEVICE_ID, listener, PID) + + testLooper.moveTimeForward(POLLING_PERIOD_MILLIS) + assertFalse("There should be no polling callbacks posted to the handler", testLooper.isIdle) + } }