[PATCH] Add --no-input option to rpi backend

[Pekka Paalanen]

On Fri, 18 Apr 2014 16:12:25 +0200
Fernando Herrera <fherrera at onirica.com> wrote:

> Adds a --no-input option to the rpi backend so weston can be started
> without any input device attached. This is useful for display boxes
> without any keyboard/mouse attached

This commit message forgets to be explicit on an important detail.
With this patch, if --no-input is passed, Weston will never use any
input devices. This is different to just being able to start
without input devices, because now you cannot later plug in a
keyboard and expect it to work.

Is there any reason why you want to forbid the use of all input
devices for all the time?

Or do you only want to let Weston start without any input devices,
but if devices are connected later they would get used?

Btw. if you will never have any input devices, how will apps be
used in your use case? Is this for digital signage, perhaps?

More little comments below.

> Signed-off-by: Fernando Herrera <fherrera at onirica.com>
> ---
>  src/compositor-rpi.c |   29 ++++++++++++++++++-----------
>  src/compositor.c     |    1 +
>  2 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
> index 287451d..f39152d 100644
> --- a/src/compositor-rpi.c
> +++ b/src/compositor-rpi.c
> @@ -488,7 +488,8 @@ struct rpi_parameters {
>  static struct weston_compositor *
>  rpi_compositor_create(struct wl_display *display, int *argc, char *argv[],
>  		      struct weston_config *config,
> -		      struct rpi_parameters *param)
> +		      struct rpi_parameters *param,
> +		      int no_input)
>  {
>  	struct rpi_compositor *compositor;
>  	uint32_t key;
> @@ -503,10 +504,12 @@ rpi_compositor_create(struct wl_display *display, int *argc, char *argv[],
>  				   config) < 0)
>  		goto out_free;
>  
> -	compositor->udev = udev_new();
> -	if (compositor->udev == NULL) {
> -		weston_log("Failed to initialize udev context.\n");
> -		goto out_compositor;
> +	if (!no_input) {
> +		compositor->udev = udev_new();
> +		if (compositor->udev == NULL) {
> +			weston_log("Failed to initialize udev context.\n");
> +			goto out_compositor;
> +		}
>  	}
>  
>  	compositor->session_listener.notify = session_notify;
> @@ -548,11 +551,13 @@ rpi_compositor_create(struct wl_display *display, int *argc, char *argv[],
>  	if (rpi_output_create(compositor, param->output_transform) < 0)
>  		goto out_renderer;
>  
> -	if (udev_input_init(&compositor->input,
> -			    &compositor->base,
> -			    compositor->udev, "seat0") != 0) {
> -		weston_log("Failed to initialize udev input.\n");
> -		goto out_renderer;
> +	if (!no_input) {
> +		if (udev_input_init(&compositor->input,
> +				    &compositor->base,
> +				    compositor->udev, "seat0") != 0) {
> +			weston_log("Failed to initialize udev input.\n");
> +			goto out_renderer;
> +		}

Like said above, these hunks prevent any input devices from being
discovered ever, not just on start.

>  	}
>  
>  	return &compositor->base;
> @@ -581,6 +586,7 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>  	     struct weston_config *config)
>  {
>  	const char *transform = "normal";
> +	int no_input;
>  	int ret;
>  
>  	struct rpi_parameters param = {
> @@ -597,6 +603,7 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>  		{ WESTON_OPTION_STRING, "transform", 0, &transform },
>  		{ WESTON_OPTION_BOOLEAN, "opaque-regions", 0,
>  		  &param.renderer.opaque_regions },
> +		{ WESTON_OPTION_BOOLEAN, "no-input", 0, &no_input },
>  	};
>  
>  	parse_options(rpi_options, ARRAY_LENGTH(rpi_options), argc, argv);
> @@ -607,5 +614,5 @@ backend_init(struct wl_display *display, int *argc, char *argv[],
>  	else
>  		param.output_transform = ret;
>  
> -	return rpi_compositor_create(display, argc, argv, config, &param);
> +	return rpi_compositor_create(display, argc, argv, config, &param, no_input);

Please do not add a new argument to rpi_compositor_create()
function. Add a field to struct rpi_parameters instead. That struct
only exists to avoid having a ton of function parameters.

>  }
> diff --git a/src/compositor.c b/src/compositor.c
> index 7c29d51..723a4f5 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3964,6 +3964,7 @@ usage(int error_code)
>  		"\tnormal 90 180 270 flipped flipped-90 flipped-180 flipped-270\n"
>  		"  --opaque-regions\tEnable support for opaque regions, can be "
>  		"very slow without support in the GPU firmware.\n"
> +		"  --no-input\t\tDont create input devices\n"
>  		"\n");
>  #endif

I agree with Jason, this would be nicer as a Weston core option,
but it's ok as a rpi-specific option, too.


Thanks,
pq