[PATCH libinput 5/6] touchpad: change palm detection trigger functions to bools

[Peter Hutterer]

On Tue, Jul 19, 2016 at 10:46:27AM +0100, Eric Engestrom wrote:
> On Tue, Jul 19, 2016 at 10:49:28AM +1000, Peter Hutterer wrote:
> > And rename to make it more obvious what the return value means.
> > 
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  src/evdev-mt-touchpad.c | 28 +++++++++++++++-------------
> >  1 file changed, 15 insertions(+), 13 deletions(-)
> > 
> > diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> > index 7ee86a9..190448b 100644
> > --- a/src/evdev-mt-touchpad.c
> > +++ b/src/evdev-mt-touchpad.c
> > @@ -573,15 +573,17 @@ tp_palm_tap_is_palm(const struct tp_dispatch *tp, const struct tp_touch *t)
> >  	return false;
> >  }
> >  
> > -static int
> > -tp_palm_detect_dwt(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
> > +static bool
> > +tp_palm_detect_dwt_triggered(struct tp_dispatch *tp,
> > +			     struct tp_touch *t,
> > +			     uint64_t time)
> >  {
> >  	if (tp->dwt.dwt_enabled &&
> >  	    tp->dwt.keyboard_active &&
> >  	    t->state == TOUCH_BEGIN) {
> >  		t->palm.state = PALM_TYPING;
> >  		t->palm.first = t->point;
> > -		return 1;
> > +		return true;
> >  	} else if (!tp->dwt.keyboard_active &&
> >  		   t->state == TOUCH_UPDATE &&
> >  		   t->palm.state == PALM_TYPING) {
> > @@ -599,22 +601,22 @@ tp_palm_detect_dwt(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
> >  		}
> >  	}
> >  
> > -	return 0;
> > +	return false;
> >  }
> >  
> > -static int
> > -tp_palm_detect_trackpoint(struct tp_dispatch *tp,
> > -			  struct tp_touch *t,
> > -			  uint64_t time)
> > +static bool
> > +tp_palm_detect_trackpoint_triggered(struct tp_dispatch *tp,
> > +				    struct tp_touch *t,
> > +				    uint64_t time)
> >  {
> >  	if (!tp->palm.monitor_trackpoint)
> > -		return 0;
> > +		return false;
> >  
> >  	if (t->palm.state == PALM_NONE &&
> >  	    t->state == TOUCH_BEGIN &&
> >  	    tp->palm.trackpoint_active) {
> >  		t->palm.state = PALM_TRACKPOINT;
> > -		return 1;
> > +		return true;
> >  	} else if (t->palm.state == PALM_TRACKPOINT &&
> >  		   t->state == TOUCH_UPDATE &&
> >  		   !tp->palm.trackpoint_active) {
> > @@ -627,7 +629,7 @@ tp_palm_detect_trackpoint(struct tp_dispatch *tp,
> >  		}
> >  	}
> >  
> > -	return 0;
> > +	return false;
> >  }
> >  
> >  static inline bool
> > @@ -684,10 +686,10 @@ static void
> >  tp_palm_detect(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
> >  {
> >  
> > -	if (tp_palm_detect_dwt(tp, t, time))
> > +	if (tp_palm_detect_dwt_triggered(tp, t, time))
> >  		goto out;
> >  
> > -	if (tp_palm_detect_trackpoint(tp, t, time))
> > +	if (tp_palm_detect_trackpoint_triggered(tp, t, time))
> >  		goto out;
> >  
> >  	if (t->palm.state == PALM_EDGE) {
> > -- 
> > 2.7.4
> 
> I feel like the "detect" + "triggered" combo is a bit confusing, and
> `tp_palm_dwt_triggered()` & `tp_palm_trackpoint_triggered()` might be
> better names.

fwiw, the reason it's palm_detect_foo because it's part of the palm
detection. there is one other function tp_palm_tap_is_palm() that only uses
the palm prefix because at this point we already think it's a palm and we're
just continuing from there. That's why I prefer to leave the full prefix
here.

> Either way this whole series is welcome, and is
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
> 
> BTW I was reading an article a few days ago about what a mess the
> success-like return values are in the C world. Maybe you read it too? ^^

heh, no, sorry. it was actually triggered by the safe_atoi discussion we had
on wayland-devel and then spawned the appropriate trainwreck of thought.
thanks for the review, much appreciated.

Cheers,
   Peter