diff options
author | 2022-08-23 16:29:10 +0000 | |
---|---|---|
committer | 2022-08-23 16:35:00 +0000 | |
commit | 5189478291e79cfe9bb5aba30cbb7de856b6bfcc (patch) | |
tree | 3d816c3750af2c4ea230bcfb89c06a5c6e938843 | |
parent | b4ef6ab1e7c0f6c5b924a5812eaa40c8eeaab617 (diff) |
EventHub: Associate AsociatedDevice using sysfs path, not descriptor
EventHub previously attempted to link an EventHub device to an
AssociatedDevice - which contains info about the EventHub device
obtained through sysfs - through the descriptor. Since each EventHub
device has a unique descriptor whose uniqueness is guaranteed by the
use of the nonce value, using the descriptor to link the
AssociatedDevice to device means that no two devices will share the same
AssociatedDevice even if they have the same sysfs paths. This is clearly
not working as intended.
We now link the AssociatedDevice to the EventHub device by the sysfs
path so that separate EventHub devices that share the same sysfs path
also share the same AssociatedDevice.
Unfortunately, we don't have good code coverage for reading from sysfs,
and there are no existing test cases.
Bug: 243005009
Test: Manual, check input dump
Change-Id: Iea2bb7fd0493baa6e03df15f876c5d895c997b14
-rw-r--r-- | services/inputflinger/reader/EventHub.cpp | 34 | ||||
-rw-r--r-- | services/inputflinger/reader/include/EventHub.h | 5 |
2 files changed, 18 insertions, 21 deletions
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index bfa44acd17..db5032d51e 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -1060,7 +1060,7 @@ const std::vector<int32_t> EventHub::getRawBatteryIds(int32_t deviceId) { std::scoped_lock _l(mLock); std::vector<int32_t> batteryIds; - for (const auto [id, info] : getBatteryInfoLocked(deviceId)) { + for (const auto& [id, info] : getBatteryInfoLocked(deviceId)) { batteryIds.push_back(id); } @@ -1081,7 +1081,7 @@ std::optional<RawBatteryInfo> EventHub::getRawBatteryInfo(int32_t deviceId, int3 } // Gets the light info map from light ID to RawLightInfo of the miscellaneous device associated -// with the deivice ID. Returns an empty map if no miscellaneous device found. +// with the device ID. Returns an empty map if no miscellaneous device found. const std::unordered_map<int32_t, RawLightInfo>& EventHub::getLightInfoLocked( int32_t deviceId) const { static const std::unordered_map<int32_t, RawLightInfo> EMPTY_LIGHT_INFO = {}; @@ -1096,7 +1096,7 @@ const std::vector<int32_t> EventHub::getRawLightIds(int32_t deviceId) { std::scoped_lock _l(mLock); std::vector<int32_t> lightIds; - for (const auto [id, info] : getLightInfoLocked(deviceId)) { + for (const auto& [id, info] : getLightInfoLocked(deviceId)) { lightIds.push_back(id); } @@ -2042,23 +2042,20 @@ void EventHub::openDeviceLocked(const std::string& devicePath) { // Load the configuration file for the device. device->loadConfigurationLocked(); - bool hasBattery = false; - bool hasLights = false; // Check the sysfs root path - std::optional<std::filesystem::path> sysfsRootPath = getSysfsRootPath(devicePath.c_str()); + const std::optional<std::filesystem::path> sysfsRootPath = getSysfsRootPath(devicePath.c_str()); if (sysfsRootPath.has_value()) { std::shared_ptr<AssociatedDevice> associatedDevice; for (const auto& [id, dev] : mDevices) { - if (device->identifier.descriptor == dev->identifier.descriptor && - !dev->associatedDevice) { + if (dev->associatedDevice && dev->associatedDevice->sysfsRootPath == *sysfsRootPath) { associatedDevice = dev->associatedDevice; } } if (!associatedDevice) { associatedDevice = std::make_shared<AssociatedDevice>(sysfsRootPath.value()); + associatedDevice->configureBatteryLocked(); + associatedDevice->configureLightsLocked(); } - hasBattery = associatedDevice->configureBatteryLocked(); - hasLights = associatedDevice->configureLightsLocked(); device->associatedDevice = associatedDevice; } @@ -2212,12 +2209,12 @@ void EventHub::openDeviceLocked(const std::string& devicePath) { } // Classify InputDeviceClass::BATTERY. - if (hasBattery) { + if (device->associatedDevice && !device->associatedDevice->batteryInfos.empty()) { device->classes |= InputDeviceClass::BATTERY; } // Classify InputDeviceClass::LIGHT. - if (hasLights) { + if (device->associatedDevice && !device->associatedDevice->lightInfos.empty()) { device->classes |= InputDeviceClass::LIGHT; } @@ -2544,12 +2541,13 @@ void EventHub::dump(std::string& dump) { device->keyMap.keyCharacterMapFile.c_str()); dump += StringPrintf(INDENT3 "ConfigurationFile: %s\n", device->configurationFile.c_str()); - dump += INDENT3 "VideoDevice: "; - if (device->videoDevice) { - dump += device->videoDevice->dump() + "\n"; - } else { - dump += "<none>\n"; - } + dump += StringPrintf(INDENT3 "VideoDevice: %s\n", + device->videoDevice ? device->videoDevice->dump().c_str() + : "<none>"); + dump += StringPrintf(INDENT3 "SysfsDevicePath: %s\n", + device->associatedDevice + ? device->associatedDevice->sysfsRootPath.c_str() + : "<none>"); } dump += INDENT "Unattached video devices:\n"; diff --git a/services/inputflinger/reader/include/EventHub.h b/services/inputflinger/reader/include/EventHub.h index 4733ecb323..7d4196a05c 100644 --- a/services/inputflinger/reader/include/EventHub.h +++ b/services/inputflinger/reader/include/EventHub.h @@ -21,6 +21,7 @@ #include <climits> #include <filesystem> #include <unordered_map> +#include <utility> #include <vector> #include <batteryservice/BatteryService.h> @@ -537,8 +538,6 @@ public: private: struct AssociatedDevice { - // The device descriptor from evdev device the misc device associated with. - std::string descriptor; // The sysfs root path of the misc device. std::filesystem::path sysfsRootPath; @@ -582,7 +581,7 @@ private: int16_t ffEffectId; // initially -1 // A shared_ptr of a device associated with the input device. - // The input devices with same descriptor has the same associated device. + // The input devices that have the same sysfs path have the same associated device. std::shared_ptr<AssociatedDevice> associatedDevice; int32_t controllerNumber; |