[PATCHv2 60/60] bluetooth: Revive module-bluetooth-discover

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 19, 2013 at 6:54 AM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Tue, 2013-08-13 at 01:54 -0300, jprvita at gmail.com wrote:
>> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org>
>>
>> Create a wrapper module called module-bluetooth-discover to avoid
>> breaking backward-compatibility of default.pa. This wrapper may
>> eventually be dropped altoghether with BlueZ 4 support.
>> ---
>>  src/Makefile.am                                   |   8 ++
>>  src/modules/bluetooth/module-bluetooth-discover.c | 107 ++++++++++++++++++++++
>>  2 files changed, 115 insertions(+)
>>  create mode 100644 src/modules/bluetooth/module-bluetooth-discover.c
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 0973d31..a283220 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1320,6 +1320,7 @@ endif
>>
>>  if HAVE_BLUEZ
>>  modlibexec_LTLIBRARIES += \
>> +             module-bluetooth-discover.la \
>>               module-bluetooth-policy.la
>>  endif
>>
>> @@ -1427,6 +1428,7 @@ SYMDEF_FILES = \
>>               module-systemd-login-symdef.h \
>>               module-bluetooth-proximity-symdef.h \
>>               module-bluetooth-policy-symdef.h \
>> +             module-bluetooth-discover-symdef.h \
>>               module-bluez4-discover-symdef.h \
>>               module-bluez4-device-symdef.h \
>>               module-bluez5-discover-symdef.h \
>> @@ -2035,6 +2037,12 @@ module_bluetooth_policy_la_LDFLAGS = $(MODULE_LDFLAGS)
>>  module_bluetooth_policy_la_LIBADD = $(MODULE_LIBADD)
>>  module_bluetooth_policy_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)
>>
>> +# Bluetooth discover
>> +module_bluetooth_discover_la_SOURCES = modules/bluetooth/module-bluetooth-discover.c
>> +module_bluetooth_discover_la_LDFLAGS = $(MODULE_LDFLAGS)
>> +module_bluetooth_discover_la_LIBADD = $(MODULE_LIBADD)
>> +module_bluetooth_discover_la_CFLAGS = $(AM_CFLAGS)
>> +
>>  # Bluetooth BlueZ 4 sink / source
>>  module_bluez4_discover_la_SOURCES = modules/bluetooth/module-bluez4-discover.c
>>  module_bluez4_discover_la_LDFLAGS = $(MODULE_LDFLAGS)
>> diff --git a/src/modules/bluetooth/module-bluetooth-discover.c b/src/modules/bluetooth/module-bluetooth-discover.c
>> new file mode 100644
>> index 0000000..ffedca2
>> --- /dev/null
>> +++ b/src/modules/bluetooth/module-bluetooth-discover.c
>> @@ -0,0 +1,107 @@
>> +/***
>> +  This file is part of PulseAudio.
>> +
>> +  Copyright 2013 Jo?o Paulo Rechi Vita
>> +
>> +  PulseAudio is free software; you can redistribute it and/or modify
>> +  it under the terms of the GNU Lesser General Public License as
>> +  published by the Free Software Foundation; either version 2.1 of the
>> +  License, or (at your option) any later version.
>> +
>> +  PulseAudio is distributed in the hope that it will be useful, but
>> +  WITHOUT ANY WARRANTY; without even the implied warranty of
>> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +  General Public License for more details.
>> +
>> +  You should have received a copy of the GNU Lesser General Public
>> +  License along with PulseAudio; if not, write to the Free Software
>> +  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
>> +  USA.
>> +***/
>> +
>> +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +
>> +#include <pulsecore/core-util.h>
>> +#include <pulsecore/macro.h>
>> +#include <pulsecore/module.h>
>> +
>> +#include "module-bluetooth-discover-symdef.h"
>> +
>> +PA_MODULE_AUTHOR("Jo?o Paulo Rechi Vita");
>> +PA_MODULE_DESCRIPTION("Detect available Bluetooth daemon and load the corresponding discovery module");
>> +PA_MODULE_VERSION(PACKAGE_VERSION);
>> +PA_MODULE_LOAD_ONCE(true);
>> +
>> +struct userdata {
>> +    pa_module *bluez5_module;
>> +    pa_module *bluez4_module;
>> +};
>> +
>> +/* This function is heavily inspired in the implementation of ".ifexists" */
>
> Would it be possible to share the implementation between
> module-bluetooth-discover and the ".ifexists" handler? We could have a
> function called pa_module_exists(), for example.
>

Yes, I've thought about sharing code here but in cli-command.h, which
would be a bit weird. Having it in module.h makes much more sense. I
have created pa_module_exists() which can handle module names (i.e.:
module-bluetooth-discover), relative file names (i.e.:
module-bluetooth-discover.so), and absolute file names (i.e.:
/tmp/module-bluetooth-discover.so). I may have missed some corner
cases, so it would be a good idea if you can double check it.

>> +static bool exists(const char *filename) {
>> +    const char *paths, *state = NULL;
>> +    char *p, *pathname;
>> +    bool result;
>> +
>> +    if (!(paths = lt_dlgetsearchpath()))
>
> undefined reference to `lt_dlgetsearchpath'
>
> Does this compile on your machine?
>

Yes, it does. I don't have this problem here.

>> +        return false;
>> +
>> +    while ((p = pa_split(paths, ":", &state))) {
>> +        pathname = pa_sprintf_malloc("%s" PA_PATH_SEP "%s", p, filename);
>> +        result = access(pathname, F_OK) == 0 ? true : false;
>> +        pa_log_debug("Checking for existence of '%s': %s", pathname, result ? "success" : "failure");
>> +        pa_xfree(pathname);
>> +        pa_xfree(p);
>> +        if (result)
>> +            return true;
>> +    }
>> +
>> +    state = NULL;
>> +    if (PA_UNLIKELY(pa_run_from_build_tree())) {
>> +        while ((p = pa_split(paths, ":", &state))) {
>> +            pathname = pa_sprintf_malloc("%s" PA_PATH_SEP ".libs" PA_PATH_SEP "%s", p, filename);
>> +            result = access(pathname, F_OK) == 0 ? true : false;
>> +            pa_log_debug("Checking for existence of '%s': %s", pathname, result ? "success" : "failure");
>> +            pa_xfree(pathname);
>> +            pa_xfree(p);
>> +            if (result)
>> +                return true;
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>> +int pa__init(pa_module* m) {
>> +    struct userdata *u;
>> +
>> +    pa_assert(m);
>> +
>> +    m->userdata = u = pa_xnew0(struct userdata, 1);
>> +
>> +    if (exists("module-bluez5-discover.so"))
>> +        u->bluez5_module = pa_module_load(m->core, "module-bluez5-discover",  NULL);
>> +
>> +    if (exists("module-bluez4-discover.so"))
>> +        u->bluez4_module = pa_module_load(m->core, "module-bluez4-discover",  NULL);
>> +
>> +    return 0;
>
> I think loading module-bluetooth-discover should fail if neither of the
> modules exist or load successfully.
>

Ok.

> Also, keeping module-bluetooth-discover running doesn't seem useful, so
> once the module has successfully loaded, it should immediately call
> pa_module_unload_request() on itself.
>

I've made the changes according to your comment, but afterwards I
started thinking if it wouldn't be a good idea to keep
module-bluetooth-discovery around so the user can unload the bluetooth
modules without having to know if the system have BlueZ 4 or BlueZ 5.
What do you think?

-- 
Jo?o Paulo Rechi Vita
http://about.me/jprvita


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux