Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 51894782 authored by Prabir Pradhan's avatar Prabir Pradhan
Browse files

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
parent b4ef6ab1
Loading
Loading
Loading
Loading
+16 −18
Original line number Diff line number Diff line
@@ -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";
+2 −3
Original line number Diff line number Diff line
@@ -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;