| Message ID | 20220613191706.31239-2-jagathjog1996@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: Add single and double tap events support | expand |
On Tue, 14 Jun 2022 00:47:05 +0530 Jagath Jog J <jagathjog1996@gmail.com> 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@gmail.com> 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@vger.kernel.org > +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@vger.kernel.org > +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@vger.kernel.org > +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?
Hi Jonathan, Sorry for the delay in replying. On Sun, Jun 19, 2022 at 6:15 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Tue, 14 Jun 2022 00:47:05 +0530 > Jagath Jog J <jagathjog1996@gmail.com> 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@gmail.com> > > 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@vger.kernel.org > > +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@vger.kernel.org > > +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@vger.kernel.org > > +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. > > 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? > > > >
On Sun, 26 Jun 2022 18:51:54 +0530 Jagath Jog J <jagathjog1996@gmail.com> wrote: > Hi Jonathan, > > Sorry for the delay in replying. > > On Sun, Jun 19, 2022 at 6:15 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Tue, 14 Jun 2022 00:47:05 +0530 > > Jagath Jog J <jagathjog1996@gmail.com> 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@gmail.com> > > > > 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@vger.kernel.org > > > +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@vger.kernel.org > > > +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@vger.kernel.org > > > +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. 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? > > > > > > > >
Hi Jonathan, On Sat, Jul 16, 2022 at 8:16 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sun, 26 Jun 2022 18:51:54 +0530 > Jagath Jog J <jagathjog1996@gmail.com> wrote: > > > Hi Jonathan, > > > > Sorry for the delay in replying. > > > > On Sun, Jun 19, 2022 at 6:15 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > > > > On Tue, 14 Jun 2022 00:47:05 +0530 > > > Jagath Jog J <jagathjog1996@gmail.com> 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@gmail.com> > > > > > > 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@vger.kernel.org > > > > +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@vger.kernel.org > > > > +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@vger.kernel.org > > > > +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? > > > > > > > > > > > > >
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@vger.kernel.org +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@vger.kernel.org +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. + +What: /sys/.../events/in_accel_gesture_doubletap_period +KernelVersion: 5.19 +Contact: linux-iio@vger.kernel.org +Description: + Minimum time period between before and after the double tap + event. Units and exact meaning of period value are device + specific. diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c index b5e059e15b0a..22d59eb0a8a2 100644 --- a/drivers/iio/industrialio-event.c +++ b/drivers/iio/industrialio-event.c @@ -231,12 +231,15 @@ static const char * const iio_ev_type_text[] = { [IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive", [IIO_EV_TYPE_CHANGE] = "change", [IIO_EV_TYPE_MAG_REFERENCED] = "mag_referenced", + [IIO_EV_TYPE_GESTURE] = "gesture", }; static const char * const iio_ev_dir_text[] = { [IIO_EV_DIR_EITHER] = "either", [IIO_EV_DIR_RISING] = "rising", - [IIO_EV_DIR_FALLING] = "falling" + [IIO_EV_DIR_FALLING] = "falling", + [IIO_EV_DIR_SINGLETAP] = "singletap", + [IIO_EV_DIR_DOUBLETAP] = "doubletap", }; static const char * const iio_ev_info_text[] = { diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h index 472cead10d8d..913864221ac4 100644 --- a/include/uapi/linux/iio/types.h +++ b/include/uapi/linux/iio/types.h @@ -105,6 +105,7 @@ enum iio_event_type { IIO_EV_TYPE_MAG_ADAPTIVE, IIO_EV_TYPE_CHANGE, IIO_EV_TYPE_MAG_REFERENCED, + IIO_EV_TYPE_GESTURE, }; enum iio_event_direction { @@ -112,6 +113,8 @@ enum iio_event_direction { IIO_EV_DIR_RISING, IIO_EV_DIR_FALLING, IIO_EV_DIR_NONE, + IIO_EV_DIR_SINGLETAP, + IIO_EV_DIR_DOUBLETAP, }; #endif /* _UAPI_IIO_TYPES_H_ */ diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c index 2f4581658859..b3b3ea399f67 100644 --- a/tools/iio/iio_event_monitor.c +++ b/tools/iio/iio_event_monitor.c @@ -69,12 +69,15 @@ static const char * const iio_ev_type_text[] = { [IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive", [IIO_EV_TYPE_CHANGE] = "change", [IIO_EV_TYPE_MAG_REFERENCED] = "mag_referenced", + [IIO_EV_TYPE_GESTURE] = "gesture", }; static const char * const iio_ev_dir_text[] = { [IIO_EV_DIR_EITHER] = "either", [IIO_EV_DIR_RISING] = "rising", - [IIO_EV_DIR_FALLING] = "falling" + [IIO_EV_DIR_FALLING] = "falling", + [IIO_EV_DIR_SINGLETAP] = "singletap", + [IIO_EV_DIR_DOUBLETAP] = "doubletap", }; static const char * const iio_modifier_names[] = { @@ -227,6 +230,7 @@ static bool event_is_known(struct iio_event_data *event) case IIO_EV_TYPE_THRESH_ADAPTIVE: case IIO_EV_TYPE_MAG_ADAPTIVE: case IIO_EV_TYPE_CHANGE: + case IIO_EV_TYPE_GESTURE: break; default: return false; @@ -236,6 +240,8 @@ static bool event_is_known(struct iio_event_data *event) case IIO_EV_DIR_EITHER: case IIO_EV_DIR_RISING: case IIO_EV_DIR_FALLING: + case IIO_EV_DIR_SINGLETAP: + case IIO_EV_DIR_DOUBLETAP: case IIO_EV_DIR_NONE: break; default:
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@gmail.com> --- 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(-)