[PATCH 1/1] cmd: allow sound command to play multiple sounds

[Simon Glass]

On Thu, 15 Dec 2022 at 17:56, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> Currently the sound command accepts only one value each for duration and
> frequency. Allowing more duration and frequency arguments enables playing a
> tune.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  cmd/sound.c             | 41 ++++++++++++++++++++++++++++-------------
>  doc/usage/cmd/sound.rst | 23 +++++++++++++++++++++--
>  2 files changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/cmd/sound.c b/cmd/sound.c
> index 20ac3f758e..cef71be5e3 100644
> --- a/cmd/sound.c
> +++ b/cmd/sound.c
> @@ -39,26 +39,39 @@ static int do_play(struct cmd_tbl *cmdtp, int flag, int argc,
>         int ret = 0;
>         int msec = 1000;
>         int freq = 400;
> -
> -       if (argc > 1)
> -               msec = dectoul(argv[1], NULL);
> -       if (argc > 2)
> -               freq = dectoul(argv[2], NULL);
> +       bool first = true;
>
>         ret = uclass_first_device_err(UCLASS_SOUND, &dev);
> -       if (!ret)
> +       if (ret)
> +               goto err;
> +       --argc;
> +       ++argv;
> +       while (argc || first) {
> +               first = false;
> +               if (argc && *argv[0] != '-') {
> +                       msec = dectoul(argv[0], NULL);
> +                       --argc;
> +                       ++argv;
> +               }
> +               if (argc && *argv[0] != '-') {
> +                       freq = dectoul(argv[0], NULL);
> +                       --argc;
> +                       ++argv;
> +               }
>                 ret = sound_beep(dev, msec, freq);
> -       if (ret) {
> -               printf("Sound device failed to play (err=%d)\n", ret);
> -               return CMD_RET_FAILURE;
> +               if (ret)
> +                       goto err;
>         }
> -
>         return 0;
> +
> +err:
> +       printf("Sound device failed to play (err=%d)\n", ret);
> +       return CMD_RET_FAILURE;
>  }
>
>  static struct cmd_tbl cmd_sound_sub[] = {
>         U_BOOT_CMD_MKENT(init, 0, 1, do_init, "", ""),
> -       U_BOOT_CMD_MKENT(play, 2, 1, do_play, "", ""),
> +       U_BOOT_CMD_MKENT(play, INT_MAX, 1, do_play, "", ""),
>  };
>
>  /* process sound command */
> @@ -83,8 +96,10 @@ static int do_sound(struct cmd_tbl *cmdtp, int flag, int argc,
>  }
>
>  U_BOOT_CMD(
> -       sound, 4, 1, do_sound,
> +       sound, INT_MAX, 1, do_sound,
>         "sound sub-system",
>         "init - initialise the sound driver\n"
> -       "sound play [len [freq]] - play a sound for len ms at freq Hz\n"
> +       "sound play [[[-q|-s] len [freq]] ...] - play sounds\n"

Is the -q / -s stuff actually implemented? Assuming that is in another patch:

Reviewed-by: Simon Glass <sjg at chromium.org>

> +       "  len - duration in ms\n"
> +       "  freq - frequency in Hz\n"
>  );
> diff --git a/doc/usage/cmd/sound.rst b/doc/usage/cmd/sound.rst
> index d3fac243b1..174975f2c6 100644
> --- a/doc/usage/cmd/sound.rst
> +++ b/doc/usage/cmd/sound.rst
> @@ -10,12 +10,12 @@ Synopsis
>  ::
>
>      sound init
> -    sound play [len [freq]]
> +    sound play [[len freq] ...] [len [freq]]
>
>  Description
>  -----------
>
> -The *sound* command is used to play a beep sound.
> +The *sound* command is used to play one or multiple beep sounds.
>
>  sound init
>      initializes the sound driver.
> @@ -30,6 +30,25 @@ len
>  freq
>      frequency of the sound in Hz, defaults to 400 Hz
>
> +Examples
> +--------
> +
> +Beep at 400 Hz for 1000 ms::
> +
> +    sound play
> +
> +Beep at 400 Hz for 600 ms::
> +
> +    sound play 600
> +
> +Beep at 500 Hz for 600 ms::
> +
> +    sound play 600 500
> +
> +Play melody::
> +
> +    sound play 500 1047 500 880 500 0 500 1047 500 880 500 0 500 784 500 698 500 784 1000 698
> +
>  Configuration
>  -------------
>
> --
> 2.37.2
>

Regards,
Simon