Hi Szymon,
On Fri, Mar 4, 2016 at 3:35 PM, Szymon Janc <szymon.janc@xxxxxxxxxxx> wrote:
> Hi Luiz,
>
> On Friday 04 March 2016 15:12:39 Luiz Augusto von Dentz wrote:
>> Hi Łukasz,
>>
>> On Fri, Mar 4, 2016 at 2:51 PM, Łukasz Rymanowski
>>
>> <lukasz.rymanowski@xxxxxxxxxxx> wrote:
>> > Hi Luiz,
>> >
>> > On 4 March 2016 at 13:24, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> wrote:
>> >> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>> >>
>> >> Some Android versions don't seem to cope well with the fact that the
>> >> peripheral can actually set a MTU thus leave the MTU as the default.
>> >
>> > Just tested that and for me connection with Android works OK even BlueZ
>> > started Exchange MTU procedure. Tested on Android 5 and 6.
>> >
>> > I saw once the issue that Android could not retrieve services after
>> > connection to BlueZ
>> > but from the logs it looked like BlueZ start search for primary
>> > services and Android not.
>> > If Android make to sent search request for primary services first then
>> > it was fine.
>> >
>> > Maybe Android don't like BlueZ to start search primary if Android is
>> > initiator? But this is their bug
>> > BTW. I saw it on Android 5 not yet on 6.
>>
>> With 4.4 it actually made a difference.
>>
>> >> ---
>> >>
>> >> src/attrib-server.c | 2 +-
>> >> src/device.c | 8 +++++---
>> >> src/device.h | 2 +-
>> >> src/gatt-database.c | 2 +-
>> >> 4 files changed, 8 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/src/attrib-server.c b/src/attrib-server.c
>> >> index 4439c27..e92ca5c 100644
>> >> --- a/src/attrib-server.c
>> >> +++ b/src/attrib-server.c
>> >> @@ -1281,7 +1281,7 @@ static void connect_event(GIOChannel *io, GError
>> >> *gerr, void *user_data)>>
>> >> if (!device)
>> >>
>> >> return;
>> >>
>> >> - device_attach_att(device, io);
>> >> + device_attach_att(device, io, false);
>> >>
>> >> }
>> >>
>> >> static gboolean register_core_services(struct gatt_server *server)
>> >>
>> >> diff --git a/src/device.c b/src/device.c
>> >> index 14e850e..b4bc593 100644
>> >> --- a/src/device.c
>> >> +++ b/src/device.c
>> >> @@ -4664,7 +4664,7 @@ static bool remote_counter(uint32_t *sign_cnt, void
>> >> *user_data)>>
>> >> return true;
>> >>
>> >> }
>> >>
>> >> -bool device_attach_att(struct btd_device *dev, GIOChannel *io)
>> >> +bool device_attach_att(struct btd_device *dev, GIOChannel *io, bool
>> >> initiator)>>
>> >> {
>> >>
>> >> GError *gerr = NULL;
>> >> GAttrib *attrib;
>> >>
>> >> @@ -4699,7 +4699,9 @@ bool device_attach_att(struct btd_device *dev,
>> >> GIOChannel *io)>>
>> >> }
>> >>
>> >> }
>> >>
>> >> - dev->att_mtu = MIN(mtu, BT_ATT_MAX_LE_MTU);
>> >> + /* Only attempt to set MTU if initiator/central */
>> >
>> > Keep in mind that MTU echange procedure is bonded to Client/Server
>> > role not central/peripheral.
>>
>> Well actually is not either GAP or GATT role related since both side
>> are in fact allowed to do that, so this is just a workaround.
>>
>> >> + dev->att_mtu = initiator ? MIN(mtu, BT_ATT_MAX_LE_MTU) :
>> >> +
>> >> BT_ATT_DEFAULT_LE_MTU;>>
>> >> attrib = g_attrib_new(io, dev->att_mtu, false);
>> >> if (!attrib) {
>> >>
>> >> error("Unable to create new GAttrib instance");
>> >>
>> >> @@ -4768,7 +4770,7 @@ static void att_connect_cb(GIOChannel *io, GError
>> >> *gerr, gpointer user_data)>>
>> >> goto done;
>> >>
>> >> }
>> >>
>> >> - if (!device_attach_att(device, io))
>> >> + if (!device_attach_att(device, io, true))
>> >>
>> >> goto done;
>> >>
>> >> if (attcb->success)
>> >>
>> >> diff --git a/src/device.h b/src/device.h
>> >> index db10827..5c01ed0 100644
>> >> --- a/src/device.h
>> >> +++ b/src/device.h
>> >> @@ -72,7 +72,7 @@ struct bt_gatt_client
>> >> *btd_device_get_gatt_client(struct btd_device *device);>>
>> >> struct bt_gatt_server *btd_device_get_gatt_server(struct btd_device
>> >> *device); void btd_device_gatt_set_service_changed(struct btd_device
>> >> *device,>>
>> >> uint16_t start, uint16_t
>> >> end);
>> >>
>> >> -bool device_attach_att(struct btd_device *dev, GIOChannel *io);
>> >> +bool device_attach_att(struct btd_device *dev, GIOChannel *io, bool
>> >> initiator);>>
>> >> void btd_device_add_uuid(struct btd_device *device, const char *uuid);
>> >> void device_add_eir_uuids(struct btd_device *dev, GSList *uuids);
>> >> void device_set_manufacturer_data(struct btd_device *dev, GSList *list);
>> >>
>> >> diff --git a/src/gatt-database.c b/src/gatt-database.c
>> >> index d906e2b..4df4a16 100644
>> >> --- a/src/gatt-database.c
>> >> +++ b/src/gatt-database.c
>> >> @@ -499,7 +499,7 @@ static void connect_cb(GIOChannel *io, GError *gerr,
>> >> gpointer user_data)>>
>> >> if (!device)
>> >>
>> >> return;
>> >>
>> >> - device_attach_att(device, io);
>> >> + device_attach_att(device, io, false);
>> >
>> > Not sure if it works when you use white list for autoconnect because
>> > iirc we got this callback when BlueZ is initiator
>> > then. But still initiator or not, the important thing is if we act as
>> > client or server.
>>
>> Probably not, I will have to add more logic to detect how it got
>> connected, maybe we can actually check who is the master so that the
>> master should be the one controlling the MTU.
>
> Both sides can initiate MTU exchange as you can be both server and client.
> I really fail to see how mixing that with central vs peripheral would help.
> And if there is a bug in Android then we would have to first understand it eg.
> if problem is just with MTU or any other request would result in same issue.
As I said I already tested this, with 4.4 at least it makes a
difference since Android stop any client request if you attempt to do
ATT Exchange: http://ix.io/oMm
Other request do work as you can see from the logs we do manage to
continue with our own request, now with these changes I got the
following: http://ix.io/oOB
I don't think there is any doubt that it is the ATT Exchange command
causing the issue, perhaps you can check if that bug still exist but
we got a thread with a few other people with similar problem.
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
