After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726199 - evdev changes needed for logind integration work
evdev changes needed for logind integration work
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2014-03-12 19:36 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2014-03-13 16:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
evdev: Extend the device open callback with a close callback as well (3.44 KB, patch)
2014-03-12 19:36 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
evdev: Set the initial pointer position for all pointer devices (1.58 KB, patch)
2014-03-12 19:37 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
evdev: Extract code for setting the libinput seat out (1.78 KB, patch)
2014-03-12 19:37 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
evdev: Always create the main seat (3.97 KB, patch)
2014-03-12 19:37 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
evdev: Extend the device open callback with a close callback as well (3.91 KB, patch)
2014-03-12 20:07 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2014-03-12 19:36:56 UTC
See patches.
Comment 1 Jasper St. Pierre (not reading bugmail) 2014-03-12 19:36:58 UTC
Created attachment 271644 [details] [review]
evdev: Extend the device open callback with a close callback as well

We need to return the device to logind with ReleaseDevice().
Comment 2 Jasper St. Pierre (not reading bugmail) 2014-03-12 19:37:02 UTC
Created attachment 271645 [details] [review]
evdev: Set the initial pointer position for all pointer devices

Rather than just those on the main seat.
Comment 3 Jasper St. Pierre (not reading bugmail) 2014-03-12 19:37:05 UTC
Created attachment 271646 [details] [review]
evdev: Extract code for setting the libinput seat out

We're going to create the main seat at an earlier time, when
we don't have the physical libinput_seat yet, so we need to
do the association later.
Comment 4 Jasper St. Pierre (not reading bugmail) 2014-03-12 19:37:10 UTC
Created attachment 271647 [details] [review]
evdev: Always create the main seat

There could be times when we may not necessarily see a device appear
at initialization time, like when we're VT switched away when we
initialize, and thus we can't ever rely on a main seat appearing.

Always create a main seat with logical pointer/keyboard devices, and
tie the first physical seat that comes in to the main seat.
Comment 5 Emmanuele Bassi (:ebassi) 2014-03-12 19:45:08 UTC
Review of attachment 271644 [details] [review]:

::: clutter/evdev/clutter-device-manager-evdev.c
@@ +1478,3 @@
  */
 void
+clutter_evdev_set_open_callback (ClutterOpenDeviceCallback  open_callback_,

this should be renamed to something like:

  clutter_evdev_set_device_callbacks()

given that it's not for setting an open callback any more.

::: clutter/evdev/clutter-evdev.h
@@ +49,3 @@
 					  gpointer     user_data,
 					  GError     **error);
+typedef void (*ClutterCloseDeviceCallback) (int          fd,

should the CloseDeviceCallback also get the path? maybe for error reporting?
Comment 6 Emmanuele Bassi (:ebassi) 2014-03-12 19:45:09 UTC
Review of attachment 271644 [details] [review]:

::: clutter/evdev/clutter-device-manager-evdev.c
@@ +1478,3 @@
  */
 void
+clutter_evdev_set_open_callback (ClutterOpenDeviceCallback  open_callback_,

this should be renamed to something like:

  clutter_evdev_set_device_callbacks()

given that it's not for setting an open callback any more.

::: clutter/evdev/clutter-evdev.h
@@ +49,3 @@
 					  gpointer     user_data,
 					  GError     **error);
+typedef void (*ClutterCloseDeviceCallback) (int          fd,

should the CloseDeviceCallback also get the path? maybe for error reporting?
Comment 7 Emmanuele Bassi (:ebassi) 2014-03-12 19:45:32 UTC
Review of attachment 271645 [details] [review]:

okay.
Comment 8 Emmanuele Bassi (:ebassi) 2014-03-12 19:45:58 UTC
Review of attachment 271646 [details] [review]:

okay.
Comment 9 Emmanuele Bassi (:ebassi) 2014-03-12 19:46:41 UTC
Review of attachment 271647 [details] [review]:

okay.
Comment 10 Jasper St. Pierre (not reading bugmail) 2014-03-12 19:48:25 UTC
(In reply to comment #6)
> this should be renamed to something like:
> 
>   clutter_evdev_set_device_callbacks()

OK.

> should the CloseDeviceCallback also get the path? maybe for error reporting?

libinput doesn't give us a path, so I'm not sure how we'd find it. Call fstat and construct it out of device major/minor numbers somehow?
Comment 11 Emmanuele Bassi (:ebassi) 2014-03-12 19:52:09 UTC
(In reply to comment #10)

> > should the CloseDeviceCallback also get the path? maybe for error reporting?
> 
> libinput doesn't give us a path, so I'm not sure how we'd find it. Call fstat
> and construct it out of device major/minor numbers somehow?

if there's no path available, then it's fine not to expose it.
Comment 12 Jasper St. Pierre (not reading bugmail) 2014-03-12 20:07:24 UTC
Created attachment 271650 [details] [review]
evdev: Extend the device open callback with a close callback as well

We need to return the device to logind with ReleaseDevice().
Comment 13 Emmanuele Bassi (:ebassi) 2014-03-13 15:38:31 UTC
Review of attachment 271650 [details] [review]:

looks good.
Comment 14 Jasper St. Pierre (not reading bugmail) 2014-03-13 16:46:58 UTC
Attachment 271645 [details] pushed as 5facd71 - evdev: Set the initial pointer position for all pointer devices
Attachment 271646 [details] pushed as defe55f - evdev: Extract code for setting the libinput seat out
Attachment 271647 [details] pushed as dcaf568 - evdev: Always create the main seat
Attachment 271650 [details] pushed as e23f77f - evdev: Extend the device open callback with a close callback as well