On Wed, 2014-08-13 at 15:23 +0300, Luiz Augusto von Dentz wrote:
> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org>
>
> ---
> configure.ac | 11 +++++++++++
> src/Makefile.am | 4 +++-
> src/modules/bluetooth/backend-null.c | 37 ++++++++++++++++++++++++++++++++++++
> src/modules/bluetooth/backend.h | 31 ++++++++++++++++++++++++++++++
> src/modules/bluetooth/bluez5-util.c | 10 ++++++++--
> 5 files changed, 90 insertions(+), 3 deletions(-)
> create mode 100644 src/modules/bluetooth/backend-null.c
> create mode 100644 src/modules/bluetooth/backend.h
>
> diff --git a/configure.ac b/configure.ac
> index 837e81e..2beca1f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1028,6 +1028,16 @@ AS_IF([test "x$HAVE_BLUEZ_4" = "x1" || test "x$HAVE_BLUEZ_5" = "x1"], HAVE_BLUEZ
> AC_SUBST(HAVE_BLUEZ)
> AM_CONDITIONAL([HAVE_BLUEZ], [test "x$HAVE_BLUEZ" = x1])
>
> +## Headset profiles backend ##
> +AC_ARG_WITH(bluetooth_headset_backend,
> +AS_HELP_STRING([--with-bluetooth-headset-backend=<null>],[Backend for Bluetooth headset profiles (null)]))
Please indent the AS_HELP_STRING line, since it's inside AC_ARG_WITH.
> +if test -z "$with_bluetooth_headset_backend" ; then
> + BLUETOOTH_HEADSET_BACKEND=null
> +else
> + BLUETOOTH_HEADSET_BACKEND=$with_bluetooth_headset_backend
It would be nice to have a check that ensures that the given backend is
one of the supported values.
> +fi
> +AC_SUBST(BLUETOOTH_HEADSET_BACKEND)
> +
> #### UDEV support (optional) ####
>
> AC_ARG_ENABLE([udev],
> @@ -1489,6 +1499,7 @@ echo "
> Enable D-Bus: ${ENABLE_DBUS}
> Enable BlueZ 4: ${ENABLE_BLUEZ_4}
> Enable BlueZ 5: ${ENABLE_BLUEZ_5}
> + headset backed: ${BLUETOOTH_HEADSET_BACKEND}
> Enable udev: ${ENABLE_UDEV}
> Enable HAL->udev compat: ${ENABLE_HAL_COMPAT}
> Enable systemd login: ${ENABLE_SYSTEMD}
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 5924bd8..b20b6df 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -2085,7 +2085,9 @@ module_bluez4_device_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) $(SBC_CFLAGS)
> libbluez5_util_la_SOURCES = \
> modules/bluetooth/bluez5-util.c \
> modules/bluetooth/bluez5-util.h \
> - modules/bluetooth/a2dp-codecs.h
> + modules/bluetooth/a2dp-codecs.h \
> + modules/bluetooth/backend.h \
> + modules/bluetooth/backend- at BLUETOOTH_HEADSET_BACKEND@.c
> libbluez5_util_la_LDFLAGS = -avoid-version
> libbluez5_util_la_LIBADD = $(MODULE_LIBADD) $(DBUS_LIBS)
> libbluez5_util_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)
> diff --git a/src/modules/bluetooth/backend-null.c b/src/modules/bluetooth/backend-null.c
> new file mode 100644
> index 0000000..9ef1ee4
> --- /dev/null
> +++ b/src/modules/bluetooth/backend-null.c
> @@ -0,0 +1,37 @@
> +/***
> + 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/log.h>
> +
> +#include "backend.h"
> +
> +hf_backend_data_t *hf_backend_new(pa_core *c) {
> + pa_log_debug("Bluetooth Headset Backend API support disabled");
> + return NULL;
> +}
> +
> +void hf_backend_free(hf_backend_data_t *data) {
> + /* Nothing to do here */
> +}
> diff --git a/src/modules/bluetooth/backend.h b/src/modules/bluetooth/backend.h
> new file mode 100644
> index 0000000..78a857c
> --- /dev/null
> +++ b/src/modules/bluetooth/backend.h
> @@ -0,0 +1,31 @@
> +#ifndef foohfagenthfoo
> +#define foohfagenthfoo
These refer to the old file name still.
> +
> +/***
> + 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.
> +***/
> +
> +#include <pulsecore/core.h>
> +
> +typedef struct hf_backend_data hf_backend_data_t;
Our convention is to not use the _t suffix with structs, and as I
already said in the previous review round, I don't like the _data suffix
either. Also, types declared in headers should have the pa_ prefix, so I
suggest that this struct is named pa_hf_backend or
pa_bluetooth_hf_backend, whichever you like more.
I'd prefer the header (and implementation) file names to follow the
struct name, i.e. call this hf-backend.h.
> +
> +hf_backend_data_t *hf_backend_new(pa_core *c);
> +void hf_backend_free(hf_backend_data_t *data);
> +#endif
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index adb8351..c6755e0 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -34,6 +34,7 @@
> #include <pulsecore/shared.h>
>
> #include "a2dp-codecs.h"
> +#include "backend.h"
>
> #include "bluez5-util.h"
>
> @@ -87,6 +88,7 @@ struct pa_bluetooth_discovery {
> pa_hashmap *devices;
> pa_hashmap *transports;
>
> + hf_backend_data_t *backend;
> PA_LLIST_HEAD(pa_dbus_pending, pending);
> };
>
> @@ -1110,9 +1112,9 @@ const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
> case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
> return "a2dp_source";
> case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
> - return "headset_head_unit";
> + return "hsp";
> case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
> - return "headset_audio_gateway";
> + return "hfgw";
These changes are unrelated to the null backend. "headset_head_unit" and
"headset_audio_gateway" were introduced in patch 1/4, so that's the
right place to give names to the new profiles.
--
Tanu
