[pulseaudio-discuss] [RFC] bluetooth: Add support for org.bluez.Adapter and org.bluez.FMRadio.

[Tanu Kaskinen]

On Mon, 2011-12-12 at 16:46 +0100, Dalleau, Frederic wrote:
> Hi Tanu,
> 
> >> Im fine with the changes except with this thing with
> >> org.bluez.FMRadio, IMO that should belong to the module that requires
> >> it
> >
> > It's a valid argument that "special" stuff like the FMRadio interface
> > adds clutter in the generic bluetooth-util.h API. But if it's
> > implemented separately, either it will reimplement a lot of stuff
> > already done by bluetooth-util, or alternatively bluetooth-util.h will
> > need to provide an API for extending the functionality, and that would
> > add another kind of clutter. I believe the extension API would be worse
> > than having FMRadio directly in bluetooth-util.
> >
> > Another argument is that having FMRadio in bluetooth-util causes
> > overhead in systems where the WL1273 chip isn't used. There is some
> > additional memory required, which I think is negligible, but there's
> > also useless D-Bus communication involved, which I think is a
> > significant drawback.
> 
> An intermediate position can be that bluetooth-util.c notifies
> module-wl1273-support that an adapter has been added or removed
> via hooks similar to what already exists.
> If this happens, module-wl1273-support could simply look for the
> org.bluez.FMRadio interface by itself.
> This way, if module-wl1273-support is not loaded, then no
> overhead is added, and if it is loaded, it still benefit the shared architecture
> of bluetooth-util.c.

That still requires that bluetooth-util.h provides an API for this kind
of extensions. But maybe it's not so bad. I haven't thought yet through
what exactly is needed, so let's do it now.

When a new adapter appears, fmradio-util will have to check if it
supports the FMRadio interface. AFAIK, there's no explicit API for
checking that, except the introspection API, but I don't want to parse
XML. So, the best way to do the check is probably by calling blindly
org.bluez.FMRadio.GetProperties. If the FMRadio interface isn't
supported, fmradio-util can detect that by looking at the error that
bluetoothd will send back.

Sending the GetProperties message requires fmradio-util to connect to
the system bus, which would have already been done if the functionality
was implemented in bluetooth-util. Well, connecting to the system bus is
very simple, so here the duplication isn't a big deal.

Sending the message requires also the bus name of bluetoothd. It seems
that bluetooth-util uses "org.bluez" instead of the unique bus name.
That seems suspicious - bluetooth-util caches the state of bluetoothd,
so the interaction with the daemon isn't stateless, so it would probably
be a better idea to use the unique bus name. Bluetooth-util seems to do
a reset whenever the "org.bluez" name owner changes, and that should get
rid of most problems. I guess there can be at most some rare corner case
problems when bluetoothd restarts and Pulseaudio sends a message after
the old instance has died but before the NameOwnerChanged signal has
been received. It may also be that that won't be a problem in practice.
But worrying about these issues could be avoided entirely if
bluetooth-util used the unique bus name. If I'll do that change, then
fmradio-util will need to get the current unique bus name from
bluetooth-util somehow (or duplicate the name owner tracking). The
current bus name could be simply a field in the pa_bluetooth_core
struct, so this is pretty harmless.

Another thing that is required is the adapter object path. That can be
made available as a field in pa_bluetooth_adapter.

I don't think calling SetProperty or subscribing to FMRadio property
changes will bring any further requirements, so the "extension API" that
I was worried about is actually very small and simple to implement. So,
I'm not against separating the FMRadio handling to fmradio-util after
all.

-- 
Tanu