Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

input-device: selection by id or by path #2504

Open
zougloub opened this issue Oct 30, 2024 · 5 comments
Open

input-device: selection by id or by path #2504

zougloub opened this issue Oct 30, 2024 · 5 comments

Comments

@zougloub
Copy link

This may relate to #2481

The code in get_input_device_section only matches an input-device section suffixed by dev->name, which is a problem if devices have the same name.

We'd need a syntax to specify a name by id or by path, maybe we can use a /dev/input/by-* path and check if the path exists, and resolve the corresponding device.

@zougloub
Copy link
Author

Example udev properties:

P: /devices/pci0000:00/0000:00:15.0/usb1/1-1/1-1.3
M: 1-1.3
R: 3
U: usb
T: usb_device
D: c 189:63
N: bus/usb/001/064
L: 0
V: usb
E: DEVPATH=/devices/pci0000:00/0000:00:15.0/usb1/1-1/1-1.3
E: SUBSYSTEM=usb
E: DEVNAME=/dev/bus/usb/001/064
E: DEVTYPE=usb_device
E: DRIVER=usb
E: PRODUCT=424f/9301/101
E: TYPE=239/2/1
E: BUSNUM=001
E: DEVNUM=064
E: MAJOR=189
E: MINOR=63
E: USEC_INITIALIZED=118584265597
E: ID_BUS=usb
E: ID_MODEL=SiW_HID_Touch_Controller
E: ID_MODEL_ENC=SiW\x20HID\x20Touch\x20Controller
E: ID_MODEL_ID=9301
E: ID_SERIAL=Siliconworks_SiW_HID_Touch_Controller
E: ID_VENDOR=Siliconworks
E: ID_VENDOR_ENC=Siliconworks
E: ID_VENDOR_ID=424f
E: ID_REVISION=0101
E: ID_USB_MODEL=SiW_HID_Touch_Controller
E: ID_USB_MODEL_ENC=SiW\x20HID\x20Touch\x20Controller
E: ID_USB_MODEL_ID=9301
E: ID_USB_SERIAL=Siliconworks_SiW_HID_Touch_Controller
E: ID_USB_VENDOR=Siliconworks
E: ID_USB_VENDOR_ENC=Siliconworks
E: ID_USB_VENDOR_ID=424f
E: ID_USB_REVISION=0101
E: ID_USB_INTERFACES=:030000:
E: ID_PATH_WITH_USB_REVISION=pci-0000:00:15.0-usbv2-0:1.3
E: ID_PATH=pci-0000:00:15.0-usb-0:1.3
E: ID_PATH_TAG=pci-0000_00_15_0-usb-0_1_3

This device must be matched by path because the constructor did not add a iSerialNumber on the USB descriptors.

I propose to match in the input-device: configuration section some properties (at least ID_PATH or ID_PATH_TAG depending on the : friendliness of config), so we can do

[input-device:pci-0000:00:15.0-usb-0:1.3]

In order to uniquely match a peripheral.

@zougloub
Copy link
Author

Using @soreau 's WIP, we can configure:

[input-device:pci-0000:00:15.0-usb-0:2.3:1.0]
output = HDMI-A-1
calibration = 1.0 0.0 0.0 0.0 1.0 0.0

[input-device:pci-0000:00:15.0-usb-0:1.3:1.0]
output = HDMI-A-2
calibration = 1.0 0.0 0.0 0.0 1.0 0.0

Giving:

DD 31-10-24 17:53:08.170 - [src/core/plugin.cpp:68] Checking for input-device config section: [input-device:pci-0000:00:15.0-usb-0:1.3:1.0]
II 31-10-24 17:53:08.170 - [src/core/seat/input-manager.cpp:108] Calibrated input device successfully: Siliconworks SiW HID Touch Controller
II 31-10-24 17:53:08.170 - [src/core/seat/input-manager.cpp:109]     1 0 0 0 1 0
DD 31-10-24 17:53:08.170 - [src/core/seat/input-manager.cpp:124] Mapping input Siliconworks SiW HID Touch Controller to output HDMI-A-2.
DD 31-10-24 17:53:08.171 - [src/core/plugin.cpp:68] Checking for input-device config section: [input-device:pci-0000:00:15.0-usb-0:2.3:1.0]
II 31-10-24 17:53:08.171 - [src/core/seat/input-manager.cpp:108] Calibrated input device successfully: Siliconworks SiW HID Touch Controller
II 31-10-24 17:53:08.171 - [src/core/seat/input-manager.cpp:109]     1 0 0 0 1 0
DD 31-10-24 17:53:08.171 - [src/core/seat/input-manager.cpp:124] Mapping input Siliconworks SiW HID Touch Controller to output HDMI-A-1.

@soreau
Copy link
Member

soreau commented Nov 3, 2024

This is an overview and status of the WIP branch in question.

In its current state, a udev handle for the device is retrieved using libinput and libudev. It loops through the udev properties until it matches ID_PATH. It then checks if an input-device section matches in the config file [input-device:$id_path]. If nothing is matched, it falls back to the existing name check, and if no name matches are found, uses the default [input-device] section.

The intent is to check for ID_PATH, then ID_SERIAL (and possibly followed by MAC if that property is available), then device name and finally the default input-device section. The log messages are important as well. The idea would be to output something in the debug log to the effect of Checking for [input-device:$property_value] and if matched, Found matching section [input-device:$property_value] for "$device_name".

This WIP branch also includes the ability to calibrate devices with the syntax calibration = f f f f f f where f is a float value in the calibration matrix.

There are reportedly many devices that @zougloub wants to test, so after the patch has been tested thoroughly, I will submit a pull request for inclusion to master branch of wayfire.

@zougloub
Copy link
Author

zougloub commented Nov 4, 2024

@soreau please consider the following changes:

diff --git a/src/core/plugin.cpp b/src/core/plugin.cpp
index c59f53b6..3f65aaae 100644
--- a/src/core/plugin.cpp
+++ b/src/core/plugin.cpp
@@ -28,29 +28,13 @@ std::shared_ptr<config::section_t> wf::config_backend_t::get_output_section(
     return config.get_section(name);
 }
 
-static const char *get_udev_id_path(udev_device *dev)
-{
-    const char *udev_id_path    = NULL;
-    udev_list_entry *properties = udev_device_get_properties_list_entry(dev);
-    while (properties)
-    {
-        const char *property_name  = udev_list_entry_get_name(properties);
-        const char *property_value = udev_list_entry_get_value(properties);
-        if (!strcmp(property_name, "ID_PATH"))
-        {
-            udev_id_path = property_value;
-        }
-
-        properties = udev_list_entry_get_next(properties);
-    }
-
-    return udev_id_path;
-}
 
 std::shared_ptr<config::section_t> wf::config_backend_t::get_input_device_section(
     wlr_input_device *device)
 {
-    std::string name = nonull(device->name);
+    auto& config = wf::get_core().config;
+    std::shared_ptr<wf::config::section_t> section;
+
     if (wlr_input_device_is_libinput(device))
     {
         auto libinput_dev = wlr_libinput_get_device_handle(device);
@@ -59,24 +43,51 @@ std::shared_ptr<config::section_t> wf::config_backend_t::get_input_device_sectio
             udev_device *udev_dev = libinput_device_get_udev_device(libinput_dev);
             if (udev_dev)
             {
-                name = nonull(get_udev_id_path(udev_dev));
+                struct property_and_desc {
+                    char const * property_name;
+                    char const * description;
+                } properties_and_descs [] =
+                {
+                    { "ID_PATH", "stable physical connection path" },
+                    { "ID_SERIAL", "stable vendor+pn+sn info" },
+                    { "LIBINPUT_DEVICE_GROUP", "stable libinput info" },
+                    // sometimes it contains info "by path", sometimes "by id"
+                    //{ "DEVPATH", "unstable devpath" },
+                    // used for debugging, to find DEVPATH and match the right
+                    // device in `udevadm info --tree`
+                };
+
+                for (struct property_and_desc const & pd : properties_and_descs)
+                {
+                    const char * value = udev_device_get_property_value(udev_dev, pd.property_name);
+                    if (value == nullptr)
+                    {
+                        continue;
+                    }
+                    std::string name = std::string("input-device:") + nonull(value);
+                    LOGD("Checking config section [", name, "] for ", pd.property_name, " (", pd.description, ")");
+                    section = config.get_section(name);
+                    if (section)
+                    {
+                        return section;
+                        break;
+                    }
+                }
             }
         }
     }
 
+    std::string name = nonull(device->name);
     name = "input-device:" + name;
-    LOGD("Checking for input-device config section: [", name, "]");
-    auto& config = wf::get_core().config;
-    if (!config.get_section(name))
+    LOGD("Checking config section [", name, "]");
+    section = config.get_section(name);
+    if (section)
     {
-        name = nonull(device->name);
+        return section;
     }
 
-    if (!config.get_section(name))
-    {
-        config.merge_section(
-            config.get_section("input-device")->clone_with_name(name));
-    }
+    config.merge_section(
+        config.get_section("input-device")->clone_with_name(name));
 
     return config.get_section(name);
 }

Here are some example outputs:

  • This is a USB barcode scanner whose USB descriptors have vendor, part number, and serial number; we can match it by these unambiguous stable fields with the second test
DD 03-11-24 22:04:21.924 - [src/core/plugin.cpp:68] Checking config section [input-device:pci-0000:00:14.0-usb-0:4.3:1.0] for ID_PATH (stable physical connection path)                                  
DD 03-11-24 22:04:21.924 - [src/core/plugin.cpp:68] Checking config section [input-device:ᄅSymbol_Technologies__Inc__2002_Symbol_Bar_Code_Scanner_S_N:09CE708D9D1CD04381448C2B774E7565_Rev:NBRPPAAQ3] for
 ID_SERIAL (stable vendor+pn+sn info)                                                                                                                                                                    
DD 03-11-24 22:04:21.924 - [src/core/plugin.cpp:68] Checking config section [input-device:3/5e0/1200:usb-0000:00:14.0-4] for LIBINPUT_DEVICE_GROUP (stable libinput info)                                
DD 03-11-24 22:04:21.924 - [src/core/plugin.cpp:82] Checking config section [input-device:ᄅSymbol Technologies, Inc, 2002 Symbol Bar Code Scanner]       
DD 03-11-24 22:04:21.924 - [src/core/seat/input-manager.cpp:128] Mapping input ᄅSymbol Technologies, Inc, 2002 Symbol Bar Code Scanner to output null.
  • This is a touchscreen USB controller, for a ViewSonic TD2455 touchscreen, where the vendor didn't bother assigning a serial number to the USB descriptors, making identifying the device from its USB connection path the only way to get stable path, in case there are more of the same touchscreens connected:
DD 03-11-24 22:09:26.346 - [src/core/plugin.cpp:68] Checking config section [input-device:pci-0000:00:14.0-usb-0:4.4.3:1.0] for ID_PATH (stable physical connection path)
DD 03-11-24 22:09:26.346 - [src/core/plugin.cpp:68] Checking config section [input-device:Siliconworks_SiW_HID_Touch_Controller] for ID_SERIAL (stable vendor+pn+sn info)
DD 03-11-24 22:09:26.346 - [src/core/plugin.cpp:68] Checking config section [input-device:3/424f/9301:usb-0000:00:14.0-4.4] for LIBINPUT_DEVICE_GROUP (stable libinput info)
DD 03-11-24 22:09:26.346 - [src/core/plugin.cpp:82] Checking config section [input-device:Siliconworks SiW HID Touch Controller]
DD 03-11-24 22:09:26.346 - [src/core/seat/input-manager.cpp:128] Mapping input Siliconworks SiW HID Touch Controller to output null.
  • This is a Yubikey NFC ; they also forgot to consider putting a serial number on the USB descriptors
DD 03-11-24 22:09:26.345 - [src/core/plugin.cpp:68] Checking config section [input-device:pci-0000:00:14.0-usb-0:1:1.0] for ID_PATH (stable physical connection path)       
DD 03-11-24 22:09:26.345 - [src/core/plugin.cpp:68] Checking config section [input-device:Yubico_YubiKey_OTP+FIDO+CCID] for ID_SERIAL (stable vendor+pn+sn info)
DD 03-11-24 22:09:26.345 - [src/core/plugin.cpp:68] Checking config section [input-device:3/1050/407:usb-0000:00:14.0-1] for LIBINPUT_DEVICE_GROUP (stable libinput info)
DD 03-11-24 22:09:26.345 - [src/core/plugin.cpp:82] Checking config section [input-device:Yubico YubiKey OTP+FIDO+CCID]
DD 03-11-24 22:09:26.345 - [src/core/seat/input-manager.cpp:128] Mapping input Yubico YubiKey OTP+FIDO+CCID to output null.
  • This is a Bluetooth mouse ; here the LIBINPUT_DEVICE_GROUP catches the bluetooth MAC address, which is also a stable thing to identify the device with, if there were many.
DD 03-11-24 22:16:38.335 - [src/core/plugin.cpp:68] Checking config section [input-device:5/46d/b020:34:12:57:68:ab:cd] for LIBINPUT_DEVICE_GROUP (stable libinput info)
DD 03-11-24 22:16:38.335 - [src/core/plugin.cpp:82] Checking config section [input-device:Logitech MX Vertical Advanced Ergonomic Mouse]
DD 03-11-24 22:16:38.335 - [src/core/seat/input-manager.cpp:128] Mapping input Logitech MX Vertical Advanced Ergonomic Mouse to output null.

@soreau
Copy link
Member

soreau commented Nov 4, 2024

I have pushed this patch as-is to input-device-updates branch.

soreau added a commit that referenced this issue Nov 6, 2024
This patch adds additional checks for input-device sections in the config
file. It checks for ID_PATH, ID_SERIAL, LIBINPUT_DEVICE_GROUP then falls
back to the device name and finally the global input-device section. In
addition, DEVPATH is printed to log when WF_PRINT_UDEV_DEVPATH is set and
'-d' is passed to wayfire, for matching the correct device in i.e.
'udevadm info --tree'. The sections that wayfire checks for are also
logged with '-d'. This makes it so users can match devices, especially
identical devices lacking many of the usual differentiating properties.

Fixes #2504.
soreau added a commit that referenced this issue Nov 6, 2024
This patch adds additional checks for input-device sections in the config
file. It checks for ID_PATH, ID_SERIAL, LIBINPUT_DEVICE_GROUP then falls
back to the device name and finally the global input-device section. In
addition, DEVPATH is printed to log when WF_PRINT_UDEV_DEVPATH is set and
'-d' is passed to wayfire, for matching the correct device in i.e.
'udevadm info --tree'. The sections that wayfire checks for are also
logged with '-d'. This makes it so users can match devices, especially
identical devices lacking many of the usual differentiating properties.

Fixes #2504.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants