On Sat, 2017-08-19 at 17:48 +0200, Georg Chini wrote: > --- > man/pactl.1.xml.in | 6 +++++ > man/pulse-cli-syntax.5.xml.in | 6 +++++ > shell-completion/bash/pulseaudio | 5 +++-- > shell-completion/zsh/_pulseaudio | 2 ++ > src/pulsecore/cli-command.c | 47 ++++++++++++++++++++++++++++++++++++++++ > src/utils/pacmd.c | 1 + > src/utils/pactl.c | 39 ++++++++++++++++++++++++++++++++- > 7 files changed, 103 insertions(+), 3 deletions(-) > > diff --git a/man/pactl.1.xml.in b/man/pactl.1.xml.in > index 39569b6b..e868babc 100644 > --- a/man/pactl.1.xml.in > +++ b/man/pactl.1.xml.in > @@ -246,6 +246,12 @@ License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>. > </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.</p></optdesc> > + </option> Are the message parameters expected to be just one string? I think it would be more user-frienly to allow the parameters to be split in multiple command line arguments. There should be a pointer to the place where all the messages are documented. We haven't agreed where to put the documentation. I think it would be highly desirable to have the documentation in the same repository with the code, so the wiki is probably not the best place. A plain text file would be the simplest way forward. The pactl man page could then point to the text file in cgit.freedesktop.org. I think it would be a good idea to use something like Sphinx for generating nice html pages from plain text with some light markup, not only for the messaging API documentation but for other documentation as well. That requires some work to learn and configure the system, however, and while I expect it to be pretty simple, I'm probably not going to do that myself anytime soon. Would you be interested in working on that? Even if not, what do you think about using something like Sphinx for documentation? Would it be better than the wiki? Maybe I will some day find the time and motivation to set it up myself. > diff --git a/man/pulse-cli-syntax.5.xml.in b/man/pulse-cli-syntax.5.xml.in > index 0a0fabaf..adf33f58 100644 > --- a/man/pulse-cli-syntax.5.xml.in > +++ b/man/pulse-cli-syntax.5.xml.in > @@ -298,6 +298,12 @@ License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>. > </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.</p></optdesc> > + </option> Pointer to the API reference here as well. > +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; The indentation is a bit off until midway through the function. > + 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"); > + return -1; > + } > + > + /* parameters may be NULL */ > + message_parameters = pa_tokenizer_get(t, 3); > + > + ret = pa_core_send_message(c, recipient_name, message, message_parameters, &response); > + > + if (ret == PA_CORE_SEND_NO_RECIPIENT) { > + pa_strbuf_puts(buf, "Invalid recipient name.\n"); > + ret = -1; > + > + } else if (ret == PA_CORE_SEND_FAILURE) { > + pa_strbuf_puts(buf, "Send message failed.\n"); > + ret = -1; > + > + } else { > + pa_strbuf_puts(buf, (const char *)response); Why is the cast there? > +static void string_callback(pa_context *c, const char *response, void *userdata) { > + if (!response) { > + pa_log(_("Failure: %s"), pa_strerror(pa_context_errno(c))); The callback is not called on failures, and there's nothing preventing the message handler from returning a null string as the response, so either you shouldn't treat a NULL response as a failure, or you should add documentation and some safeguards that prevent NULL responses from successful operations. > @@ -2015,6 +2036,19 @@ 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")); > + 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]); message_args doesn't get set if argc > optiond + 4. -- Tanu https://www.patreon.com/tanuk
