[PATCH weston 5/8] tests: Add checks for pointer motion and button event timestamps

[Alexandros Frantzis]

On Tue, Dec 12, 2017 at 02:13:26PM +0200, Pekka Paalanen wrote:
> On Mon,  4 Dec 2017 15:34:05 +0200
> Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:
> 
> > Enhance the existing pointer motion and button event tests to
> > additionally verify the event timestamps. This requires updating the
> > weston-test protocol to support passing motion and button event
> > timestamps.
> > 
> > Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> > ---
> >  protocol/weston-test.xml          |  6 ++++++
> >  tests/internal-screenshot-test.c  |  2 +-
> >  tests/pointer-test.c              | 45 ++++++++++++++++++++++++++++++---------
> >  tests/subsurface-shot-test.c      |  2 +-
> >  tests/weston-test-client-helper.c |  2 ++
> >  tests/weston-test-client-helper.h |  2 ++
> >  tests/weston-test.c               |  6 ++++--
> >  7 files changed, 51 insertions(+), 14 deletions(-)
> > 
> > diff --git a/protocol/weston-test.xml b/protocol/weston-test.xml
> > index 74a15214..ae3349ed 100644
> > --- a/protocol/weston-test.xml
> > +++ b/protocol/weston-test.xml
> > @@ -40,10 +40,16 @@
> >        <arg name="y" type="int"/>
> >      </request>
> >      <request name="move_pointer">
> > +      <arg name="tv_sec_hi" type="uint"/>
> > +      <arg name="tv_sec_lo" type="uint"/>
> > +      <arg name="tv_nsec" type="uint"/>
> >        <arg name="x" type="int"/>
> >        <arg name="y" type="int"/>
> >      </request>
> >      <request name="send_button">
> > +      <arg name="tv_sec_hi" type="uint"/>
> > +      <arg name="tv_sec_lo" type="uint"/>
> > +      <arg name="tv_nsec" type="uint"/>
> >        <arg name="button" type="int"/>
> >        <arg name="state" type="uint"/>
> >      </request>
> 
> Hi Alf,
> 
> this patch looks good, just one idea below.

Hi Pekka,

> > diff --git a/tests/weston-test-client-helper.h b/tests/weston-test-client-helper.h
> > index a288af7e..08817242 100644
> > --- a/tests/weston-test-client-helper.h
> > +++ b/tests/weston-test-client-helper.h
> > @@ -90,6 +90,8 @@ struct pointer {
> >  	int y;
> >  	uint32_t button;
> >  	uint32_t state;
> > +	uint32_t motion_time;
> > +	uint32_t button_time;
> 
> I assume these will either be replaced or complemented with the precise
> timestamp values in the future. For that and also in general, it would
> be nice to use names like motion_msec or motion_time_msec to carry the
> units. Personally I've found it very easy to forget what they are
> otherwise.
> 
> This applies to all patches in this series.

In the current iteration of the Weston input-timestamps proof of concept
branch I am using a different test helper object to store the precise
timestamps (I wanted to keep non-core functionality separate), so there
is no confusion, but of course this may change.

Regardless of the above, I named the variables after the event name
fields (e.g. motion.time), but it's indeed helpful to add the unit info
in the name (since unfortunately it's not part of the type) for clarity.
I will update the code to use the prefix.

Thanks,
Alexandros