[pulseaudio-discuss] [RFC next v0 01/11] bluetooth: Add backend infrastructure with core->backend reporting

[Tanu Kaskinen]

On Wed, 2013-05-15 at 10:46 +0200, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
> 
> Components other than BlueZ can also implement Bluetooth profiles and
> some infrastructure is required to keep the codebase cleanly split.
> ---
>  src/modules/bluetooth/bluetooth-util.c | 32 ++++++++++++++++++++++++++++++++
>  src/modules/bluetooth/bluetooth-util.h | 12 ++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
> index 5924736..b8fe450 100644
> --- a/src/modules/bluetooth/bluetooth-util.c
> +++ b/src/modules/bluetooth/bluetooth-util.c
> @@ -91,6 +91,11 @@ typedef enum pa_bluez_version {
>      BLUEZ_VERSION_5,
>  } pa_bluez_version_t;
>  
> +struct profile_data {
> +    pa_bluetooth_backend *backend;
> +    void *backend_private;
> +};

Why isn't the private pointer part of pa_bluetooth_backend?

> +
>  struct pa_bluetooth_discovery {
>      PA_REFCNT_DECLARE;
>  
> @@ -101,6 +106,7 @@ struct pa_bluetooth_discovery {
>      bool adapters_listed;
>      pa_hashmap *devices;
>      pa_hashmap *transports;
> +    struct profile_data profiles[PA_BLUETOOTH_PROFILE_COUNT];
>      pa_hook hooks[PA_BLUETOOTH_HOOK_MAX];
>      bool filter_added;
>  };
> @@ -2281,3 +2287,29 @@ bool pa_bluetooth_uuid_has(pa_bluetooth_uuid *uuids, const char *uuid) {
>  
>      return false;
>  }
> +
> +int pa_bt_backend_register(pa_bluetooth_discovery *y, pa_bluetooth_backend *b, enum profile p, void *bp) {

The p and bp parameters seem redundant. I already asked about the
private pointer, but here's another similar question: why doesn't the
backend have a profile field?

> +    pa_assert(y);
> +    pa_assert(b);
> +
> +    if (y->profiles[p].backend) {
> +        pa_log_error("Bluetooth backend already exists for profile %s", pa_bt_profile_to_string(p));
> +        return -1;
> +    }
> +
> +    y->profiles[p].backend = b;
> +    y->profiles[p].backend_private = bp;
> +
> +    return 0;
> +}
> +
> +void pa_bt_backend_unregister(pa_bluetooth_discovery *y, pa_bluetooth_backend *b, enum profile p) {
> +    pa_assert(y);
> +    pa_assert(b);
> +
> +    if (y->profiles[p].backend != b)
> +        return;

Shouldn't this be an assertion?

> +
> +    y->profiles[p].backend = NULL;
> +    y->profiles[p].backend_private = NULL;
> +}
> diff --git a/src/modules/bluetooth/bluetooth-util.h b/src/modules/bluetooth/bluetooth-util.h
> index 3361b0f..969f489 100644
> --- a/src/modules/bluetooth/bluetooth-util.h
> +++ b/src/modules/bluetooth/bluetooth-util.h
> @@ -175,4 +175,16 @@ char *pa_bluetooth_cleanup_name(const char *name);
>  bool pa_bluetooth_uuid_has(pa_bluetooth_uuid *uuids, const char *uuid);
>  const char *pa_bt_profile_to_string(enum profile profile);
>  
> +/*
> + * Backend registration mechanism
> + */
> +struct pa_bluetooth_backend;

This line is redundant, given the typedef that follows.

-- 
Tanu