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

[Jagath Jog J]

Hi Jonathan,

On Sun, Aug 14, 2022 at 10:24 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Sat, 13 Aug 2022 12:48:02 +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 Jagath,
>
> This ABI is definitely something I want more eyes than ours on, so
> whatever happens I'll leave it on the list for a few weeks.

Sure, I will leave KernelVersion blank in the next series.

>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 41 +++++++++++++++++++++++++
> >  drivers/iio/industrialio-event.c        |  7 ++++-
> >  include/linux/iio/types.h               |  2 ++
> >  include/uapi/linux/iio/types.h          |  3 ++
> >  tools/iio/iio_event_monitor.c           |  8 ++++-
> >  5 files changed, 59 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index e81ba6f5e1c8..54cb925f714c 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -2038,3 +2038,44 @@ 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.21
> > +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.21
> > +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 wonder if we should list a direction?  As in smaller is more sensitive?

Yeah in most of the devices which support tap, this value represents the
threshold, the lower the value higher the tap sensitivity. I will add it to the
description in the next series.

> (at least to first approximation)
Do I need to add available attributes into ABI docs?

> That way a user would at least be able to consistently decide if they should
> raise or lower the number to get the perf the want.
>
> > +
> > +What:                /sys/.../events/in_accel_gesture_singletap_reset_timeout
> > +What:                /sys/.../events/in_accel_gesture_doubletap_reset_timeout
> > +KernelVersion:       5.21
> > +Contact:     linux-iio@xxxxxxxxxxxxxxx
> > +Description:
> > +             Specifies the timeout value in seconds for the tap detector
> > +             to not to look for another tap event after the event as
> > +             occoured. Basically the minimum quiet time between the two
> spelling.  occured

Sorry, I will correct this.

Thank you
Jagath

>
> > +             single-tap's or two double-tap's.
> > +
> > +What:                /sys/.../events/in_accel_gesture_doubletap_tap_2min_delay
>
> I'm not sure this naming is intuitive enough. Might be a simple
> as doubletap_tap2_min_delay?  My brain didn't parse 2min correctly.
>
> This one is a bit odd, so definitely want to hear more view points on whether
> this is general enough to cover sensors and intuitive enough that people
> have some hope of setting it right.
>
> > +KernelVersion:       5.21
> > +Contact:     linux-iio@xxxxxxxxxxxxxxx
> > +Description:
> > +             Specifies the minimum quiet time in seconds between the two
> > +             taps of a double tap.
> > +
> > +What:                /sys/.../events/in_accel_gesture_maxtomin_time
> > +KernelVersion:       5.21
> > +Contact:     linux-iio@xxxxxxxxxxxxxxx
> > +Description:
> > +             Specifies the maximum time difference allowed between upper
> > +             and lower peak of tap to consider it as the valid tap event.
> > +             Units in seconds.
> Needs to be associated with 'tap' in the naming.
> Easiest is probably only to define it as
> singletap_maxtomin_time + doubletap_maxtomin_time and not have the
> shared version as we'd lose the 'tap' part of the name.
>
>