Re: [PATCH v1 1/2] iio: Add new event type gesture and use direction for single and double tap

[Jonathan Cameron]

On Tue, 14 Jun 2022 00:47:05 +0530
Jagath Jog J <jagathjog1996@xxxxxxxxx> wrote:

> Add new event type for tap called gesture and the direction can be used
> to differentiate single and double tap. This may be used by accelerometer
> sensors to express single and double tap events. For directional tap,
> modifiers like IIO_MOD_(X/Y/Z) can be used along with singletap and
> doubletap direction.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@xxxxxxxxx>

Hi,

With fresh eyes I think we need to rethink the use of _period to make sure
we have 'space' for another very like ABI element which is the maximum
time between events for them to be considered a double tap.

Jonathan

> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 24 ++++++++++++++++++++++++
>  drivers/iio/industrialio-event.c        |  5 ++++-
>  include/uapi/linux/iio/types.h          |  3 +++
>  tools/iio/iio_event_monitor.c           |  8 +++++++-
>  4 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 3e00d7f7ee22..4eaf85e01911 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -2035,3 +2035,27 @@ Description:
>  		Available range for the forced calibration value, expressed as:
>  
>  		- a range specified as "[min step max]"
> +
> +What:		/sys/.../events/in_accel_gesture_singletap_en
> +What:		/sys/.../events/in_accel_gesture_doubletap_en
> +KernelVersion:	5.19
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		Device generates an event on a single or double tap.
> +
> +What:		/sys/.../events/in_accel_gesture_singletap_value
> +What:		/sys/.../events/in_accel_gesture_doubletap_value
> +KernelVersion:	5.19
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		Specifies the threshold value that the device is comparing
> +		against to generate the tap gesture event. Units and exact
> +		meaning of value are device specific.

I'm fine with this one being device specific, as likely a complex alg
involved.

> +
> +What:		/sys/.../events/in_accel_gesture_doubletap_period
> +KernelVersion:	5.19
> +Contact:	linux-iio@xxxxxxxxxxxxxxx
> +Description:
> +		Minimum time period between before and after the double tap
> +		event. Units and exact meaning of period value are device
> +		specific.

I think the units need to be standard.  Also would this work better
as a description?

	Minimum time in seconds between the two taps making up a double
        tap event.

Raises a question though. How would we specify the maximum time? I.e.
if taps are further apart than Xseconds, they are reported as two single
taps.

Maybe reusing period here isn't a good idea and we need to have new ABI for
this?