Hi Claudio,
> This patch adds the initial SCO server socket handling. BtIO and BlueZ
> functions should not be used, with the new Profile API the objetive is
> to get rid of these dependencies.
> ---
> plugins/hfp_hf_bluez5.c | 93
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index 0e496ee..398dee0 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -64,6 +64,7 @@ struct hfp {
> static GHashTable *modem_hash = NULL;
> static GHashTable *devices_proxies = NULL;
> static GDBusClient *bluez = NULL;
> +static guint sco_watch = 0;
>
> static void hfp_debug(const char *str, void *user_data)
> {
> @@ -378,6 +379,89 @@ static const GDBusMethodTable profile_methods[] = {
> { }
> };
>
> +static gboolean sco_accept(GIOChannel *io, GIOCondition cond,
> + gpointer user_data)
> +{
> + struct sockaddr_sco saddr;
> + socklen_t optlen;
> + int sk, nsk;
> +
> + if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
> + return FALSE;
> +
> + sk = g_io_channel_unix_get_fd(io);
> +
> + memset(&saddr, 0, sizeof(saddr));
> + optlen = sizeof(saddr);
> +
> + nsk = accept(sk, (struct sockaddr *) &saddr, &optlen);
> + if (nsk < 0)
> + return TRUE;
> +
> + /* TODO: Verify if the device has a modem */
> +
> + return TRUE;
> +}
> +
> +static GIOChannel *sco_listen(int *err)
> +{
I do not like this int *err. Why is this a separate function anyway.
Just do the whole SCO server socket setup with accept watch setup in one
function. There is no advantage in having two functions.
> + struct sockaddr_sco addr;
> + GIOChannel *io;
> + int sk, defer_setup = 1;
> +
> + sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_SCO);
> + if (sk < 0) {
> + *err = -errno;
> + return NULL;
> + }
> +
> + /* Bind to local address */
> + memset(&addr, 0, sizeof(addr));
> + addr.sco_family = AF_BLUETOOTH;
> + bt_bacpy(&addr.sco_bdaddr, BDADDR_ANY);
> +
> + if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> + close(sk);
> + *err = -errno;
> + return NULL;
> + }
> +
> + if (setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
> + &defer_setup, sizeof(defer_setup)) < 0) {
> + ofono_warn("Can't enable deferred setup: %s (%d)",
> + strerror(errno), errno);
> + *err = -errno;
This is semantically incorrect. You can not return non-NULL and an
error.
Can we actually make HFP 1.6 work properly without defer setup enabled?
> + }
> +
> + io = g_io_channel_unix_new(sk);
> + g_io_channel_set_close_on_unref(io, TRUE);
> + g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK, NULL);
Can you just open the SOCK_NONBLOCK and SOCK_CLOEXEC as we should be
always doing.
> +
> + if (listen(sk, 5) < 0) {
> + g_io_channel_unref(io);
> + *err = -errno;
> + return NULL;
> + }
> +
> + return io;
> +}
> +
> +static int sco_init(void)
> +{
> + GIOCondition cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
What is this extra declaration good for? I rather have it inside the
call that is adding the watch.
> + GIOChannel *sco_io;
> + int err = 0;
> +
> + sco_io = sco_listen(&err);
> + if (sco_io == NULL)
> + return err;
> +
> + sco_watch = g_io_add_watch(sco_io, cond, sco_accept, NULL);
> + g_io_channel_unref(sco_io);
> +
> + return 0;
> +}
> +
> static void connect_handler(DBusConnection *conn, void *user_data)
> {
> DBG("Registering External Profile handler ...");
> @@ -500,6 +584,12 @@ static int hfp_init(void)
> if (DBUS_TYPE_UNIX_FD < 0)
> return -EBADF;
>
> + err = sco_init();
> + if (err < 0) {
> + ofono_error("SCO: %s(%d)", strerror(-err), -err);
> + return err;
> + }
> +
> /* Registers External Profile handler */
> if (!g_dbus_register_interface(conn, HFP_EXT_PROFILE_PATH,
> BLUEZ_PROFILE_INTERFACE,
> @@ -550,6 +640,9 @@ static void hfp_exit(void)
>
> g_hash_table_destroy(modem_hash);
> g_hash_table_destroy(devices_proxies);
> +
> + if (sco_watch)
Normally we do sco_watch > 0 for the GLib sources.
> + g_source_remove(sco_watch);
> }
>
> OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin",
> VERSION,
Regards
Marcel
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
