Re: [PATCH] lib/igt_aux: Improve wait_for_keypress helper a bit

[Damien Lespiau <damien.lespiau@intel.com>]

On Fri, Sep 05, 2014 at 08:52:31AM +0200, Daniel Vetter wrote:
> - Use keys in just one env variable to enable/disable it.
> - Add an informational message so that the users knows when to press
>   the key (more useful over ssh than when run on the terminal ofc).
> - Improve the documentation so that it's clearer how to use this
>   when running tests.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Looks good to me.

Acked-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  lib/igt_aux.c | 26 +++++++++++++++++++++-----
>  lib/igt_aux.h |  2 +-
>  lib/igt_kms.c |  3 +--
>  3 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 5ddc8b610895..05cb4bd1a533 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -344,21 +344,37 @@ void igt_drop_root(void)
>  }
>  
>  /**
> - * igt_wait_for_keypress:
> + * igt_debug_wait_for_keypress:
> + * @key: env var lookup to to enable this wait
>   *
> - * Waits for a key press when run interactively. When not connected to a
> - * terminal immediately continues.
> + * Waits for a key press when run interactively and when the corresponding debug
> + * key is set in the IGT_DEBUG_INTERACTIVE environment variable. Multiple keys
> + * can be specified as a comma-separated list or alternatively "all" if a wait
> + * should happen for all keys.  When not connected to a terminal the enviroment
> + * setting is ignored and execution immediately continues.
>   *
>   * This is useful for display tests where under certain situation manual
> - * inspection of the display is useful.
> + * inspection of the display is useful. Or when running a testcase in the
> + * background.
>   */
> -void igt_wait_for_keypress(void)
> +void igt_debug_wait_for_keypress(const char *key)
>  {
>  	struct termios oldt, newt;
> +	const char *env;
>  
>  	if (!isatty(STDIN_FILENO))
>  		return;
>  
> +	env = getenv("IGT_DEBUG_INTERACTIVE");
> +
> +	if (!env)
> +		return;
> +
> +	if (!strstr(env, key) && !strstr(env, "all"))
> +		return;
> +
> +	igt_info("Press any key to continue ...\n");
> +
>  	tcgetattr ( STDIN_FILENO, &oldt );
>  	newt = oldt;
>  	newt.c_lflag &= ~( ICANON | ECHO );
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index a90d8d9e6d95..d958abeb0e84 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -56,7 +56,7 @@ void igt_system_suspend_autoresume(void);
>  /* dropping priviledges */
>  void igt_drop_root(void);
>  
> -void igt_wait_for_keypress(void);
> +void igt_debug_wait_for_keypress(const char *key);
>  
>  enum igt_runtime_pm_status {
>  	IGT_RUNTIME_PM_STATUS_ACTIVE,
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index d763013cf15f..f483e2daf755 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1401,8 +1401,7 @@ static int do_display_commit(igt_display_t *display,
>  
>  	LOG_UNINDENT(display);
>  
> -	if (getenv("IGT_DISPLAY_WAIT_AT_COMMIT"))
> -		igt_wait_for_keypress();
> +	igt_debug_wait_for_keypress("modeset");
>  
>  	return 0;
>  }
> -- 
> 1.9.3
>