2020-07-07 16:45:26

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ 1/2] mesh: Get rid of "unreliable opcodes" in config server

This removes an old notion of unreliable opcodes in config server
model , i.e., a correctly formatted acknowledged message always
gets a response.
---
mesh/cfgmod-server.c | 31 +++++++++++--------------------
mesh/model.h | 2 --
2 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index c525d9d24..8ba9bc6ec 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -98,9 +98,8 @@ static void config_pub_get(struct mesh_node *node, uint16_t net_idx,
}

static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
- uint16_t src, uint16_t dst,
- const uint8_t *pkt, bool virt,
- bool vendor, bool unreliable)
+ uint16_t src, uint16_t dst,
+ const uint8_t *pkt, bool virt, bool vendor)
{
uint32_t mod_id;
uint16_t ele_addr, idx, ota = UNASSIGNED_ADDRESS;
@@ -143,9 +142,8 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
status, ele_addr, ota, mod_id, idx);

if (status != MESH_STATUS_SUCCESS) {
- if (!unreliable)
- send_pub_status(node, net_idx, src, dst, status,
- ele_addr, mod_id, 0, 0, 0, 0, 0, 0);
+ send_pub_status(node, net_idx, src, dst, status, ele_addr,
+ mod_id, 0, 0, 0, 0, 0, 0);

return;
}
@@ -180,10 +178,8 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
status = MESH_STATUS_STORAGE_FAIL;
}

- if (!unreliable)
- send_pub_status(node, net_idx, src, dst, status, ele_addr,
- mod_id, ota, idx, cred_flag, ttl,
- period, retransmit);
+ send_pub_status(node, net_idx, src, dst, status, ele_addr, mod_id, ota,
+ idx, cred_flag, ttl, period, retransmit);
}

static void send_sub_status(struct mesh_node *node, uint16_t net_idx,
@@ -311,7 +307,6 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
bool virt, uint32_t opcode)
{
uint16_t grp, ele_addr;
- bool unreliable = !!(opcode & OP_UNRELIABLE);
uint32_t mod_id;
const uint8_t *addr = NULL;
int status = MESH_STATUS_SUCCESS;
@@ -369,7 +364,7 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
} else
grp = UNASSIGNED_ADDRESS;

- switch (opcode & ~OP_UNRELIABLE) {
+ switch (opcode) {
default:
l_debug("Bad opcode: %x", opcode);
return;
@@ -411,8 +406,8 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
grp = UNASSIGNED_ADDRESS;
/* Fall Through */
case OP_CONFIG_MODEL_SUB_DELETE:
- status = mesh_model_sub_del(node, ele_addr, mod_id,
- addr, virt, &grp);
+ status = mesh_model_sub_del(node, ele_addr, mod_id, addr, virt,
+ &grp);

if (status == MESH_STATUS_SUCCESS)
save_config_sub(node, ele_addr, mod_id, vendor, addr,
@@ -421,10 +416,7 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
break;
}

- if (!unreliable)
- send_sub_status(node, net_idx, src, dst, status, ele_addr,
- grp, mod_id);
-
+ send_sub_status(node, net_idx, src, dst, status, ele_addr, grp, mod_id);
}

static void send_model_app_status(struct mesh_node *node, uint16_t net_idx,
@@ -786,8 +778,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
return true;

config_pub_set(node, net_idx, src, dst, pkt, virt,
- size == 13 || size == 27,
- !!(opcode & OP_UNRELIABLE));
+ size == 13 || size == 27);
break;

case OP_CONFIG_MODEL_PUB_GET:
diff --git a/mesh/model.h b/mesh/model.h
index f717fb00c..0377d3fdd 100644
--- a/mesh/model.h
+++ b/mesh/model.h
@@ -19,8 +19,6 @@

struct mesh_model;

-#define OP_UNRELIABLE 0x0100
-
#define MAX_BINDINGS 10
#define MAX_GRP_PER_MOD 10

--
2.26.2


2020-07-07 16:47:18

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ 2/2] tools/mesh: Get rid of "unreliable opcodes" in models

This removes an old notion of unreliable opcodes in models , i.e.,
a correctly formatted acknowledged message always gets a response.
---
tools/mesh-gatt/config-client.c | 2 +-
tools/mesh-gatt/config-server.c | 2 +-
tools/mesh-gatt/onoff-model.c | 2 +-
tools/mesh-gatt/util.h | 2 --
tools/mesh/cfgcli.c | 4 ++--
tools/mesh/model.h | 1 -
6 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/mesh-gatt/config-client.c b/tools/mesh-gatt/config-client.c
index ed31c67d9..bfc788258 100644
--- a/tools/mesh-gatt/config-client.c
+++ b/tools/mesh-gatt/config-client.c
@@ -100,7 +100,7 @@ static bool client_msg_recvd(uint16_t src, uint8_t *data,
if (primary != src)
return false;

- switch (opcode & ~OP_UNRELIABLE) {
+ switch (opcode) {
default:
return false;

diff --git a/tools/mesh-gatt/config-server.c b/tools/mesh-gatt/config-server.c
index 8fc6edcc0..9e9b93959 100644
--- a/tools/mesh-gatt/config-server.c
+++ b/tools/mesh-gatt/config-server.c
@@ -73,7 +73,7 @@ static bool server_msg_recvd(uint16_t src, uint8_t *data,

n = 0;

- switch (opcode & ~OP_UNRELIABLE) {
+ switch (opcode) {
default:
return false;

diff --git a/tools/mesh-gatt/onoff-model.c b/tools/mesh-gatt/onoff-model.c
index 92c9a3105..be519c969 100644
--- a/tools/mesh-gatt/onoff-model.c
+++ b/tools/mesh-gatt/onoff-model.c
@@ -123,7 +123,7 @@ static bool client_msg_recvd(uint16_t src, uint8_t *data,
len, opcode);
print_byte_array("\t",data, len);

- switch (opcode & ~OP_UNRELIABLE) {
+ switch (opcode) {
default:
return false;

diff --git a/tools/mesh-gatt/util.h b/tools/mesh-gatt/util.h
index c3facfa73..dba2c480b 100644
--- a/tools/mesh-gatt/util.h
+++ b/tools/mesh-gatt/util.h
@@ -25,8 +25,6 @@

struct mesh_publication;

-#define OP_UNRELIABLE 0x0100
-
void set_menu_prompt(const char *name, const char *id);
void print_byte_array(const char *prefix, const void *ptr, int len);
bool str2hex(const char *str, uint16_t in_len, uint8_t *out_buf,
diff --git a/tools/mesh/cfgcli.c b/tools/mesh/cfgcli.c
index 218e82c50..e36c8dca5 100644
--- a/tools/mesh/cfgcli.c
+++ b/tools/mesh/cfgcli.c
@@ -416,7 +416,7 @@ static bool msg_recvd(uint16_t src, uint16_t idx, uint8_t *data,

bt_shell_printf("Received %s (len %u)\n", opcode_str(opcode), len);

- req = get_req_by_rsp(src, (opcode & ~OP_UNRELIABLE));
+ req = get_req_by_rsp(src, opcode);
if (req) {
cmd = req->cmd;
free_request(req);
@@ -424,7 +424,7 @@ static bool msg_recvd(uint16_t src, uint16_t idx, uint8_t *data,
} else
cmd = NULL;

- switch (opcode & ~OP_UNRELIABLE) {
+ switch (opcode) {
default:
return false;

diff --git a/tools/mesh/model.h b/tools/mesh/model.h
index 449fe19b2..35bb80efc 100644
--- a/tools/mesh/model.h
+++ b/tools/mesh/model.h
@@ -18,7 +18,6 @@
*
*/

-#define OP_UNRELIABLE 0x0100
#define VENDOR_ID_INVALID 0xFFFF

typedef bool (*model_send_msg_func_t) (void *user_data, uint16_t dst,
--
2.26.2

2020-07-08 17:57:21

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/2] mesh: Get rid of "unreliable opcodes" in config server

Patchset Applied

On Tue, 2020-07-07 at 09:44 -0700, Inga Stotland wrote:
> This removes an old notion of unreliable opcodes in config server
> model , i.e., a correctly formatted acknowledged message always
> gets a response.
> ---
> mesh/cfgmod-server.c | 31 +++++++++++--------------------
> mesh/model.h | 2 --
> 2 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
> index c525d9d24..8ba9bc6ec 100644
> --- a/mesh/cfgmod-server.c
> +++ b/mesh/cfgmod-server.c
> @@ -98,9 +98,8 @@ static void config_pub_get(struct mesh_node *node, uint16_t net_idx,
> }
>
> static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
> - uint16_t src, uint16_t dst,
> - const uint8_t *pkt, bool virt,
> - bool vendor, bool unreliable)
> + uint16_t src, uint16_t dst,
> + const uint8_t *pkt, bool virt, bool vendor)
> {
> uint32_t mod_id;
> uint16_t ele_addr, idx, ota = UNASSIGNED_ADDRESS;
> @@ -143,9 +142,8 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
> status, ele_addr, ota, mod_id, idx);
>
> if (status != MESH_STATUS_SUCCESS) {
> - if (!unreliable)
> - send_pub_status(node, net_idx, src, dst, status,
> - ele_addr, mod_id, 0, 0, 0, 0, 0, 0);
> + send_pub_status(node, net_idx, src, dst, status, ele_addr,
> + mod_id, 0, 0, 0, 0, 0, 0);
>
> return;
> }
> @@ -180,10 +178,8 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
> status = MESH_STATUS_STORAGE_FAIL;
> }
>
> - if (!unreliable)
> - send_pub_status(node, net_idx, src, dst, status, ele_addr,
> - mod_id, ota, idx, cred_flag, ttl,
> - period, retransmit);
> + send_pub_status(node, net_idx, src, dst, status, ele_addr, mod_id, ota,
> + idx, cred_flag, ttl, period, retransmit);
> }
>
> static void send_sub_status(struct mesh_node *node, uint16_t net_idx,
> @@ -311,7 +307,6 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
> bool virt, uint32_t opcode)
> {
> uint16_t grp, ele_addr;
> - bool unreliable = !!(opcode & OP_UNRELIABLE);
> uint32_t mod_id;
> const uint8_t *addr = NULL;
> int status = MESH_STATUS_SUCCESS;
> @@ -369,7 +364,7 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
> } else
> grp = UNASSIGNED_ADDRESS;
>
> - switch (opcode & ~OP_UNRELIABLE) {
> + switch (opcode) {
> default:
> l_debug("Bad opcode: %x", opcode);
> return;
> @@ -411,8 +406,8 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
> grp = UNASSIGNED_ADDRESS;
> /* Fall Through */
> case OP_CONFIG_MODEL_SUB_DELETE:
> - status = mesh_model_sub_del(node, ele_addr, mod_id,
> - addr, virt, &grp);
> + status = mesh_model_sub_del(node, ele_addr, mod_id, addr, virt,
> + &grp);
>
> if (status == MESH_STATUS_SUCCESS)
> save_config_sub(node, ele_addr, mod_id, vendor, addr,
> @@ -421,10 +416,7 @@ static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
> break;
> }
>
> - if (!unreliable)
> - send_sub_status(node, net_idx, src, dst, status, ele_addr,
> - grp, mod_id);
> -
> + send_sub_status(node, net_idx, src, dst, status, ele_addr, grp, mod_id);
> }
>
> static void send_model_app_status(struct mesh_node *node, uint16_t net_idx,
> @@ -786,8 +778,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
> return true;
>
> config_pub_set(node, net_idx, src, dst, pkt, virt,
> - size == 13 || size == 27,
> - !!(opcode & OP_UNRELIABLE));
> + size == 13 || size == 27);
> break;
>
> case OP_CONFIG_MODEL_PUB_GET:
> diff --git a/mesh/model.h b/mesh/model.h
> index f717fb00c..0377d3fdd 100644
> --- a/mesh/model.h
> +++ b/mesh/model.h
> @@ -19,8 +19,6 @@
>
> struct mesh_model;
>
> -#define OP_UNRELIABLE 0x0100
> -
> #define MAX_BINDINGS 10
> #define MAX_GRP_PER_MOD 10
>