Identify tuples to generate unique touch identifier for touch devices |
||||||||||||
Issue descriptionCurrently the unique identifier generated for a touch device uses the device's name, product id and vendor id. This can however lead to collisions and is not always safe. We need to use tuples that can uniquely identify a given input device. Is there something else in EventDeviceInfo[1] that can be used? [1] https://cs.chromium.org/chromium/src/ui/events/ozone/evdev/event_device_info.h
,
Mar 30 2018
,
Mar 30 2018
> This can however lead to collisions and is not always safe. Why is product-id/vendor-id not always safe? What kind of collisions are you seeing?
,
Mar 30 2018
A recent bug associated with an input touch device that offers two different interfaces. The two interfaces are present in chrome as two separate input event sources, but they have the same name, product id and vendor id. Also @spang pointed out that two identical model touchscreens are basically guaranteed to conflict with this tuple; it is not unique enough.
,
Mar 30 2018
Right, "name" here is something like "Atmel maXTouch Touchscreen". It just names the product. Nothing in (vid, pid, name) is going to vary between different units of the same product. We need it to vary if we're using that to lookup configuration information, when the configuration information decides what monitor to use in dual monitor setups. This feature has reportedly broken even single display cases, where the touchscreen exposes two interfaces (product in question has two modes - "absolute mouse" + touch). The single display case is really easy from a pairing perspective - let's fix this regression quickly for that case at least.
,
Mar 30 2018
,
Mar 30 2018
Offline discussion notes (@spang & @dtor) - Use name, vendor id and product id to generate the hash. - On collision use the physical path string to differentiate the two. Problems with this approach: - If the user swapped the ports of the device with same hash, all touch association and calibration information would get swapped. In which case the user would have to manually reassociate and recalibrate the devices. This however does not solve the issue where the path for two interfaces of the same device can change even if connected to the same port.(as observed in bug 827583 )
,
Apr 7 2018
Final logic being implemented in https://chromium-review.googlesource.com/c/chromium/src/+/1000548 Step 1) Associate all touch devices normally. Step 2) If devices with same hashes are detected, associate them with the display based on the port they are connected to. Step 3) If no port information is given, associate them to the internal display. If not internal display is available, associate them with a random display. The user can then calibrate and re-associate them to the right display. Step 4) After calibration/association, store the port information for future use in step 2. The port information we store: - (Device hash, Port Phys Hash, Display Id) In the future if we have the Device hash and the port hash, we can easily find the display Id.
,
Apr 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a87a563dcb32efda52330e097482395a34855c53 commit a87a563dcb32efda52330e097482395a34855c53 Author: Malay Keshav <malaykeshav@chromium.org> Date: Fri Apr 13 07:44:22 2018 Differentiate between duplicate touch devices based on the port This patch performs multiple tasks: - Add EVIOCGPHYS field to EventDeviceInfo and plumbs it to InputDevice. - Adds a secondary identifier to TouchDeviceIdentifier that corresponds to the phys(or port) information of the device. - Add support for TouchDeviceManager to manage devices with same ID by using their port information. - Persistently stores the port mapping for each device in case of duplicate ID. Adds and updates unit tests. Bug: 827452 Change-Id: Ia224cd35125615705d1b94a13c4c0afa2bd25182 Component: TouchDeviceManager, EventDeviceInfo, InputDevice Reviewed-on: https://chromium-review.googlesource.com/1000548 Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Michael Spang <spang@chromium.org> Cr-Commit-Position: refs/heads/master@{#550538} [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ash/display/display_prefs.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ash/public/cpp/ash_pref_names.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ash/public/cpp/ash_pref_names.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/chrome/browser/chromeos/display/display_prefs_unittest.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/chrome/common/pref_names.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/test/touch_device_manager_test_api.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/touch_device_manager.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/touch_device_manager.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/touch_device_manager_unittest.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/devices/input_device.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/devices/input_device.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_converter_evdev.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_converter_evdev.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_converter_evdev_impl.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_device_info.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_device_info.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/gamepad_event_converter_evdev.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/libgestures_glue/event_reader_libevdev_cros.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/tablet_event_converter_evdev.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/touch_event_converter_evdev.cc
,
Apr 13 2018
,
Apr 13 2018
,
Apr 13 2018
This bug requires manual review: We are only 3 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 13 2018
M66 merge rejected - This does not seem to be a regression and CL is quite big for merging this late in the cycle Please update if any strong concerns/business implications by punting to M67
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eadf8dceb0d06926cdc20fd36eaa9f114529dcf8 commit eadf8dceb0d06926cdc20fd36eaa9f114529dcf8 Author: Malay Keshav <malaykeshav@chromium.org> Date: Tue Apr 17 17:43:36 2018 ozone: evdev: Switch to char[] for EVIOCGPHYS return std::string is not used correctly with ioctl() and hence fails to get the field info. Replace it with C string. Bug: 827452 Change-Id: Iace6ce24a6405c40a68eeff71c090883b847a46f Component: Event Device Info, phys Reviewed-on: https://chromium-review.googlesource.com/1014409 Reviewed-by: Michael Spang <spang@chromium.org> Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Cr-Commit-Position: refs/heads/master@{#551381} [modify] https://crrev.com/eadf8dceb0d06926cdc20fd36eaa9f114529dcf8/ui/events/ozone/evdev/event_device_info.cc
,
Apr 17 2018
The recent bug fix missed the branch cut. Merge request for the same.
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a87a563dcb32efda52330e097482395a34855c53 commit a87a563dcb32efda52330e097482395a34855c53 Author: Malay Keshav <malaykeshav@chromium.org> Date: Fri Apr 13 07:44:22 2018 Differentiate between duplicate touch devices based on the port This patch performs multiple tasks: - Add EVIOCGPHYS field to EventDeviceInfo and plumbs it to InputDevice. - Adds a secondary identifier to TouchDeviceIdentifier that corresponds to the phys(or port) information of the device. - Add support for TouchDeviceManager to manage devices with same ID by using their port information. - Persistently stores the port mapping for each device in case of duplicate ID. Adds and updates unit tests. Bug: 827452 Change-Id: Ia224cd35125615705d1b94a13c4c0afa2bd25182 Component: TouchDeviceManager, EventDeviceInfo, InputDevice Reviewed-on: https://chromium-review.googlesource.com/1000548 Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Michael Spang <spang@chromium.org> Cr-Commit-Position: refs/heads/master@{#550538} [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ash/display/display_prefs.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ash/public/cpp/ash_pref_names.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ash/public/cpp/ash_pref_names.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/chrome/browser/chromeos/display/display_prefs_unittest.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/chrome/common/pref_names.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/test/touch_device_manager_test_api.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/touch_device_manager.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/touch_device_manager.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/display/manager/chromeos/touch_device_manager_unittest.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/devices/input_device.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/devices/input_device.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_converter_evdev.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_converter_evdev.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_converter_evdev_impl.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_device_info.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/event_device_info.h [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/gamepad_event_converter_evdev.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/libgestures_glue/event_reader_libevdev_cros.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/tablet_event_converter_evdev.cc [modify] https://crrev.com/a87a563dcb32efda52330e097482395a34855c53/ui/events/ozone/evdev/touch_event_converter_evdev.cc
,
Apr 17 2018
This appears to be a feature rather than a M67 bug regression? Need to limit merge requests on regressions rather than late feature inclusion.
,
Apr 17 2018
This is a regression in M65 and M66. Touch devices of certain kinds have stopped working. This is a fix for that.
,
Apr 18 2018
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb commit 7b6800f79dd7fa0095b0619d3fc9b28f764b74bb Author: Malay Keshav <malaykeshav@chromium.org> Date: Wed Apr 18 20:29:20 2018 (Merge) Differentiate between duplicate touch devices based on the port Merge to M67 This patch performs multiple tasks: - Add EVIOCGPHYS field to EventDeviceInfo and plumbs it to InputDevice. - Adds a secondary identifier to TouchDeviceIdentifier that corresponds to the phys(or port) information of the device. - Add support for TouchDeviceManager to manage devices with same ID by using their port information. - Persistently stores the port mapping for each device in case of duplicate ID. Adds and updates unit tests. Bug: 827452 Change-Id: Ia224cd35125615705d1b94a13c4c0afa2bd25182 Component: TouchDeviceManager, EventDeviceInfo, InputDevice Reviewed-on: https://chromium-review.googlesource.com/1000548 Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Michael Spang <spang@chromium.org> Cr-Commit-Position: refs/heads/master@{#550538} TBR=oshima@chromium.org,sadrul@chromium.org,spang@chromium.org Change-Id: Ia224cd35125615705d1b94a13c4c0afa2bd25182 Reviewed-on: https://chromium-review.googlesource.com/1017774 Reviewed-by: Malay Keshav <malaykeshav@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#99} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/chrome/browser/chromeos/display/display_prefs.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/chrome/browser/chromeos/display/display_prefs_unittest.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/chrome/common/pref_names.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/chrome/common/pref_names.h [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/display/manager/chromeos/test/touch_device_manager_test_api.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/display/manager/chromeos/touch_device_manager.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/display/manager/chromeos/touch_device_manager.h [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/display/manager/chromeos/touch_device_manager_unittest.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/devices/input_device.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/devices/input_device.h [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/event_converter_evdev.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/event_converter_evdev.h [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/event_converter_evdev_impl.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/event_device_info.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/event_device_info.h [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/gamepad_event_converter_evdev.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/libgestures_glue/event_reader_libevdev_cros.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/tablet_event_converter_evdev.cc [modify] https://crrev.com/7b6800f79dd7fa0095b0619d3fc9b28f764b74bb/ui/events/ozone/evdev/touch_event_converter_evdev.cc
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a027459a583f5086b9c6fb8a26e992a5d1d6944 commit 7a027459a583f5086b9c6fb8a26e992a5d1d6944 Author: Malay Keshav <malaykeshav@chromium.org> Date: Wed Apr 18 20:32:23 2018 (Merge) ozone: evdev: Switch to char[] for EVIOCGPHYS return Merge to M67 std::string is not used correctly with ioctl() and hence fails to get the field info. Replace it with C string. Bug: 827452 Change-Id: Iace6ce24a6405c40a68eeff71c090883b847a46f Component: Event Device Info, phys Reviewed-on: https://chromium-review.googlesource.com/1014409 Reviewed-by: Michael Spang <spang@chromium.org> Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#551381}(cherry picked from commit eadf8dceb0d06926cdc20fd36eaa9f114529dcf8) Reviewed-on: https://chromium-review.googlesource.com/1017663 Reviewed-by: Malay Keshav <malaykeshav@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#100} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/7a027459a583f5086b9c6fb8a26e992a5d1d6944/ui/events/ozone/evdev/event_device_info.cc
,
Jun 6 2018
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by malaykeshav@chromium.org
, Mar 30 2018