Hi Robert,
On Thu, Mar 22, 2018 at 7:54 AM, Robert Lubaś <robert.lubas@xxxxxxxxxxx> wrote:
> 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?
Lets please keep the comments inline instead of top-posting, as for
the formatting Id say we should fix this in bt_shell so it actually
detects if the line should be broken or not. We may as well truncate
if that doesn't fit into the terminal, but that should be done generic
so the commands don't need to come up with their own way of
formatting.
> 2018-03-21 19:39 GMT+01:00 Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>:
>>
>> Hi Robert,
>>
>> On Wed, Mar 21, 2018 at 6:59 PM, Robert Lubaś <robert.lubas@xxxxxxxxxxx>
>> 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 = 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"},
>>
>> 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 majordomo@xxxxxxxxxxxxxxx
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
>
>
> --
> Robert Lubaś
> Software Developer
>
> Silvair by Seed Labs
> Jasnogórska 44
> 31-358 Krakow
> POLAND
>
> www.silvair.com
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
