On Fri, 2014-08-22 at 17:04 +0300, Luiz Augusto von Dentz wrote:
> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org>
>
> ---
> src/modules/bluetooth/backend-ofono.c | 66 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
> index 79765bb..5fb99de 100644
> --- a/src/modules/bluetooth/backend-ofono.c
> +++ b/src/modules/bluetooth/backend-ofono.c
> @@ -23,9 +23,13 @@
> #include <config.h>
> #endif
>
> +#include <errno.h>
> +#include <poll.h>
> +
> #include <pulsecore/core-util.h>
> #include <pulsecore/dbus-shared.h>
> #include <pulsecore/shared.h>
> +#include <pulsecore/core-error.h>
>
> #include "bluez5-util.h"
>
> @@ -118,8 +122,68 @@ static void hf_audio_card_free(void *data) {
> pa_xfree(card);
> }
>
> +static int socket_accept(int sock)
> +{
> + char c;
> + struct pollfd pfd;
> +
> + if (sock < 0)
> + return -ENOTCONN;
> +
> + memset(&pfd, 0, sizeof(pfd));
> + pfd.fd = sock;
> + pfd.events = POLLOUT;
> +
> + if (poll(&pfd, 1, 0) < 0)
> + return -errno;
> +
> + /* If socket already writable then it is not in defer setup state */
I think this "defer setup" thing needs to be explained somewhere. What
is it, why is it needed?
> + if ((pfd.revents & POLLOUT))
> + return 0;
> +
> + /* Enable socket by reading 1 byte */
> + if (read(sock, &c, 1) < 0)
> + return -errno;
> +
> + return 0;
> +}
> +
> static int hf_audio_agent_transport_acquire(pa_bluetooth_transport *t, bool optional, size_t *imtu, size_t *omtu) {
> - return -1;
> + pa_bluetooth_backend *backend = t->userdata;
> + hf_audio_card *card;
> + int err;
> +
> + pa_assert(backend);
> + pa_assert_se(card = pa_hashmap_get(backend->cards, t->path));
I think it would be good to set the transport userdata to point to the
hf_audio_card, and get the backend from the hf_audio_card object.
> +
> + if (!optional) {
> + DBusMessage *m;
> +
> + pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, "org.ofono.HandsfreeAudioCard", "Connect"));
> + pa_assert_se(dbus_connection_send(pa_dbus_connection_get(backend->connection), m, NULL));
> +
> + return -1;
I'd like to have a comment here explaining what !optional means and why
it's a good idea to send a Connect message and then return -1. Is this
some kind of an "asynchronous acquire" thing, i.e. the acquire operation
will complete once oFono calls NewConnection()? If Connect() returns an
error, do we need to care about that?
> + }
> +
> + /* The correct block size should take into account the SCO MTU from
> + * the Bluetooth adapter and (for adapters in the USB bus) the MxPS
> + * value from the Isoc USB endpoint in use by btusb and should be
> + * made available to userspace by the Bluetooth kernel subsystem.
> + * Meanwhile the empiric value 48 will be used. */
Do you know what downsides the hardcoding has? Those could be documented
here. Also, is the kernel bug tracked somewhere?
--
Tanu
