[PATCH weston 5/8] tests: Support setting the test client input dynamically

[Pekka Paalanen]

On Fri, 26 Jan 2018 18:47:59 +0200
Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:

> The current test client code waits for all wl_seat globals to arrive
> before checking them and deciding which one is the test seat global to
> use for the input object. This method doesn't support dynamic addition
> of the test seat global (i.e., after client start-up), which will be
> needed in upcoming commits.
> 
> This commit changes the code to check for the test seat and set up the
> input object while handling the wl_seat information events.
> 
> Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> ---
>  tests/weston-test-client-helper.c | 78 ++++++++++++++++++++++-----------------
>  tests/weston-test-client-helper.h |  1 +
>  2 files changed, 46 insertions(+), 33 deletions(-)

Hi,

I feel this patch might be doing more than what it says on the tin:
- move input_update_devices() call from client_set_input() into the
  event handlers
- remove the check for all seats must have a name

I suppose justifying these in the commit message would be enough in
this case.

> diff --git a/tests/weston-test-client-helper.c b/tests/weston-test-client-helper.c
> index 6e0a5246..854978d0 100644
> --- a/tests/weston-test-client-helper.c
> +++ b/tests/weston-test-client-helper.c
> @@ -538,6 +538,14 @@ static const struct weston_test_listener test_listener = {
>  	test_handle_capture_screenshot_done,
>  };
>  
> +static void
> +input_destroy(struct input *inp)
> +{
> +	wl_list_remove(&inp->link);
> +	wl_seat_destroy(inp->wl_seat);
> +	free(inp);
> +}
> +
>  static void
>  input_update_devices(struct input *input)
>  {
> @@ -598,22 +606,56 @@ seat_handle_capabilities(void *data, struct wl_seat *seat,
>  
>  	/* we will create/update the devices only with the right (test) seat.
>  	 * If we haven't discovered which seat is the test seat, just
> -	 * store capabilities and bail out */
> -	if (input->seat_name && strcmp(input->seat_name, "test-seat") == 0)
> +	 * store capabilities and bail out. Note that we don't need to
> +	 * check the name contents here, since only the test seat input will
> +	 * have its name set */
> +	if (input->seat_name) {
>  		input_update_devices(input);

In fact, this bit of old code was not actually necessary, because
client_set_input() would have done the right thing anyway. Your patch
fixes this "imbalance" by making both event handlers call
input_update_devices().

> +		input->client->input = input;
> +	}
> +
>  
>  	fprintf(stderr, "test-client: got seat %p capabilities: %x\n",
>  		input, caps);
>  }
>  
> +static struct input *
> +client_find_input_with_seat_name(struct client *client, const char *name)
> +{

This function seems unnecessary, because...

> +	struct input *input;
> +
> +	wl_list_for_each(input, &client->inputs, link) {
> +		if (input->seat_name && strcmp(input->seat_name, name) == 0)
> +			return input;
> +	}
> +
> +	return NULL;
> +}
> +
>  static void
>  seat_handle_name(void *data, struct wl_seat *seat, const char *name)
>  {
>  	struct input *input = data;
>  
> +	/* We don't care about seats other than the test seat */
> +	if (strcmp(name, "test-seat") != 0) {
> +		input_destroy(input);
> +		return;
> +	}
> +
> +	assert(!client_find_input_with_seat_name(input->client, name) &&
> +	       "Multiple test seats detected!");

..this test could as well be done...

> +
>  	input->seat_name = strdup(name);
>  	assert(input->seat_name && "No memory");
>  
> +	/* We assume that the test seat always has some capabilities,
> +	 * so caps == 0 just means that we haven't received them yet */

We don't actually need this assumption, because whether the caps really
are zero or are not yet handled, the result should be exactly the same.
We can uncoditionally set client->input here, and remove the setting
from the capabilities handler. input_update_devices() also already does
the right thing regardless whether capabilities have been received yet.

> +	if (input->caps != 0) {
> +		input_update_devices(input);
> +		input->client->input = input;

..as an assert on this assignment that client->input must be NULL
before we set it.


Thanks,
pq

> +	}
> +
>  	fprintf(stderr, "test-client: got seat %p name: \'%s\'\n",
>  		input, name);
>  }
> @@ -705,6 +747,7 @@ handle_global(void *data, struct wl_registry *registry,
>  					 &wl_compositor_interface, version);
>  	} else if (strcmp(interface, "wl_seat") == 0) {
>  		input = xzalloc(sizeof *input);
> +		input->client = client;
>  		input->wl_seat =
>  			wl_registry_bind(registry, id,
>  					 &wl_seat_interface, version);
> @@ -809,34 +852,6 @@ log_handler(const char *fmt, va_list args)
>  	vfprintf(stderr, fmt, args);
>  }
>  
> -static void
> -input_destroy(struct input *inp)
> -{
> -	wl_list_remove(&inp->link);
> -	wl_seat_destroy(inp->wl_seat);
> -	free(inp);
> -}
> -
> -/* find the test-seat and set it in client.
> - * Destroy other inputs */
> -static void
> -client_set_input(struct client *cl)
> -{
> -	struct input *inp, *inptmp;
> -	wl_list_for_each_safe(inp, inptmp, &cl->inputs, link) {
> -		assert(inp->seat_name && "BUG: input with no name");
> -		if (strcmp(inp->seat_name, "test-seat") == 0) {
> -			cl->input = inp;
> -			input_update_devices(inp);
> -		} else {
> -			input_destroy(inp);
> -		}
> -	}
> -
> -	/* we keep only one input */
> -	assert(wl_list_length(&cl->inputs) == 1);
> -}
> -
>  struct client *
>  create_client(void)
>  {
> @@ -862,9 +877,6 @@ create_client(void)
>  	 * events */
>  	client_roundtrip(client);
>  
> -	/* find the right input for us */
> -	client_set_input(client);
> -
>  	/* must have WL_SHM_FORMAT_ARGB32 */
>  	assert(client->has_argb);
>  
> diff --git a/tests/weston-test-client-helper.h b/tests/weston-test-client-helper.h
> index 09a5df4a..f16356e5 100644
> --- a/tests/weston-test-client-helper.h
> +++ b/tests/weston-test-client-helper.h
> @@ -74,6 +74,7 @@ struct test {
>  };
>  
>  struct input {
> +	struct client *client;
>  	struct wl_seat *wl_seat;
>  	struct pointer *pointer;
>  	struct keyboard *keyboard;

-------------- 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/20180201/ce72b370/attachment.sig>