Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping

2014-06-04 Thread Jonas Ådahl 
On Wed, Jun 04, 2014 at 11:17:58AM +1000, Peter Hutterer wrote:
 On Tue, Jun 03, 2014 at 10:41:16PM +0200, Jonas Ådahl wrote:
  On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote:
   Provide an interface to enable/disable tapping, with a default mapping of
   1/2/3 fingers mapping to L/R/M button events, respectively.
   
   Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
   ---
src/libinput-private.h | 13 +
src/libinput.c | 33 +++
src/libinput.h | 73 
   +-
3 files changed, 118 insertions(+), 1 deletion(-)
   
   diff --git a/src/libinput-private.h b/src/libinput-private.h
   index 61cdc79..020167e 100644
   --- a/src/libinput-private.h
   +++ b/src/libinput-private.h
   @@ -69,12 +69,25 @@ struct libinput_seat {
 uint32_t button_count[KEY_CNT];
};

   +struct libinput_device_config_tap {
   + int (*count)(struct libinput_device *device);
   + enum libinput_config_status (*enable)(struct libinput_device *device,
   +   int enable);
   + int (*is_enabled)(struct libinput_device *device);
   + void (*reset)(struct libinput_device *device);
   +};
   +
   +struct libinput_device_config {
   + struct libinput_device_config_tap *tap;
   +};
   +
struct libinput_device {
 struct libinput_seat *seat;
 struct list link;
 void *user_data;
 int terminated;
 int refcount;
   + struct libinput_device_config config;
};

typedef void (*libinput_source_dispatch_t)(void *data);
   diff --git a/src/libinput.c b/src/libinput.c
   index 6b7e8b8..6a713bb 100644
   --- a/src/libinput.c
   +++ b/src/libinput.c
   @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct 
   libinput_event_touch *event)
{
 return event-base;
}
   +
   +LIBINPUT_EXPORT int
   +libinput_device_config_tap_get_finger_count(struct libinput_device 
   *device)
   +{
   + return device-config.tap ? device-config.tap-count(device) : 0;
   +}
   +
   +LIBINPUT_EXPORT enum libinput_config_status
   +libinput_device_config_tap_enable(struct libinput_device *device,
   +   int enable)
   +{
   + if (enable 
   + libinput_device_config_tap_get_finger_count(device) == 0)
   + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
  
  Shouldn't this be returned also if !enable? It would be a bit confusing
  if disabling is allowed, but enabling is not.
 
 This was a conscious decision that applies to the other functions as well:
 if you're doing something that's not technically invalid, we're not
 reporting an error. This means that an error only needs to be handled if
 something really is off.

I'd argue that disabling tapping on a device that doesn't support
tapping is a technically invalid operation that is simply handled in the
cases where it makes no sense.

 
  Also, in what circumstances will config.tap be set, but finger count
  returning 0? What type of devices would have tapping configuration
  enabled with no tapping possible?
 
 I expect none, this approach was just the safest and most copy-paste-proof
 way of checking.
  
   +
   + return device-config.tap-enable(device, enable);
   +}
   +
   +LIBINPUT_EXPORT int
   +libinput_device_config_tap_is_enabled(struct libinput_device *device)
   +{
   + if (libinput_device_config_tap_get_finger_count(device) == 0)
   + return 0;
   +
   + return device-config.tap-is_enabled(device);
   +}
   +
   +LIBINPUT_EXPORT void
   +libinput_device_config_tap_reset(struct libinput_device *device)
   +{
   + if (device-config.tap)
   + device-config.tap-reset(device);
   +}
   diff --git a/src/libinput.h b/src/libinput.h
   index c9ec71a..0c84547 100644
   --- a/src/libinput.h
   +++ b/src/libinput.h
   @@ -1362,8 +1362,79 @@ enum libinput_config_status {
const char *
libinput_config_status_to_str(enum libinput_config_status status);

   +/**
   + * @ingroup config
   + *
   + * Check if the device supports tap-to-click. See
   + * libinput_device_config_tap_set() for more information.
  
  Should it be libinput_device_config_tap_enable() here instead?
 
 ah, thanks. one always escapes the refacturing...
 
   + *
   + * @param device The device to configure
   + * @return The number of fingers that can generate a tap event, or 0 if 
   the
   + * device does not support tapping.
   + *
   + * @see libinput_device_config_tap_enable
   + * @see libinput_device_config_tap_is_enabled
   + * @see libinput_device_config_tap_reset
   + */
   +int
   +libinput_device_config_tap_get_finger_count(struct libinput_device 
   *device);
  
  I wonder where doing things like this, and
  libinput_device_config_scroll_get_methods() will lead us. It'd mean we
  expose some device information via the config API, and some via direct
  device getters, which might not be the best thing for consistency.
 
 Just to be clear - you're talking about ditching

Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping

2014-06-04 Thread Hans de Goede 
Hi,

On 06/04/2014 10:21 AM, Jonas Ådahl wrote:
 On Wed, Jun 04, 2014 at 11:17:58AM +1000, Peter Hutterer wrote:
 On Tue, Jun 03, 2014 at 10:41:16PM +0200, Jonas Ådahl wrote:
 On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote:
 Provide an interface to enable/disable tapping, with a default mapping of
 1/2/3 fingers mapping to L/R/M button events, respectively.

 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
  src/libinput-private.h | 13 +
  src/libinput.c | 33 +++
  src/libinput.h | 73 
 +-
  3 files changed, 118 insertions(+), 1 deletion(-)

 diff --git a/src/libinput-private.h b/src/libinput-private.h
 index 61cdc79..020167e 100644
 --- a/src/libinput-private.h
 +++ b/src/libinput-private.h
 @@ -69,12 +69,25 @@ struct libinput_seat {
uint32_t button_count[KEY_CNT];
  };
  
 +struct libinput_device_config_tap {
 +  int (*count)(struct libinput_device *device);
 +  enum libinput_config_status (*enable)(struct libinput_device *device,
 +int enable);
 +  int (*is_enabled)(struct libinput_device *device);
 +  void (*reset)(struct libinput_device *device);
 +};
 +
 +struct libinput_device_config {
 +  struct libinput_device_config_tap *tap;
 +};
 +
  struct libinput_device {
struct libinput_seat *seat;
struct list link;
void *user_data;
int terminated;
int refcount;
 +  struct libinput_device_config config;
  };
  
  typedef void (*libinput_source_dispatch_t)(void *data);
 diff --git a/src/libinput.c b/src/libinput.c
 index 6b7e8b8..6a713bb 100644
 --- a/src/libinput.c
 +++ b/src/libinput.c
 @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct 
 libinput_event_touch *event)
  {
return event-base;
  }
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_get_finger_count(struct libinput_device 
 *device)
 +{
 +  return device-config.tap ? device-config.tap-count(device) : 0;
 +}
 +
 +LIBINPUT_EXPORT enum libinput_config_status
 +libinput_device_config_tap_enable(struct libinput_device *device,
 +int enable)
 +{
 +  if (enable 
 +  libinput_device_config_tap_get_finger_count(device) == 0)
 +  return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;

 Shouldn't this be returned also if !enable? It would be a bit confusing
 if disabling is allowed, but enabling is not.

 This was a conscious decision that applies to the other functions as well:
 if you're doing something that's not technically invalid, we're not
 reporting an error. This means that an error only needs to be handled if
 something really is off.
 
 I'd argue that disabling tapping on a device that doesn't support
 tapping is a technically invalid operation that is simply handled in the
 cases where it makes no sense.

I'm with Peter here, that its easier for our API consumers if they caan
always safely call disable.

 

 Also, in what circumstances will config.tap be set, but finger count
 returning 0? What type of devices would have tapping configuration
 enabled with no tapping possible?

 I expect none, this approach was just the safest and most copy-paste-proof
 way of checking.
  
 +
 +  return device-config.tap-enable(device, enable);
 +}
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_is_enabled(struct libinput_device *device)
 +{
 +  if (libinput_device_config_tap_get_finger_count(device) == 0)
 +  return 0;
 +
 +  return device-config.tap-is_enabled(device);
 +}
 +
 +LIBINPUT_EXPORT void
 +libinput_device_config_tap_reset(struct libinput_device *device)
 +{
 +  if (device-config.tap)
 +  device-config.tap-reset(device);
 +}
 diff --git a/src/libinput.h b/src/libinput.h
 index c9ec71a..0c84547 100644
 --- a/src/libinput.h
 +++ b/src/libinput.h
 @@ -1362,8 +1362,79 @@ enum libinput_config_status {
  const char *
  libinput_config_status_to_str(enum libinput_config_status status);
  
 +/**
 + * @ingroup config
 + *
 + * Check if the device supports tap-to-click. See
 + * libinput_device_config_tap_set() for more information.

 Should it be libinput_device_config_tap_enable() here instead?

 ah, thanks. one always escapes the refacturing...

 + *
 + * @param device The device to configure
 + * @return The number of fingers that can generate a tap event, or 0 if 
 the
 + * device does not support tapping.
 + *
 + * @see libinput_device_config_tap_enable
 + * @see libinput_device_config_tap_is_enabled
 + * @see libinput_device_config_tap_reset
 + */
 +int
 +libinput_device_config_tap_get_finger_count(struct libinput_device 
 *device);

 I wonder where doing things like this, and
 libinput_device_config_scroll_get_methods() will lead us. It'd mean we
 expose some device information via the config API, and some via direct
 device getters, which might not be the best thing for consistency.

 Just to be clear - you're talking about ditching the _config_ part of the
 interface? If so, yeah, doable but I

Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping

2014-06-04 Thread Hans de Goede 
Hi,

On 06/03/2014 07:34 AM, Peter Hutterer wrote:
 Provide an interface to enable/disable tapping, with a default mapping of
 1/2/3 fingers mapping to L/R/M button events, respectively.
 
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
  src/libinput-private.h | 13 +
  src/libinput.c | 33 +++
  src/libinput.h | 73 
 +-
  3 files changed, 118 insertions(+), 1 deletion(-)
 
 diff --git a/src/libinput-private.h b/src/libinput-private.h
 index 61cdc79..020167e 100644
 --- a/src/libinput-private.h
 +++ b/src/libinput-private.h
 @@ -69,12 +69,25 @@ struct libinput_seat {
   uint32_t button_count[KEY_CNT];
  };
  
 +struct libinput_device_config_tap {
 + int (*count)(struct libinput_device *device);
 + enum libinput_config_status (*enable)(struct libinput_device *device,
 +   int enable);
 + int (*is_enabled)(struct libinput_device *device);
 + void (*reset)(struct libinput_device *device);
 +};
 +
 +struct libinput_device_config {
 + struct libinput_device_config_tap *tap;
 +};
 +
  struct libinput_device {
   struct libinput_seat *seat;
   struct list link;
   void *user_data;
   int terminated;
   int refcount;
 + struct libinput_device_config config;
  };
  
  typedef void (*libinput_source_dispatch_t)(void *data);
 diff --git a/src/libinput.c b/src/libinput.c
 index 6b7e8b8..6a713bb 100644
 --- a/src/libinput.c
 +++ b/src/libinput.c
 @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct 
 libinput_event_touch *event)
  {
   return event-base;
  }
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_get_finger_count(struct libinput_device *device)
 +{
 + return device-config.tap ? device-config.tap-count(device) : 0;
 +}
 +
 +LIBINPUT_EXPORT enum libinput_config_status
 +libinput_device_config_tap_enable(struct libinput_device *device,
 +   int enable)
 +{
 + if (enable 
 + libinput_device_config_tap_get_finger_count(device) == 0)
 + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
 +
 + return device-config.tap-enable(device, enable);
 +}
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_is_enabled(struct libinput_device *device)
 +{
 + if (libinput_device_config_tap_get_finger_count(device) == 0)
 + return 0;
 +
 + return device-config.tap-is_enabled(device);
 +}
 +
 +LIBINPUT_EXPORT void
 +libinput_device_config_tap_reset(struct libinput_device *device)
 +{
 + if (device-config.tap)
 + device-config.tap-reset(device);
 +}

Didn't we agree in an off-list discussion to add a get_default instead,
so to that ie a cmdline app for querying config info can not only display
the current value but also the device default value for a config option ?

This get_default would replace the reset, apps can simple implement that
functionality themselves through the get_default.

Regards,

Hans


 diff --git a/src/libinput.h b/src/libinput.h
 index c9ec71a..0c84547 100644
 --- a/src/libinput.h
 +++ b/src/libinput.h
 @@ -1362,8 +1362,79 @@ enum libinput_config_status {
  const char *
  libinput_config_status_to_str(enum libinput_config_status status);
  
 +/**
 + * @ingroup config
 + *
 + * Check if the device supports tap-to-click. See
 + * libinput_device_config_tap_set() for more information.
 + *
 + * @param device The device to configure
 + * @return The number of fingers that can generate a tap event, or 0 if the
 + * device does not support tapping.
 + *
 + * @see libinput_device_config_tap_enable
 + * @see libinput_device_config_tap_is_enabled
 + * @see libinput_device_config_tap_reset
 + */
 +int
 +libinput_device_config_tap_get_finger_count(struct libinput_device *device);
 +
 +/**
 + * @ingroup config
 + *
 + * Enable tap-to-click on this device, with a default mapping of 1, 2, 3
 + * finger tap mapping to left, right, middle click, respectively.
 + * Tapping is limited by the number of simultaneous touches
 + * supported by the device, see
 + * libinput_device_config_tap_get_finger_count().
 + *
 + * @param device The device to configure
 + * @param enable Non-zero to enable, zero to disable
 + *
 + * @return A config status code. Disabling tapping on a device that does not
 + * support tapping always succeeds.
 + *
 + * @see libinput_device_config_tap_get_finger_count
 + * @see libinput_device_config_tap_is_enabled
 + * @see libinput_device_config_tap_reset
 + */
 +enum libinput_config_status
 +libinput_device_config_tap_enable(struct libinput_device *device,
 +   int enable);
 +
 +/**
 + * @ingroup config
 + *
 + * Check if tap-to-click is enabled on this device. If the device does not
 + * support tapping, this function always returns 0.
 + *
 + * @param device The device to configure
 + *
 + * @return 1 if enabled, 0 otherwise.
 + *
 + * @see

Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping

2014-06-04 Thread Peter Hutterer 

On 4/06/2014 18:40 , Hans de Goede wrote:

Hi,

On 06/03/2014 07:34 AM, Peter Hutterer wrote:

Provide an interface to enable/disable tapping, with a default mapping of
1/2/3 fingers mapping to L/R/M button events, respectively.

Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
  src/libinput-private.h | 13 +
  src/libinput.c | 33 +++
  src/libinput.h | 73 +-
  3 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/src/libinput-private.h b/src/libinput-private.h
index 61cdc79..020167e 100644
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -69,12 +69,25 @@ struct libinput_seat {
uint32_t button_count[KEY_CNT];
  };

+struct libinput_device_config_tap {
+   int (*count)(struct libinput_device *device);
+   enum libinput_config_status (*enable)(struct libinput_device *device,
+ int enable);
+   int (*is_enabled)(struct libinput_device *device);
+   void (*reset)(struct libinput_device *device);
+};
+
+struct libinput_device_config {
+   struct libinput_device_config_tap *tap;
+};
+
  struct libinput_device {
struct libinput_seat *seat;
struct list link;
void *user_data;
int terminated;
int refcount;
+   struct libinput_device_config config;
  };

  typedef void (*libinput_source_dispatch_t)(void *data);
diff --git a/src/libinput.c b/src/libinput.c
index 6b7e8b8..6a713bb 100644
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct 
libinput_event_touch *event)
  {
return event-base;
  }
+
+LIBINPUT_EXPORT int
+libinput_device_config_tap_get_finger_count(struct libinput_device *device)
+{
+   return device-config.tap ? device-config.tap-count(device) : 0;
+}
+
+LIBINPUT_EXPORT enum libinput_config_status
+libinput_device_config_tap_enable(struct libinput_device *device,
+ int enable)
+{
+   if (enable 
+   libinput_device_config_tap_get_finger_count(device) == 0)
+   return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
+
+   return device-config.tap-enable(device, enable);
+}
+
+LIBINPUT_EXPORT int
+libinput_device_config_tap_is_enabled(struct libinput_device *device)
+{
+   if (libinput_device_config_tap_get_finger_count(device) == 0)
+   return 0;
+
+   return device-config.tap-is_enabled(device);
+}
+
+LIBINPUT_EXPORT void
+libinput_device_config_tap_reset(struct libinput_device *device)
+{
+   if (device-config.tap)
+   device-config.tap-reset(device);
+}


Didn't we agree in an off-list discussion to add a get_default instead,
so to that ie a cmdline app for querying config info can not only display
the current value but also the device default value for a config option ?

This get_default would replace the reset, apps can simple implement that
functionality themselves through the get_default.


huh, yes, now I remember. I knew something was missing. I'll fix this 
series up with this in mind, sorry about that.


Cheers,
  Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping

2014-06-04 Thread Hans de Goede 
Hi,

On 06/03/2014 07:34 AM, Peter Hutterer wrote:
 Provide an interface to enable/disable tapping, with a default mapping of
 1/2/3 fingers mapping to L/R/M button events, respectively.
 
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net


I see that in all the other config interfaces you use get / set, yet here
you use enable / is_enabled. I think it would be more consistent to use
get / set with a bool return / argument here too.

This would also mix better with the get_default I've suggested.

Regards,

Hans

 ---
  src/libinput-private.h | 13 +
  src/libinput.c | 33 +++
  src/libinput.h | 73 
 +-
  3 files changed, 118 insertions(+), 1 deletion(-)
 
 diff --git a/src/libinput-private.h b/src/libinput-private.h
 index 61cdc79..020167e 100644
 --- a/src/libinput-private.h
 +++ b/src/libinput-private.h
 @@ -69,12 +69,25 @@ struct libinput_seat {
   uint32_t button_count[KEY_CNT];
  };
  
 +struct libinput_device_config_tap {
 + int (*count)(struct libinput_device *device);
 + enum libinput_config_status (*enable)(struct libinput_device *device,
 +   int enable);
 + int (*is_enabled)(struct libinput_device *device);
 + void (*reset)(struct libinput_device *device);
 +};
 +
 +struct libinput_device_config {
 + struct libinput_device_config_tap *tap;
 +};
 +
  struct libinput_device {
   struct libinput_seat *seat;
   struct list link;
   void *user_data;
   int terminated;
   int refcount;
 + struct libinput_device_config config;
  };
  
  typedef void (*libinput_source_dispatch_t)(void *data);
 diff --git a/src/libinput.c b/src/libinput.c
 index 6b7e8b8..6a713bb 100644
 --- a/src/libinput.c
 +++ b/src/libinput.c
 @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct 
 libinput_event_touch *event)
  {
   return event-base;
  }
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_get_finger_count(struct libinput_device *device)
 +{
 + return device-config.tap ? device-config.tap-count(device) : 0;
 +}
 +
 +LIBINPUT_EXPORT enum libinput_config_status
 +libinput_device_config_tap_enable(struct libinput_device *device,
 +   int enable)
 +{
 + if (enable 
 + libinput_device_config_tap_get_finger_count(device) == 0)
 + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
 +
 + return device-config.tap-enable(device, enable);
 +}
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_is_enabled(struct libinput_device *device)
 +{
 + if (libinput_device_config_tap_get_finger_count(device) == 0)
 + return 0;
 +
 + return device-config.tap-is_enabled(device);
 +}
 +
 +LIBINPUT_EXPORT void
 +libinput_device_config_tap_reset(struct libinput_device *device)
 +{
 + if (device-config.tap)
 + device-config.tap-reset(device);
 +}
 diff --git a/src/libinput.h b/src/libinput.h
 index c9ec71a..0c84547 100644
 --- a/src/libinput.h
 +++ b/src/libinput.h
 @@ -1362,8 +1362,79 @@ enum libinput_config_status {
  const char *
  libinput_config_status_to_str(enum libinput_config_status status);
  
 +/**
 + * @ingroup config
 + *
 + * Check if the device supports tap-to-click. See
 + * libinput_device_config_tap_set() for more information.
 + *
 + * @param device The device to configure
 + * @return The number of fingers that can generate a tap event, or 0 if the
 + * device does not support tapping.
 + *
 + * @see libinput_device_config_tap_enable
 + * @see libinput_device_config_tap_is_enabled
 + * @see libinput_device_config_tap_reset
 + */
 +int
 +libinput_device_config_tap_get_finger_count(struct libinput_device *device);
 +
 +/**
 + * @ingroup config
 + *
 + * Enable tap-to-click on this device, with a default mapping of 1, 2, 3
 + * finger tap mapping to left, right, middle click, respectively.
 + * Tapping is limited by the number of simultaneous touches
 + * supported by the device, see
 + * libinput_device_config_tap_get_finger_count().
 + *
 + * @param device The device to configure
 + * @param enable Non-zero to enable, zero to disable
 + *
 + * @return A config status code. Disabling tapping on a device that does not
 + * support tapping always succeeds.
 + *
 + * @see libinput_device_config_tap_get_finger_count
 + * @see libinput_device_config_tap_is_enabled
 + * @see libinput_device_config_tap_reset
 + */
 +enum libinput_config_status
 +libinput_device_config_tap_enable(struct libinput_device *device,
 +   int enable);
 +
 +/**
 + * @ingroup config
 + *
 + * Check if tap-to-click is enabled on this device. If the device does not
 + * support tapping, this function always returns 0.
 + *
 + * @param device The device to configure
 + *
 + * @return 1 if enabled, 0 otherwise.
 + *
 + * @see libinput_device_config_tap_get_finger_count
 + * @see libinput_device_config_tap_enable
 + * @see

Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping

2014-06-04 Thread Jonas Ådahl 
On Wed, Jun 04, 2014 at 10:38:19AM +0200, Hans de Goede wrote:
 Hi,
 
 On 06/04/2014 10:21 AM, Jonas Ådahl wrote:
  On Wed, Jun 04, 2014 at 11:17:58AM +1000, Peter Hutterer wrote:
  On Tue, Jun 03, 2014 at 10:41:16PM +0200, Jonas Ådahl wrote:
  On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote:
  Provide an interface to enable/disable tapping, with a default mapping of
  1/2/3 fingers mapping to L/R/M button events, respectively.
 
  Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
  ---
   src/libinput-private.h | 13 +
   src/libinput.c | 33 +++
   src/libinput.h | 73 
  +-
   3 files changed, 118 insertions(+), 1 deletion(-)
 
  diff --git a/src/libinput-private.h b/src/libinput-private.h
  index 61cdc79..020167e 100644
  --- a/src/libinput-private.h
  +++ b/src/libinput-private.h
  @@ -69,12 +69,25 @@ struct libinput_seat {
   uint32_t button_count[KEY_CNT];
   };
   
  +struct libinput_device_config_tap {
  +int (*count)(struct libinput_device *device);
  +enum libinput_config_status (*enable)(struct libinput_device 
  *device,
  +  int enable);
  +int (*is_enabled)(struct libinput_device *device);
  +void (*reset)(struct libinput_device *device);
  +};
  +
  +struct libinput_device_config {
  +struct libinput_device_config_tap *tap;
  +};
  +
   struct libinput_device {
   struct libinput_seat *seat;
   struct list link;
   void *user_data;
   int terminated;
   int refcount;
  +struct libinput_device_config config;
   };
   
   typedef void (*libinput_source_dispatch_t)(void *data);
  diff --git a/src/libinput.c b/src/libinput.c
  index 6b7e8b8..6a713bb 100644
  --- a/src/libinput.c
  +++ b/src/libinput.c
  @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct 
  libinput_event_touch *event)
   {
   return event-base;
   }
  +
  +LIBINPUT_EXPORT int
  +libinput_device_config_tap_get_finger_count(struct libinput_device 
  *device)
  +{
  +return device-config.tap ? device-config.tap-count(device) : 
  0;
  +}
  +
  +LIBINPUT_EXPORT enum libinput_config_status
  +libinput_device_config_tap_enable(struct libinput_device *device,
  +  int enable)
  +{
  +if (enable 
  +libinput_device_config_tap_get_finger_count(device) == 0)
  +return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
 
  Shouldn't this be returned also if !enable? It would be a bit confusing
  if disabling is allowed, but enabling is not.
 
  This was a conscious decision that applies to the other functions as well:
  if you're doing something that's not technically invalid, we're not
  reporting an error. This means that an error only needs to be handled if
  something really is off.
  
  I'd argue that disabling tapping on a device that doesn't support
  tapping is a technically invalid operation that is simply handled in the
  cases where it makes no sense.
 
 I'm with Peter here, that its easier for our API consumers if they caan
 always safely call disable.

Who would want to disable tapping on a device that doesn't support it?
Anyway, seems its 2-1 on this one so I'll stop now.

 
  
 
  Also, in what circumstances will config.tap be set, but finger count
  returning 0? What type of devices would have tapping configuration
  enabled with no tapping possible?
 
  I expect none, this approach was just the safest and most copy-paste-proof
  way of checking.
   
  +
  +return device-config.tap-enable(device, enable);
  +}
  +
  +LIBINPUT_EXPORT int
  +libinput_device_config_tap_is_enabled(struct libinput_device *device)
  +{
  +if (libinput_device_config_tap_get_finger_count(device) == 0)
  +return 0;
  +
  +return device-config.tap-is_enabled(device);
  +}
  +
  +LIBINPUT_EXPORT void
  +libinput_device_config_tap_reset(struct libinput_device *device)
  +{
  +if (device-config.tap)
  +device-config.tap-reset(device);
  +}
  diff --git a/src/libinput.h b/src/libinput.h
  index c9ec71a..0c84547 100644
  --- a/src/libinput.h
  +++ b/src/libinput.h
  @@ -1362,8 +1362,79 @@ enum libinput_config_status {
   const char *
   libinput_config_status_to_str(enum libinput_config_status status);
   
  +/**
  + * @ingroup config
  + *
  + * Check if the device supports tap-to-click. See
  + * libinput_device_config_tap_set() for more information.
 
  Should it be libinput_device_config_tap_enable() here instead?
 
  ah, thanks. one always escapes the refacturing...
 
  + *
  + * @param device The device to configure
  + * @return The number of fingers that can generate a tap event, or 0 if 
  the
  + * device does not support tapping.
  + *
  + * @see libinput_device_config_tap_enable
  + * @see libinput_device_config_tap_is_enabled
  + * @see

Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping

2014-06-03 Thread Jonas Ådahl 
On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote:
 Provide an interface to enable/disable tapping, with a default mapping of
 1/2/3 fingers mapping to L/R/M button events, respectively.
 
 Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
 ---
  src/libinput-private.h | 13 +
  src/libinput.c | 33 +++
  src/libinput.h | 73 
 +-
  3 files changed, 118 insertions(+), 1 deletion(-)
 
 diff --git a/src/libinput-private.h b/src/libinput-private.h
 index 61cdc79..020167e 100644
 --- a/src/libinput-private.h
 +++ b/src/libinput-private.h
 @@ -69,12 +69,25 @@ struct libinput_seat {
   uint32_t button_count[KEY_CNT];
  };
  
 +struct libinput_device_config_tap {
 + int (*count)(struct libinput_device *device);
 + enum libinput_config_status (*enable)(struct libinput_device *device,
 +   int enable);
 + int (*is_enabled)(struct libinput_device *device);
 + void (*reset)(struct libinput_device *device);
 +};
 +
 +struct libinput_device_config {
 + struct libinput_device_config_tap *tap;
 +};
 +
  struct libinput_device {
   struct libinput_seat *seat;
   struct list link;
   void *user_data;
   int terminated;
   int refcount;
 + struct libinput_device_config config;
  };
  
  typedef void (*libinput_source_dispatch_t)(void *data);
 diff --git a/src/libinput.c b/src/libinput.c
 index 6b7e8b8..6a713bb 100644
 --- a/src/libinput.c
 +++ b/src/libinput.c
 @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct 
 libinput_event_touch *event)
  {
   return event-base;
  }
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_get_finger_count(struct libinput_device *device)
 +{
 + return device-config.tap ? device-config.tap-count(device) : 0;
 +}
 +
 +LIBINPUT_EXPORT enum libinput_config_status
 +libinput_device_config_tap_enable(struct libinput_device *device,
 +   int enable)
 +{
 + if (enable 
 + libinput_device_config_tap_get_finger_count(device) == 0)
 + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;

Shouldn't this be returned also if !enable? It would be a bit confusing
if disabling is allowed, but enabling is not.

Also, in what circumstances will config.tap be set, but finger count
returning 0? What type of devices would have tapping configuration
enabled with no tapping possible?

 +
 + return device-config.tap-enable(device, enable);
 +}
 +
 +LIBINPUT_EXPORT int
 +libinput_device_config_tap_is_enabled(struct libinput_device *device)
 +{
 + if (libinput_device_config_tap_get_finger_count(device) == 0)
 + return 0;
 +
 + return device-config.tap-is_enabled(device);
 +}
 +
 +LIBINPUT_EXPORT void
 +libinput_device_config_tap_reset(struct libinput_device *device)
 +{
 + if (device-config.tap)
 + device-config.tap-reset(device);
 +}
 diff --git a/src/libinput.h b/src/libinput.h
 index c9ec71a..0c84547 100644
 --- a/src/libinput.h
 +++ b/src/libinput.h
 @@ -1362,8 +1362,79 @@ enum libinput_config_status {
  const char *
  libinput_config_status_to_str(enum libinput_config_status status);
  
 +/**
 + * @ingroup config
 + *
 + * Check if the device supports tap-to-click. See
 + * libinput_device_config_tap_set() for more information.

Should it be libinput_device_config_tap_enable() here instead?

 + *
 + * @param device The device to configure
 + * @return The number of fingers that can generate a tap event, or 0 if the
 + * device does not support tapping.
 + *
 + * @see libinput_device_config_tap_enable
 + * @see libinput_device_config_tap_is_enabled
 + * @see libinput_device_config_tap_reset
 + */
 +int
 +libinput_device_config_tap_get_finger_count(struct libinput_device *device);

I wonder where doing things like this, and
libinput_device_config_scroll_get_methods() will lead us. It'd mean we
expose some device information via the config API, and some via direct
device getters, which might not be the best thing for consistency.

Would it be better to keep device info getters, even though they are only
directly relevant to the configuration API, outside of the coniguration
API, so that it'll be the same for all device specific information
retrieval? There could potentially also be some device information that
are relevant to multiple config options, and in those cases, where would
they be retrieved from?


Jonas

 +
 +/**
 + * @ingroup config
 + *
 + * Enable tap-to-click on this device, with a default mapping of 1, 2, 3
 + * finger tap mapping to left, right, middle click, respectively.
 + * Tapping is limited by the number of simultaneous touches
 + * supported by the device, see
 + * libinput_device_config_tap_get_finger_count().
 + *
 + * @param device The device to configure
 + * @param enable Non-zero to enable, zero to disable
 + *
 + * @return A config status code. Disabling tapping on a device that does not