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.
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,
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 <[email protected]
> om>:
> > Hi Jehudi,
> >
> > On Fri, Sep 8, 2017 at 3:30 PM, Laczen JMS <[email protected]>
> > 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 [email protected]
> > > 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
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 <[email protected]>:
> Hi Jehudi,
>
> On Fri, Sep 8, 2017 at 3:30 PM, Laczen JMS <[email protected]> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
Hi,
On Fri, Sep 8, 2017 at 4:47 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Jehudi,
>
> On Fri, Sep 8, 2017 at 3:30 PM, Laczen JMS <[email protected]> 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,
Btw please have a look in our HACKING document on how to contribute patches:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/HACKING
--
Luiz Augusto von Dentz
Hi Jehudi,
On Fri, Sep 8, 2017 at 3:30 PM, Laczen JMS <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz