[pulseaudio-discuss] [PATCH 3/5] pactl, pacmd, cli-command: Add send-message command
Tanu Kaskinen
tanuk at iki.fi
Tue Jan 9 19:30:36 UTC 2018
On Sun, 2017-10-29 at 20:51 +0100, Georg Chini wrote:
> ---
> doc/messaging_api.txt | 16 ++++++++++++++
> man/pactl.1.xml.in | 7 ++++++
> man/pulse-cli-syntax.5.xml.in | 7 ++++++
> shell-completion/bash/pulseaudio | 5 +++--
> shell-completion/zsh/_pulseaudio | 2 ++
> src/pulsecore/cli-command.c | 44 ++++++++++++++++++++++++++++++++++++++
> src/utils/pacmd.c | 1 +
> src/utils/pactl.c | 46 +++++++++++++++++++++++++++++++++++++++-
> 8 files changed, 125 insertions(+), 3 deletions(-)
> create mode 100644 doc/messaging_api.txt
>
> diff --git a/doc/messaging_api.txt b/doc/messaging_api.txt
> new file mode 100644
> index 00000000..11835cda
> --- /dev/null
> +++ b/doc/messaging_api.txt
> @@ -0,0 +1,16 @@
> +Message API reference
> +
> +The message API allows any object within pulseaudio to register a message
> +handler. A message handler is a function that can be called by clients using
> +PA_COMMAND_SEND_OBJECT_MESSAGE. A message consists at least of a recipient
> +and a message command, both specified as strings. Additional parameters can
> +be specified using a single string, but are not mandatory. The message handler
> +returns an error number as defined in def.h and may also return a string in
> +the "response" variable. If "response" is NULL, this should be treated like
> +an empty string. The following reference lists available messages, their
> +parameters and return values.
> +
> +Recipient:
> +Message:
> +Parameters:
> +Return value:
> diff --git a/man/pactl.1.xml.in b/man/pactl.1.xml.in
> index 39569b6b..9669aca9 100644
> --- a/man/pactl.1.xml.in
> +++ b/man/pactl.1.xml.in
> @@ -245,6 +245,13 @@ License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
> 'ac3-iec61937, format.rate = "[ 32000, 44100, 48000 ]"').
> </p></optdesc> </option>
>
> + <option>
> + <p><opt>send-message</opt> <arg>RECIPIENT</arg> <arg>MESSAGE</arg> <arg>MESSAGE_PARAMETERS</arg></p>
> + <optdesc><p>Send a message string to the specified recipient object. If applicable an additional string containing
> + message parameters can be specified. A string is returned as a response to the message. For available message
> + commands see doc/messaging_api.txt.</p></optdesc>
Instead of just "doc/messaging_api.txt", the man page should provide a
link to the file.
> + </option>
> +
> <option>
> <p><opt>subscribe</opt></p>
> <optdesc><p>Subscribe to events, pactl does not exit by itself, but keeps waiting for new events.</p></optdesc>
> diff --git a/man/pulse-cli-syntax.5.xml.in b/man/pulse-cli-syntax.5.xml.in
> index 0a0fabaf..0d870740 100644
> --- a/man/pulse-cli-syntax.5.xml.in
> +++ b/man/pulse-cli-syntax.5.xml.in
> @@ -297,6 +297,13 @@ License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
> <optdesc><p>Debug: Show shared properties.</p></optdesc>
> </option>
>
> + <option>
> + <p><opt>send-message</opt> <arg>recipient</arg> <arg>message</arg> <arg>message_parameters</arg></p>
> + <optdesc><p>Send a message string to the specified recipient object. If applicable an additional string containing
> + message parameters can be specified. A string is returned as a response to the message. For available message
> + commands see doc/messaging_api.txt.</p></optdesc>
Same as above.
> + </option>
> +
> <option>
> <p><opt>exit</opt></p>
> <optdesc><p>Terminate the daemon. If you want to terminate a CLI
> diff --git a/shell-completion/bash/pulseaudio b/shell-completion/bash/pulseaudio
> index e473b9c2..797ec067 100644
> --- a/shell-completion/bash/pulseaudio
> +++ b/shell-completion/bash/pulseaudio
> @@ -120,7 +120,8 @@ _pactl() {
> set-source-port set-sink-volume set-source-volume
> set-sink-input-volume set-source-output-volume set-sink-mute
> set-source-mute set-sink-input-mute set-source-output-mute
> - set-sink-formats set-port-latency-offset subscribe help)
> + set-sink-formats set-port-latency-offset subscribe send-message
> + help)
>
> _init_completion -n = || return
> preprev=${words[$cword-2]}
> @@ -270,7 +271,7 @@ _pacmd() {
> move-sink-input move-source-output suspend-sink suspend-source
> suspend set-card-profile set-sink-port set-source-port
> set-port-latency-offset set-log-target set-log-level set-log-meta
> - set-log-time set-log-backtrace)
> + set-log-time set-log-backtrace send-message)
> _init_completion -n = || return
> preprev=${words[$cword-2]}
>
> diff --git a/shell-completion/zsh/_pulseaudio b/shell-completion/zsh/_pulseaudio
> index 0e9e89bd..a2817ebb 100644
> --- a/shell-completion/zsh/_pulseaudio
> +++ b/shell-completion/zsh/_pulseaudio
> @@ -263,6 +263,7 @@ _pactl_completion() {
> 'set-sink-input-mute: mute a stream'
> 'set-source-output-mute: mute a recording stream'
> 'set-sink-formats: set supported formats of a sink'
> + 'send-message: send a message to a pulseaudio object'
> 'subscribe: subscribe to events'
> )
>
> @@ -561,6 +562,7 @@ _pacmd_completion() {
> 'dump: show daemon configuration'
> 'dump-volumes: show the state of all volumes'
> 'shared: show shared properties'
> + 'send-message: send a message to a pulseaudio object'
> 'exit: ask the PulseAudio daemon to exit'
> )
> _describe 'pacmd commands' _pacmd_commands
> diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c
> index 44795b0d..459abc95 100644
> --- a/src/pulsecore/cli-command.c
> +++ b/src/pulsecore/cli-command.c
> @@ -53,6 +53,7 @@
> #include <pulsecore/sound-file-stream.h>
> #include <pulsecore/shared.h>
> #include <pulsecore/core-util.h>
> +#include <pulsecore/message-handler.h>
> #include <pulsecore/core-error.h>
> #include <pulsecore/modinfo.h>
> #include <pulsecore/dynarray.h>
> @@ -135,6 +136,7 @@ static int pa_cli_command_sink_port(pa_core *c, pa_tokenizer *t, pa_strbuf *buf,
> static int pa_cli_command_source_port(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
> static int pa_cli_command_port_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
> static int pa_cli_command_dump_volumes(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
> +static int pa_cli_command_send_message_to_object(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail);
>
> /* A method table for all available commands */
>
> @@ -191,6 +193,7 @@ static const struct command commands[] = {
> { "set-log-meta", pa_cli_command_log_meta, "Show source code location in log messages (args: bool)", 2},
> { "set-log-time", pa_cli_command_log_time, "Show timestamps in log messages (args: bool)", 2},
> { "set-log-backtrace", pa_cli_command_log_backtrace, "Show backtrace in log messages (args: frames)", 2},
> + { "send-message", pa_cli_command_send_message_to_object, "Send a message to an object (args: recipient, message, message_parameters)", 4},
> { "play-file", pa_cli_command_play_file, "Play a sound file (args: filename, sink|index)", 3},
> { "dump", pa_cli_command_dump, "Dump daemon configuration", 1},
> { "dump-volumes", pa_cli_command_dump_volumes, "Debug: Show the state of all volumes", 1 },
> @@ -1779,6 +1782,47 @@ static int pa_cli_command_port_offset(pa_core *c, pa_tokenizer *t, pa_strbuf *bu
> return 0;
> }
>
> +static int pa_cli_command_send_message_to_object(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail) {
> + const char *recipient_name, *message, *message_parameters;
> + char *response = NULL;
> + int ret;
> +
> + pa_core_assert_ref(c);
> + pa_assert(t);
> + pa_assert(buf);
> + pa_assert(fail);
> +
> +
> + if (!(recipient_name = pa_tokenizer_get(t, 1))) {
> + pa_strbuf_puts(buf, "You need to specify a recipient for the message.\n");
> + return -1;
> + }
> +
> + if (!(message = pa_tokenizer_get(t, 2))) {
> + pa_strbuf_puts(buf, "You need to specify a message string.\n");
"message name" instead of "message string" would be a clearer term in
my opinion.
> + return -1;
> + }
> +
> + /* parameters may be NULL */
> + message_parameters = pa_tokenizer_get(t, 3);
> +
> + ret = pa_message_handler_send_message(c, recipient_name, message, message_parameters, &response);
> +
> + if (ret < 0) {
> + pa_strbuf_printf(buf, "Send message failed: %s\n", pa_strerror(ret));
> + ret = -1;
> +
> + } else {
> + if (response)
> + pa_strbuf_puts(buf, response);
> + pa_strbuf_puts(buf, "\n");
> + ret = 0;
> + }
> +
> + pa_xfree(response);
> + return ret;
> +}
> +
> static int pa_cli_command_dump(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, bool *fail) {
> pa_module *m;
> pa_sink *sink;
> diff --git a/src/utils/pacmd.c b/src/utils/pacmd.c
> index 616573cc..6eae6c42 100644
> --- a/src/utils/pacmd.c
> +++ b/src/utils/pacmd.c
> @@ -77,6 +77,7 @@ static void help(const char *argv0) {
> printf("%s %s %s\n", argv0, "set-log-meta", _("1|0"));
> printf("%s %s %s\n", argv0, "set-log-time", _("1|0"));
> printf("%s %s %s\n", argv0, "set-log-backtrace", _("FRAMES"));
> + printf("%s %s %s\n", argv0, "send-message", _("RECIPIENT MESSAGE [MESSAGE_PARAMETERS]"));
>
> printf(_("\n"
> " -h, --help Show this help\n"
> diff --git a/src/utils/pactl.c b/src/utils/pactl.c
> index e9bf005b..2e15b189 100644
> --- a/src/utils/pactl.c
> +++ b/src/utils/pactl.c
> @@ -56,7 +56,10 @@ static char
> *card_name = NULL,
> *profile_name = NULL,
> *port_name = NULL,
> - *formats = NULL;
> + *formats = NULL,
> + *recipient = NULL,
> + *message = NULL,
> + *message_args = NULL;
>
> static uint32_t
> sink_input_idx = PA_INVALID_INDEX,
> @@ -130,6 +133,7 @@ static enum {
> SET_SOURCE_OUTPUT_MUTE,
> SET_SINK_FORMATS,
> SET_PORT_LATENCY_OFFSET,
> + SEND_OBJECT_MESSAGE,
> SUBSCRIBE
> } action = NONE;
>
> @@ -834,6 +838,22 @@ static void index_callback(pa_context *c, uint32_t idx, void *userdata) {
> complete_action();
> }
>
> +static void string_callback(pa_context *c, int success, const char *response, void *userdata) {
"send_message_callback" would be a better name. It's not as if this
function is trying to be somehow reusable (the error message says "Send
message failed").
> +
> + if (success < 0) {
success should be set to 0 on failure (this comment applies to patch 2,
but I didn't notice the issue when I reviewed that patch). The reason
why 0 should be used is that that's what pa_context_success_cb_t does
with its success parameter, and pa_context_string_cb_t should be
consistent with that.
> + pa_log(_("Send message failed: %s"), pa_strerror(pa_context_errno(c)));
> + quit(1);
> + return;
> + }
> +
> + if (!response)
> + response = "";
I think it would be better to guarantee that response is always non-
NULL, rather than requiring clients to always check for NULL.
> +
> + printf("%s\n", response);
> +
> + complete_action();
> +}
> +
> static void volume_relative_adjust(pa_cvolume *cv) {
> pa_assert(volume_flags & VOL_RELATIVE);
>
> @@ -1404,6 +1424,10 @@ static void context_state_callback(pa_context *c, void *userdata) {
> o = pa_context_set_port_latency_offset(c, card_name, port_name, latency_offset, simple_callback, NULL);
> break;
>
> + case SEND_OBJECT_MESSAGE:
> + o = pa_context_send_message_to_object(c, recipient, message, message_args, string_callback, NULL);
> + break;
> +
> case SUBSCRIBE:
> pa_context_set_subscribe_callback(c, context_subscribe_callback, NULL);
>
> @@ -1580,6 +1604,7 @@ static void help(const char *argv0) {
> printf("%s %s %s %s\n", argv0, _("[options]"), "set-(sink-input|source-output)-mute", _("#N 1|0|toggle"));
> printf("%s %s %s %s\n", argv0, _("[options]"), "set-sink-formats", _("#N FORMATS"));
> printf("%s %s %s %s\n", argv0, _("[options]"), "set-port-latency-offset", _("CARD-NAME|CARD-#N PORT OFFSET"));
> + printf("%s %s %s %s\n", argv0, _("[options]"), "send-message", _("RECIPIENT MESSAGE MESSAGE_PARAMETERS"));
MESSAGE_PARAMETERS should be marked as optional.
> printf("%s %s %s\n", argv0, _("[options]"), "subscribe");
> printf(_("\nThe special names @DEFAULT_SINK@, @DEFAULT_SOURCE@ and @DEFAULT_MONITOR@\n"
> "can be used to specify the default sink, source and monitor.\n"));
> @@ -2015,6 +2040,22 @@ int main(int argc, char *argv[]) {
> goto quit;
> }
>
> + } else if (pa_streq(argv[optind], "send-message")) {
> + action = SEND_OBJECT_MESSAGE;
> +
> + if (argc < optind+3) {
> + pa_log(_("You have to specify at least a recipient and a message"));
I'd prefer "message name" rather than just "message".
> + goto quit;
> + }
> +
> + recipient = pa_xstrdup(argv[optind + 1]);
> + message = pa_xstrdup(argv[optind + 2]);
> + if (argc >= optind+4)
> + message_args = pa_xstrdup(argv[optind + 3]);
> +
> + if (argc > optind+4)
> + pa_log(_("Excess arguments will be ignored"));
I think "Excess arguments given, they will be ignored. Note that all
message parameters must be given as a single string." would be better.
--
Tanu
https://liberapay.com/tanuk
https://www.patreon.com/tanuk
More information about the pulseaudio-discuss
mailing list