From d2bf2b4c2a052e48f29896aafdb7e05480beb64f Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Wed, 5 Oct 2022 20:23:40 +0000 Subject: [PATCH] BatteryController: Notify listeners from DeviceMonitor Notify listeners directly from DeviceMonitor on poll, UEvent, or on configuration changes whenever the reported state changes. Do not report any changes if only the timestamp changes. There should be no behavior changes in this CL. Bug: 243005009 Test: atest FrameworkServicesTests Change-Id: Ie234319ccf1b89532e7392e221bce5f5dd740fbe --- .../server/input/BatteryController.java | 161 +++++++++++------- 1 file changed, 104 insertions(+), 57 deletions(-) diff --git a/services/core/java/com/android/server/input/BatteryController.java b/services/core/java/com/android/server/input/BatteryController.java index 324eefc809e82..dcd47d4acba9b 100644 --- a/services/core/java/com/android/server/input/BatteryController.java +++ b/services/core/java/com/android/server/input/BatteryController.java @@ -44,6 +44,7 @@ import java.io.PrintWriter; import java.util.Arrays; import java.util.Objects; import java.util.Set; +import java.util.function.Consumer; /** * A thread-safe component of {@link InputManagerService} responsible for managing the battery state @@ -165,14 +166,15 @@ 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") @@ -192,6 +194,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)) @@ -252,7 +261,7 @@ final class BatteryController { if (!hasRegisteredListenerForDeviceLocked(deviceId)) { // There are no more listeners monitoring this device. final DeviceMonitor monitor = getDeviceMonitorOrThrowLocked(deviceId); - monitor.stopMonitoring(); + monitor.onMonitorDestroy(); mDeviceMonitors.remove(deviceId); } @@ -298,9 +307,7 @@ final class BatteryController { if (monitor == null) { return; } - if (monitor.updateBatteryState(eventTime)) { - notifyAllListenersForDeviceLocked(monitor.getBatteryStateForReporting()); - } + monitor.onUEvent(eventTime); } } @@ -310,14 +317,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 +329,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(); } } @@ -392,9 +388,7 @@ final class BatteryController { return; } final long eventTime = SystemClock.uptimeMillis(); - if (monitor.updateBatteryState(eventTime)) { - notifyAllListenersForDeviceLocked(monitor.getBatteryStateForReporting()); - } + monitor.onConfiguration(eventTime); } } }; @@ -422,8 +416,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 +427,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 +439,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 +483,36 @@ 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); + } + // Returns the current battery state that can be used to notify listeners BatteryController. public State getBatteryStateForReporting() { return new State(mState); @@ -503,8 +520,10 @@ 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"); } } @@ -548,18 +567,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 +603,24 @@ 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 + + "}"; } } }