* [RFC]Mesh: meshctl configuration output fails in gatt.c:pipe_write
@ 2017-11-24 14:39 Steve Brown
2017-11-24 15:10 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 4+ messages in thread
From: Steve Brown @ 2017-11-24 14:39 UTC (permalink / raw)
To: linux-bluetooth
If the first command output in a new connection exceeds 20 bytes,
mesh_gatt_write sets the SAR to FIRST as the write_mtu is initially 0
and the default is GATT_MTU-3 (20).
When pipe_write gets called, a new larger write_mtu has been set, but
the SAR is still set to FIRST. It's assumed that data->gatt_len >
max_len. However, it's not which causes lots of bogus output.
I've added code to reset the SAR and length in acquire_write_reply in
case write_mtu might have changed.
This seems to work, but I'm sure there is a better way.
Steve
---
[config: Target = 0100]# get-ttl
mesh_gatt_write: max_len:20 len:21 sar:0 write_mtu:0 write_io:(nil)
Characteristic property changed /org/bluez/hci0/dev_C9_63_A3_A6_CD_DA/service000a/char000b
AcquireWrite success: fd 8 MTU 69
pipe_write: len:21 max_len:65
GATT-TX: 40 f4 a8 fa b2 33 ac 51 80 ff 16 2e 74 b4 a0 bb
GATT-TX: 2f 2f 54 e2 96
GATT-TX: iov[0]: 40
GATT-TX: iov[1]: f4 a8 fa b2 33 ac 51 80 ff 16 2e 74 b4 a0 bb 2f
GATT-TX: iov[1]: 2f 54 e2
GATT-TX: iov[0]: 80
GATT-TX: iov[1]: 00 00 fe ff ff ff fe ff ff ff fe ff ff ff fe ff
GATT-TX: iov[1]: ff ff fe ff ff ff fe ff ff ff fe ff ff ff fe ff
GATT-TX: iov[1]: ff ff 14 00 00 00 fe ff ff ff 01 00 00 00 70 15
GATT-TX: iov[1]: d6 01 36 00 00 00 3e 00 00 00 00 00 00 00 00 00
GATT-TX: iov[1]: 00
GATT-TX: iov[0]: 80
GATT-TX: iov[1]: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
GATT-TX: iov[1]: 00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00
GATT-TX: iov[1]: 00 00 00 00 00 00 00 00 00 51 00 00 00 06 00 00
GATT-TX: iov[1]: 00 0c 98 d7 76 00 00 00 00 00 00 00 00 04 00 97
GATT-TX: iov[1]: d7
GATT-TX: iov[0]: 80
GATT-TX: iov[1]: 76 c4 97 d7 00 00 00 00 30 00 00 00 b0 4d d4 01
GATT-TX: iov[1]: e0 b6 dc 76 20 b6 dc 76 21 00 00 00 dc 97 d7 76
GATT-TX: iov[1]: dc 97 d7 76 28 4e d4 01 20 00 00 00 00 00 00 00
GATT-TX: iov[1]: 00 00 00 00 50 00 00 00 29 00 00 00 00 00 00 00
GATT-TX: iov[1]: 00
GATT-TX: iov[0]: c0
GATT-TX: iov[1]: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
GATT-TX: iov[1]: 00
diff --git a/mesh/gatt.c b/mesh/gatt.c
index 001eb17a8..3f59268f2 100644
--- a/mesh/gatt.c
+++ b/mesh/gatt.c
@@ -358,6 +362,7 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
struct write_data *data = user_data;
DBusError error;
int fd;
+ uint8_t max_len;
dbus_error_init(&error);
@@ -383,6 +388,18 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
write_io = pipe_io_new(fd);
+ /* Reset type and length as write_mtu may have changed */
+ max_len = write_mtu ? write_mtu - 3 : GATT_MTU - 3;
+ data->gatt_data[0] &= GATT_TYPE_MASK;
+
+ if (max_len < data->gatt_len) {
+ data->iov.iov_len = max_len;
+ data->gatt_data[0] |= GATT_SAR_FIRST;
+ }
+
+ else
+ data->iov.iov_len = data->gatt_len;
+
pipe_write(write_io, data);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [RFC]Mesh: meshctl configuration output fails in gatt.c:pipe_write
2017-11-24 14:39 [RFC]Mesh: meshctl configuration output fails in gatt.c:pipe_write Steve Brown
@ 2017-11-24 15:10 ` Luiz Augusto von Dentz
2017-11-24 16:23 ` Steve Brown
2017-12-06 15:02 ` Steve Brown
0 siblings, 2 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2017-11-24 15:10 UTC (permalink / raw)
To: Steve Brown; +Cc: linux-bluetooth
Hi Steve,
On Fri, Nov 24, 2017 at 4:39 PM, Steve Brown <sbrown@cortland.com> wrote:
> If the first command output in a new connection exceeds 20 bytes,
> mesh_gatt_write sets the SAR to FIRST as the write_mtu is initially 0
> and the default is GATT_MTU-3 (20).
>
> When pipe_write gets called, a new larger write_mtu has been set, but
> the SAR is still set to FIRST. It's assumed that data->gatt_len >
> max_len. However, it's not which causes lots of bogus output.
>
> I've added code to reset the SAR and length in acquire_write_reply in
> case write_mtu might have changed.
>
> This seems to work, but I'm sure there is a better way.
>
> Steve
>
> ---
>
> [config: Target = 0100]# get-ttl
> mesh_gatt_write: max_len:20 len:21 sar:0 write_mtu:0 write_io:(nil)
> Characteristic property changed /org/bluez/hci0/dev_C9_63_A3_A6_CD_DA/service000a/char000b
> AcquireWrite success: fd 8 MTU 69
> pipe_write: len:21 max_len:65
> GATT-TX: 40 f4 a8 fa b2 33 ac 51 80 ff 16 2e 74 b4 a0 bb
> GATT-TX: 2f 2f 54 e2 96
> GATT-TX: iov[0]: 40
> GATT-TX: iov[1]: f4 a8 fa b2 33 ac 51 80 ff 16 2e 74 b4 a0 bb 2f
> GATT-TX: iov[1]: 2f 54 e2
> GATT-TX: iov[0]: 80
> GATT-TX: iov[1]: 00 00 fe ff ff ff fe ff ff ff fe ff ff ff fe ff
> GATT-TX: iov[1]: ff ff fe ff ff ff fe ff ff ff fe ff ff ff fe ff
> GATT-TX: iov[1]: ff ff 14 00 00 00 fe ff ff ff 01 00 00 00 70 15
> GATT-TX: iov[1]: d6 01 36 00 00 00 3e 00 00 00 00 00 00 00 00 00
> GATT-TX: iov[1]: 00
> GATT-TX: iov[0]: 80
> GATT-TX: iov[1]: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> GATT-TX: iov[1]: 00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00
> GATT-TX: iov[1]: 00 00 00 00 00 00 00 00 00 51 00 00 00 06 00 00
> GATT-TX: iov[1]: 00 0c 98 d7 76 00 00 00 00 00 00 00 00 04 00 97
> GATT-TX: iov[1]: d7
> GATT-TX: iov[0]: 80
> GATT-TX: iov[1]: 76 c4 97 d7 00 00 00 00 30 00 00 00 b0 4d d4 01
> GATT-TX: iov[1]: e0 b6 dc 76 20 b6 dc 76 21 00 00 00 dc 97 d7 76
> GATT-TX: iov[1]: dc 97 d7 76 28 4e d4 01 20 00 00 00 00 00 00 00
> GATT-TX: iov[1]: 00 00 00 00 50 00 00 00 29 00 00 00 00 00 00 00
> GATT-TX: iov[1]: 00
> GATT-TX: iov[0]: c0
> GATT-TX: iov[1]: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> GATT-TX: iov[1]: 00
>
> diff --git a/mesh/gatt.c b/mesh/gatt.c
> index 001eb17a8..3f59268f2 100644
> --- a/mesh/gatt.c
> +++ b/mesh/gatt.c
> @@ -358,6 +362,7 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
> struct write_data *data = user_data;
> DBusError error;
> int fd;
> + uint8_t max_len;
>
> dbus_error_init(&error);
>
> @@ -383,6 +388,18 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
>
> write_io = pipe_io_new(fd);
>
> + /* Reset type and length as write_mtu may have changed */
> + max_len = write_mtu ? write_mtu - 3 : GATT_MTU - 3;
> + data->gatt_data[0] &= GATT_TYPE_MASK;
Ive never really liked the idea of putting the SAR byte into data but
it was required due to use of WriteValue, now perhaps we can even
remove that since AcquireWrite is stable the sar byte can be set in an
independent variable in pipe_write so we can move this logic there.
> + if (max_len < data->gatt_len) {
> + data->iov.iov_len = max_len;
> + data->gatt_data[0] |= GATT_SAR_FIRST;
> + }
> +
> + else
> + data->iov.iov_len = data->gatt_len;
> +
> pipe_write(write_io, data);
> }
>
>
>
>
> --
> 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.html
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC]Mesh: meshctl configuration output fails in gatt.c:pipe_write
2017-11-24 15:10 ` Luiz Augusto von Dentz
@ 2017-11-24 16:23 ` Steve Brown
2017-12-06 15:02 ` Steve Brown
1 sibling, 0 replies; 4+ messages in thread
From: Steve Brown @ 2017-11-24 16:23 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Steve Brown; +Cc: linux-bluetooth
Hi Luiz,
On Fri, 2017-11-24 at 17:10 +0200, Luiz Augusto von Dentz wrote:
> Hi Steve,
>
> On Fri, Nov 24, 2017 at 4:39 PM, Steve Brown <sbrown@cortland.com>
> wrote:
> > If the first command output in a new connection exceeds 20 bytes,
> > mesh_gatt_write sets the SAR to FIRST as the write_mtu is initially
> > 0
> > and the default is GATT_MTU-3 (20).
> >
> > When pipe_write gets called, a new larger write_mtu has been set,
> > but
> > the SAR is still set to FIRST. It's assumed that data->gatt_len >
> > max_len. However, it's not which causes lots of bogus output.
> >
> > I've added code to reset the SAR and length in acquire_write_reply
> > in
> > case write_mtu might have changed.
> >
> > This seems to work, but I'm sure there is a better way.
> >
> > Steve
> >
> > ---
> >
> >
> > diff --git a/mesh/gatt.c b/mesh/gatt.c
> > index 001eb17a8..3f59268f2 100644
> > --- a/mesh/gatt.c
> > +++ b/mesh/gatt.c
> > @@ -358,6 +362,7 @@ static void acquire_write_reply(DBusMessage
> > *message, void *user_data)
> > struct write_data *data = user_data;
> > DBusError error;
> > int fd;
> > + uint8_t max_len;
> >
> > dbus_error_init(&error);
> >
> > @@ -383,6 +388,18 @@ static void acquire_write_reply(DBusMessage
> > *message, void *user_data)
> >
> > write_io = pipe_io_new(fd);
> >
> > + /* Reset type and length as write_mtu may have changed */
> > + max_len = write_mtu ? write_mtu - 3 : GATT_MTU - 3;
> > + data->gatt_data[0] &= GATT_TYPE_MASK;
>
> Ive never really liked the idea of putting the SAR byte into data but
> it was required due to use of WriteValue, now perhaps we can even
> remove that since AcquireWrite is stable the sar byte can be set in
> an
> independent variable in pipe_write so we can move this logic there.
>
> > + if (max_len < data->gatt_len) {
> > + data->iov.iov_len = max_len;
> > + data->gatt_data[0] |= GATT_SAR_FIRST;
> > + }
> > +
> > + else
> > + data->iov.iov_len = data->gatt_len;
> > +
> > pipe_write(write_io, data);
> > }
> >
Thanks for your quick response.
I've just started reading the gatt-api doc. I might be ready to have at
this sometime in the spring.
I discovered the problem while trying to configure the new zephyr
mesh_shell. It should probably be fixed sooner than later. I best defer
to somebody more experienced.
Steve
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC]Mesh: meshctl configuration output fails in gatt.c:pipe_write
2017-11-24 15:10 ` Luiz Augusto von Dentz
2017-11-24 16:23 ` Steve Brown
@ 2017-12-06 15:02 ` Steve Brown
1 sibling, 0 replies; 4+ messages in thread
From: Steve Brown @ 2017-12-06 15:02 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Steve Brown; +Cc: linux-bluetooth
Hi Luiz,
On Fri, 2017-11-24 at 17:10 +0200, Luiz Augusto von Dentz wrote:
> Hi Steve,
>
> On Fri, Nov 24, 2017 at 4:39 PM, Steve Brown <sbrown@cortland.com>
> wrote:
> > If the first command output in a new connection exceeds 20 bytes,
> > mesh_gatt_write sets the SAR to FIRST as the write_mtu is initially
> > 0
> > and the default is GATT_MTU-3 (20).
> >
> > When pipe_write gets called, a new larger write_mtu has been set,
> > but
> > the SAR is still set to FIRST. It's assumed that data->gatt_len >
> > max_len. However, it's not which causes lots of bogus output.
> >
> > I've added code to reset the SAR and length in acquire_write_reply
> > in
> > case write_mtu might have changed.
> >
> > This seems to work, but I'm sure there is a better way.
> >
> > Steve
> >
> Ive never really liked the idea of putting the SAR byte into data but
> it was required due to use of WriteValue, now perhaps we can even
> remove that since AcquireWrite is stable the sar byte can be set in
> an
> independent variable in pipe_write so we can move this logic there.
>
>
>
Have you had a chance to review my changes to gatt.c?
http://www.spinics.net/lists/linux-bluetooth/msg72981.html
I removed the WriteValue code and moved the sar logic to pipe_write as
you requested. The latter fixed the bug I reported. If you have no
additional comments, let me know and I'll submit it as a patch.
I'm working on some additional commands for meshctl and they won't work
without the bug fixed.
Thanks,
Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-06 15:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-24 14:39 [RFC]Mesh: meshctl configuration output fails in gatt.c:pipe_write Steve Brown
2017-11-24 15:10 ` Luiz Augusto von Dentz
2017-11-24 16:23 ` Steve Brown
2017-12-06 15:02 ` Steve Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.