Return-Path: MIME-Version: 1.0 In-Reply-To: References: <20150417220040.A379A220209@puck.mtv.corp.google.com> Date: Tue, 21 Apr 2015 09:08:14 +0300 Message-ID: Subject: Re: [PATCH 2/3] hog: re-enable HoG report notifications on HoG device reconnect From: Luiz Augusto von Dentz To: Petri Gynther Cc: "linux-bluetooth@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Petri, On Tue, Apr 21, 2015 at 2:17 AM, Petri Gynther wrote: > Hi Luiz, > > On Mon, Apr 20, 2015 at 2:54 PM, Petri Gynther wrote: >> Hi Luiz, >> >> On Mon, Apr 20, 2015 at 1:12 PM, Luiz Augusto von Dentz >> wrote: >>> Hi Petri, >>> >>> On Mon, Apr 20, 2015 at 7:34 PM, Petri Gynther wrote: >>>> Hi Luiz, >>>> >>>> On Mon, Apr 20, 2015 at 7:50 AM, Luiz Augusto von Dentz >>>> wrote: >>>>> Hi Petri, >>>>> >>>>> On Sat, Apr 18, 2015 at 1:00 AM, Petri Gynther wrote: >>>>>> On HoG device reconnect, re-enable HoG report notifications every >>>>>> time by writing the client characteristic configuration attribute >>>>>> of each HoG report. >>>>>> >>>>>> Doing this on every reconnect: >>>>>> 1. ensures that HoG report notifications are always enabled >>>>>> (e.g. lost report notification state in battery swap). >>>>>> 2. signals to HoG device that it can start sending HoG reports >>>>>> (e.g. buffered keypresses while the reconnect is still pending). >>>>> >>>>> Is that happening for a specific device? For 1 I suppose it would >>>>> loose the paring keys as well which means we would not be able to >>>>> connect at all as HoG mandates pairing, or does it loose only the CCC >>>>> configuration? Reason 2 is not necessary the behavior every device >>>>> would have, for example I don't expect a mouse to buffer anything and >>>>> even a keyboard may buffer only the last key pressed since it might >>>>> have very little memory to spend in buffering, anyway I suppose reason >>>>> 1 is what you should really concentrate to tell us what is going on. >>>>> >>>> >>>> On the device that I'm working on, pairing info is stored in >>>> persistent storage, so it survives battery swap. However, I'm not sure >>>> yet about HoG report notification enable/disable state. Waiting for >>>> confirmation from vendor. >>> >>> I would expect it to store the CCC state as well in the persistent >>> storage if one exists. >>> >>>> Anyways, we are seeing (2). The first keypress from HoG device is >>>> getting lost on reconnect, leading to bad user experience. The device >>>> is sending the keypress immediately when the connection is good, but >>>> too early before BlueZ has managed to re-register the handler with >>>> g_attrib_register(). During the reconnect, the HoG device has no way >>>> of knowing when BlueZ is ready and has registered the handler. So, to >>>> provide this indication, I propose to re-enable the HoG report >>>> notification every time. >>> >>> But I already fixed this problem not long ago: >>> >>> commit 14569a33418a4c1dc07cca090b9336373d63a085 >>> Author: Luiz Augusto von Dentz >>> Date: Wed Mar 11 21:23:55 2015 +0200 >>> >>> core/device: Fix not handling notification >>> >>> attio callbacks needs to be triggered as soon as possible once connected >>> otherwise profiles such as HoG may miss notification that are sent while >>> bt_gatt_client is doing MTU exchange. >>> >>> Perhaps you don't have this patch? >>> >> >> Thanks for pointing this out. We are still using BlueZ 5.28, so don't >> have this. I'll patch this in and see how HoG reconnect works then. >> > > I reverted my 3 patches and applied your "core/device: Fix not > handling notification" patch on top of our BlueZ 5.28 stack. > Unfortunately, your patch breaks our HoG device completely. Our > device's report map ends up being read incorrectly, and as a result, > not a single keypress works any more. > > Applying your patch causes attio_connected_cb() getting called very > early, before the MTU exchange has been negotiated. Somehow, this > doesn't work for our device. You might need to following as well: attrib: Fix not honoring MTU If MTU has changed, by bt_gatt_client for example, the code should be able to figure it out without waiting g_attrib_set_mtu to be called. -- Luiz Augusto von Dentz