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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).