extmod/modbluetooth: Merge gatts_notify/indicate implementation.

Makes gatts_notify and gatts_indicate work in the same way: by default they
send the DB value, but you can manually override the payload.

In other words, makes gatts_indicate work the same as gatts_notify.

Note: This removes support for queuing notifications and indications on
btstack when the ACL buffer is full. This functionality will be
reimplemented in a future commit.

Signed-off-by: Jim Mussared <[email protected]>

Author: jimmo

docs/library/bluetooth.rst

@@ -514,19 +514,24 @@ writes from a client to a given characteristic, use
 
     Sends a notification request to a connected client.
 
-    If *data* is not ``None``, then that value is sent to the client as part of
-    the notification. The local value will not be modified.
+    If *data* is ``None`` (the default), then the current local value (as set
+    with :meth:`gatts_write <BLE.gatts_write>`) will be sent.
 
-    Otherwise, if *data* is ``None``, then the current local value (as
-    set with :meth:`gatts_write <BLE.gatts_write>`) will be sent.
+    Otherwise, if *data* is not ``None``, then that value is sent to the client
+    as part of the notification. The local value will not be modified.
 
     **Note:** The notification will be sent regardless of the subscription
     status of the client to this characteristic.
 
-.. method:: BLE.gatts_indicate(conn_handle, value_handle, /)
+.. method:: BLE.gatts_indicate(conn_handle, value_handle, data=None, /)
 
-    Sends an indication request containing the characteristic's current value to
-    a connected client.
+    Sends a indication request to a connected client.
+
+    If *data* is ``None`` (the default), then the current local value (as set
+    with :meth:`gatts_write <BLE.gatts_write>`) will be sent.
+
+    Otherwise, if *data* is not ``None``, then that value is sent to the client
+    as part of the indication. The local value will not be modified.
 
     On acknowledgment (or failure, e.g. timeout), the
     ``_IRQ_GATTS_INDICATE_DONE`` event will be raised.

extmod/btstack/modbluetooth_btstack.c

@@ -130,8 +130,6 @@ STATIC mp_obj_bluetooth_uuid_t create_mp_uuid(uint16_t uuid16, const uint8_t *uu
 // Pending operation types.
 enum {
     // Queued for sending when possible.
-    MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY, // Waiting for context callback
-    MP_BLUETOOTH_BTSTACK_PENDING_INDICATE, // Waiting for context callback
     MP_BLUETOOTH_BTSTACK_PENDING_WRITE_NO_RESPONSE, // Waiting for conn handle
     // Hold buffer pointer until complete.
     MP_BLUETOOTH_BTSTACK_PENDING_WRITE, // Waiting for write done event
@@ -150,11 +148,7 @@ struct _mp_btstack_pending_op_t {
     uint16_t conn_handle;
     uint16_t value_handle;
 
-    // For notify/indicate only.
-    // context_registration.context will point back to this struct.
-    btstack_context_callback_registration_t context_registration;
-
-    // For notify/indicate/write-without-response, this is the actual buffer to send.
+    // For write-without-response, this is the actual buffer to send.
     // For write-with-response, just holding onto the buffer for GC ref.
     size_t len;
     uint8_t buf[];
@@ -170,30 +164,6 @@ STATIC void btstack_remove_pending_operation(mp_btstack_pending_op_t *pending_op
     }
 }
 
-// Called in response to a gatts_notify/indicate being unable to complete, which then calls
-// att_server_request_to_send_notification.
-// We now have an opportunity to re-try the operation with an empty ACL buffer.
-STATIC void btstack_notify_indicate_ready_handler(void *context) {
-    MICROPY_PY_BLUETOOTH_ENTER
-    mp_btstack_pending_op_t *pending_op = (mp_btstack_pending_op_t *)context;
-    DEBUG_printf("btstack_notify_indicate_ready_handler op_type=%d conn_handle=%d value_handle=%d len=%zu\n", pending_op->op_type, pending_op->conn_handle, pending_op->value_handle, pending_op->len);
-    if (pending_op->op_type == MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY) {
-        int err = att_server_notify(pending_op->conn_handle, pending_op->value_handle, pending_op->buf, pending_op->len);
-        DEBUG_printf("btstack_notify_indicate_ready_handler: sending notification err=%d\n", err);
-        assert(err == ERROR_CODE_SUCCESS);
-        (void)err;
-    } else {
-        assert(pending_op->op_type == MP_BLUETOOTH_BTSTACK_PENDING_INDICATE);
-        int err = att_server_indicate(pending_op->conn_handle, pending_op->value_handle, NULL, 0);
-        DEBUG_printf("btstack_notify_indicate_ready_handler: sending indication err=%d\n", err);
-        assert(err == ERROR_CODE_SUCCESS);
-        (void)err;
-    }
-    // Can't free the pending op as we're in IRQ context. Leave it for the GC.
-    btstack_remove_pending_operation(pending_op, false /* del */);
-    MICROPY_PY_BLUETOOTH_EXIT
-}
-
 // Register a pending background operation -- copies the buffer, and makes it known to the GC.
 STATIC mp_btstack_pending_op_t *btstack_enqueue_pending_operation(uint16_t op_type, uint16_t conn_handle, uint16_t value_handle, const uint8_t *buf, size_t len) {
     DEBUG_printf("btstack_enqueue_pending_operation op_type=%d conn_handle=%d value_handle=%d len=%zu\n", op_type, conn_handle, value_handle, len);
@@ -204,11 +174,6 @@ STATIC mp_btstack_pending_op_t *btstack_enqueue_pending_operation(uint16_t op_ty
     pending_op->len = len;
     memcpy(pending_op->buf, buf, len);
 
-    if (op_type == MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY || op_type == MP_BLUETOOTH_BTSTACK_PENDING_INDICATE) {
-        pending_op->context_registration.callback = &btstack_notify_indicate_ready_handler;
-        pending_op->context_registration.context = pending_op;
-    }
-
     MICROPY_PY_BLUETOOTH_ENTER
     bool added = btstack_linked_list_add(&MP_STATE_PORT(bluetooth_btstack_root_pointers)->pending_ops, (btstack_linked_item_t *)pending_op);
     assert(added);
@@ -854,7 +819,7 @@ void mp_bluetooth_set_io_capability(uint8_t capability) {
 #endif // MICROPY_PY_BLUETOOTH_ENABLE_PAIRING_BONDING
 
 size_t mp_bluetooth_gap_get_device_name(const uint8_t **buf) {
-    uint8_t *value = NULL;
+    const uint8_t *value = NULL;
     size_t value_len = 0;
     mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, BTSTACK_GAP_DEVICE_NAME_HANDLE, &value, &value_len);
     *buf = value;
@@ -1095,7 +1060,7 @@ int mp_bluetooth_gatts_register_service_end(void) {
     return 0;
 }
 
-int mp_bluetooth_gatts_read(uint16_t value_handle, uint8_t **value, size_t *value_len) {
+int mp_bluetooth_gatts_read(uint16_t value_handle, const uint8_t **value, size_t *value_len) {
     DEBUG_printf("mp_bluetooth_gatts_read\n");
     if (!mp_bluetooth_is_active()) {
         return ERRNO_BLUETOOTH_NOT_ACTIVE;
@@ -1114,85 +1079,41 @@ int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t
     return mp_bluetooth_gatts_db_write(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, value, value_len);
 }
 
-int mp_bluetooth_gatts_notify(uint16_t conn_handle, uint16_t value_handle) {
-    DEBUG_printf("mp_bluetooth_gatts_notify\n");
+int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len) {
+    DEBUG_printf("mp_bluetooth_gatts_notify_indicate\n");
 
     if (!mp_bluetooth_is_active()) {
         return ERRNO_BLUETOOTH_NOT_ACTIVE;
     }
 
-    // Note: btstack doesn't appear to support sending a notification without a value, so include the stored value.
-    uint8_t *data = NULL;
-    size_t len = 0;
-    mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, &data, &len);
-    return mp_bluetooth_gatts_notify_send(conn_handle, value_handle, data, len);
-}
-
-int mp_bluetooth_gatts_notify_send(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len) {
-    DEBUG_printf("mp_bluetooth_gatts_notify_send\n");
-
-    if (!mp_bluetooth_is_active()) {
-        return ERRNO_BLUETOOTH_NOT_ACTIVE;
+    if (!value) {
+        // NULL value means "use DB value".
+        mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, &value, &value_len);
     }
 
+    int err = ERROR_CODE_UNKNOWN_HCI_COMMAND;
+
     // Attempt to send immediately. If it succeeds, btstack will copy the buffer.
     MICROPY_PY_BLUETOOTH_ENTER
-    int err = att_server_notify(conn_handle, value_handle, value, value_len);
-    MICROPY_PY_BLUETOOTH_EXIT
-
-    if (err == BTSTACK_ACL_BUFFERS_FULL) {
-        DEBUG_printf("mp_bluetooth_gatts_notify_send: ACL buffer full, scheduling callback\n");
-        // Schedule callback, making a copy of the buffer.
-        mp_btstack_pending_op_t *pending_op = btstack_enqueue_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY, conn_handle, value_handle, value, value_len);
-
-        err = att_server_request_to_send_notification(&pending_op->context_registration, conn_handle);
-
-        if (err != ERROR_CODE_SUCCESS) {
-            // Failure. Unref and free the pending operation.
-            btstack_remove_pending_operation(pending_op, true /* del */);
-        }
-
-        return 0;
-    } else {
-        return btstack_error_to_errno(err);
-    }
-}
-
-int mp_bluetooth_gatts_indicate(uint16_t conn_handle, uint16_t value_handle) {
-    DEBUG_printf("mp_bluetooth_gatts_indicate\n");
-
-    if (!mp_bluetooth_is_active()) {
-        return ERRNO_BLUETOOTH_NOT_ACTIVE;
+    switch (gatts_op) {
+        case MP_BLUETOOTH_GATTS_OP_NOTIFY:
+            err = att_server_notify(conn_handle, value_handle, value, value_len);
+            break;
+        case MP_BLUETOOTH_GATTS_OP_INDICATE:
+            // Indicate will raise ATT_EVENT_HANDLE_VALUE_INDICATION_COMPLETE when
+            // acknowledged (or timeout/error).
+            err = att_server_indicate(conn_handle, value_handle, value, value_len);
+            break;
     }
-
-    uint8_t *data = NULL;
-    size_t len = 0;
-    mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, &data, &len);
-
-    // Indicate will raise ATT_EVENT_HANDLE_VALUE_INDICATION_COMPLETE when
-    // acknowledged (or timeout/error).
-
-    // Attempt to send immediately, will copy buffer.
-    MICROPY_PY_BLUETOOTH_ENTER
-    int err = att_server_indicate(conn_handle, value_handle, data, len);
     MICROPY_PY_BLUETOOTH_EXIT
 
     if (err == BTSTACK_ACL_BUFFERS_FULL) {
-        DEBUG_printf("mp_bluetooth_gatts_indicate: ACL buffer full, scheduling callback\n");
-        // Schedule callback, making a copy of the buffer.
-        mp_btstack_pending_op_t *pending_op = btstack_enqueue_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_INDICATE, conn_handle, value_handle, data, len);
-
-        err = att_server_request_to_send_indication(&pending_op->context_registration, conn_handle);
-
-        if (err != ERROR_CODE_SUCCESS) {
-            // Failure. Unref and free the pending operation.
-            btstack_remove_pending_operation(pending_op, true /* del */);
-        }
+        DEBUG_printf("mp_bluetooth_gatts_notify_indicate: ACL buffer full, scheduling callback\n");
 
-        return 0;
-    } else {
-        return btstack_error_to_errno(err);
+        // TODO: re-implement the handling for this.
     }
+
+    return btstack_error_to_errno(err);
 }
 
 int mp_bluetooth_gatts_set_buffer(uint16_t value_handle, size_t len, bool append) {

extmod/modbluetooth.c

@@ -733,7 +733,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gap_passkey_obj, 4, 4,
 STATIC mp_obj_t bluetooth_ble_gatts_read(mp_obj_t self_in, mp_obj_t value_handle_in) {
     (void)self_in;
     size_t len = 0;
-    uint8_t *buf;
+    const uint8_t *buf;
     mp_bluetooth_gatts_read(mp_obj_get_int(value_handle_in), &buf, &len);
     return mp_obj_new_bytes(buf, len);
 }
@@ -751,32 +751,30 @@ STATIC mp_obj_t bluetooth_ble_gatts_write(size_t n_args, const mp_obj_t *args) {
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gatts_write_obj, 3, 4, bluetooth_ble_gatts_write);
 
-STATIC mp_obj_t bluetooth_ble_gatts_notify(size_t n_args, const mp_obj_t *args) {
+STATIC mp_obj_t bluetooth_ble_gatts_notify_indicate(size_t n_args, const mp_obj_t *args, int gatts_op) {
     mp_int_t conn_handle = mp_obj_get_int(args[1]);
     mp_int_t value_handle = mp_obj_get_int(args[2]);
 
+    const uint8_t *value = NULL;
+    size_t value_len = 0;
     if (n_args == 4 && args[3] != mp_const_none) {
         mp_buffer_info_t bufinfo = {0};
         mp_get_buffer_raise(args[3], &bufinfo, MP_BUFFER_READ);
-        int err = mp_bluetooth_gatts_notify_send(conn_handle, value_handle, bufinfo.buf, bufinfo.len);
-        bluetooth_handle_errno(err);
-        return mp_const_none;
-    } else {
-        int err = mp_bluetooth_gatts_notify(conn_handle, value_handle);
-        return bluetooth_handle_errno(err);
+        value = bufinfo.buf;
+        value_len = bufinfo.len;
     }
+    return bluetooth_handle_errno(mp_bluetooth_gatts_notify_indicate(conn_handle, value_handle, gatts_op, value, value_len));
 }
-STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gatts_notify_obj, 3, 4, bluetooth_ble_gatts_notify);
 
-STATIC mp_obj_t bluetooth_ble_gatts_indicate(mp_obj_t self_in, mp_obj_t conn_handle_in, mp_obj_t value_handle_in) {
-    (void)self_in;
-    mp_int_t conn_handle = mp_obj_get_int(conn_handle_in);
-    mp_int_t value_handle = mp_obj_get_int(value_handle_in);
+STATIC mp_obj_t bluetooth_ble_gatts_notify(size_t n_args, const mp_obj_t *args) {
+    return bluetooth_ble_gatts_notify_indicate(n_args, args, MP_BLUETOOTH_GATTS_OP_NOTIFY);
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gatts_notify_obj, 3, 4, bluetooth_ble_gatts_notify);
 
-    int err = mp_bluetooth_gatts_indicate(conn_handle, value_handle);
-    return bluetooth_handle_errno(err);
+STATIC mp_obj_t bluetooth_ble_gatts_indicate(size_t n_args, const mp_obj_t *args) {
+    return bluetooth_ble_gatts_notify_indicate(n_args, args, MP_BLUETOOTH_GATTS_OP_INDICATE);
 }
-STATIC MP_DEFINE_CONST_FUN_OBJ_3(bluetooth_ble_gatts_indicate_obj, bluetooth_ble_gatts_indicate);
+STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gatts_indicate_obj, 3, 4, bluetooth_ble_gatts_indicate);
 
 STATIC mp_obj_t bluetooth_ble_gatts_set_buffer(size_t n_args, const mp_obj_t *args) {
     mp_int_t value_handle = mp_obj_get_int(args[1]);
@@ -1718,7 +1716,7 @@ mp_bluetooth_gatts_db_entry_t *mp_bluetooth_gatts_db_lookup(mp_gatts_db_t db, ui
     return MP_OBJ_TO_PTR(elem->value);
 }
 
-int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, uint8_t **value, size_t *value_len) {
+int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, const uint8_t **value, size_t *value_len) {
     MICROPY_PY_BLUETOOTH_ENTER
     mp_bluetooth_gatts_db_entry_t *entry = mp_bluetooth_gatts_db_lookup(db, handle);
     if (entry) {

extmod/modbluetooth.h

@@ -186,6 +186,10 @@
 #define MP_BLUETOOTH_PASSKEY_ACTION_DISPLAY             (3)
 #define MP_BLUETOOTH_PASSKEY_ACTION_NUMERIC_COMPARISON  (4)
 
+// These are the ops for mp_bluetooth_gatts_notify_indicate.
+#define MP_BLUETOOTH_GATTS_OP_NOTIFY                    (1)
+#define MP_BLUETOOTH_GATTS_OP_INDICATE                  (2)
+
 /*
 These aren't included in the module for space reasons, but can be used
 in your Python code if necessary.
@@ -333,15 +337,11 @@ int mp_bluetooth_gatts_register_service(mp_obj_bluetooth_uuid_t *service_uuid, m
 int mp_bluetooth_gatts_register_service_end(void);
 
 // Read the value from the local gatts db (likely this has been written by a central).
-int mp_bluetooth_gatts_read(uint16_t value_handle, uint8_t **value, size_t *value_len);
+int mp_bluetooth_gatts_read(uint16_t value_handle, const uint8_t **value, size_t *value_len);
 // Write a value to the local gatts db (ready to be queried by a central). Optionally send notifications/indications.
 int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t value_len, bool send_update);
-// Notify the central that it should do a read.
-int mp_bluetooth_gatts_notify(uint16_t conn_handle, uint16_t value_handle);
-// Notify the central, including a data payload. (Note: does not set the gatts db value).
-int mp_bluetooth_gatts_notify_send(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len);
-// Indicate the central.
-int mp_bluetooth_gatts_indicate(uint16_t conn_handle, uint16_t value_handle);
+// Send a notification/indication to the central, optionally with custom payload (otherwise the DB value is used).
+int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len);
 
 // Resize and enable/disable append-mode on a value.
 // Append-mode means that remote writes will append and local reads will clear after reading.
@@ -508,7 +508,7 @@ STATIC inline void mp_bluetooth_gatts_db_reset(mp_gatts_db_t db) {
 
 void mp_bluetooth_gatts_db_create_entry(mp_gatts_db_t db, uint16_t handle, size_t len);
 mp_bluetooth_gatts_db_entry_t *mp_bluetooth_gatts_db_lookup(mp_gatts_db_t db, uint16_t handle);
-int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, uint8_t **value, size_t *value_len);
+int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, const uint8_t **value, size_t *value_len);
 int mp_bluetooth_gatts_db_write(mp_gatts_db_t db, uint16_t handle, const uint8_t *value, size_t value_len);
 int mp_bluetooth_gatts_db_resize(mp_gatts_db_t db, uint16_t handle, size_t len, bool append);
 

extmod/nimble/modbluetooth_nimble.c

@@ -1008,7 +1008,7 @@ int mp_bluetooth_gap_disconnect(uint16_t conn_handle) {
     return ble_hs_err_to_errno(ble_gap_terminate(conn_handle, BLE_ERR_REM_USER_CONN_TERM));
 }
 
-int mp_bluetooth_gatts_read(uint16_t value_handle, uint8_t **value, size_t *value_len) {
+int mp_bluetooth_gatts_read(uint16_t value_handle, const uint8_t **value, size_t *value_len) {
     if (!mp_bluetooth_is_active()) {
         return ERRNO_BLUETOOTH_NOT_ACTIVE;
     }
@@ -1026,35 +1026,40 @@ int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t
     return err;
 }
 
-// TODO: Could use ble_gatts_chr_updated to send to all subscribed centrals.
-
-int mp_bluetooth_gatts_notify(uint16_t conn_handle, uint16_t value_handle) {
+int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len) {
     if (!mp_bluetooth_is_active()) {
         return ERRNO_BLUETOOTH_NOT_ACTIVE;
     }
-    // Confusingly, notify/notify_custom/indicate are "gattc" function (even though they're used by peripherals (i.e. gatt servers)).
-    // See https://www.mail-archive.com/[email protected]/msg01293.html
-    return ble_hs_err_to_errno(ble_gattc_notify(conn_handle, value_handle));
-}
 
-int mp_bluetooth_gatts_notify_send(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len) {
-    if (!mp_bluetooth_is_active()) {
-        return ERRNO_BLUETOOTH_NOT_ACTIVE;
-    }
-    struct os_mbuf *om = ble_hs_mbuf_from_flat(value, value_len);
-    if (om == NULL) {
-        return MP_ENOMEM;
+    int err = BLE_HS_EINVAL;
+
+    // NULL om in the _custom methods means "use DB value" (NimBLE will call
+    // back into mp_bluetooth_gatts_read for us).
+    struct os_mbuf *om = NULL;
+
+    if (value) {
+        om = ble_hs_mbuf_from_flat(value, value_len);
+        if (om == NULL) {
+            return MP_ENOMEM;
+        }
     }
-    return ble_hs_err_to_errno(ble_gattc_notify_custom(conn_handle, value_handle, om));
-}
 
-int mp_bluetooth_gatts_indicate(uint16_t conn_handle, uint16_t value_handle) {
-    if (!mp_bluetooth_is_active()) {
-        return ERRNO_BLUETOOTH_NOT_ACTIVE;
+    // Note: Confusingly, Nimble's notify/notify_custom and indicate/indicate_custom
+    // are "gattc" functions (even though they're used by peripherals, i.e. gatt servers).
+    // See https://www.mail-archive.com/[email protected]/msg01293.html
+
+    switch (gatts_op) {
+        case MP_BLUETOOTH_GATTS_OP_NOTIFY:
+            err = ble_gattc_notify_custom(conn_handle, value_handle, om);
+            break;
+        case MP_BLUETOOTH_GATTS_OP_INDICATE:
+            // This will raise BLE_GAP_EVENT_NOTIFY_TX with a status when it is
+            // acknowledged (or timeout/error).
+            err = ble_gattc_indicate_custom(conn_handle, value_handle, om);
+            break;
     }
-    // This will raise BLE_GAP_EVENT_NOTIFY_TX with a status when it is
-    // acknowledged (or timeout/error).
-    return ble_hs_err_to_errno(ble_gattc_indicate(conn_handle, value_handle));
+
+    return ble_hs_err_to_errno(err);
 }
 
 int mp_bluetooth_gatts_set_buffer(uint16_t value_handle, size_t len, bool append) {