[PATCH weston v2 4/6] libweston: Implement keyboard timestamps for input_timestamps_unstable_v1

[Pekka Paalanen]

On Fri, 16 Feb 2018 18:44:17 +0200
Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:

> Implement the zwp_input_timestamps_manager_v1.get_keyboard_timestamps
> request to subscribe to timestamp events for wl_keyboard resources.
> Ensure that the request handling code can gracefully handle inert
> keyboard resources.
> 
> This commit introduces a few internal helper functions which will also
> be useful in the implementation of the remaining
> zwp_input_timestamps_manager_v1 requests.
> 
> Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> ---
> Changes in v2:
>  - Merge helper functions from v1 3/6 commit in this one to avoid
>    warnings in 3/6 commit.
>  - Remove the head of timestamps_list in weston_keyboard_destroy.
>  - Gracefully handle inert keyboard resources in destroy_keyboard_resource
>    and input_timestamps_manager_get_keyboard_timestamps.
> 
>  libweston/compositor.h |  2 ++
>  libweston/input.c      | 95 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  tests/keyboard-test.c  | 45 ++++++++++++++++++++++++
>  3 files changed, 139 insertions(+), 3 deletions(-)
> 

Hi Alf,

the below pattern repeats in all patches 4-6.

> +TEST(keyboard_timestamps_stop_after_client_releases_wl_keyboard)
> +{
> +	struct client *client = create_client_with_keyboard_focus();
> +	struct keyboard *keyboard = client->input->keyboard;
> +	struct input_timestamps *input_ts =
> +		input_timestamps_create_for_keyboard(client);
> +
> +	send_key(client, &t1, 1, WL_KEYBOARD_KEY_STATE_PRESSED);
> +	assert(keyboard->key_time_msec == timespec_to_msec(&t1));
> +	assert(timespec_eq(&keyboard->key_time_timespec, &t1));
> +
> +	wl_keyboard_release(client->input->keyboard->wl_keyboard);

This destroys the wl_keyboard, meaning that we no longer get events on
it.

> +
> +	send_key(client, &t2, 1, WL_KEYBOARD_KEY_STATE_RELEASED);
> +	assert(keyboard->key_time_msec == timespec_to_msec(&t1));
> +	assert(timespec_eq(&keyboard->key_time_timespec, &t1));

This tests key_time_timespec, however, even if the server erroneously
sent a timestamp, we would not run the wl_keyboard.key event handler
which would modify key_time_timespec. To me it looks like this test
doesn't actually test the server as is.

Should it be checking keyboard->input_timestamp instead?

> +
> +	input_timestamps_destroy(input_ts);
>  }

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180220/cf1ed75f/attachment.sig>