Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH BlueZ 1/7] gdbus: Add g_dbus_client_get_proxy
From: Marcel Holtmann @ 2013-01-13 23:03 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZ+=oDRu-x3toCQfWi7wdzk9SCm2Fx1AhS6SNVf+6SEpkw@mail.gmail.com>

Hi Luiz,

> >> >> g_dbus_client_get_proxy gives the possibilitity to just check if a
> >> >> proxy exist for the given path and interface pair instead of using
> >> >> g_dbus_proxy_new which end up creating a proxy if it doesn't exists
> >> >> which is not always necessary.
> >> >
> >> > why would we do that. You get the proxy via the client callbacks for
> >> > proxy created or proxy removed.
> >>
> >> That way we the user don't have to maintain a list of found proxies,
> >> such a list exist and is maintained by GDBusClient.
> >
> > see how bluetoothctl does it. Pretty damn simple. So I am not convinced
> > that this is a good idea.
> >
> >> > The proxy_new method is for dealing with services that do not have
> >> > ObjectManager support.
> >>
> >> Yep, so in one way or the other you are already letting the user
> >> application search in the list of proxies maintained by GDBusClient,
> >> the only thing this does is to make the proxy_lookup public so the
> >> client don't have to maintain its how list only to be able to look
> >> back when a proxy point to another, btw the reason proxy_new is not
> >> enough is because it may lead to extra calls when the user application
> >> may just want to bail out if the proxy is not found.
> >
> > Still not convinced. What kind of application would this. This seems to
> > be all half thought trough. I am really failing to see the real purpose
> > here. Either you go all the way with ObjectManager and do it the right
> > way or you don't have an object manager, then you need to manually
> > trigger the get all properties.
> 
> Im actually using these functions in an upcoming patch:
> 
> static void register_player(GDBusProxy *proxy)
> {
> ...
> client = g_dbus_proxy_get_client(proxy);
> 
> if (!g_dbus_proxy_get_property(proxy, "Device", &iter))
>     return;
> 
> dbus_message_iter_get_basic(&iter, &path);
> 
> device = g_dbus_client_get_proxy(client, path, "org.bluez.Device1");
> if (device == NULL)
>     return;
> 
> if (!g_dbus_proxy_get_property(device, "Name", &iter))
>     return;
> ...
> static void proxy_added(GDBusProxy *proxy, void *user_data)
> {
> ...
>     } else if (!strcmp(interface, BLUEZ_MEDIA_PLAYER_INTERFACE)) {
>          printf("Bluetooth Player %s found\n", g_dbus_proxy_get_path(proxy));
>          register_player(proxy);
>    }
> ...
> 
> With this I don't have to keep any list of found devices what so ever,
> if I need a proxy I can just query it via g_dbus_client_get_proxy. Now
> compare this to what bluetoothctl does:
> 
> static void proxy_added(GDBusProxy *proxy, void *user_data)
> {
> ...
>     if (!strcmp(interface, "org.bluez.Device1")) {
>         if (device_is_child(proxy, default_ctrl) == TRUE) {
>             dev_list = g_list_append(dev_list, proxy);
> 
>             print_device(proxy, "NEW");
>     } else if (!strcmp(interface, "org.bluez.Adapter1")) {
>             ctrl_list = g_list_append(ctrl_list, proxy);
> 
>             if (!default_ctrl)
>                 default_ctrl = proxy;
> 
>              print_adapter(proxy, "NEW");
> ...
> 
> So you are keeping the list of proxies while GDBusClient keeps them
> too, now it maybe useful for bluetoothctl but for mpris-tool it is
> completely useless, why would I keep a proxy of a device that doesn't
> contain any player, specially while discovering devices this would
> just populate with useless temporary devices.

bluetoothctl only keeps the list of proxies that contain an interface it
is interested in. And the same could be done for mpris-tool. If you get
an object path with a player interface, you remember it, otherwise you
just ignore it.

> Btw, with these functions we could actually have the following changes
> to device_is_child:
> 
> diff --git a/client/main.c b/client/main.c
> index 9a927a8..1cd6d10 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -224,8 +224,10 @@ static void print_uuids(GDBusProxy *proxy)
> 
>  static gboolean device_is_child(GDBusProxy *device, GDBusProxy *master)
>  {
> +       GDBusClient *client;
> +       GDBusProxy *proxy;
>         DBusMessageIter iter;
> -       const char *adapter, *path;
> +       const char *path;
> 
>         if (!master)
>                 return FALSE;
> @@ -233,13 +235,13 @@ static gboolean device_is_child(GDBusProxy
> *device, GDBusProxy *master)
>         if (g_dbus_proxy_get_property(device, "Adapter", &iter) == FALSE)
>                 return FALSE;
> 
> -       dbus_message_iter_get_basic(&iter, &adapter);
> -       path = g_dbus_proxy_get_path(master);
> +       dbus_message_iter_get_basic(&iter, &path);
> 
> -       if (!strcmp(path, adapter))
> -               return TRUE;
> +       client = g_dbus_proxy_get_client(device);

And this I have single client, I could just make this global. So I am
still not buying into this one.

> -       return FALSE;
> +       proxy = g_dbus_client_get_proxy(client, path, "org.bluez.Adapter1");
> +
> +       return proxy == master ? TRUE : FALSE;
>  }

Don't see how this makes it simpler code. Or ends up in less
instructions for that matter.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 1/7] gdbus: Add g_dbus_client_get_proxy
From: Luiz Augusto von Dentz @ 2013-01-13 22:36 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1358111392.1806.87.camel@aeonflux>

Hi Marcel,

On Sun, Jan 13, 2013 at 11:09 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> >> g_dbus_client_get_proxy gives the possibilitity to just check if a
>> >> proxy exist for the given path and interface pair instead of using
>> >> g_dbus_proxy_new which end up creating a proxy if it doesn't exists
>> >> which is not always necessary.
>> >
>> > why would we do that. You get the proxy via the client callbacks for
>> > proxy created or proxy removed.
>>
>> That way we the user don't have to maintain a list of found proxies,
>> such a list exist and is maintained by GDBusClient.
>
> see how bluetoothctl does it. Pretty damn simple. So I am not convinced
> that this is a good idea.
>
>> > The proxy_new method is for dealing with services that do not have
>> > ObjectManager support.
>>
>> Yep, so in one way or the other you are already letting the user
>> application search in the list of proxies maintained by GDBusClient,
>> the only thing this does is to make the proxy_lookup public so the
>> client don't have to maintain its how list only to be able to look
>> back when a proxy point to another, btw the reason proxy_new is not
>> enough is because it may lead to extra calls when the user application
>> may just want to bail out if the proxy is not found.
>
> Still not convinced. What kind of application would this. This seems to
> be all half thought trough. I am really failing to see the real purpose
> here. Either you go all the way with ObjectManager and do it the right
> way or you don't have an object manager, then you need to manually
> trigger the get all properties.

Im actually using these functions in an upcoming patch:

static void register_player(GDBusProxy *proxy)
{
...
client = g_dbus_proxy_get_client(proxy);

if (!g_dbus_proxy_get_property(proxy, "Device", &iter))
    return;

dbus_message_iter_get_basic(&iter, &path);

device = g_dbus_client_get_proxy(client, path, "org.bluez.Device1");
if (device == NULL)
    return;

if (!g_dbus_proxy_get_property(device, "Name", &iter))
    return;
...
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
...
    } else if (!strcmp(interface, BLUEZ_MEDIA_PLAYER_INTERFACE)) {
         printf("Bluetooth Player %s found\n", g_dbus_proxy_get_path(proxy));
         register_player(proxy);
   }
...

With this I don't have to keep any list of found devices what so ever,
if I need a proxy I can just query it via g_dbus_client_get_proxy. Now
compare this to what bluetoothctl does:

static void proxy_added(GDBusProxy *proxy, void *user_data)
{
...
    if (!strcmp(interface, "org.bluez.Device1")) {
        if (device_is_child(proxy, default_ctrl) == TRUE) {
            dev_list = g_list_append(dev_list, proxy);

            print_device(proxy, "NEW");
    } else if (!strcmp(interface, "org.bluez.Adapter1")) {
            ctrl_list = g_list_append(ctrl_list, proxy);

            if (!default_ctrl)
                default_ctrl = proxy;

             print_adapter(proxy, "NEW");
...

So you are keeping the list of proxies while GDBusClient keeps them
too, now it maybe useful for bluetoothctl but for mpris-tool it is
completely useless, why would I keep a proxy of a device that doesn't
contain any player, specially while discovering devices this would
just populate with useless temporary devices.

Btw, with these functions we could actually have the following changes
to device_is_child:

diff --git a/client/main.c b/client/main.c
index 9a927a8..1cd6d10 100644
--- a/client/main.c
+++ b/client/main.c
@@ -224,8 +224,10 @@ static void print_uuids(GDBusProxy *proxy)

 static gboolean device_is_child(GDBusProxy *device, GDBusProxy *master)
 {
+       GDBusClient *client;
+       GDBusProxy *proxy;
        DBusMessageIter iter;
-       const char *adapter, *path;
+       const char *path;

        if (!master)
                return FALSE;
@@ -233,13 +235,13 @@ static gboolean device_is_child(GDBusProxy
*device, GDBusProxy *master)
        if (g_dbus_proxy_get_property(device, "Adapter", &iter) == FALSE)
                return FALSE;

-       dbus_message_iter_get_basic(&iter, &adapter);
-       path = g_dbus_proxy_get_path(master);
+       dbus_message_iter_get_basic(&iter, &path);

-       if (!strcmp(path, adapter))
-               return TRUE;
+       client = g_dbus_proxy_get_client(device);

-       return FALSE;
+       proxy = g_dbus_client_get_proxy(client, path, "org.bluez.Adapter1");
+
+       return proxy == master ? TRUE : FALSE;
 }

So we compare the proxies not the path alone.

--
Luiz Augusto von Dentz

^ permalink raw reply related

* Re: [PATCH BlueZ 2/7] gdbus: Add g_dbus_proxy_get_client function
From: Marcel Holtmann @ 2013-01-13 21:12 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZKZ1kXbF7E5jJbPhduZcLmFcTT45jyA67KzoJeJVZiz3A@mail.gmail.com>

Hi Luiz,

> >> This is convenient as some callbacks don't provide the client which
> >> the proxy belongs.
> >
> > and I did not add that one on purpose. You should know where your proxy
> > belongs to and worst case hand it over via user_data.
> 
> I understand this as the user_data will always be the client pointer,
> or a struct containing it, or you have to use a global variable, is
> that what you really want? The use application has given the client
> when registering the callbacks, so it is already passing it as
> context. Now that being said, Im not saying the design utterly broken,
> quite the contrary it work like a charm, it just could be more
> convenient and that is what Im trying to achieve with this patch.

it is a convenience for one special kind of quick-and-dirty hack that
normal application will never need. I have made these mistakes in the
past and rather have not sneaking up on me again. So lets not do this.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 1/7] gdbus: Add g_dbus_client_get_proxy
From: Marcel Holtmann @ 2013-01-13 21:09 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZJyaBUt+oRSzyYJ0PNLTys8BfODLJFNY5NzC5touoraGA@mail.gmail.com>

Hi Luiz,

> >> g_dbus_client_get_proxy gives the possibilitity to just check if a
> >> proxy exist for the given path and interface pair instead of using
> >> g_dbus_proxy_new which end up creating a proxy if it doesn't exists
> >> which is not always necessary.
> >
> > why would we do that. You get the proxy via the client callbacks for
> > proxy created or proxy removed.
> 
> That way we the user don't have to maintain a list of found proxies,
> such a list exist and is maintained by GDBusClient.

see how bluetoothctl does it. Pretty damn simple. So I am not convinced
that this is a good idea.

> > The proxy_new method is for dealing with services that do not have
> > ObjectManager support.
> 
> Yep, so in one way or the other you are already letting the user
> application search in the list of proxies maintained by GDBusClient,
> the only thing this does is to make the proxy_lookup public so the
> client don't have to maintain its how list only to be able to look
> back when a proxy point to another, btw the reason proxy_new is not
> enough is because it may lead to extra calls when the user application
> may just want to bail out if the proxy is not found.

Still not convinced. What kind of application would this. This seems to
be all half thought trough. I am really failing to see the real purpose
here. Either you go all the way with ObjectManager and do it the right
way or you don't have an object manager, then you need to manually
trigger the get all properties.

Regards

Marcel



^ permalink raw reply

* Re: Obexd OPP filesend fails with Windows7 stack
From: Syam Sidhardhan @ 2013-01-13 18:35 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Syam Sidhardhan, linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZKMi572+BUbymbA=sX4s3NKSGcHsrGQWSptj5xVSTkMBw@mail.gmail.com>

Hi Luiz,

On Sun, Jan 13, 2013 at 9:12 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Syam,
>
> On Fri, Jan 11, 2013 at 10:32 AM, Syam Sidhardhan <s.syam@samsung.com> wrote:
>> Hi,
>>
>> We are using the obexd 0.48 version and testing the obexd.
>> During testing OPP file send, we found one interoperability
>> issue with the Windows7 PC stack and some commercialized devices
>> available in the market.
>>
>> The issue is: OBEXD file transfer is getting failed
>> once after the complete file content got transfferd. This is
>> because of, we do a direct RFCOMM disconection rather than
>> doing a proper OBEX disconection before.
>>
>> Does someone can help me a with a fix for this?
>
> We probably need to send a disconnect command before disconnecting
> RFCOMM, iirc there was someone else looking into a similar problem
> with a Nokia phone, the strange part is that the remote stack has
> acknowledged the transfer complete and in theory we could go ahead and
> start another one using the same session.
>

Yes, you are correct. We need to send a OBEX disconnect command
before the RFCOMM disconnection. We have tried with the same and
its working fine.

Regards,
Syam

^ permalink raw reply

* Re: [PATCH BlueZ 2/7] gdbus: Add g_dbus_proxy_get_client function
From: Luiz Augusto von Dentz @ 2013-01-13 16:24 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1357937240.1806.83.camel@aeonflux>

Hi Marcel,

On Fri, Jan 11, 2013 at 10:47 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> This is convenient as some callbacks don't provide the client which
>> the proxy belongs.
>
> and I did not add that one on purpose. You should know where your proxy
> belongs to and worst case hand it over via user_data.

I understand this as the user_data will always be the client pointer,
or a struct containing it, or you have to use a global variable, is
that what you really want? The use application has given the client
when registering the callbacks, so it is already passing it as
context. Now that being said, Im not saying the design utterly broken,
quite the contrary it work like a charm, it just could be more
convenient and that is what Im trying to achieve with this patch.

--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH BlueZ 1/7] gdbus: Add g_dbus_client_get_proxy
From: Luiz Augusto von Dentz @ 2013-01-13 16:09 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1357937184.1806.82.camel@aeonflux>

Hi Marcel,

On Fri, Jan 11, 2013 at 10:46 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> g_dbus_client_get_proxy gives the possibilitity to just check if a
>> proxy exist for the given path and interface pair instead of using
>> g_dbus_proxy_new which end up creating a proxy if it doesn't exists
>> which is not always necessary.
>
> why would we do that. You get the proxy via the client callbacks for
> proxy created or proxy removed.

That way we the user don't have to maintain a list of found proxies,
such a list exist and is maintained by GDBusClient.

> The proxy_new method is for dealing with services that do not have
> ObjectManager support.

Yep, so in one way or the other you are already letting the user
application search in the list of proxies maintained by GDBusClient,
the only thing this does is to make the proxy_lookup public so the
client don't have to maintain its how list only to be able to look
back when a proxy point to another, btw the reason proxy_new is not
enough is because it may lead to extra calls when the user application
may just want to bail out if the proxy is not found.


--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH 1/8] obexd: Handle empty path name in SetPath
From: Luiz Augusto von Dentz @ 2013-01-13 15:55 UTC (permalink / raw)
  To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org, Christian Fetzer
In-Reply-To: <1357919757-10334-2-git-send-email-christian.fetzer@oss.bmw-carit.de>

Hi Christian,

On Fri, Jan 11, 2013 at 5:55 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>
> If the empty path is given, an empty name should be sent via OBEX.
> Currently the name field is not set at all and later checks which
> depend on data->index will access invalid memory regions as g_strsplit
> returns NULL when an empty string is given.
>
> 0  0x000000000041a181 in g_obex_setpath (obex=obex@entry=0x662eb0, path=
>     0x20 <Address 0x20 out of bounds>, func=func@entry=0x42d300 <setpath_cb>,
>     user_data=user_data@entry=0x668f10, err=err@entry=0x7fffffffda08)
>     at gobex/gobex.c:1397
> 1  0x000000000042d395 in setpath_cb (obex=0x662eb0, err=0x0, rsp=<optimized out>,
>     user_data=0x668f10) at obexd/client/session.c:902
> 2  0x0000000000418e54 in handle_response (obex=obex@entry=0x662eb0, err=err@entry=0x0,
>     rsp=rsp@entry=0x668f40) at gobex/gobex.c:948
> 3  0x0000000000419d7a in incoming_data (io=<optimized out>, cond=<optimized out>,
>     user_data=0x662eb0) at gobex/gobex.c:1191
> 4  0x00007ffff703c845 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
> 5  0x00007ffff703cb78 in ?? () from /usr/lib/libglib-2.0.so.0
> 6  0x00007ffff703cf72 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
> 7  0x000000000040def2 in main (argc=1, argv=0x7fffffffdd88) at obexd/src/main.c:323
> ---
>  obexd/client/session.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/obexd/client/session.c b/obexd/client/session.c
> index 9cc824e..db37a86 100644
> --- a/obexd/client/session.c
> +++ b/obexd/client/session.c
> @@ -934,7 +934,7 @@ guint obc_session_setpath(struct obc_session *session, const char *path,
>         p = pending_request_new(session, NULL, setpath_complete, data);
>
>         /* Relative path */
> -       if (path[0] != '/') {
> +       if (path[0] != '/' && path[0] != 0) {
>                 first = data->remaining[data->index];
>                 data->index++;
>         }
> --
> 1.8.1

But what empty means in this case, I would like to stick with cd
behavior, so empty should set back to root. Otherwise the patchset
seems fine.

--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: Obexd OPP filesend fails with Windows7 stack
From: Luiz Augusto von Dentz @ 2013-01-13 15:42 UTC (permalink / raw)
  To: Syam Sidhardhan; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <D43D72D2B7084BA8B750CE7FD0D1C287@sisodomain.com>

Hi Syam,

On Fri, Jan 11, 2013 at 10:32 AM, Syam Sidhardhan <s.syam@samsung.com> wrote:
> Hi,
>
> We are using the obexd 0.48 version and testing the obexd.
> During testing OPP file send, we found one interoperability
> issue with the Windows7 PC stack and some commercialized devices
> available in the market.
>
> The issue is: OBEXD file transfer is getting failed
> once after the complete file content got transfferd. This is
> because of, we do a direct RFCOMM disconection rather than
> doing a proper OBEX disconection before.
>
> Does someone can help me a with a fix for this?

We probably need to send a disconnect command before disconnecting
RFCOMM, iirc there was someone else looking into a similar problem
with a Nokia phone, the strange part is that the remote stack has
acknowledged the transfer complete and in theory we could go ahead and
start another one using the same session.

--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH 1/1] test: Make map script a command line client
From: Luiz Augusto von Dentz @ 2013-01-13 15:29 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Christian Fetzer, linux-bluetooth@vger.kernel.org,
	Christian Fetzer
In-Reply-To: <1357937329.1806.84.camel@aeonflux>

Hi Marcel, Christian,

On Fri, Jan 11, 2013 at 10:48 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Christian,
>
>> Rework the map-client test script into an interactive command line client.
>> Now multiple MCE functions can be called in one active session.
>> The script also allows to specify all filters or optional parameters including
>> auto completion.
>>
>> Change-Id: I9c9ede2bc958009c757384177cf06c081d984c98
>
> no Change-Id crap please.

I wonder if we should complicate more the testing scripts adding such
futures or start working in a proper C tool such as bluetoothctl e.g.
obexctl?

--
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH BlueZ 01/11] transport: Initialize the "Volume" property with 50%
From: Luiz Augusto von Dentz @ 2013-01-13 15:22 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: linux-bluetooth@vger.kernel.org, Vinicius Gomes, Claudio Takahasi,
	Luiz Augusto Von Dentz
In-Reply-To: <1357935934-20033-2-git-send-email-jprvita@openbossa.org>

Hi Joao,

On Fri, Jan 11, 2013 at 10:25 PM, João Paulo Rechi Vita
<jprvita@openbossa.org> wrote:
> ---
>  profiles/audio/transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index a4370a5..6ffa98a 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -787,7 +787,7 @@ struct media_transport *media_transport_create(struct media_endpoint *endpoint,
>                 struct a2dp_transport *a2dp;
>
>                 a2dp = g_new0(struct a2dp_transport, 1);
> -               a2dp->volume = -1;
> +               a2dp->volume = 63;
>
>                 transport->resume = resume_a2dp;
>                 transport->suspend = suspend_a2dp;
> --
> 1.7.11.7

Does the spec say anything regarding this? Actually it seems this
value must be set by PA if it does support volume notification, which
means a new version of PA, then it should set the value when the card
is initialized, otherwise if the endpoint doesn't set a value it
should remain -1/not available. If volume is not set by the endpoint
we should either return and error upon register notification or return
maximum volume always and refuse to SetAbsoluteVolume, my guess is
that the latter is better for IOP reasons since the remote device may
register to volume while the endpoint is setting up the transport so
the volume may be set latter.


--
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH] build: Substitute systemd unit directories if overriden by user
From: Lubomir Rintel @ 2013-01-13 10:51 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Marcel Holtmann, Lubomir Rintel

Otherwise --with-systemdsystemunitdir and --with-systemduserunitdir would be
effectively ignored.
---
 configure.ac |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 09e9d4a..514aa5a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -202,8 +202,8 @@ if (test "${enable_systemd}" != "no" && test -z "${path_systemunitdir}"); then
 		AC_MSG_ERROR([systemd system unit directory is required])
 	fi
 	AC_MSG_RESULT([${path_systemunitdir}])
-	AC_SUBST(SYSTEMD_SYSTEMUNITDIR, [${path_systemunitdir}])
 fi
+AC_SUBST(SYSTEMD_SYSTEMUNITDIR, [${path_systemunitdir}])
 
 AC_ARG_WITH([systemduserunitdir],
 			AC_HELP_STRING([--with-systemduserunitdir=DIR],
@@ -216,8 +216,8 @@ if (test "${enable_systemd}" != "no" && test -z "${path_userunitdir}"); then
 		AC_MSG_ERROR([systemd user unit directory is required])
 	fi
 	AC_MSG_RESULT([${path_userunitdir}])
-	AC_SUBST(SYSTEMD_USERUNITDIR, [${path_userunitdir}])
 fi
+AC_SUBST(SYSTEMD_USERUNITDIR, [${path_userunitdir}])
 
 AC_ARG_ENABLE(datafiles, AC_HELP_STRING([--disable-datafiles],
 			[do not install configuration and data files]),
-- 
1.7.1

^ permalink raw reply related

* 3.8.0-rc3: possible circular locking dependency: &tty->legacy_mutex / &tty->hangup_work with serial/RFCOMM connection via USB bluetooth dongle
From: Sander Eikelenboom @ 2013-01-12 18:46 UTC (permalink / raw)
  To: linux-kernel, linux-serial, linux-bluetooth
  Cc: Alan Cox, Greg Kroah-Hartman, marcel

Hi,

Running a 3.8.0-rc3 kernel (latest commit b719f43059903820c31edb30f4663a2818836e7f) kernel (debian squeeze os), i'm running into this lockdep warning when:

- Running a perl script that uses rfcomm to communicatie via bluetooth with a bluetooth/TTL converter.
- It can run ok for a few hours before this lockdep occurs and the perl script freezes.
- The info related to bluetooth from syslog:

Jan 12 10:24:08 serveerstertje kernel: [    7.919775] Bluetooth: Virtual HCI driver ver 1.3
Jan 12 10:24:08 serveerstertje kernel: [    7.920314] Bluetooth: HCI UART driver ver 2.2
Jan 12 10:24:08 serveerstertje kernel: [    7.920316] Bluetooth: HCI H4 protocol initialized
Jan 12 10:24:08 serveerstertje kernel: [    7.920317] Bluetooth: HCI BCSP protocol initialized
Jan 12 10:24:08 serveerstertje kernel: [    7.920318] Bluetooth: HCILL protocol initialized
Jan 12 10:24:08 serveerstertje kernel: [    7.920318] Bluetooth: HCIATH3K protocol initialized
Jan 12 10:24:08 serveerstertje kernel: [    7.920319] Bluetooth: HCI Three-wire UART (H5) protocol initialized

Jan 12 10:24:08 serveerstertje kernel: [    8.191897] Bluetooth: RFCOMM TTY layer initialized
Jan 12 10:24:08 serveerstertje kernel: [    8.191930] Bluetooth: RFCOMM socket layer initialized
Jan 12 10:24:08 serveerstertje kernel: [    8.191931] Bluetooth: RFCOMM ver 1.11
Jan 12 10:24:08 serveerstertje kernel: [    8.191932] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
Jan 12 10:24:08 serveerstertje kernel: [    8.191933] Bluetooth: BNEP filters: protocol multicast
Jan 12 10:24:08 serveerstertje kernel: [    8.191944] Bluetooth: BNEP socket layer initialized
Jan 12 10:24:08 serveerstertje kernel: [    8.191945] Bluetooth: HIDP (Human Interface Emulation) ver 1.2
Jan 12 10:24:08 serveerstertje kernel: [    8.191954] Bluetooth: HIDP socket layer initialized

Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Bluetooth deamon 4.66
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Starting SDP server
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Starting experimental netlink support
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to find Bluetooth netlink family
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to init netlink plugin
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: bridge pan0 created
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: HCI dev 0 registered
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to open RFKILL control device
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: HCI dev 0 up
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Starting security manager 0
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Adapter /org/bluez/3912/hci0 has been enabled
Jan 12 10:24:09 serveerstertje bluetoothd[3912]: Failed to access HAL


- And the lockdep warning itself:

[28678.458250]
[28678.476588] ======================================================
[28678.494887] [ INFO: possible circular locking dependency detected ]
[28678.513013] 3.8.0-rc3-20130112-netpatched-rocketscience-radeon #1 Not tainted
[28678.530909] -------------------------------------------------------
[28678.548636] kworker/2:1/19513 is trying to acquire lock:
[28678.566070]  (&tty->legacy_mutex){+.+.+.}, at: [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
[28678.583577]
[28678.583577] but task is already holding lock:
[28678.617615]  ((&tty->hangup_work)){+.+...}, at: [<ffffffff81080bf8>] process_one_work+0x158/0x4b0
[28678.634569]
[28678.634569] which lock already depends on the new lock.
[28678.634569]
[28678.683868]
[28678.683868] the existing dependency chain (in reverse order) is:
[28678.715354]
[28678.715354] -> #2 ((&tty->hangup_work)){+.+...}:
[28678.745890]        [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
[28678.760975]        [<ffffffff810b576a>] lock_acquire+0xba/0x100
[28678.775834]        [<ffffffff8108322a>] flush_work+0x3a/0x250
[28678.790408]        [<ffffffff81451568>] tty_ldisc_flush_works+0x18/0x40
[28678.804877]        [<ffffffff814517ae>] tty_ldisc_release+0x2e/0x90
[28678.818952]        [<ffffffff8144b827>] tty_release+0x3c7/0x590
[28678.832813]        [<ffffffff8114e009>] __fput+0xa9/0x2c0
[28678.846411]        [<ffffffff8114e289>] ____fput+0x9/0x10
[28678.859644]        [<ffffffff810854d5>] task_work_run+0x95/0xb0
[28678.872661]        [<ffffffff8100dc4d>] do_notify_resume+0x6d/0x80
[28678.885516]        [<ffffffff819bb5a2>] int_signal+0x12/0x17
[28678.898047]
[28678.898047] -> #1 (&tty->legacy_mutex/1){+.+...}:
[28678.922334]        [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
[28678.934268]        [<ffffffff810b576a>] lock_acquire+0xba/0x100
[28678.945916]        [<ffffffff819b754c>] mutex_lock_nested+0x4c/0x450
[28678.957318]        [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
[28678.968500]        [<ffffffff819ba6aa>] tty_lock_pair+0x6a/0x70
[28678.979405]        [<ffffffff8144b5cb>] tty_release+0x16b/0x590
[28678.990012]        [<ffffffff8114e009>] __fput+0xa9/0x2c0
[28679.000367]        [<ffffffff8114e289>] ____fput+0x9/0x10
[28679.009455] FW: BLOCKED low udp input: IN=eth0 OUT= MAC=40:61:86:f4:67:d9:00:08:ae:10:46:60:08:00 SRC=112.203.174.221 DST=88.159.69.252 LEN=131 TOS=0x00 PREC=0x00 TTL=38 ID=17898 PROTO=UDP SPT=27001 DPT=1024 LEN=111
[28679.030869]        [<ffffffff810854d5>] task_work_run+0x95/0xb0
[28679.040727]        [<ffffffff8100dc4d>] do_notify_resume+0x6d/0x80
[28679.050419]        [<ffffffff819bb5a2>] int_signal+0x12/0x17
[28679.059880]
[28679.059880] -> #0 (&tty->legacy_mutex){+.+.+.}:
[28679.077823]        [<ffffffff810b41d8>] validate_chain+0x1258/0x1300
[28679.086583]        [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
[28679.095126]        [<ffffffff810b576a>] lock_acquire+0xba/0x100
[28679.103399]        [<ffffffff819b754c>] mutex_lock_nested+0x4c/0x450
[28679.111468]        [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
[28679.119247]        [<ffffffff819ba63b>] tty_lock+0xb/0x10
[28679.126712]        [<ffffffff814492b5>] __tty_hangup+0x65/0x3c0
[28679.133940]        [<ffffffff81449620>] do_tty_hangup+0x10/0x20
[28679.140970]        [<ffffffff81080c60>] process_one_work+0x1c0/0x4b0
[28679.147755]        [<ffffffff8108134e>] worker_thread+0x11e/0x3d0
[28679.154383]        [<ffffffff81088a36>] kthread+0xd6/0xe0
[28679.160649]        [<ffffffff819bb1bc>] ret_from_fork+0x7c/0xb0
[28679.166666]
[28679.166666] other info that might help us debug this:
[28679.166666]
[28679.183748] Chain exists of:
[28679.183748]   &tty->legacy_mutex --> &tty->legacy_mutex/1 --> (&tty->hangup_work)
[28679.183748]
[28679.200495]  Possible unsafe locking scenario:
[28679.200495]
[28679.211416]        CPU0                    CPU1
[28679.216751]        ----                    ----
[28679.222049]   lock((&tty->hangup_work));
[28679.227206]                                lock(&tty->legacy_mutex/1);
[28679.232380]                                lock((&tty->hangup_work));
[28679.237532]   lock(&tty->legacy_mutex);
[28679.242673]
[28679.242673]  *** DEADLOCK ***
[28679.242673]
[28679.257840] 2 locks held by kworker/2:1/19513:
[28679.262888]  #0:  (events){.+.+.+}, at: [<ffffffff81080bf8>] process_one_work+0x158/0x4b0
[28679.268053]  #1:  ((&tty->hangup_work)){+.+...}, at: [<ffffffff81080bf8>] process_one_work+0x158/0x4b0
[28679.273381]
[28679.273381] stack backtrace:
[28679.283820] Pid: 19513, comm: kworker/2:1 Not tainted 3.8.0-rc3-20130112-netpatched-rocketscience-radeon #1
[28679.289347] Call Trace:
[28679.294804]  [<ffffffff810b2c74>] print_circular_bug+0x204/0x300
[28679.300384]  [<ffffffff810b41d8>] validate_chain+0x1258/0x1300
[28679.305997]  [<ffffffff810b4d2e>] __lock_acquire+0x44e/0xdd0
[28679.311599]  [<ffffffff810b4d4b>] ? __lock_acquire+0x46b/0xdd0
[28679.317222]  [<ffffffff810b576a>] lock_acquire+0xba/0x100
[28679.322889]  [<ffffffff819ba5ee>] ? tty_lock_nested+0x3e/0x80
[28679.328481]  [<ffffffff819ba5ee>] ? tty_lock_nested+0x3e/0x80
[28679.334023]  [<ffffffff819b754c>] mutex_lock_nested+0x4c/0x450
[28679.339415]  [<ffffffff819ba5ee>] ? tty_lock_nested+0x3e/0x80
[28679.344784]  [<ffffffff810b5788>] ? lock_acquire+0xd8/0x100
[28679.350154]  [<ffffffff81449279>] ? __tty_hangup+0x29/0x3c0
[28679.355506]  [<ffffffff819ba5ee>] tty_lock_nested+0x3e/0x80
[28679.360939]  [<ffffffff819ba63b>] tty_lock+0xb/0x10
[28679.366282]  [<ffffffff814492b5>] __tty_hangup+0x65/0x3c0
[28679.371651]  [<ffffffff81080bf8>] ? process_one_work+0x158/0x4b0
[28679.377032]  [<ffffffff810b1918>] ? trace_hardirqs_on_caller+0xf8/0x200
[28679.382273]  [<ffffffff81449620>] do_tty_hangup+0x10/0x20
[28679.387216]  [<ffffffff81080c60>] process_one_work+0x1c0/0x4b0
[28679.392118]  [<ffffffff81080bf8>] ? process_one_work+0x158/0x4b0
[28679.397039]  [<ffffffff81449610>] ? __tty_hangup+0x3c0/0x3c0
[28679.401952]  [<ffffffff8108134e>] worker_thread+0x11e/0x3d0
[28679.406859]  [<ffffffff810b1918>] ? trace_hardirqs_on_caller+0xf8/0x200
[28679.411787]  [<ffffffff81081230>] ? manage_workers+0x2e0/0x2e0
[28679.416683]  [<ffffffff81088a36>] kthread+0xd6/0xe0
[28679.421538]  [<ffffffff81088960>] ? __init_kthread_worker+0x70/0x70
[28679.426429]  [<ffffffff819bb1bc>] ret_from_fork+0x7c/0xb0
[28679.431278]  [<ffffffff81088960>] ? __init_kthread_worker+0x70/0x70


- This is followed by blocked task messages for the perl script:

Jan 12 18:25:29 serveerstertje kernel: [28926.144229] INFO: task zabbix_slimmeme:26976 blocked for more than 120 seconds.
Jan 12 18:25:29 serveerstertje kernel: [28926.162883] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
Jan 12 18:25:29 serveerstertje kernel: [28926.181312] zabbix_slimmeme D ffff88003851d230     0 26976  22112 0x00000000
Jan 12 18:25:29 serveerstertje kernel: [28926.199596]  ffff88002d473818 0000000000000216 ffff880000000002 ffffffff8202be38
Jan 12 18:25:29 serveerstertje kernel: [28926.217728]  ffff88003851d230 0000000000013040 ffff88002d473fd8 ffff88002d472010
Jan 12 18:25:29 serveerstertje kernel: [28926.235627]  0000000000013040 0000000000013040 ffff88002d473fd8 0000000000013040
Jan 12 18:25:29 serveerstertje kernel: [28926.253346] Call Trace:
Jan 12 18:25:29 serveerstertje kernel: [28926.270648]  [<ffffffff810be1ed>] ? __module_text_address+0xd/0x60
Jan 12 18:25:29 serveerstertje kernel: [28926.322462]  [<ffffffff810be1ed>] ? __module_text_address+0xd/0x60
Jan 12 18:25:29 serveerstertje kernel: [28926.339617]  [<ffffffff810be44b>] ? is_module_text_address+0x2b/0x60
Jan 12 18:25:29 serveerstertje kernel: [28926.356452]  [<ffffffff81085958>] ? __kernel_text_address+0x58/0x80
Jan 12 18:25:29 serveerstertje kernel: [28926.373057]  [<ffffffff81140049>] ? sysfs_slab_add+0x149/0x200
Jan 12 18:25:29 serveerstertje kernel: [28926.389435]  [<ffffffff81140067>] ? sysfs_slab_add+0x167/0x200
Jan 12 18:25:29 serveerstertje kernel: [28926.405516]  [<ffffffff819b8d04>] schedule+0x24/0x70
Jan 12 18:25:29 serveerstertje kernel: [28926.421242]  [<ffffffff819b5f6d>] schedule_timeout+0x1bd/0x220
Jan 12 18:25:29 serveerstertje kernel: [28926.436793]  [<ffffffff810b5788>] ? lock_acquire+0xd8/0x100
Jan 12 18:25:29 serveerstertje kernel: [28926.452138]  [<ffffffff819b8201>] ? wait_for_common+0x31/0x170
Jan 12 18:25:29 serveerstertje kernel: [28926.467171]  [<ffffffff810b5c17>] ? lock_release+0x117/0x250
Jan 12 18:25:29 serveerstertje kernel: [28926.481938]  [<ffffffff819b82d1>] wait_for_common+0x101/0x170
Jan 12 18:25:29 serveerstertje kernel: [28926.496482]  [<ffffffff81098730>] ? try_to_wake_up+0x310/0x310
Jan 12 18:25:29 serveerstertje kernel: [28926.510873]  [<ffffffff819b83e8>] wait_for_completion+0x18/0x20
Jan 12 18:25:29 serveerstertje kernel: [28926.525014]  [<ffffffff81083385>] flush_work+0x195/0x250
Jan 12 18:25:29 serveerstertje kernel: [28926.538855]  [<ffffffff810833a0>] ? flush_work+0x1b0/0x250
Jan 12 18:25:29 serveerstertje kernel: [28926.552411]  [<ffffffff81080400>] ? cwq_dec_nr_in_flight+0xd0/0xd0
Jan 12 18:25:29 serveerstertje kernel: [28926.565910]  [<ffffffff81451568>] tty_ldisc_flush_works+0x18/0x40
Jan 12 18:25:29 serveerstertje kernel: [28926.579013]  [<ffffffff814517ae>] tty_ldisc_release+0x2e/0x90
Jan 12 18:25:29 serveerstertje kernel: [28926.591876]  [<ffffffff8144b827>] tty_release+0x3c7/0x590
Jan 12 18:25:29 serveerstertje kernel: [28926.604527]  [<ffffffff810b1a2d>] ? trace_hardirqs_on+0xd/0x10
Jan 12 18:25:29 serveerstertje kernel: [28926.616853]  [<ffffffff819b63a9>] ? __mutex_unlock_slowpath+0x149/0x1d0
Jan 12 18:25:29 serveerstertje kernel: [28926.628997]  [<ffffffff81098730>] ? try_to_wake_up+0x310/0x310
Jan 12 18:25:29 serveerstertje kernel: [28926.640952]  [<ffffffff8144bdb4>] tty_open+0x3c4/0x5f0
Jan 12 18:25:30 serveerstertje kernel: [28926.652584]  [<ffffffff81150a18>] chrdev_open+0x98/0x170
Jan 12 18:25:30 serveerstertje kernel: [28926.663972]  [<ffffffff810912cd>] ? lg_local_unlock+0x3d/0x70
Jan 12 18:25:30 serveerstertje kernel: [28926.675152]  [<ffffffff81150980>] ? cdev_put+0x30/0x30
Jan 12 18:25:30 serveerstertje kernel: [28926.686173]  [<ffffffff8114b1fe>] do_dentry_open+0x25e/0x310
Jan 12 18:25:30 serveerstertje kernel: [28926.696868]  [<ffffffff8114b3c0>] finish_open+0x30/0x50
Jan 12 18:25:30 serveerstertje kernel: [28926.707267]  [<ffffffff8115a79e>] do_last+0x30e/0xe90
Jan 12 18:25:30 serveerstertje kernel: [28926.717404]  [<ffffffff81157aba>] ? link_path_walk+0x9a/0x9f0
Jan 12 18:25:30 serveerstertje kernel: [28926.727353]  [<ffffffff8115b3ce>] path_openat+0xae/0x4e0
Jan 12 18:25:30 serveerstertje kernel: [28926.737008]  [<ffffffff810b5c17>] ? lock_release+0x117/0x250
Jan 12 18:25:30 serveerstertje kernel: [28926.746482]  [<ffffffff81160264>] ? do_select+0x5f4/0x6d0
Jan 12 18:25:30 serveerstertje kernel: [28926.755613]  [<ffffffff8115b934>] do_filp_open+0x44/0xa0
Jan 12 18:25:30 serveerstertje kernel: [28926.764542]  [<ffffffff811691e3>] ? __alloc_fd+0xb3/0x150
Jan 12 18:25:30 serveerstertje kernel: [28926.773224]  [<ffffffff8114ad13>] do_sys_open+0x103/0x1f0
Jan 12 18:25:30 serveerstertje kernel: [28926.781585]  [<ffffffff8114ae3c>] sys_open+0x1c/0x20
Jan 12 18:25:30 serveerstertje kernel: [28926.789685]  [<ffffffff819bb269>] system_call_fastpath+0x16/0x1b
Jan 12 18:25:30 serveerstertje kernel: [28926.797570] INFO: lockdep is turned off.

^ permalink raw reply

* Re: [PATCH 1/1] test: Make map script a command line client
From: Marcel Holtmann @ 2013-01-11 20:48 UTC (permalink / raw)
  To: Christian Fetzer; +Cc: linux-bluetooth, Christian Fetzer
In-Reply-To: <1357922675-11718-1-git-send-email-christian.fetzer@oss.bmw-carit.de>

Hi Christian,

> Rework the map-client test script into an interactive command line client.
> Now multiple MCE functions can be called in one active session.
> The script also allows to specify all filters or optional parameters including
> auto completion.
> 
> Change-Id: I9c9ede2bc958009c757384177cf06c081d984c98

no Change-Id crap please.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 2/7] gdbus: Add g_dbus_proxy_get_client function
From: Marcel Holtmann @ 2013-01-11 20:47 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1357905018-23237-2-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

> This is convenient as some callbacks don't provide the client which
> the proxy belongs.

and I did not add that one on purpose. You should know where your proxy
belongs to and worst case hand it over via user_data.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH BlueZ 1/7] gdbus: Add g_dbus_client_get_proxy
From: Marcel Holtmann @ 2013-01-11 20:46 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1357905018-23237-1-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

> g_dbus_client_get_proxy gives the possibilitity to just check if a
> proxy exist for the given path and interface pair instead of using
> g_dbus_proxy_new which end up creating a proxy if it doesn't exists
> which is not always necessary.

why would we do that. You get the proxy via the client callbacks for
proxy created or proxy removed.

The proxy_new method is for dealing with services that do not have
ObjectManager support.

Regards

Marcel



^ permalink raw reply

* [PATCH BlueZ 11/11] audio: Bump AVRCP CT version to 1.5
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/avrcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 8bd5fb6..19b6721 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -231,7 +231,7 @@ static sdp_record_t *avrcp_ct_record(void)
 	sdp_record_t *record;
 	sdp_data_t *psm, *version, *features;
 	uint16_t lp = AVCTP_CONTROL_PSM;
-	uint16_t avrcp_ver = 0x0103, avctp_ver = 0x0103;
+	uint16_t avrcp_ver = 0x0105, avctp_ver = 0x0103;
 	uint16_t feat = ( AVRCP_FEATURE_CATEGORY_1 |
 						AVRCP_FEATURE_CATEGORY_2 |
 						AVRCP_FEATURE_CATEGORY_3 |
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 10/11] avrcp: Handle SetAbsoluteVolume command
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/avrcp.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index d715177..8bd5fb6 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1392,6 +1392,32 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static uint8_t avrcp_handle_set_absolute_volume(struct avrcp *session,
+						struct avrcp_header *pdu,
+						uint8_t transaction)
+{
+	struct avrcp_player *player = session->player;
+	uint16_t len = ntohs(pdu->params_len);
+
+	if (len != 1)
+		goto err;
+
+	if (pdu->params[0] > 127)
+		goto err;
+
+	if (!player)
+		goto err;
+
+	media_transport_update_device_volume(session->dev, pdu->params[0]);
+
+	return AVC_CTYPE_ACCEPTED;
+
+err:
+	pdu->params_len = htons(1);
+	pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
+	return AVC_CTYPE_REJECTED;
+}
+
 static const struct control_pdu_handler tg_control_handlers[] = {
 		{ AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
 					avrcp_handle_get_capabilities },
@@ -1429,6 +1455,8 @@ static const struct control_pdu_handler ct_control_handlers[] = {
 					avrcp_handle_get_capabilities },
 		{ AVRCP_REGISTER_NOTIFICATION, AVC_CTYPE_NOTIFY,
 					avrcp_handle_register_notification },
+		{ AVRCP_SET_ABSOLUTE_VOLUME, AVC_CTYPE_CONTROL,
+					avrcp_handle_set_absolute_volume },
 		{ },
 };
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 09/11] transport: Update volume passing only audio_device
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/transport.c | 19 +++++++++++++++++++
 profiles/audio/transport.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 9419af0..c344a92 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -896,3 +896,22 @@ uint8_t media_transport_get_device_volume(struct audio_device *dev)
 
 	return 128;
 }
+
+void media_transport_update_device_volume(struct audio_device *dev,
+								uint8_t volume)
+{
+	GSList *l;
+
+	if (dev == NULL)
+		return;
+
+	for (l = transports; l; l = l->next) {
+		struct media_transport *transport = l->data;
+		if (transport->device != dev)
+			continue;
+
+		/* Volume is A2DP only */
+		if (media_endpoint_get_sep(transport->endpoint))
+			media_transport_update_volume(transport, volume);
+	}
+}
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index cbbd0b6..b8cbf1f 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -42,3 +42,5 @@ void transport_get_properties(struct media_transport *transport,
 							DBusMessageIter *iter);
 
 uint8_t media_transport_get_device_volume(struct audio_device *dev);
+void media_transport_update_device_volume(struct audio_device *dev,
+								uint8_t volume);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 08/11] avrcp: Notify remote of volume changes
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/avrcp.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index b1016df..d715177 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2540,7 +2540,7 @@ int avrcp_set_volume(struct audio_device *dev, uint8_t volume)
 {
 	struct avrcp_server *server;
 	struct avrcp *session;
-	uint8_t buf[AVRCP_HEADER_LENGTH + 1];
+	uint8_t buf[AVRCP_HEADER_LENGTH + 2];
 	struct avrcp_header *pdu = (void *) buf;
 
 	server = find_server(servers, device_get_adapter(dev->btd_dev));
@@ -2555,13 +2555,27 @@ int avrcp_set_volume(struct audio_device *dev, uint8_t volume)
 
 	set_company_id(pdu->company_id, IEEEID_BTSIG);
 
-	pdu->pdu_id = AVRCP_SET_ABSOLUTE_VOLUME;
-	pdu->params[0] = volume;
-	pdu->params_len = htons(1);
-
 	DBG("volume=%u", volume);
 
-	return avctp_send_vendordep_req(session->conn, AVC_CTYPE_CONTROL,
-					AVC_SUBUNIT_PANEL, buf, sizeof(buf),
+	if (session->target) {
+		pdu->pdu_id = AVRCP_SET_ABSOLUTE_VOLUME;
+		pdu->params[0] = volume;
+		pdu->params_len = htons(1);
+
+		return avctp_send_vendordep_req(session->conn,
+					AVC_CTYPE_CONTROL, AVC_SUBUNIT_PANEL,
+					buf, sizeof(buf),
 					avrcp_handle_set_volume, session);
+	} else {
+		uint8_t id = AVRCP_EVENT_VOLUME_CHANGED;
+		pdu->pdu_id = AVRCP_REGISTER_NOTIFICATION;
+		pdu->params[0] = AVRCP_EVENT_VOLUME_CHANGED;
+		pdu->params[1] = volume;
+		pdu->params_len = htons(2);
+
+		return avctp_send_vendordep(session->conn,
+					session->transaction_events[id],
+					AVC_CTYPE_CHANGED, AVC_SUBUNIT_PANEL,
+					buf, sizeof(buf));
+	}
 }
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 07/11] avrcp: Handle RegisterNotification for volume
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

This commit answers a NOTIFY command to register for nofications of
EVENT_VOLUME_CHANGED with an INTERIM response containing the current
absolute volume value.
---
 profiles/audio/avrcp.c     | 15 +++++++++++++++
 profiles/audio/transport.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index bb8ab24..b1016df 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -62,6 +62,7 @@
 #include "avdtp.h"
 #include "sink.h"
 #include "player.h"
+#include "transport.h"
 
 /* Company IDs for vendor dependent commands */
 #define IEEEID_BTSIG		0x001958
@@ -1247,6 +1248,7 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 						uint8_t transaction)
 {
 	struct avrcp_player *player = session->player;
+	struct audio_device *dev = session->dev;
 	uint16_t len = ntohs(pdu->params_len);
 	uint64_t uid;
 	GList *settings;
@@ -1293,6 +1295,17 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 		}
 
 		break;
+	case AVRCP_EVENT_VOLUME_CHANGED:
+		if (session->version < 0x0104)
+			goto err;
+
+		pdu->params[1] = media_transport_get_device_volume(dev);
+		if (pdu->params[1] > 127)
+			goto err;
+
+		len = 2;
+
+		break;
 	default:
 		/* All other events are not supported yet */
 		goto err;
@@ -1414,6 +1427,8 @@ static const struct control_pdu_handler tg_control_handlers[] = {
 static const struct control_pdu_handler ct_control_handlers[] = {
 		{ AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
 					avrcp_handle_get_capabilities },
+		{ AVRCP_REGISTER_NOTIFICATION, AVC_CTYPE_NOTIFY,
+					avrcp_handle_register_notification },
 		{ },
 };
 
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index 78c6fa7..cbbd0b6 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -23,6 +23,7 @@
  */
 
 struct media_transport;
+struct media_endpoint;
 
 struct media_transport *media_transport_create(struct media_endpoint *endpoint,
 						struct audio_device *device,
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 06/11] transport: Get volume passing only audio_device
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/transport.c | 20 ++++++++++++++++++++
 profiles/audio/transport.h |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index c3e26f4..9419af0 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -876,3 +876,23 @@ void media_transport_update_volume(struct media_transport *transport,
 					transport->path,
 					MEDIA_TRANSPORT_INTERFACE, "Volume");
 }
+
+uint8_t media_transport_get_device_volume(struct audio_device *dev)
+{
+	GSList *l;
+
+	if (dev == NULL)
+		return 128;
+
+	for (l = transports; l; l = l->next) {
+		struct media_transport *transport = l->data;
+		if (transport->device != dev)
+			continue;
+
+		/* Volume is A2DP only */
+		if (media_endpoint_get_sep(transport->endpoint))
+			return media_transport_get_volume(transport);
+	}
+
+	return 128;
+}
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index 0fe8973..78c6fa7 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -39,3 +39,5 @@ void media_transport_update_volume(struct media_transport *transport,
 								uint8_t volume);
 void transport_get_properties(struct media_transport *transport,
 							DBusMessageIter *iter);
+
+uint8_t media_transport_get_device_volume(struct audio_device *dev);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 05/11] transport: Keep a list o all existent transports
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/transport.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 4a81d85..c3e26f4 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -107,6 +107,8 @@ struct media_transport {
 	void			*data;
 };
 
+static GSList *transports = NULL;
+
 static const char *state2str(transport_state_t state)
 {
 	switch (state) {
@@ -703,6 +705,8 @@ static void media_transport_free(void *data)
 {
 	struct media_transport *transport = data;
 
+	transports = g_slist_remove(transports, transport);
+
 	if (transport->owner)
 		media_transport_remove_owner(transport);
 
@@ -816,6 +820,8 @@ struct media_transport *media_transport_create(struct media_endpoint *endpoint,
 		goto fail;
 	}
 
+	transports = g_slist_append(transports, transport);
+
 	return transport;
 
 fail:
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 04/11] transport: Get volume from transport
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/transport.c | 6 ++++++
 profiles/audio/transport.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index f3caa1b..4a81d85 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -849,6 +849,12 @@ struct audio_device *media_transport_get_dev(struct media_transport *transport)
 	return transport->device;
 }
 
+uint16_t media_transport_get_volume(struct media_transport *transport)
+{
+	struct a2dp_transport *a2dp = transport->data;
+	return a2dp->volume;
+}
+
 void media_transport_update_volume(struct media_transport *transport,
 								uint8_t volume)
 {
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index a6b71e5..0fe8973 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -32,6 +32,7 @@ struct media_transport *media_transport_create(struct media_endpoint *endpoint,
 void media_transport_destroy(struct media_transport *transport);
 const char *media_transport_get_path(struct media_transport *transport);
 struct audio_device *media_transport_get_dev(struct media_transport *transport);
+uint16_t media_transport_get_volume(struct media_transport *transport);
 void media_transport_update_delay(struct media_transport *transport,
 							uint16_t delay);
 void media_transport_update_volume(struct media_transport *transport,
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 03/11] avrcp: Add EVENT_VOLUME_CHANGED to CT capabilities
From: João Paulo Rechi Vita @ 2013-01-11 20:25 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: vinicius.gomes, claudio.takahasi, luiz.von.dentz,
	João Paulo Rechi Vita
In-Reply-To: <1357935934-20033-1-git-send-email-jprvita@openbossa.org>

---
 profiles/audio/avrcp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index ef324b0..bb8ab24 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2164,6 +2164,9 @@ static void session_ct_init(struct avrcp *session)
 
 	session->control_handlers = ct_control_handlers;
 
+	if (session->version >= 0x0104)
+		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
+
 	DBG("%p version 0x%04x", session, session->version);
 
 	session->control_id = avctp_register_pdu_handler(session->conn,
-- 
1.7.11.7


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox