On Thu, Jul 25, 2013 at 9:11 PM, Jo?o Paulo Rechi Vita
<jprvita at gmail.com> wrote:
> On Mon, Jul 22, 2013 at 11:36 AM, Tanu Kaskinen
> <tanu.kaskinen at linux.intel.com> wrote:
>> On Fri, 2013-07-12 at 15:06 -0300, jprvita at gmail.com wrote:
>>> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org>
>>>
>>> ---
>>> src/modules/bluetooth/module-bluez5-discover.c | 68 ++++++++++++++++++++++++++
>>> 1 file changed, 68 insertions(+)
>>>
>>> diff --git a/src/modules/bluetooth/module-bluez5-discover.c b/src/modules/bluetooth/module-bluez5-discover.c
>>> index 8409bd3..4c90744 100644
>>> --- a/src/modules/bluetooth/module-bluez5-discover.c
>>> +++ b/src/modules/bluetooth/module-bluez5-discover.c
>>> @@ -24,6 +24,7 @@
>>> #endif
>>>
>>> #include <pulsecore/core.h>
>>> +#include <pulsecore/core-util.h>
>>> #include <pulsecore/macro.h>
>>> #include <pulsecore/module.h>
>>>
>>> @@ -39,9 +40,57 @@ PA_MODULE_LOAD_ONCE(true);
>>> struct userdata {
>>> pa_module *module;
>>> pa_core *core;
>>> + pa_hashmap *device_modules;
>>> + pa_hook_slot *device_connection_changed_slot;
>>> pa_bluetooth_discovery *discovery;
>>> };
>>>
>>> +struct module_info {
>>> + char *path;
>>> + uint32_t module;
>>
>> It seems that the module index is not actually used for anything. So
>> device_modules could store just the paths, and there's no need for
>> module_info.
>>
>
> Makes sense, this was also legacy logic from the BlueZ 4 modules.
>
>>> +};
>>> +
>>> +static pa_hook_result_t device_connection_changed_cb(pa_bluetooth_discovery *y, const pa_bluetooth_device *d, struct userdata *u) {
>>> + struct module_info *mi;
>>> +
>>> + pa_assert(d);
>>> + pa_assert(u);
>>> +
>>> + mi = pa_hashmap_get(u->device_modules, d->path);
>>> +
>>> + if (mi && !pa_bluetooth_device_any_transport_connected(d)) {
>>> + /* disconnection, the module unloads itself */
>>> + pa_log_debug("Unregistering module for %s", d->path);
>>> + pa_hashmap_remove(u->device_modules, mi->path);
>>> + pa_xfree(mi->path);
>>> + pa_xfree(mi);
>>> + return PA_HOOK_OK;
>>> + }
>>> +
>>> + if (!mi && pa_bluetooth_device_any_transport_connected(d)) {
>>> + /* a new device has been connected */
>>> + pa_module *m;
>>> + char *args = pa_sprintf_malloc("path=%s", d->path);
>>> +
>>> + pa_log_debug("Loading module-bluez5-device %s", args);
>>> + m = pa_module_load(u->module->core, "module-bluez5-device", args);
>>> + pa_xfree(args);
>>> +
>>> + if (m) {
>>> + mi = pa_xnew(struct module_info, 1);
>>> + mi->module = m->index;
>>> + mi->path = pa_xstrdup(d->path);
>>> +
>>> + pa_hashmap_put(u->device_modules, mi->path, mi);
>>> + } else
>>> + pa_log_warn("Failed to load module for device %s", d->path);
>>> +
>>> + return PA_HOOK_OK;
>>> + }
>>> +
>>> + return PA_HOOK_OK;
>>> +}
>>> +
>>> int pa__init(pa_module* m) {
>>> struct userdata *u;
>>>
>>> @@ -50,10 +99,15 @@ int pa__init(pa_module* m) {
>>> m->userdata = u = pa_xnew0(struct userdata, 1);
>>> u->module = m;
>>> u->core = m->core;
>>> + u->device_modules = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
>>>
>>> if (!(u->discovery = pa_bluetooth_discovery_get(u->core)))
>>> goto fail;
>>>
>>> + u->device_connection_changed_slot =
>>> + pa_hook_connect(pa_bluetooth_discovery_hook(u->discovery, PA_BLUETOOTH_HOOK_DEVICE_CONNECTION_CHANGED),
>>> + PA_HOOK_NORMAL, (pa_hook_cb_t) device_connection_changed_cb, u);
>>
>> If the discovery object already existed, and it contains devices,
>> module-bluez5-discover should load module-bluez5-device modules for
>> those devices. It's possible that the device modules are already loaded,
>> though, so there needs to be some way of checking whether a device
>> module actually needs to be loaded. pa_bluetooth_discovery could have a
>> registry for loaded modules, perhaps. Or pa_bluetooth_device could have
>> a module_loaded boolean field.
>>
>
> The discovery object will only exist if a module-bluez5-device has
> been loaded before module-bluez5-discover (I'm not considering the
> possibility of clash between bluez4/bluez5 discovery objects, since
> this is a different problem). And as you mentioned in the comments of
> patch 41/56, manually loading the device module doesn't work anymore
> (and I really don't remember if it ever worked). So perhaps we should
> explicitly not support manually loading it, is there a way to enforce
> this via PulseAudio's module-loading system?
>
The comment I wanted to reference is on patch 42/56, actually.
--
Jo?o Paulo Rechi Vita
http://about.me/jprvita
