Re: [PATCH 1/1] mesh: Remove StopNotify method to avoid reading data twice on some mesh nodes

["Stotland, Inga" <inga.stotland@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3543 bytes --]

Hi Jehudi,

On Fri, 2017-09-08 at 16:25 +0200, Laczen JMS wrote:
> Hi Luiz,
> 
> Probably my analysis of the cause is wrong. I added a rl_printf when
> the StopNotify was called, meshctl shows the following:
> 
> Notify started
> Notify for Mesh Provisioning Out Data started
> Open-Node: 0x102c470
> Open-Prov: 0xff7000
> Open-Prov: proxy 0xffe830
> GATT-TX:  03 00 10
> Attempting to write
> /org/bluez/hci0/dev_C3_F0_13_5D_5F_5E/service000a/char000b
> Initiated provisioning
> Characteristic property changed
> /org/bluez/hci0/dev_C3_F0_13_5D_5F_5E/service000a/char000d
> GATT-RX: 03 01 01 00 01 00 00 00 00 00 00 00 00
> Got provisioning data (12 bytes)
> 01 01 00 01 00 00 00 00 00 00 00 00
> Provisioning failed
> Calling StopNotify --------------> added by me using rl_printf()
> Characteristic property changed
> /org/bluez/hci0/dev_C3_F0_13_5D_5F_5E/service000a/char000d
> GATT-RX: 03 01 01 00 01 00 00 00 00 00 00 00 00
> Node not found?
> Notify stopped
> Attempting to disconnect from C3:F0:13:5D:5F:5E
> Services resolved no
> 
> As you can see the same data is received twice: GATT-RX: 03 01 01 00
> 01 ..., just after the Characteristic property changed line. I
> thought
> that this Characteristic property change was the result of the not
> changing of the node and keeping notifications on.
> 
> I also enable btmon during this process (see included file). In the
> log I cannot find the data being send twice.
> 
> Kind regards,
> 
> Jehudi
> 
> 2017-09-08 15:47 GMT+02:00 Luiz Augusto von Dentz <luiz.dentz@gmail.c
> om>:
> > Hi Jehudi,
> > 
> > On Fri, Sep 8, 2017 at 3:30 PM, Laczen JMS <laczenjms@gmail.com>
> > wrote:
> > > Some mesh nodes do not change (e.g. zephyr) the notification when
> > > given the StopNotify. As a result the data from the notification
> > > channel is processed twice. This patch removes the StopNotify.
> > 
> > What do you mean the node do not change? Perhaps this is the result
> > of
> > bluetoothd remembering the subscriptions so you don't have to
> > StopNotify while disconnected, though this should not cause the
> > data
> > to be processed twice otherwise we have a bug somewhere in the
> > daemon.
> > 
> > > Kind regards,
> > > 
> > > Jehudi
> > > 
> > >   diff --git a/mesh/gatt.c b/mesh/gatt.c
> > > index f9615b3..7d3b869 100644
> > > --- a/mesh/gatt.c
> > > +++ b/mesh/gatt.c
> > > @@ -589,15 +589,10 @@ bool mesh_gatt_notify(GDBusProxy *proxy,
> > > bool
> > > enable, GDBusReturnFunction cb,
> > >   cb = notify_reply;
> > >   }
> > >   } else {
> > > - if (notify_io) {
> > > - notify_io_destroy();
> > > - if (cb)
> > > - cb(NULL, user_data);
> > > - return true;
> > > - } else {
> > > - method = "StopNotify";
> > > - cb = notify_reply;
> > > - }
> > > + if (notify_io) notify_io_destroy();
> > > + if (cb) cb(NULL, user_data);
> > > +
> > > + return true;
> > >   }
> > > 
> > >   if (g_dbus_proxy_method_call(proxy, method, NULL, cb,
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> > > bluetooth" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.htm
> > > l
> > 
> > 
> > 
> > --
> > Luiz Augusto von Dentz


Thank you for your analysis of the issue. A patch (subject "PATCH
BlueZ] mesh: Add characteristic property name check") has been
submitted and (I believe) should take care of seeing duplicate GATT-RX
output.

Regards,

Inga

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3266 bytes --]