[PATCH libinput 03/10] touchpad: hook up to the tapping configuration

[Hans de Goede]

Hi,

On 06/04/2014 10:57 PM, Jonas Ådahl wrote:
> On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 06/03/2014 07:34 AM, Peter Hutterer wrote:
>>> Now that we have run-time changes of the tap.enabled state move the check
>>> to the IDLE state only. Otherwise the tap machine may hang if tapping is
>>> disabled while a gesture is in progress.
>>>
>>> Two basic tests are added to check for the tap default setting - which is now
>>> "tap disabled by default", because a bike shed's correct color is green.
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>>  src/evdev-mt-touchpad-tap.c | 56 +++++++++++++++++++++++++++++++++++++++------
>>>  src/evdev-mt-touchpad.h     |  1 +
>>>  test/touchpad.c             | 32 ++++++++++++++++++++++++++
>>>  3 files changed, 82 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
>>> index eee334f..24ea8c3 100644
>>> --- a/src/evdev-mt-touchpad-tap.c
>>> +++ b/src/evdev-mt-touchpad-tap.c
>>> @@ -438,13 +438,13 @@ static void
>>>  tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time)
>>>  {
>>>  	enum tp_tap_state current;
>>> -	if (!tp->tap.enabled)
>>> -		return;
>>>  
>>>  	current = tp->tap.state;
>>>  
>>>  	switch(tp->tap.state) {
>>>  	case TAP_STATE_IDLE:
>>> +		if (!tp->tap.enabled)
>>> +			break;
>>>  		tp_tap_idle_handle_event(tp, event, time);
>>>  		break;
>>>  	case TAP_STATE_TOUCH:
>>> @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
>>>  unsigned int
>>>  tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
>>>  {
>>> -	if (!tp->tap.enabled)
>>> -		return 0;
>>> -
>>>  	if (tp->tap.timeout && tp->tap.timeout <= time) {
>>>  		tp_tap_clear_timer(tp);
>>>  		tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
>>> @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
>>>  	return tp->tap.timeout;
>>>  }
>>>  
>>> +static int
>>> +tp_tap_config_count(struct libinput_device *device)
>>> +{
>>> +	struct evdev_dispatch *dispatch = ((struct evdev_device *)device)->dispatch;
>>> +	struct tp_dispatch *tp = container_of(dispatch, tp, base);
>>> +
>>> +	return min(tp->ntouches, 3); /* we only do up to 3 finger tap */
>>> +}
>>> +
>>> +static enum libinput_config_status
>>> +tp_tap_config_enable(struct libinput_device *device, int enabled)
>>> +{
>>> +	struct evdev_dispatch *dispatch = ((struct evdev_device *)device)->dispatch;
>>
>> Please user container_of here for proper type checking rather then
>> just a blatant cast of $random_type_foo to $random_type_bar. Same for all
>> the other occurrences of the same pattern below.
> 
> Would just like to point out that there are several places this cast is
> already in place, and starting to use container_of would introduce
> inconsistencies. I think this kind of casting isn't necessarily bad for
> single (or primary) inheritance types of objects.

I really believe that we should make use of compiler type checking were
possible. Lets go with Peters suggestion to do a separate patch to move
all these kind of casts over to container_of in one big patch.

Regards,

Hans