2018-03-21 16:59:00

by Robert Lubaś

[permalink] [raw]
Subject: [PATCH BlueZ] Mesh: Fix TTL in Config Heartbeat Publication Set

In 4.2.17.4 Heartbeat Publication TTL value range is 0x00-0x7F.
In cmd_hb_pub_set heartbeat ttl was set to DEFAULT_TTL 0x77, this
patch fix this by adding ttl param to hb-pub-set.
---
mesh/config-client.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/mesh/config-client.c b/mesh/config-client.c
index 19e617d62..9757e2625 100644
--- a/mesh/config-client.c
+++ b/mesh/config-client.c
@@ -1042,7 +1042,7 @@ static void cmd_hb_pub_set(int argc, char *argv[])
n = mesh_opcode_set(OP_CONFIG_HEARTBEAT_PUB_SET, msg);

parm_cnt = read_input_parameters(argc, argv);
- if (parm_cnt != 5) {
+ if (parm_cnt != 6) {
bt_shell_printf("Bad arguments: %s\n", argv[1]);
return bt_shell_noninteractive_quit(EXIT_FAILURE);
}
@@ -1056,12 +1056,12 @@ static void cmd_hb_pub_set(int argc, char *argv[])
/* Period Log */
msg[n++] = parms[2];
/* Heartbeat TTL */
- msg[n++] = DEFAULT_TTL;
+ msg[n++] = parms[3];
/* Features */
- put_le16(parms[3], msg + n);
+ put_le16(parms[4], msg + n);
n += 2;
/* NetKey Index */
- put_le16(parms[4], msg + n);
+ put_le16(parms[5], msg + n);
n += 2;

if (!config_send(msg, n)) {
@@ -1167,8 +1167,9 @@ static const struct bt_shell_menu cfg_menu = {
"Set relay"},
{"relay-get", NULL, cmd_relay_get,
"Get relay"},
- {"hb-pub-set", "<pub_addr> <count> <period> <features> <net_idx>",
- cmd_hb_pub_set, "Set heartbeat publish"},
+ {"hb-pub-set", "<pub_addr> <count> <period> <ttl> <features> "
+ "<net_idx>",
+ cmd_hb_pub_set, "\n\t\t\t\t\t\t Set heartbeat publish"},
{"hb-pub-get", NULL, cmd_hb_pub_get,
"Get heartbeat publish"},
{"hb-sub-set", "<src_addr> <dst_addr> <period>",
--
2.11.0



2018-03-22 05:59:16

by Robert Lubaś

[permalink] [raw]
Subject: Re: [PATCH BlueZ] Mesh: Fix TTL in Config Heartbeat Publication Set

Hi Luiz,

Sorry, I have another thing in my my mind. DEFAULT_TTL was set to 0xFF
(not 0x77)
and this is out of valid range(my device does not acknowledge the message)=
.
I will follow with V2.

About the extra formatting - I just took as a example the pub-set command.

with extra formatting config help looks like :
relay-get Get relay
hb-pub-set <pub_addr> <count> <period> <ttl> <features> <net_idx>
Set heartbeat publish
hb-pub-get Get heartbeat publish

without extra formatting:
relay-get Get relay
hb-pub-set <pub_addr> <count> <period> <ttl> <features> <net_idx> Set
heartbeat publish
hb-pub-get Get heartbeat publish

What do you think?

Robert

2018-03-21 19:39 GMT+01:00 Luiz Augusto von Dentz <[email protected]>:
> Hi Robert,
>
> On Wed, Mar 21, 2018 at 6:59 PM, Robert Luba=C5=9B <robert.lubas@silvair.=
com> wrote:
>> In 4.2.17.4 Heartbeat Publication TTL value range is 0x00-0x7F.
>> In cmd_hb_pub_set heartbeat ttl was set to DEFAULT_TTL 0x77, this
>> patch fix this by adding ttl param to hb-pub-set.
>
> I don't follow, is 0x77 not valid? It should be if the range is really
> 0x00-0x7F.
>
>> ---
>> mesh/config-client.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/mesh/config-client.c b/mesh/config-client.c
>> index 19e617d62..9757e2625 100644
>> --- a/mesh/config-client.c
>> +++ b/mesh/config-client.c
>> @@ -1042,7 +1042,7 @@ static void cmd_hb_pub_set(int argc, char *argv[])
>> n =3D mesh_opcode_set(OP_CONFIG_HEARTBEAT_PUB_SET, msg);
>>
>> parm_cnt =3D read_input_parameters(argc, argv);
>> - if (parm_cnt !=3D 5) {
>> + if (parm_cnt !=3D 6) {
>> bt_shell_printf("Bad arguments: %s\n", argv[1]);
>> return bt_shell_noninteractive_quit(EXIT_FAILURE);
>> }
>> @@ -1056,12 +1056,12 @@ static void cmd_hb_pub_set(int argc, char *argv[=
])
>> /* Period Log */
>> msg[n++] =3D parms[2];
>> /* Heartbeat TTL */
>> - msg[n++] =3D DEFAULT_TTL;
>> + msg[n++] =3D parms[3];
>> /* Features */
>> - put_le16(parms[3], msg + n);
>> + put_le16(parms[4], msg + n);
>> n +=3D 2;
>> /* NetKey Index */
>> - put_le16(parms[4], msg + n);
>> + put_le16(parms[5], msg + n);
>> n +=3D 2;
>>
>> if (!config_send(msg, n)) {
>> @@ -1167,8 +1167,9 @@ static const struct bt_shell_menu cfg_menu =3D {
>> "Set relay"},
>> {"relay-get", NULL, cmd_relay_get,
>> "Get relay"},
>> - {"hb-pub-set", "<pub_addr> <count> <period> <features> <net_idx>=
",
>> - cmd_hb_pub_set, "Set heartbeat publi=
sh"},
>> + {"hb-pub-set", "<pub_addr> <count> <period> <ttl> <features> "
>> + "<net_idx>",
>> + cmd_hb_pub_set, "\n\t\t\t\t\t\t Set hea=
rtbeat publish"},
>
> I don't really like the idea of having extra formatting on the command
> description, bt_shell should do the right thing and if that doesn't
> fit in the terminal it should wrap around automatically.
>
>> {"hb-pub-get", NULL, cmd_hb_pub_get,
>> "Get heartbeat publish"}=
,
>> {"hb-sub-set", "<src_addr> <dst_addr> <period>",
>> --
>> 2.11.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetoot=
h" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz



--=20
Robert Luba=C5=9B
Software Developer

Silvair by Seed Labs
Jasnog=C3=B3rska 44
31-358 Krakow
POLAND

http://www.silvair.com

2018-03-21 18:39:02

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] Mesh: Fix TTL in Config Heartbeat Publication Set

Hi Robert,

On Wed, Mar 21, 2018 at 6:59 PM, Robert Luba=C5=9B <[email protected]=
m> wrote:
> In 4.2.17.4 Heartbeat Publication TTL value range is 0x00-0x7F.
> In cmd_hb_pub_set heartbeat ttl was set to DEFAULT_TTL 0x77, this
> patch fix this by adding ttl param to hb-pub-set.

I don't follow, is 0x77 not valid? It should be if the range is really
0x00-0x7F.

> ---
> mesh/config-client.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/mesh/config-client.c b/mesh/config-client.c
> index 19e617d62..9757e2625 100644
> --- a/mesh/config-client.c
> +++ b/mesh/config-client.c
> @@ -1042,7 +1042,7 @@ static void cmd_hb_pub_set(int argc, char *argv[])
> n =3D mesh_opcode_set(OP_CONFIG_HEARTBEAT_PUB_SET, msg);
>
> parm_cnt =3D read_input_parameters(argc, argv);
> - if (parm_cnt !=3D 5) {
> + if (parm_cnt !=3D 6) {
> bt_shell_printf("Bad arguments: %s\n", argv[1]);
> return bt_shell_noninteractive_quit(EXIT_FAILURE);
> }
> @@ -1056,12 +1056,12 @@ static void cmd_hb_pub_set(int argc, char *argv[]=
)
> /* Period Log */
> msg[n++] =3D parms[2];
> /* Heartbeat TTL */
> - msg[n++] =3D DEFAULT_TTL;
> + msg[n++] =3D parms[3];
> /* Features */
> - put_le16(parms[3], msg + n);
> + put_le16(parms[4], msg + n);
> n +=3D 2;
> /* NetKey Index */
> - put_le16(parms[4], msg + n);
> + put_le16(parms[5], msg + n);
> n +=3D 2;
>
> if (!config_send(msg, n)) {
> @@ -1167,8 +1167,9 @@ static const struct bt_shell_menu cfg_menu =3D {
> "Set relay"},
> {"relay-get", NULL, cmd_relay_get,
> "Get relay"},
> - {"hb-pub-set", "<pub_addr> <count> <period> <features> <net_idx>"=
,
> - cmd_hb_pub_set, "Set heartbeat publis=
h"},
> + {"hb-pub-set", "<pub_addr> <count> <period> <ttl> <features> "
> + "<net_idx>",
> + cmd_hb_pub_set, "\n\t\t\t\t\t\t Set hear=
tbeat publish"},

I don't really like the idea of having extra formatting on the command
description, bt_shell should do the right thing and if that doesn't
fit in the terminal it should wrap around automatically.

> {"hb-pub-get", NULL, cmd_hb_pub_get,
> "Get heartbeat publish"},
> {"hb-sub-set", "<src_addr> <dst_addr> <period>",
> --
> 2.11.0
>
> --
> 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



--=20
Luiz Augusto von Dentz