From: "Stotland, Inga" <inga.stotland@intel.com>
To: "sbrown@cortland.com" <sbrown@cortland.com>,
"linux-bluetooth@vger.kernel.org"
<linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH V2 1/8] mesh: meshctl: Change command names to <cmd>-<get/set>
Date: Fri, 15 Dec 2017 02:14:49 +0000 [thread overview]
Message-ID: <1513304088.3023.39.camel@intel.com> (raw)
In-Reply-To: <1513242768.11292.10.camel@ewol.com>
[-- Attachment #1: Type: text/plain, Size: 8132 bytes --]
Hi Steve,
On Thu, 2017-12-14 at 02:12 -0700, Steve Brown wrote:
> Hi Inga,
> On Thu, 2017-12-14 at 08:08 +0000, Stotland, Inga wrote:
> > Hi Steve,
> >
> > On Tue, 2017-12-12 at 12:58 +0000, sbrown@cortland.com wrote:
> > > From: Steve Brown <sbrown@cortland.com>
> > >
> > > Fix lines over 80 chars
> > > Move cmd_default()
> > > Add parameter to pub-set to control retransmit count
> > > ---
> > > mesh/config-client.c | 78 +++++++++++++++++++++++++++-----------
> > > --
> > > --
> > > ----------
> > > 1 file changed, 41 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/mesh/config-client.c b/mesh/config-client.c
> > > index 3d618b6a6..51adf2c52 100644
> > > --- a/mesh/config-client.c
> > > +++ b/mesh/config-client.c
> > > @@ -170,9 +170,9 @@ static bool client_msg_recvd(uint16_t src,
> > > uint8_t *data,
> > > if (len != 12 && len != 14)
> > > return true;
> > >
> > > - bt_shell_printf("\nSet publication for node
> > > %4.4x
> > > status: %s\n", src,
> > > - data[0] == MESH_STATUS_SUCCESS ?
> > > "Success" :
> > > - mesh_status_str(
> > > da
> > > ta
> > > [0]));
> > > + bt_shell_printf("\nSet publication for node
> > > %4.4x
> > > status: %s\n",
> > > + src, data[0] ==
> > > MESH_STATUS_SUCCESS
> > > ?
> > > + "Success" :
> > > mesh_status_str(data[0]));
> > >
> > > if (data[0] != MESH_STATUS_SUCCESS)
> > > return true;
> > > @@ -189,6 +189,7 @@ static bool client_msg_recvd(uint16_t src,
> > > uint8_t *data,
> > > pub.ttl = data[7];
> > > pub.period = data[8];
> > > n = (data[8] & 0x3f);
> > > + bt_shell_printf("Publication address: 0x%04x\n",
> > > pub.u.addr16);
> > > switch (data[8] >> 6) {
> > > case 0:
> > > bt_shell_printf("Period: %d ms\n", n *
> > > 100);
> > > @@ -206,7 +207,8 @@ static bool client_msg_recvd(uint16_t src,
> > > uint8_t *data,
> > >
> > > pub.retransmit = data[9];
> > > bt_shell_printf("Retransmit count: %d\n",
> > > data[9]
> > > > >
> > >
> > > 5);
> > > - bt_shell_printf("Retransmit Interval Steps:
> > > %d\n",
> > > data[9] & 0x1f);
> > > + bt_shell_printf("Retransmit Interval Steps:
> > > %d\n",
> > > + data[9] & 0x1f);
> > >
> > > ele_idx = ele_addr - node_get_primary(node);
> > >
> > > @@ -219,6 +221,7 @@ static bool client_msg_recvd(uint16_t src,
> > > uint8_t *data,
> > > node_model_pub_get(node,
> > > ele_idx, mod_id));
> > > break;
> > > }
> > > +
> > > return true;
> > > }
> > >
> > > @@ -287,6 +290,23 @@ static bool config_send(uint8_t *buf,
> > > uint16_t
> > > len)
> > >
> > > }
> > >
> > > +static void cmd_default(uint32_t opcode)
> > > +{
> > > + uint16_t n;
> > > + uint8_t msg[32];
> > > +
> > > + if (IS_UNASSIGNED(target)) {
> > > + bt_shell_printf("Destination not set\n");
> > > + return;
> > > + }
> > > +
> > > + n = mesh_opcode_set(opcode, msg);
> > > +
> > > + if (!config_send(msg, n))
> > > + bt_shell_printf("Failed to send command (opcode
> > > 0x%x)\n",
> > > +
> > > op
> > > co
> > > de);
> > > +}
> > > +
> > > static void cmd_get_composition(int argc, char *argv[])
> > > {
> > > uint16_t n;
> > > @@ -556,7 +576,7 @@ static void cmd_set_pub(int argc, char
> > > *argv[])
> > > n = mesh_opcode_set(OP_CONFIG_MODEL_PUB_SET, msg);
> > >
> > > parm_cnt = read_input_parameters(argc, argv);
> > > - if (parm_cnt != 5) {
> > > + if (parm_cnt != 6) {
> > > bt_shell_printf("Bad arguments\n");
> > > return;
> > > }
> > > @@ -574,14 +594,14 @@ static void cmd_set_pub(int argc, char
> > > *argv[])
> > > /* Publish period step count and step resolution */
> > > msg[n++] = parms[3];
> > > /* Publish retransmit count & interval steps */
> > > - msg[n++] = (1 << 5) + 2;
> > > + msg[n++] = parms[4];
> > > /* Model Id */
> > > - if (parms[4] > 0xffff) {
> > > - put_le16(parms[4] >> 16, msg + n);
> > > - put_le16(parms[4], msg + n + 2);
> > > + if (parms[5] > 0xffff) {
> > > + put_le16(parms[5] >> 16, msg + n);
> > > + put_le16(parms[5], msg + n + 2);
> > > n += 4;
> > > } else {
> > > - put_le16(parms[4], msg + n);
> > > + put_le16(parms[5], msg + n);
> > > n += 2;
> > > }
> > >
> > > @@ -589,23 +609,6 @@ static void cmd_set_pub(int argc, char
> > > *argv[])
> > > bt_shell_printf("Failed to send \"SET MODEL
> > > PUBLICATION\"\n");
> > > }
> > >
> > > -static void cmd_default(uint32_t opcode)
> > > -{
> > > - uint16_t n;
> > > - uint8_t msg[32];
> > > -
> > > - if (IS_UNASSIGNED(target)) {
> > > - bt_shell_printf("Destination not set\n");
> > > - return;
> > > - }
> > > -
> > > - n = mesh_opcode_set(opcode, msg);
> > > -
> > > - if (!config_send(msg, n))
> > > - bt_shell_printf("Failed to send command (opcode
> > > 0x%x)\n",
> > > -
> > > op
> > > co
> > > de);
> > > -}
> > > -
> > > static void cmd_get_ttl(int argc, char *argv[])
> > > {
> > > cmd_default(OP_CONFIG_DEFAULT_TTL_GET);
> > > @@ -614,27 +617,28 @@ static void cmd_get_ttl(int argc, char
> > > *argv[])
> > > static const struct bt_shell_menu cfg_menu = {
> > > .name = "config",
> > > .entries = {
> > > - {"target", "<unicast>",
> > > cmd_set_node,
> > > + {"target", "<unicast>", cm
> > > d_
> > > se
> > > t_node,
> > > "Set target node
> > > to
> > > configure"},
> > > - {"get-composition", "[<page_num>]",
> > > cm
> > > d_get_composition,
> > > + {"composition-get", "[<page_num>]",
> > > cm
> > > d_get_composition,
> > > "Get Composition
> > > Data"},
> > > - {"add-netkey", "<net_idx>",
> > >
> > > cmd_add_net_key,
> > > + {"netkey-add", "<net_idx>",
> > > cm
> > > d_add_net_key,
> > > "Add network
> > > key"},
> > > - {"del-netkey", "<net_idx>",
> > >
> > > cmd_del_net_key,
> > > + {"netkey-del", "<net_idx>",
> > > cm
> > > d_del_net_key,
> > > "Delete network
> > > key"},
> > > - {"add-appkey", "<app_idx>",
> > >
> > > cmd_add_app_key,
> > > + {"appkey-add", "<app_idx>",
> > > cm
> > > d_add_app_key,
> > > "Add application
> > > key"},
> > > - {"del-appkey", "<app_idx>",
> > >
> > > cmd_del_app_key,
> > > + {"appkey-del", "<app_idx>",
> > > cm
> > > d_del_app_key,
> > > "Delete
> > > application
> > > key"},
> > > {"bind", "<ele_idx> <app_idx> <mod_id>
> > > [cid]",
> > > cmd_bind, "Bind app key
> > > to
> > > a
> > > model"},
> > > - {"set-ttl", "<ttl>",
> > > c
> > > md_set_ttl,
> > > + {"ttl-set", "<ttl>", cmd_s
> > > et
> > > _t
> > > tl,
> > > "Set default
> > > TTL"},
> > > - {"get-ttl", NULL,
> > > cm
> > > d_
> > > get_ttl,
> > > + {"ttl-get", NULL,
> > > cm
> > > d_
> > > get_ttl,
> > > "Get default
> > > TTL"},
> > > - {"set-pub", "<ele_addr> <pub_addr> <app_idx> "
> > > - "<period
> > > (step|res)>
> > > <model>",
> > > + {"pub-set", "<ele_addr> <pub_addr> <app_idx> "
> > > + "<period (step|res)> <re-xmt
> > > (count|per)>
> > > <model>",
> > > cmd_set_pub, "Set
> > > publication"},
> > > +
> > > {} },
> > > };
> > >
> >
> > Since you are modifying pub-set command, could you please fix it to
> > correctly indicate SIG and vendor models, similarly to "bind"
> > command
> > (adding an optional "cid" parameter for vendor models).
> >
> > Regards,
> >
> > Inga Stotland
>
> It looks like the pub command already assumes it's a vendor model if
> the model id is > 0xffff. Is that a correct assumption?
>
> If it is, should I make the same change to bind and remove the
> optional
> parameter?
>
> Steve
>
>
>
>
> --
> 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
The check for model id is > 0xffff is not entirely correct since it
does not account for a case when CID is 0x0000.
Regards,
Inga
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3266 bytes --]
next prev parent reply other threads:[~2017-12-15 2:14 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-12 12:58 [PATCH V2 0/8] mesh: Add configuration commands to meshctl sbrown
2017-12-12 12:58 ` [PATCH V2 1/8] mesh: meshctl: Change command names to <cmd>-<get/set> sbrown
2017-12-14 8:08 ` Stotland, Inga
2017-12-14 9:12 ` Steve Brown
2017-12-15 2:14 ` Stotland, Inga [this message]
2017-12-15 6:57 ` Steve Brown
2017-12-12 12:58 ` [PATCH V2 2/8] mesh: meshctl: Add add/get Subscribe sbrown
2017-12-12 12:58 ` [PATCH V2 3/8] mesh: meshctl: Add set heartbeat command sbrown
2017-12-14 8:20 ` Stotland, Inga
2017-12-12 12:58 ` [PATCH V2 4/8] mesh: meshctl: Add get app keys command sbrown
2017-12-12 12:58 ` [PATCH V2 5/8] mesh: meshctl: Add get publish command sbrown
2017-12-12 12:58 ` [PATCH V2 6/8] mesh: meshctl: Add set/get proxy command sbrown
2017-12-12 12:58 ` [PATCH V2 7/8] mesh: meshctl: Add get/set identity sbrown
2017-12-12 12:58 ` [PATCH V2 8/8] mesh: meshctl: Add get/set relay command sbrown
2017-12-13 12:27 ` [PATCH V2 0/8] mesh: Add configuration commands to meshctl Luiz Augusto von Dentz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1513304088.3023.39.camel@intel.com \
--to=inga.stotland@intel.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=sbrown@cortland.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.