From 3dd8190cb5a9aa019445d66a522fcc040bb1a9e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Narajowski?= Date: Mon, 20 Aug 2018 16:05:28 +0200 Subject: [PATCH 1/6] mesh: Fix missing semicolon for debug log Without this it's not possible to build cfg_srv.c with debug logs enabled. --- nimble/host/mesh/src/cfg_srv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimble/host/mesh/src/cfg_srv.c b/nimble/host/mesh/src/cfg_srv.c index 94fccb339..97b939703 100644 --- a/nimble/host/mesh/src/cfg_srv.c +++ b/nimble/host/mesh/src/cfg_srv.c @@ -1013,7 +1013,7 @@ static void relay_set(struct bt_mesh_model *model, cfg->relay, change ? "changed" : "not changed", cfg->relay_retransmit, BT_MESH_TRANSMIT_COUNT(cfg->relay_retransmit), - BT_MESH_TRANSMIT_INT(cfg->relay_retransmit)) + BT_MESH_TRANSMIT_INT(cfg->relay_retransmit)); sub = bt_mesh_subnet_get(cfg->hb_pub.net_idx); if ((cfg->hb_pub.feat & BT_MESH_FEAT_RELAY) && sub && change) { From 677152bc7e4122a5671938088a571d5a9210c4c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Narajowski?= Date: Mon, 20 Aug 2018 16:48:33 +0200 Subject: [PATCH 2/6] mesh: Improve outgoing segment count configuration The Mesh specification doesn't support more than 32 transport layer segments, the way the number was so far derived from the advertising buffer count could result in a highre numbe than 32, thereby wasting memory. Make the number of supported segments build-time configurable through a new BT_MESH_TX_SEG_MAX configuration option. --- apps/blemesh/syscfg.yml | 3 +++ apps/blemesh_light/syscfg.yml | 4 ++- apps/blemesh_shell/syscfg.yml | 3 +++ nimble/host/mesh/include/mesh/glue.h | 3 +++ nimble/host/mesh/src/transport.c | 9 ++++++- nimble/host/mesh/src/transport.h | 3 +-- nimble/host/mesh/syscfg.yml | 37 ++++++++++++++++++++++------ 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/apps/blemesh/syscfg.yml b/apps/blemesh/syscfg.yml index b8bdb6512..e17e32c7f 100644 --- a/apps/blemesh/syscfg.yml +++ b/apps/blemesh/syscfg.yml @@ -34,6 +34,9 @@ syscfg.vals: BLE_MESH: 1 MSYS_1_BLOCK_COUNT: 48 + BLE_MESH_ADV_BUF_COUNT: 20 + BLE_MESH_TX_SEG_MAX: 6 + BLE_MESH_DEBUG: 1 BLE_MESH_DEBUG_NET: 1 BLE_MESH_DEBUG_TRANS: 1 diff --git a/apps/blemesh_light/syscfg.yml b/apps/blemesh_light/syscfg.yml index 556a844ee..8f2c1ec4c 100644 --- a/apps/blemesh_light/syscfg.yml +++ b/apps/blemesh_light/syscfg.yml @@ -37,6 +37,9 @@ syscfg.vals: MSYS_1_BLOCK_COUNT: 80 + BLE_MESH_ADV_BUF_COUNT: 20 + BLE_MESH_TX_SEG_MAX: 6 + BLE_MESH: 1 BLE_MESH_SHELL: 1 BLE_MESH_PROV: 1 @@ -48,7 +51,6 @@ syscfg.vals: BLE_MESH_CFG_CLI: 1 BLE_MESH_HEALTH_CLI: 0 BLE_MESH_SHELL_MODELS: 1 - BLE_MESH_TESTING: 1 BLE_MESH_OOB_OUTPUT_ACTIONS: 0 BLE_MESH_SETTINGS: 0 CONFIG_NFFS: 0 diff --git a/apps/blemesh_shell/syscfg.yml b/apps/blemesh_shell/syscfg.yml index 227359c52..b08fa29fb 100644 --- a/apps/blemesh_shell/syscfg.yml +++ b/apps/blemesh_shell/syscfg.yml @@ -33,6 +33,9 @@ syscfg.vals: MSYS_1_BLOCK_COUNT: 80 + BLE_MESH_ADV_BUF_COUNT: 20 + BLE_MESH_TX_SEG_MAX: 6 + BLE_MESH: 1 BLE_MESH_SHELL: 1 BLE_MESH_PROV: 1 diff --git a/nimble/host/mesh/include/mesh/glue.h b/nimble/host/mesh/include/mesh/glue.h index b458ffebf..d330ecbc5 100644 --- a/nimble/host/mesh/include/mesh/glue.h +++ b/nimble/host/mesh/include/mesh/glue.h @@ -373,6 +373,7 @@ static inline unsigned int find_msb_set(u32_t op) #define CONFIG_BT_MESH_STORE_TIMEOUT MYNEWT_VAL(BLE_MESH_STORE_TIMEOUT) #define CONFIG_BT_MESH_IVU_DIVIDER MYNEWT_VAL(BLE_MESH_IVU_DIVIDER) #define CONFIG_BT_DEVICE_NAME MYNEWT_VAL(BLE_MESH_DEVICE_NAME) +#define CONFIG_BT_MESH_TX_SEG_MAX MYNEWT_VAL(BLE_MESH_TX_SEG_MAX) #define printk console_printf @@ -454,4 +455,6 @@ settings_load(void) #endif /* MYNEWT_VAL(MYNEWT_VAL_BLE_MESH_SETTINGS) */ +#define BUILD_ASSERT(cond) _Static_assert(cond, "") + #endif diff --git a/nimble/host/mesh/src/transport.c b/nimble/host/mesh/src/transport.c index 702f11ca5..e6f857b9a 100644 --- a/nimble/host/mesh/src/transport.c +++ b/nimble/host/mesh/src/transport.c @@ -28,6 +28,13 @@ #include "transport.h" #include "testing.h" +/* The transport layer needs at least three buffers for itself to avoid + * deadlocks. Ensure that there are a sufficient number of advertising + * buffers available compared to the maximum supported outgoing segment + * count. + */ +BUILD_ASSERT(CONFIG_BT_MESH_ADV_BUF_COUNT >= (CONFIG_BT_MESH_TX_SEG_MAX + 3)); + #define AID_MASK ((u8_t)(BIT_MASK(6))) #define SEG(data) ((data)[0] >> 7) @@ -58,7 +65,7 @@ static struct seg_tx { struct bt_mesh_subnet *sub; - struct os_mbuf *seg[BT_MESH_TX_SEG_COUNT]; + struct os_mbuf *seg[CONFIG_BT_MESH_TX_SEG_MAX]; u64_t seq_auth; u16_t dst; u8_t seg_n:5, /* Last segment index */ diff --git a/nimble/host/mesh/src/transport.h b/nimble/host/mesh/src/transport.h index d676f782e..ac6b3f8ee 100644 --- a/nimble/host/mesh/src/transport.h +++ b/nimble/host/mesh/src/transport.h @@ -11,8 +11,7 @@ #define TRANS_SEQ_AUTH_NVAL 0xffffffffffffffff -#define BT_MESH_TX_SEG_COUNT (MYNEWT_VAL(BLE_MESH_ADV_BUF_COUNT) - 3) -#define BT_MESH_TX_SDU_MAX (BT_MESH_TX_SEG_COUNT * 12) +#define BT_MESH_TX_SDU_MAX (CONFIG_BT_MESH_TX_SEG_MAX * 12) #define TRANS_CTL_OP_MASK ((u8_t)BIT_MASK(7)) #define TRANS_CTL_OP(data) ((data)[0] & TRANS_CTL_OP_MASK) diff --git a/nimble/host/mesh/syscfg.yml b/nimble/host/mesh/syscfg.yml index 598ec8694..5f9ade452 100644 --- a/nimble/host/mesh/syscfg.yml +++ b/nimble/host/mesh/syscfg.yml @@ -117,14 +117,13 @@ syscfg.defs: BLE_MESH_ADV_BUF_COUNT: description: > - Number of advertising buffers available. The transport layer - reserves ADV_BUF_COUNT - 3 buffers for outgoing segments. The - maximum outgoing SDU size is 12 times this number (out of which - 4 or 8 bytes is used for the Transport Layer MIC). For - example, 5 segments means the maximum SDU size is 60 bytes, - which leaves 56 bytes for application layer data using a - 4-byte MIC and 52 bytes using an 8-byte MIC. - value: 10 + Number of advertising buffers available. This should be chosen + based on what kind of features the local node shoule have. E.g. + a relay will perform better the more buffers it has. Another + thing to consider is outgoing segmented messages. There must + be at least three more advertising buffers than the maximum + supported outgoing segment count (BT_MESH_TX_SEG_MAX). + value: 6 BLE_MESH_IVU_DIVIDER: description: > @@ -169,6 +168,28 @@ syscfg.defs: usage. value: 384 + BLE_MESH_TX_SEG_MAX: + description: > + Maximum number of segments supported for outgoing messages. + This value should typically be fine-tuned based on what + models the local node supports, i.e. what's the largest + message payload that the node needs to be able to send. + This value affects memory and call stack consumption, which + is why the default is lower than the maximum that the + specification would allow (32 segments). + + The maximum outgoing SDU size is 12 times this number (out of + which 4 or 8 bytes is used for the Transport Layer MIC). For + example, 5 segments means the maximum SDU size is 60 bytes, + which leaves 56 bytes for application layer data using a + 4-byte MIC and 52 bytes using an 8-byte MIC. + + Be sure to specify a sufficient number of advertising buffers + when setting this option to a higher value. There must be at + least three more advertising buffers (BT_MESH_ADV_BUF_COUNT) + as there are outgoing segments. + value: 3 + BLE_MESH_RELAY: description: > Support for acting as a Mesh Relay Node. From e932b20ce3200303478560f6911c1cff61efeb95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Narajowski?= Date: Mon, 20 Aug 2018 16:49:38 +0200 Subject: [PATCH 3/6] mesh: Kconfig: Tweak RX SDU value Update the RX SDU Kconfig value to something that reflects better current use cases and doesn't waste memory needlessly. Also lower the minimum to two segments, since while many samples need three for their composition data (typically the biggest transferred payload), it's possible to have a very simple node whose composition fits in two segments. --- nimble/host/mesh/syscfg.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/nimble/host/mesh/syscfg.yml b/nimble/host/mesh/syscfg.yml index 5f9ade452..46f9b1862 100644 --- a/nimble/host/mesh/syscfg.yml +++ b/nimble/host/mesh/syscfg.yml @@ -163,10 +163,13 @@ syscfg.defs: BLE_MESH_RX_SDU_MAX: description: > - Maximum incoming Upper Transport Access PDU length. Leave this - to the value; value, unless you really need to optimize memory - usage. - value: 384 + Maximum incoming Upper Transport Access PDU length. This + determines also how many segments incoming segmented messages + can have. Each segment can contain 12 bytes, so this value should + be set to a multiple of 12 to avoid wasted memory. The minimum + requirement is 2 segments (24 bytes) whereas the maximum supported + by the Mesh specification is 32 segments (384 bytes). + value: 72 BLE_MESH_TX_SEG_MAX: description: > From 0fa9b57e100c91c07a11fdaaf579e64dff38d48d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Narajowski?= Date: Mon, 20 Aug 2018 16:51:06 +0200 Subject: [PATCH 4/6] mesh: Clarify confusing log message Normally App Keys are identified using the AppKey Index value (a 12 bit value in practice), whereas the stack-internal array index has very little relevance. --- nimble/host/mesh/src/transport.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nimble/host/mesh/src/transport.c b/nimble/host/mesh/src/transport.c index e6f857b9a..967c0ca97 100644 --- a/nimble/host/mesh/src/transport.c +++ b/nimble/host/mesh/src/transport.c @@ -646,7 +646,8 @@ static int sdu_recv(struct bt_mesh_net_rx *rx, u32_t seq, u8_t hdr, rx->ctx.recv_dst, seq, BT_MESH_NET_IVI_RX(rx)); if (err) { - BT_WARN("Unable to decrypt with AppKey %u", i); + BT_WARN("Unable to decrypt with AppKey 0x%03x", + key->app_idx); continue; } From 4b9f59408629edc7d490789f4dc130091ac60076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Narajowski?= Date: Tue, 21 Aug 2018 16:45:00 +0200 Subject: [PATCH 5/6] mesh: Initialize PB-GATT advertising data at the right time Some things, such as UUID, URI or even the local name may be different when bt_mesh_prov_enable() is called compared to when bt_mesh_init() was called. Create the advertising data on-demand each time when enabling PB-GATT. --- nimble/host/mesh/src/proxy.c | 92 ++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/nimble/host/mesh/src/proxy.c b/nimble/host/mesh/src/proxy.c index 186f2f560..03cd081ba 100644 --- a/nimble/host/mesh/src/proxy.c +++ b/nimble/host/mesh/src/proxy.c @@ -987,9 +987,6 @@ static const struct bt_data prov_ad[] = { BT_DATA_BYTES(BT_DATA_UUID16_ALL, 0x27, 0x18), BT_DATA(BT_DATA_SVC_DATA16, prov_svc_data, sizeof(prov_svc_data)), }; - -static struct bt_data prov_sd[2]; -static size_t prov_sd_len; #endif /* PB_GATT */ #if (MYNEWT_VAL(BLE_MESH_GATT_PROXY)) @@ -1183,6 +1180,52 @@ static s32_t gatt_proxy_advertise(struct bt_mesh_subnet *sub) } #endif /* GATT_PROXY */ +#if (MYNEWT_VAL(BLE_MESH_PB_GATT)) +static size_t gatt_prov_adv_create(struct bt_data prov_sd[2]) +{ + const struct bt_mesh_prov *prov = bt_mesh_prov_get(); + const char *name = CONFIG_BT_DEVICE_NAME; + size_t name_len = strlen(name); + size_t prov_sd_len = 0; + size_t sd_space = 31; + + memcpy(prov_svc_data + 2, prov->uuid, 16); + sys_put_be16(prov->oob_info, prov_svc_data + 18); + + if (prov->uri) { + size_t uri_len = strlen(prov->uri); + + if (uri_len > 29) { + /* There's no way to shorten an URI */ + BT_WARN("Too long URI to fit advertising packet"); + } else { + prov_sd[0].type = BT_DATA_URI; + prov_sd[0].data_len = uri_len; + prov_sd[0].data = (void *)prov->uri; + sd_space -= 2 + uri_len; + prov_sd_len++; + } + } + + if (sd_space > 2 && name_len > 0) { + sd_space -= 2; + + if (sd_space < name_len) { + prov_sd[prov_sd_len].type = BT_DATA_NAME_SHORTENED; + prov_sd[prov_sd_len].data_len = sd_space; + } else { + prov_sd[prov_sd_len].type = BT_DATA_NAME_COMPLETE; + prov_sd[prov_sd_len].data_len = name_len; + } + + prov_sd[prov_sd_len].data = (void *)name; + prov_sd_len++; + } + + return prov_sd_len; +} +#endif /* PB_GATT */ + s32_t bt_mesh_proxy_adv_start(void) { BT_DBG(""); @@ -1194,6 +1237,8 @@ s32_t bt_mesh_proxy_adv_start(void) #if (MYNEWT_VAL(BLE_MESH_PB_GATT)) if (!bt_mesh_is_provisioned()) { const struct ble_gap_adv_params *param; + struct bt_data prov_sd[2]; + size_t prov_sd_len; if (prov_fast_adv) { param = &fast_adv_param; @@ -1201,6 +1246,8 @@ s32_t bt_mesh_proxy_adv_start(void) param = &slow_adv_param; } + prov_sd_len = gatt_prov_adv_create(prov_sd); + if (bt_le_adv_start(param, prov_ad, ARRAY_SIZE(prov_ad), prov_sd, prov_sd_len) == 0) { proxy_adv_enabled = true; @@ -1338,45 +1385,6 @@ int bt_mesh_proxy_init(void) clients[i].buf = NET_BUF_SIMPLE(CLIENT_BUF_SIZE); } -#if (MYNEWT_VAL(BLE_MESH_PB_GATT)) - const struct bt_mesh_prov *prov = bt_mesh_prov_get(); - size_t name_len = strlen(CONFIG_BT_DEVICE_NAME); - size_t sd_space = 31; - - memcpy(prov_svc_data + 2, prov->uuid, 16); - sys_put_be16(prov->oob_info, prov_svc_data + 18); - - if (prov->uri) { - size_t uri_len = strlen(prov->uri); - - if (uri_len > 29) { - /* There's no way to shorten an URI */ - BT_WARN("Too long URI to fit advertising packet"); - } else { - prov_sd[0].type = BT_DATA_URI; - prov_sd[0].data_len = uri_len; - prov_sd[0].data = (void *)prov->uri; - sd_space -= 2 + uri_len; - prov_sd_len++; - } - } - - if (sd_space > 2 && name_len > 0) { - sd_space -= 2; - - if (sd_space < name_len) { - prov_sd[prov_sd_len].type = BT_DATA_NAME_SHORTENED; - prov_sd[prov_sd_len].data_len = sd_space; - } else { - prov_sd[prov_sd_len].type = BT_DATA_NAME_COMPLETE; - prov_sd[prov_sd_len].data_len = name_len; - } - - prov_sd[prov_sd_len].data = (void *)CONFIG_BT_DEVICE_NAME; - prov_sd_len++; - } -#endif - resolve_svc_handles(); ble_gatts_svc_set_visibility(svc_handles.proxy_h, 0); From babce918f6397ac88c2170545e485e90352d58c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Narajowski?= Date: Tue, 21 Aug 2018 16:49:21 +0200 Subject: [PATCH 6/6] mesh: Fix heartbeat subscription state handling PTS version 7.3.0 incorporates some errata which change the expected behavior of the heartbeat subscription state. Update the code so that the following tests pass successfully: MESH/NODE/CFG/HBS/BV-01 MESH/NODE/CFG/HBS/BV-02 MESH/NODE/CFG/HBS/BV-03 MESH/NODE/CFG/HBS/BV-04 --- nimble/host/mesh/src/cfg_srv.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/nimble/host/mesh/src/cfg_srv.c b/nimble/host/mesh/src/cfg_srv.c index 97b939703..cafa4ec07 100644 --- a/nimble/host/mesh/src/cfg_srv.c +++ b/nimble/host/mesh/src/cfg_srv.c @@ -3146,15 +3146,11 @@ static void hb_sub_send_status(struct bt_mesh_model *model, net_buf_simple_add_le16(msg, cfg->hb_sub.src); net_buf_simple_add_le16(msg, cfg->hb_sub.dst); - if (cfg->hb_sub.src == BT_MESH_ADDR_UNASSIGNED || - cfg->hb_sub.dst == BT_MESH_ADDR_UNASSIGNED) { - memset(net_buf_simple_add(msg, 4), 0, 4); - } else { - net_buf_simple_add_u8(msg, hb_log(period)); - net_buf_simple_add_u8(msg, hb_log(cfg->hb_sub.count)); - net_buf_simple_add_u8(msg, cfg->hb_sub.min_hops); - net_buf_simple_add_u8(msg, cfg->hb_sub.max_hops); - } + net_buf_simple_add_u8(msg, hb_log(period)); + net_buf_simple_add_u8(msg, hb_log(cfg->hb_sub.count)); + net_buf_simple_add_u8(msg, cfg->hb_sub.min_hops); + net_buf_simple_add_u8(msg, cfg->hb_sub.max_hops); + if (bt_mesh_model_send(model, ctx, msg, NULL, NULL)) { BT_ERR("Unable to send Heartbeat Subscription Status"); @@ -3211,12 +3207,17 @@ static void heartbeat_sub_set(struct bt_mesh_model *model, if (sub_src == BT_MESH_ADDR_UNASSIGNED || sub_dst == BT_MESH_ADDR_UNASSIGNED || sub_period == 0x00) { - /* Setting the same addresses with zero period should retain - * the addresses according to MESH/NODE/CFG/HBS/BV-02-C. + /* Only an explicit address change to unassigned should + * trigger clearing of the values according to + * MESH/NODE/CFG/HBS/BV-02-C. */ - if (cfg->hb_sub.src != sub_src || cfg->hb_sub.dst != sub_dst) { + if (sub_src == BT_MESH_ADDR_UNASSIGNED || + sub_dst == BT_MESH_ADDR_UNASSIGNED) { cfg->hb_sub.src = BT_MESH_ADDR_UNASSIGNED; cfg->hb_sub.dst = BT_MESH_ADDR_UNASSIGNED; + cfg->hb_sub.min_hops = BT_MESH_TTL_MAX; + cfg->hb_sub.max_hops = 0; + cfg->hb_sub.count = 0; } period_ms = 0; @@ -3241,6 +3242,13 @@ static void heartbeat_sub_set(struct bt_mesh_model *model, } hb_sub_send_status(model, ctx, STATUS_SUCCESS); + + /* MESH/NODE/CFG/HBS/BV-01-C expects the MinHops to be 0x7f after + * disabling subscription, but 0x00 for subsequent Get requests. + */ + if (!period_ms) { + cfg->hb_sub.min_hops = 0; + } } const struct bt_mesh_model_op bt_mesh_cfg_srv_op[] = {