Avoid UI freezing when reading battery capacity/status
Reading battery capacity and status could fail and end up
with timeout after 5s for some input devices, for example:
8BitDo SN30 Pro+ gamepad.
Before reading those items, it would be better to release
`mLock` so that other threads could run.
Bug: 244088945
Test: Manually verified no UI freezing when calling
`inputDevice.batteryState.capacity` or
`inputDevice.batteryState.status`
Change-Id: Ibb6b8c999b7fd4fe7fbcc57264a1fe6ad74903dc
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp
index 06a38c8..2f594d6 100644
--- a/services/inputflinger/reader/EventHub.cpp
+++ b/services/inputflinger/reader/EventHub.cpp
@@ -1516,25 +1516,35 @@
}
std::optional<int32_t> EventHub::getBatteryCapacity(int32_t deviceId, int32_t batteryId) const {
- std::scoped_lock _l(mLock);
+ std::filesystem::path batteryPath;
+ {
+ // Do not read the sysfs node to get the battery state while holding
+ // the EventHub lock. For some peripheral devices, reading battery state
+ // can be broken and take 5+ seconds. Holding the lock in this case would
+ // block all other event processing during this time. For now, we assume this
+ // call never happens on the InputReader thread and read the sysfs node outside
+ // the lock to prevent event processing from being blocked by this call.
+ std::scoped_lock _l(mLock);
- const auto infos = getBatteryInfoLocked(deviceId);
- auto it = infos.find(batteryId);
- if (it == infos.end()) {
- return std::nullopt;
- }
+ const auto infos = getBatteryInfoLocked(deviceId);
+ auto it = infos.find(batteryId);
+ if (it == infos.end()) {
+ return std::nullopt;
+ }
+ batteryPath = it->second.path;
+ } // release lock
+
std::string buffer;
// Some devices report battery capacity as an integer through the "capacity" file
- if (base::ReadFileToString(it->second.path / BATTERY_NODES.at(InputBatteryClass::CAPACITY),
+ if (base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::CAPACITY),
&buffer)) {
return std::stoi(base::Trim(buffer));
}
// Other devices report capacity as an enum value POWER_SUPPLY_CAPACITY_LEVEL_XXX
// These values are taken from kernel source code include/linux/power_supply.h
- if (base::ReadFileToString(it->second.path /
- BATTERY_NODES.at(InputBatteryClass::CAPACITY_LEVEL),
+ if (base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::CAPACITY_LEVEL),
&buffer)) {
// Remove any white space such as trailing new line
const auto levelIt = BATTERY_LEVEL.find(base::Trim(buffer));
@@ -1547,15 +1557,27 @@
}
std::optional<int32_t> EventHub::getBatteryStatus(int32_t deviceId, int32_t batteryId) const {
- std::scoped_lock _l(mLock);
- const auto infos = getBatteryInfoLocked(deviceId);
- auto it = infos.find(batteryId);
- if (it == infos.end()) {
- return std::nullopt;
- }
+ std::filesystem::path batteryPath;
+ {
+ // Do not read the sysfs node to get the battery state while holding
+ // the EventHub lock. For some peripheral devices, reading battery state
+ // can be broken and take 5+ seconds. Holding the lock in this case would
+ // block all other event processing during this time. For now, we assume this
+ // call never happens on the InputReader thread and read the sysfs node outside
+ // the lock to prevent event processing from being blocked by this call.
+ std::scoped_lock _l(mLock);
+
+ const auto infos = getBatteryInfoLocked(deviceId);
+ auto it = infos.find(batteryId);
+ if (it == infos.end()) {
+ return std::nullopt;
+ }
+ batteryPath = it->second.path;
+ } // release lock
+
std::string buffer;
- if (!base::ReadFileToString(it->second.path / BATTERY_NODES.at(InputBatteryClass::STATUS),
+ if (!base::ReadFileToString(batteryPath / BATTERY_NODES.at(InputBatteryClass::STATUS),
&buffer)) {
ALOGE("Failed to read sysfs battery info: %s", strerror(errno));
return std::nullopt;
diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp
index 5c9e63e..4ab55cb 100644
--- a/services/inputflinger/reader/InputDevice.cpp
+++ b/services/inputflinger/reader/InputDevice.cpp
@@ -548,14 +548,6 @@
for_each_mapper([when, readTime](InputMapper& mapper) { mapper.cancelTouch(when, readTime); });
}
-std::optional<int32_t> InputDevice::getBatteryCapacity() {
- return mController ? mController->getBatteryCapacity(DEFAULT_BATTERY_ID) : std::nullopt;
-}
-
-std::optional<int32_t> InputDevice::getBatteryStatus() {
- return mController ? mController->getBatteryStatus(DEFAULT_BATTERY_ID) : std::nullopt;
-}
-
bool InputDevice::setLightColor(int32_t lightId, int32_t color) {
return mController ? mController->setLightColor(lightId, color) : false;
}
@@ -623,6 +615,10 @@
for_each_mapper([reset](InputMapper& mapper) { mapper.updateLedState(reset); });
}
+std::optional<int32_t> InputDevice::getBatteryEventHubId() const {
+ return mController ? std::make_optional(mController->getEventHubId()) : std::nullopt;
+}
+
InputDeviceContext::InputDeviceContext(InputDevice& device, int32_t eventHubId)
: mDevice(device),
mContext(device.getContext()),
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index dc41051..4750d90 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -706,23 +706,43 @@
}
std::optional<int32_t> InputReader::getBatteryCapacity(int32_t deviceId) {
- std::scoped_lock _l(mLock);
+ std::optional<int32_t> eventHubId;
+ {
+ // Do not query the battery state while holding the lock. For some peripheral devices,
+ // reading battery state can be broken and take 5+ seconds. Holding the lock in this case
+ // would block all other event processing during this time. For now, we assume this
+ // call never happens on the InputReader thread and get the battery state outside the
+ // lock to prevent event processing from being blocked by this call.
+ std::scoped_lock _l(mLock);
+ InputDevice* device = findInputDeviceLocked(deviceId);
+ if (!device) return {};
+ eventHubId = device->getBatteryEventHubId();
+ } // release lock
- InputDevice* device = findInputDeviceLocked(deviceId);
- if (device) {
- return device->getBatteryCapacity();
- }
- return std::nullopt;
+ if (!eventHubId) return {};
+ const auto batteryIds = mEventHub->getRawBatteryIds(*eventHubId);
+ if (batteryIds.empty()) return {};
+ return mEventHub->getBatteryCapacity(*eventHubId, batteryIds.front());
}
std::optional<int32_t> InputReader::getBatteryStatus(int32_t deviceId) {
- std::scoped_lock _l(mLock);
+ std::optional<int32_t> eventHubId;
+ {
+ // Do not query the battery state while holding the lock. For some peripheral devices,
+ // reading battery state can be broken and take 5+ seconds. Holding the lock in this case
+ // would block all other event processing during this time. For now, we assume this
+ // call never happens on the InputReader thread and get the battery state outside the
+ // lock to prevent event processing from being blocked by this call.
+ std::scoped_lock _l(mLock);
+ InputDevice* device = findInputDeviceLocked(deviceId);
+ if (!device) return {};
+ eventHubId = device->getBatteryEventHubId();
+ } // release lock
- InputDevice* device = findInputDeviceLocked(deviceId);
- if (device) {
- return device->getBatteryStatus();
- }
- return std::nullopt;
+ if (!eventHubId) return {};
+ const auto batteryIds = mEventHub->getRawBatteryIds(*eventHubId);
+ if (batteryIds.empty()) return {};
+ return mEventHub->getBatteryStatus(*eventHubId, batteryIds.front());
}
std::vector<InputDeviceLightInfo> InputReader::getLights(int32_t deviceId) {
diff --git a/services/inputflinger/reader/controller/PeripheralController.cpp b/services/inputflinger/reader/controller/PeripheralController.cpp
index 7673174..eaf5b51 100644
--- a/services/inputflinger/reader/controller/PeripheralController.cpp
+++ b/services/inputflinger/reader/controller/PeripheralController.cpp
@@ -521,4 +521,8 @@
return light->getLightPlayerId();
}
+int32_t PeripheralController::getEventHubId() const {
+ return getDeviceContext().getEventHubId();
+}
+
} // namespace android
diff --git a/services/inputflinger/reader/controller/PeripheralController.h b/services/inputflinger/reader/controller/PeripheralController.h
index b1bc8c7..ac951eb 100644
--- a/services/inputflinger/reader/controller/PeripheralController.h
+++ b/services/inputflinger/reader/controller/PeripheralController.h
@@ -31,6 +31,7 @@
explicit PeripheralController(InputDeviceContext& deviceContext);
~PeripheralController() override;
+ int32_t getEventHubId() const override;
void populateDeviceInfo(InputDeviceInfo* deviceInfo) override;
void dump(std::string& dump) override;
bool setLightColor(int32_t lightId, int32_t color) override;
@@ -43,6 +44,7 @@
private:
inline int32_t getDeviceId() { return mDeviceContext.getId(); }
inline InputDeviceContext& getDeviceContext() { return mDeviceContext; }
+ inline InputDeviceContext& getDeviceContext() const { return mDeviceContext; }
InputDeviceContext& mDeviceContext;
void configureLights();
diff --git a/services/inputflinger/reader/controller/PeripheralControllerInterface.h b/services/inputflinger/reader/controller/PeripheralControllerInterface.h
index 7688a43..306e361 100644
--- a/services/inputflinger/reader/controller/PeripheralControllerInterface.h
+++ b/services/inputflinger/reader/controller/PeripheralControllerInterface.h
@@ -33,6 +33,8 @@
PeripheralControllerInterface() {}
virtual ~PeripheralControllerInterface() {}
+ virtual int32_t getEventHubId() const = 0;
+
// Interface methods for Battery
virtual std::optional<int32_t> getBatteryCapacity(int32_t batteryId) = 0;
virtual std::optional<int32_t> getBatteryStatus(int32_t batteryId) = 0;
diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h
index 51872ac..968f53a 100644
--- a/services/inputflinger/reader/include/InputDevice.h
+++ b/services/inputflinger/reader/include/InputDevice.h
@@ -32,8 +32,6 @@
#include "InputReaderContext.h"
namespace android {
-// TODO b/180733860 support multiple battery in API and remove this.
-constexpr int32_t DEFAULT_BATTERY_ID = 1;
class PeripheralController;
class PeripheralControllerInterface;
@@ -100,8 +98,7 @@
void disableSensor(InputDeviceSensorType sensorType);
void flushSensor(InputDeviceSensorType sensorType);
- std::optional<int32_t> getBatteryCapacity();
- std::optional<int32_t> getBatteryStatus();
+ std::optional<int32_t> getBatteryEventHubId() const;
bool setLightColor(int32_t lightId, int32_t color);
bool setLightPlayerId(int32_t lightId, int32_t playerId);