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
