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

Consider ID_INPUT_POINTINGSTICK as touchpad (BugFix) #1447

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

khfeng
Copy link

@khfeng khfeng commented Sep 3, 2024

Most resistive touchpads don't support ABS coordination nor multitouch. When both capabilities are absent, the touchpad gets tagged as ID_INPUT_MOUSE.

Since there's no such device as I2C mouse, if the touchpad is on I2C bus, it will also get tagged as ID_INPUT_POINTINGSTICK.

To really distinguish a true pointing stick and a resistive touchpad, the parser needs to find the slibling node created by HID Report Descriptor and ensure it's a "touchpad" (i.e. ABS or MT capability). It's not easy to traverse to a sibling node in the current parser, so simply treat ID_INPUT_POINTINGSTICK as resistive touchpad to resolve the issue.

Description

Resolved issues

Documentation

Tests

@khfeng khfeng force-pushed the resistive-touchpad branch from 28c86a9 to eda0824 Compare September 3, 2024 06:06
@khfeng khfeng changed the title Consider ID_INPUT_POINTINGSTICK as touchpad Consider ID_INPUT_POINTINGSTICK as touchpad (BugFix) Sep 3, 2024
Most resistive touchpads don't support ABS coordination nor multitouch.
When both capabilities are absent, the touchpad gets tagged as
ID_INPUT_MOUSE.

Since there's no such device as I2C mouse, if the touchpad is on I2C
bus, it will also get tagged as ID_INPUT_POINTINGSTICK.

To really distinguish a true pointing stick and a resistive touchpad,
the parser needs to find the slibling node created by HID Report
Descriptor and ensure it's a "touchpad" (i.e. ABS or MT capability).
It's not easy to traverse to a sibling node in the current parser, so
simply treat ID_INPUT_POINTINGSTICK as resistive touchpad to resolve the
issue.
@khfeng khfeng force-pushed the resistive-touchpad branch from eda0824 to 7877a9c Compare September 3, 2024 06:09
@nancyc12 nancyc12 requested review from Hook25 and pieqq September 4, 2024 02:40
Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See below inline comment: it seems introducing this fix would create a regression on devices with a trackpoint, which is currently recognizes as a mouse, but would start being recognized as a touchpad if we landed your change.

Is there a better way to identify the model you are focusing on, to make sure it's detected as a touchpad nonetheless?

Comment on lines +358 to +360
self.assertEqual(self.count(devices, "TOUCHPAD"), 2)
self.assertEqual(self.count(devices, "CARDREADER"), 1)
self.assertEqual(self.count(devices, "MOUSE"), 1)
self.assertEqual(self.count(devices, "MOUSE"), 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem right.

I went and check what device this udev DB came from, and it is a Dell Latitude 5580 which comes with a trackpoint, similar to the ones on the Lenovo laptops.

I asked one of our colleagues and they provided a sample of a Lenovo unit with such a trackpoint:

4649 P: /devices/pci0000:00/0000:00:1f.4/i2c-0/0-0015/input/input8
4650 L: 0
4651 E: DEVPATH=/devices/pci0000:00/0000:00:1f.4/i2c-0/0-0015/input/input8
4652 E: SUBSYSTEM=input
4653 E: PRODUCT=18/4f3/67/0
4654 E: NAME="Elan TrackPoint"
4655 E: PROP=21
4656 E: EV=7
4657 E: KEY=70000 0 0 0 0
4658 E: REL=3
4659 E: MODALIAS=input:b0018v04F3p0067e0000-e0,1,2,k110,111,112,r0,1,amlsfw
4660 E: USEC_INITIALIZED=3293319
4661 E: ID_INPUT=1
4662 E: ID_INPUT_POINTINGSTICK=1
4663 E: ID_INPUT_MOUSE=1
4664 E: ID_SERIAL=noserial
4665 E: ID_PATH=pci-0000:00:1f.4
4666 E: ID_PATH_TAG=pci-0000_00_1f_4
4667 E: ID_FOR_SEAT=input-pci-0000_00_1f_4
4668 E: TAGS=:seat:

And, as confirmed with them, in this case, when ID_INPUT_POINTINGSTICK=1 and ID_INPUT_MOUSE=1, the TrackPoint should be a mouse, not a touchpad. (otherwise the touchpad test cases would be run twice when running Checkbox on this device, which doesn't really make sense)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is, why should a trackpoint be a mouse?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add a separate test case for pointingsitck? If that's present then it's okay to have no touchpad?

@khfeng
Copy link
Author

khfeng commented Sep 6, 2024

See below inline comment: it seems introducing this fix would create a regression on devices with a trackpoint, which is currently recognizes as a mouse, but would start being recognized as a touchpad if we landed your change.

Is there a better way to identify the model you are focusing on, to make sure it's detected as a touchpad nonetheless?

What makes a pointingstick a mouse?

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

Successfully merging this pull request may close these issues.

2 participants