[pulseaudio-discuss] [PATCH 1/2] protocol-native: Add commands for communication with modules

[Tanu Kaskinen]

On Sun, 2017-05-21 at 14:56 +0200, Georg Chini wrote:
> This patch adds two new commands to the native protocol that enable direct
> communication with modules during run time. SEND_MODULE_COMMAND is used to
> send a command string to a module if no reply is needed. This can for example
> be employed to change parameters on the fly. SEND_MODULE_QUERY is similar to
> SEND_MODULE_COMMAND but expects a reply string from the module. This command
> can for example be used to query the current value of parameters.
> Within the module, the functions set_command_callback() and get_command_callback()
> need to be implemented to handle the two types of requests.
> 
> This is a rather generic interface because it only passes strings between the
> module and a client without making any assumptions about the content of the
> strings.

So far we've used "extensions" for module-specific commands. This seems
to be an alternative solution to the same problem. I would expect some
explanation in the commit message for why this solution is better than
the existing one.

I'm not necessarily opposed to replacing the extension system with
something new. The extension system is cumbersome to work with, and
it's probably possible to come up with something better.

If I recall correctly, you mentioned (probably in IRC) configuring
equalizer parameters as one possible use case, and my reply was that
equalizer instances don't necessarily map directly to module instances,
so "pa_context_send_module_command()" isn't really appropriate; at
least the function name needs to be changed. D-Bus uses "objects" as
message targets, and each object has a unique path string as an
identifier. I think a similar approach would be good here: messages
should just carry an arbitrary string as the recipient indentifier, it
doesn't matter whether the recipient is a module or something else.

D-Bus has three message types: method calls, method replies and
signals. I think that's worth copying.

We already have a reply command, so that probably doesn't need to be
added.

Your proposal adds two different "method call" kind of messages, but I
don't really see why that is necessary. Every command will anyway have
at least a simple ack reply, so it's not like you're saving any
traffic. Commands that don't return anything could just respond with an
empty string.

Your proposal doesn't have a way to send notification messages from the
server to the client. I think this is rather important to have (but
this can also be added in a later patch).

I wrote and deleted some stuff about things that D-Bus has: interfaces,
signatures and introspection. We may want those at some point, and I
thought that the basic message protocol would need some changes to be
compatible with them, but I don't think that's the case after all.

In conclusion, I have much less complaints about the high-level idea
than I first thought I would have (I didn't read the code in detail).
Just avoid making the protocol module-centric and remove the redundant
command-without-reply message type. Signal support would be nice too.

This new protocol has the capability to be used not only as a
replacement for the extension system, but as a replacement for
everything else in the native protocol too. I'd like to have some
discussion about what the new protocol's scope should be going forward
(I added Arun to Cc for this reason). Should new core features be added
using this mechanism, or should we keep adding new commands to the
native protocol as we've done so far?

-- 
Tanu

https://www.patreon.com/tanuk