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

[Jagath Jog J]

Hi Jonathan,

On Sat, Jul 16, 2022 at 8:16 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Sun, 26 Jun 2022 18:51:54 +0530
> Jagath Jog J <jagathjog1996@xxxxxxxxx> wrote:
>
> > Hi Jonathan,
> >
> > Sorry for the delay in replying.
> >
> > On Sun, Jun 19, 2022 at 6:15 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > >
> > > 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.
> >
> > The values for the time between the two taps are not in terms of seconds,
> > here period value is in terms of data samples which depends on
> > the data rate.
>
> These time based controls in IIO are always defined in seconds. You'll have
> to correct for the data rate in the driver.
>
> Either just make the available options change with data rate, or have the
> driver do a 'nearest possible' choice based on what was requested and what
> is possible at the current data rate.
>
> It's more complex but it gives a consistent ABI across devices that work
> in many different ways.

TAP interrupts work with a 200 Hz data rate, so all the time-related
config values
are converted to seconds and provided the available options.
I have prepared the v2 and soon I will send the same.

Thank you,
Jagath


>
> Jonathan
>
> >
> > >
> > > 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?
> > >
> > >
> > >
> > >
>