From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <cyrus@holtmann.org>
MIME-Version: 1.0
In-Reply-To: <1331215032-27695-9-git-send-email-arik@wizery.com>
References: <1331215032-27695-1-git-send-email-arik@wizery.com>
	<1331215032-27695-9-git-send-email-arik@wizery.com>
Date: Wed, 14 Mar 2012 09:19:34 -0400
Message-ID: <CAJdJm_PKA2eCSHDin6WLj4xTCONuURYfMkyBAeu=T1Sd74vBwQ@mail.gmail.com>
Subject: Re: [PATCH 8/8] proximity: reporter: implement D-Bus API
From: Anderson Lizardo <anderson.lizardo@openbossa.org>
To: Arik Nemtsov <arik@wizery.com>
Cc: linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset=ISO-8859-1
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

Hi Arik,

On Thu, Mar 8, 2012 at 9:57 AM, Arik Nemtsov <arik@wizery.com> wrote:
>  int reporter_init(struct btd_adapter *adapter)
>  {
> +       struct reporter_adapter *radapter;
> +       DBusConnection *conn;
> +
>        if (!main_opts.attrib_server) {
>                DBG("Attribute server is disabled");
>                return -1;
>        }
>
> -       connection = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
> -       if (connection == NULL)
> -               return -EIO;
> -       DBG("Proximity Reporter for adapter %p", adapter);
> +       conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
> +       if (!conn)
> +               return -1;
>
> -       register_link_loss(adapter, connection);
> +       radapter = g_new0(struct reporter_adapter, 1);
> +       radapter->adapter = adapter;
> +       radapter->conn = conn;
> +
> +       register_link_loss(adapter, radapter->conn);
>        register_tx_power(adapter);
> -       register_imm_alert(adapter, connection);
> +       register_imm_alert(adapter, radapter->conn);
> +
> +       /*
> +        * Register watches for getting device change events. We watch
> +        * for existing, new and removed devices.
> +        */
> +       radapter->watch_added = g_dbus_add_signal_watch(radapter->conn,
> +                                       BLUEZ_SERVICE, NULL, ADAPTER_INTERFACE,
> +                                       "DeviceAdded", handle_device_change,
> +                                       radapter, NULL);
> +
> +       radapter->watch_created = g_dbus_add_signal_watch(radapter->conn,
> +                                       BLUEZ_SERVICE, NULL, ADAPTER_INTERFACE,
> +                                       "DeviceCreated", handle_device_change,
> +                                       radapter, NULL);
> +
> +       radapter->watch_removed = g_dbus_add_signal_watch(radapter->conn,
> +                                       BLUEZ_SERVICE, NULL, ADAPTER_INTERFACE,
> +                                       "DeviceRemoved", handle_device_change,
> +                                       radapter, NULL);

Again, setting a watcher for BlueZ own signals is strange. Why can't
you use a device driver like other plugins?

> +
> +       reporter_adapters = g_slist_prepend(reporter_adapters, radapter);
> +       DBG("Proximity Reporter for adapter %p", adapter);
>
>        return 0;
>  }

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil