Return-Path: From: Steve Brown Message-ID: <1513242768.11292.10.camel@ewol.com> Subject: Re: [PATCH V2 1/8] mesh: meshctl: Change command names to - To: "Stotland, Inga" , "sbrown@cortland.com" , "linux-bluetooth@vger.kernel.org" Date: Thu, 14 Dec 2017 02:12:48 -0700 In-Reply-To: <1513238880.3023.18.camel@intel.com> References: <20171212125832.13440-1-sbrown@cortland.com> <20171212125832.13440-2-sbrown@cortland.com> <1513238880.3023.18.camel@intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: 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 > > > > 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", "", > > cmd_set_node, > > + {"target", "", cmd_ > > se > > t_node, > > "Set target node > > to > > configure"}, > > - {"get-composition", "[]", > > cm > > d_get_composition, > > + {"composition-get", "[]", > > cm > > d_get_composition, > > "Get Composition > > Data"}, > > - {"add-netkey", "", > > cmd_add_net_key, > > + {"netkey-add", "", > > cm > > d_add_net_key, > > "Add network > > key"}, > > - {"del-netkey", "", > > cmd_del_net_key, > > + {"netkey-del", "", > > cm > > d_del_net_key, > > "Delete network > > key"}, > > - {"add-appkey", "", > > cmd_add_app_key, > > + {"appkey-add", "", > > cm > > d_add_app_key, > > "Add application > > key"}, > > - {"del-appkey", "", > > cmd_del_app_key, > > + {"appkey-del", "", > > cm > > d_del_app_key, > > "Delete > > application > > key"}, > > {"bind", " > > [cid]", > > cmd_bind, "Bind app key to > > a > > model"}, > > - {"set-ttl", "", > > c > > md_set_ttl, > > + {"ttl-set", "", cmd_set > > _t > > tl, > > "Set default > > TTL"}, > > - {"get-ttl", NULL, cm > > d_ > > get_ttl, > > + {"ttl-get", NULL, cm > > d_ > > get_ttl, > > "Get default > > TTL"}, > > - {"set-pub", " " > > - " > (step|res)> > > ", > > + {"pub-set", " " > > + "

> > ", > > 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