1. Remove advertising data construction in C: it's all done in Python now

2. Add scan response capability to advertising.

6 files changed, 39 insertions, 143 deletions

diff --git a/ports/nrf/common-hal/bleio/Broadcaster.c b/ports/nrf/common-hal/bleio/Broadcaster.c
index a70209a7f..a9e2a04c3 100644
--- a/ports/nrf/common-hal/bleio/Broadcaster.c
+++ b/ports/nrf/common-hal/bleio/Broadcaster.c
@@ -39,7 +39,7 @@
 static uint8_t m_adv_handle = BLE_GAP_ADV_SET_HANDLE_NOT_SET;
 
 void common_hal_bleio_broadcaster_construct(bleio_broadcaster_obj_t *self, mp_float_t interval) {
-    common_hal_bleio_adapter_set_enabled(true);   // TODO -- Do this somewhere else maybe bleio __init__
+    common_hal_bleio_adapter_set_enabled(true);
     const mp_float_t min = BLE_GAP_ADV_INTERVAL_MIN * ADV_INTERVAL_UNIT_FLOAT_SECS;
     const mp_float_t max = BLE_GAP_ADV_INTERVAL_MAX * ADV_INTERVAL_UNIT_FLOAT_SECS;
 
diff --git a/ports/nrf/common-hal/bleio/Peripheral.c b/ports/nrf/common-hal/bleio/Peripheral.c
index 4aff75c9b..ad28523ae 100644
--- a/ports/nrf/common-hal/bleio/Peripheral.c
+++ b/ports/nrf/common-hal/bleio/Peripheral.c
@@ -51,63 +51,15 @@
 
 static uint8_t m_adv_handle = BLE_GAP_ADV_SET_HANDLE_NOT_SET;
 
-STATIC void check_data_fit(size_t pos, size_t data_len) {
-    if (pos + data_len > BLE_GAP_ADV_SET_DATA_SIZE_MAX) {
+STATIC void check_data_fit(size_t data_len) {
+    if (data_len > BLE_GAP_ADV_SET_DATA_SIZE_MAX) {
         mp_raise_ValueError(translate("Data too large for advertisement packet"));
     }
 }
 
-STATIC uint32_t add_services_to_advertisement(bleio_peripheral_obj_t *self, size_t* adv_data_pos_p, size_t uuid_len) {
-    uint32_t uuids_total_size = 0;
-    const mp_obj_list_t *service_list = MP_OBJ_TO_PTR(self->service_list);
-    uint32_t err_code = NRF_SUCCESS;
-
-    check_data_fit(*adv_data_pos_p, 1 + 1);
-
-    // Remember where length byte is; fill in later when we know the size.
-    const size_t length_pos = *adv_data_pos_p;
-    (*adv_data_pos_p)++;
-
-    self->adv_data[(*adv_data_pos_p)++] = (uuid_len == 16)
-        ? BLE_GAP_AD_TYPE_16BIT_SERVICE_UUID_COMPLETE
-        : BLE_GAP_AD_TYPE_128BIT_SERVICE_UUID_COMPLETE;
-
-    for (size_t i = 0; i < service_list->len; ++i) {
-        const bleio_service_obj_t *service = MP_OBJ_TO_PTR(service_list->items[i]);
-        uint8_t encoded_size = 0;
-
-        // Skip services of the wrong length and secondary services.
-        if (common_hal_bleio_uuid_get_size(service->uuid) != uuid_len || service->is_secondary) {
-            continue;
-        }
-
-        ble_uuid_t uuid;
-        bleio_uuid_convert_to_nrf_ble_uuid(service->uuid, &uuid);
-
-        err_code = sd_ble_uuid_encode(&uuid, &encoded_size, &(self->adv_data[*adv_data_pos_p]));
-        if (err_code != NRF_SUCCESS) {
-            return err_code;
-        }
-
-        check_data_fit(*adv_data_pos_p, encoded_size);
-        uuids_total_size += encoded_size;
-        (*adv_data_pos_p) += encoded_size;
-    }
-
-    self->adv_data[length_pos] = 1 + uuids_total_size;  // 1 for the field type.
-    return err_code;
-}
-
-
-
-// if raw_data is a zero-length buffer, generate an advertising packet that advertises the
-// services passed in when this Peripheral was created.
-// If raw_data contains some bytes, use those bytes as the advertising packet.
-// TODO: Generate the advertising packet in Python, not here.
-STATIC uint32_t set_advertisement_data(bleio_peripheral_obj_t *self, bool connectable, mp_buffer_info_t *raw_data) {
+STATIC uint32_t start_advertising(bleio_peripheral_obj_t *self, bool connectable, mp_buffer_info_t *advertising_data_bufinfo, mp_buffer_info_t *scan_response_data_bufinfo) {
     common_hal_bleio_adapter_set_enabled(true);
 
-    size_t adv_data_pos = 0;
     uint32_t err_code;
 
     GET_STR_DATA_LEN(self->name, name_data, name_len);
@@ -115,86 +67,18 @@ STATIC uint32_t set_advertisement_data(bleio_peripheral_obj_t *self, bool connec
         ble_gap_conn_sec_mode_t sec_mode;
         BLE_GAP_CONN_SEC_MODE_SET_OPEN(&sec_mode);
 
-        // We'll add the name after everything else, shortening it if necessary.
         err_code = sd_ble_gap_device_name_set(&sec_mode, name_data, name_len);
         if (err_code != NRF_SUCCESS) {
             return err_code;
         }
     }
 
-    if (raw_data->len != 0) {
-        // User-supplied advertising packet.
-        check_data_fit(adv_data_pos, raw_data->len);
-        memcpy(&(self->adv_data[adv_data_pos]), raw_data->buf, raw_data->len);
-        adv_data_pos += raw_data->len;
-    } else {
-        // Build up advertising packet.
-        check_data_fit(adv_data_pos, 1 + 1 + 1);
-        self->adv_data[adv_data_pos++] = 2;
-        self->adv_data[adv_data_pos++] = BLE_GAP_AD_TYPE_FLAGS;
-        self->adv_data[adv_data_pos++] = BLE_GAP_ADV_FLAGS_LE_ONLY_GENERAL_DISC_MODE;
-
-        // The 16-bit ids and 128-bit ids are grouped together by length, so find it whether we have
-        // 16 and/or 128-bit service UUIDs.
-
-        const mp_obj_list_t *service_list = MP_OBJ_TO_PTR(self->service_list);
-        if (service_list->len > 0) {
-            bool has_128bit_services = false;
-            bool has_16bit_services = false;
-
-            for (size_t i = 0; i < service_list->len; ++i) {
-                const bleio_service_obj_t *service = MP_OBJ_TO_PTR(service_list->items[i]);
-
-                if (service->is_secondary) {
-                    continue;
-                }
-
-                switch (common_hal_bleio_uuid_get_size(service->uuid)) {
-                case 16:
-                    has_16bit_services = true;
-                    break;
-                case 128:
-                    has_128bit_services = true;
-                    break;
-                }
-            }
-
-            // Add 16-bit service UUID's in a group, then 128-bit service UUID's.
-
-            if (has_16bit_services) {
-                err_code = add_services_to_advertisement(self, &adv_data_pos, 16);
-                if (err_code != NRF_SUCCESS) {
-                    return err_code;
-                }
-            }
-
-            if (has_128bit_services) {
-                err_code = add_services_to_advertisement(self, &adv_data_pos, 128);
-                if (err_code != NRF_SUCCESS) {
-                    return err_code;
-                }
-            }
-        }
+    check_data_fit(advertising_data_bufinfo->len);
+    memcpy(self->advertising_data, advertising_data_bufinfo->buf, advertising_data_bufinfo->len);
+
+    check_data_fit(scan_response_data_bufinfo->len);
+    memcpy(self->scan_response_data, scan_response_data_bufinfo->buf, scan_response_data_bufinfo->len);
 
-        // Always include TX power.
-        check_data_fit(adv_data_pos, 1 + 1 + 1);
-        self->adv_data[adv_data_pos++] = 1 + 1;
-        self->adv_data[adv_data_pos++] = BLE_GAP_AD_TYPE_TX_POWER_LEVEL;
-        self->adv_data[adv_data_pos++] = 0;   // TODO - allow power level to be set later.
-
-        // We need room for at least a one-character name.
-        check_data_fit(adv_data_pos, 1 + 1 + 1);
-
-        // How big a name can we fit?
-        size_t bytes_left = BLE_GAP_ADV_SET_DATA_SIZE_MAX - adv_data_pos - 1 - 1;
-        size_t partial_name_len = MIN(bytes_left, name_len);
-        self->adv_data[adv_data_pos++] = 1 + partial_name_len;
-        self->adv_data[adv_data_pos++] = (partial_name_len == name_len)
-            ? BLE_GAP_AD_TYPE_COMPLETE_LOCAL_NAME
-            : BLE_GAP_AD_TYPE_SHORT_LOCAL_NAME;
-        memcpy(&(self->adv_data[adv_data_pos]), name_data, partial_name_len);
-        adv_data_pos += partial_name_len;
-    } // end of advertising packet construction
 
     static ble_gap_adv_params_t m_adv_params = {
         .interval = MSEC_TO_UNITS(1000, UNIT_0_625_MS),
@@ -211,8 +95,10 @@ STATIC uint32_t set_advertisement_data(bleio_peripheral_obj_t *self, bool connec
     common_hal_bleio_peripheral_stop_advertising(self);
 
     const ble_gap_adv_data_t ble_gap_adv_data = {
-        .adv_data.p_data = self->adv_data,
-        .adv_data.len = adv_data_pos,
+        .adv_data.p_data = self->advertising_data,
+        .adv_data.len = advertising_data_bufinfo->len,
+        .scan_rsp_data.p_data = scan_response_data_bufinfo-> len > 0 ? self->scan_response_data : NULL,
+        .scan_rsp_data.len = scan_response_data_bufinfo->len,
     };
 
     err_code = sd_ble_gap_adv_set_configure(&m_adv_handle, &ble_gap_adv_data, &m_adv_params);
@@ -322,12 +208,13 @@ bool common_hal_bleio_peripheral_get_connected(bleio_peripheral_obj_t *self) {
     return self->conn_handle != BLE_CONN_HANDLE_INVALID;
 }
 
-void common_hal_bleio_peripheral_start_advertising(bleio_peripheral_obj_t *self, bool connectable, mp_buffer_info_t *raw_data) {
+void common_hal_bleio_peripheral_start_advertising(bleio_peripheral_obj_t *self, bool connectable, mp_buffer_info_t *advertising_data_bufinfo, mp_buffer_info_t *scan_response_data_bufinfo) {
     if (connectable) {
         ble_drv_add_event_handler(peripheral_on_ble_evt, self);
     }
 
-    const uint32_t err_code = set_advertisement_data(self, connectable, raw_data);
+    const uint32_t err_code = start_advertising(self, connectable,
+                                                advertising_data_bufinfo, scan_response_data_bufinfo);
     if (err_code != NRF_SUCCESS) {
         mp_raise_OSError_msg_varg(translate("Failed to start advertising, err 0x%04x"), err_code);
     }
diff --git a/ports/nrf/common-hal/bleio/Peripheral.h b/ports/nrf/common-hal/bleio/Peripheral.h
index b255fe9f4..bf1931dda 100644
--- a/ports/nrf/common-hal/bleio/Peripheral.h
+++ b/ports/nrf/common-hal/bleio/Peripheral.h
@@ -43,10 +43,11 @@ typedef struct {
     mp_obj_t service_list;
     mp_obj_t notif_handler;
     mp_obj_t conn_handler;
-    // The advertising data buffer is held by us, not by the SD, so we must
-    // maintain it and not change it. If we need to change its contents during advertising,
+    // The advertising data and scan response buffers are held by us, not by the SD, so we must
+    // maintain them and not change it. If we need to change the contents during advertising,
     // there are tricks to get the SD to notice (see DevZone - TBS).
-    uint8_t adv_data[BLE_GAP_ADV_SET_DATA_SIZE_MAX];
+    uint8_t advertising_data[BLE_GAP_ADV_SET_DATA_SIZE_MAX];
+    uint8_t scan_response_data[BLE_GAP_ADV_SET_DATA_SIZE_MAX];
 
 } bleio_peripheral_obj_t;
 
diff --git a/shared-bindings/bleio/Broadcaster.c b/shared-bindings/bleio/Broadcaster.c
index 209e66490..294f1a83a 100644
--- a/shared-bindings/bleio/Broadcaster.c
+++ b/shared-bindings/bleio/Broadcaster.c
@@ -82,7 +82,7 @@ STATIC mp_obj_t bleio_broadcaster_make_new(const mp_obj_type_t *type, size_t n_a
 //|
 //|     Start advertising using the given data packet.
 //|
-//|     :param buf data:  advertising data packet, starting with advertising data flags (0x01)
+//|     :param buf data:  advertising data packet
 //|
 STATIC mp_obj_t bleio_broadcaster_start_advertising(mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     bleio_broadcaster_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
diff --git a/shared-bindings/bleio/Peripheral.c b/shared-bindings/bleio/Peripheral.c
index 340485f35..960b3723c 100644
--- a/shared-bindings/bleio/Peripheral.c
+++ b/shared-bindings/bleio/Peripheral.c
@@ -68,7 +68,8 @@ static const char default_name[] = "CIRCUITPY";
 //|
 //|    # Create a peripheral and start it up.
 //|    periph = bleio.Peripheral([service])
-//|    periph.start_advertising()
+//|    adv = ServerAdvertisement(periph)
+//|    periph.start_advertising(adv.advertising_data_bytes, adv.scan_response_bytes)
 //|
 //|    while not periph.connected:
 //|        # Wait for connection.
@@ -181,33 +182,40 @@ const mp_obj_property_t bleio_peripheral_name_obj = {
                (mp_obj_t)&mp_const_none_obj },
 };
 
-//|   .. method:: start_advertising(*, connectable=True, data=None)
+//|   .. method:: start_advertising(data, *, scan_response=None, connectable=True)
 //|
 //|     Starts advertising the peripheral. The peripheral's name and
 //|     services are included in the advertisement packets.
 //|
+//|     :param buf data: advertising data packet bytes
+//|     :param buf scan_response: scan response data packet bytes. ``None`` if no scan response is needed.
 //|     :param bool connectable:  If `True` then other devices are allowed to connect to this peripheral.
-//|     :param buf data: If `None`, advertise the services passed to this Peripheral when it was created.
-//|       If not `None`, then send the bytes in ``data`` as the advertising packet.
+//|
 //|
 STATIC mp_obj_t bleio_peripheral_start_advertising(mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     bleio_peripheral_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
 
-    enum { ARG_connectable, ARG_data };
+    enum { ARG_data, ARG_scan_response, ARG_connectable };
     static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_data, MP_ARG_REQUIRED | MP_ARG_OBJ },
+        { MP_QSTR_scan_response, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} },
         { MP_QSTR_connectable, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = true} },
-        { MP_QSTR_data, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} },
     };
 
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
-    mp_buffer_info_t bufinfo = { 0 };
+    mp_buffer_info_t data_bufinfo;
+    mp_get_buffer_raise(args[ARG_data].u_obj, &data_bufinfo, MP_BUFFER_READ);
+
+    // Pass an empty buffer if scan_response not provided.
+    mp_buffer_info_t scan_response_bufinfo = { 0 };
     if (args[ARG_data].u_obj != mp_const_none) {
-        mp_get_buffer_raise(args[ARG_data].u_obj, &bufinfo, MP_BUFFER_READ);
+        mp_get_buffer_raise(args[ARG_scan_response].u_obj, &scan_response_bufinfo, MP_BUFFER_READ);
     }
 
-    common_hal_bleio_peripheral_start_advertising(self, args[ARG_connectable].u_bool, &bufinfo);
+    common_hal_bleio_peripheral_start_advertising(self, args[ARG_connectable].u_bool,
+                                                  &data_bufinfo, &scan_response_bufinfo);
 
     return mp_const_none;
 }
diff --git a/shared-bindings/bleio/Peripheral.h b/shared-bindings/bleio/Peripheral.h
index 02a5c1ef8..c09c60088 100644
--- a/shared-bindings/bleio/Peripheral.h
+++ b/shared-bindings/bleio/Peripheral.h
@@ -34,7 +34,7 @@ extern const mp_obj_type_t bleio_peripheral_type;
 
 extern void common_hal_bleio_peripheral_construct(bleio_peripheral_obj_t *self);
 extern bool common_hal_bleio_peripheral_get_connected(bleio_peripheral_obj_t *self);
-extern void common_hal_bleio_peripheral_start_advertising(bleio_peripheral_obj_t *device, bool connectable, mp_buffer_info_t *raw_data);
+extern void common_hal_bleio_peripheral_start_advertising(bleio_peripheral_obj_t *device, bool connectable, mp_buffer_info_t *advertising_data_bufinfo, mp_buffer_info_t *scan_response_data_bufinfo);
 extern void common_hal_bleio_peripheral_stop_advertising(bleio_peripheral_obj_t *device);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_PERIPHERAL_H