[riot-notifications] [RIOT-OS/RIOT] net/BLE: add support for RPL-over-BLE (#16364)

benpicco notifications at github.com
Sun Jul 4 16:26:15 CEST 2021


@benpicco commented on this pull request.



> +#ifdef MODULE_NIMBLE_RPBLE
+        nimble_rpble_ctx_t ctx;
+        memset(&ctx, 0, sizeof(ctx));
+        ctx.inst_id = dodag->instance->id;
+        memcpy(ctx.dodag_id, &dodag->dodag_id, 16);
+        ctx.version = dodag->version;
+        ctx.rank = dodag->my_rank;
+        ctx.role = dodag->node_status;
+        nimble_rpble_update(&ctx);
+#endif

since you have this very same block again in `gnrc_rpl.c`, better move it to a function that can then be defined to be a no-op when `nimble_rpble` is not used

> @@ -0,0 +1,178 @@
+/*
+ * Copyright (C) 2019-2021 Freie Universität Berlin
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @defgroup    net_ble_nimble_netif_rpble RPL-over-BLE for NimBLE
+ * @ingroup     net_ble_nimble

this group does not exist

> +    uint32_t scan_itvl;         /**< scan interval when scanning for parents */
+    uint32_t scan_win;          /**< scan window when scanning for parents */
+    uint32_t adv_itvl;          /**< advertising interval used when advertising
+                                 *   RPL context to child nodes */

what units are those? ms? µs?
would 16 bit be enough here?

> +                                 *   connection */
+
+    /* time the node searches and ranks potential parents
+     * -> itvl := rand([eval_itvl:2*eval_itvl)) */
+    uint32_t eval_itvl_min;     /**< amount of time a node searches for
+                                 *   potential parents, lower bound */
+    uint32_t eval_itvl_max;     /**< amount of time a node searches for
+                                 *   potential parents, upper bound */
+} nimble_rpble_cfg_t;
+
+/**
+ * @brief   RPL DODAG information
+ */
+typedef struct {
+    uint8_t inst_id;            /**< instance ID */
+    uint8_t dodag_id[16];       /**< DODAG ID */

not `ipv6_addr_t`? 

> +
+#include "nimble_rpble.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @name    Default parameters used for the nimble_netif_rpble module
+ * @{
+ */
+#ifndef NIMBLE_RPBLE_SCAN_ITVL_MS
+#define NIMBLE_RPBLE_SCAN_ITVL_MS       1200U
+#endif
+#ifndef NIMBLE_RPBLE_SCAN_WIN_MS
+#define NIMBLE_RPBLE_SCAN_WIN_MS        120U    /* 10% duty cycling */

I don't get the connection between 120ms and 10%

> +    /* generate the new advertising data */
+    bluetil_ad_t ad;
+    uint8_t buf[BLE_HS_ADV_MAX_SZ];
+    res = bluetil_ad_init_with_flags(&ad, buf, BLE_HS_ADV_MAX_SZ,
+                                     BLUETIL_AD_FLAGS_DEFAULT);
+    assert(res == BLUETIL_AD_OK);
+    res = bluetil_ad_add(&ad, BLE_GAP_AD_SERVICE_DATA, sd, sizeof(sd));
+    assert(res == BLUETIL_AD_OK);
+
+    /* start advertising this node */
+    res = nimble_netif_accept(ad.buf, ad.pos, &_adv_params);
+    assert(res == NIMBLE_NETIF_OK);
+}
+
+static void _on_scan_evt(uint8_t type,
+                         const ble_addr_t *addr, int8_t rssi,

nice so we can hook this up to `netstats_nb_record()` and use it for MR-HOF :wink: 

> @@ -69,7 +69,6 @@ typedef union __attribute__((packed)) {
     le_uint16_t l16[4]; /**< little endian 16 bit representation */
     le_uint32_t l32[2]; /**< little endian 32 bit representation */
 } le_uint64_t;
-

unrelated :wink: 

> @@ -69,7 +69,6 @@ typedef union __attribute__((packed)) {
     le_uint16_t l16[4]; /**< little endian 16 bit representation */
     le_uint32_t l32[2]; /**< little endian 32 bit representation */
 } le_uint64_t;
-

unrelated :wink: 

> +    bluetil_ad_t ads = { .buf = (uint8_t *)ad, .pos = ad_len, .size = ad_len };
+    res = bluetil_ad_find(&ads, BLE_GAP_AD_SERVICE_DATA, &sd_field);
+    if (res != BLUETIL_AD_OK) {
+        return;
+    }
+    if ((sd_field.len != AD_SVC_DATA_LEN) ||
+        (byteorder_lebuftohs(sd_field.data) != BLE_GATT_SVC_IPSS)) {
+        return;
+    }
+
+    /**
+     * @note    Here we need to improve the filtering: so far, we consider every
+     *          node we see that is capable of rplbe to be a parent. We should
+     *          however also filter for instance ID and possibly the DODAG ID as
+     *          well. On top, we should probably only consider nodes with >=
+     *          version as parent

What's stopping us from doing just that?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/16364#pullrequestreview-698678820
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210704/7b8b6c6d/attachment-0001.htm>


More information about the notifications mailing list