nRF5x: Ensure Bluetooth notifications work correctly when two separate connections use the same handle for their characteristics

            nRF5x: Remove handlers from our handlers array when a device is disconnected

Author: gfwilliams

ChangeLog

@@ -1,4 +1,5 @@
-            ...
+            nRF5x: Ensure Bluetooth notifications work correctly when two separate connections use the same handle for their characteristics
+            nRF5x: Remove handlers from our handlers array when a device is disconnected
 
      2v26 : nRF5x: ensure TIMER1_IRQHandler doesn't always wake idle loop up (fix #1900)
             Puck.js: On v2.1 ensure Puck.mag behaves like other variants - just returning the last reading (avoids glitches when used with Puck.magOn)

libs/bluetooth/bluetooth.h

@@ -214,6 +214,10 @@ typedef enum {
 #endif
 } BLEPending;
 
+/// amount to shift the connection index for the 16 bit 'data' field in jsble_queue_pending_buf(BLEP_CENTRAL_NOTIFICATION, ...
+#define BLEP_CENTRAL_NOTIFICATION_CONN_SHIFT (15)
+/// we need to mask off the handle (as we're using the top bits for the connection)
+#define BLEP_CENTRAL_NOTIFICATION_HANDLE_MASK (0x7FFF)
 
 extern volatile BLEStatus bleStatus;
 /// Filter to use when discovering BLE Services/Characteristics

libs/bluetooth/bluetooth_utils.c

@@ -539,6 +539,19 @@ bool jsble_exec_pending_common(BLEPending blep, uint16_t data, unsigned char *bu
       jsvUnLock2(gattServer, bluetoothDevice);
     }
     bleSetActiveBluetoothGattServer(centralIdx, 0);
+    // when we disconnect, remove handles for notifications for this connection
+    JsVar *handles = jsvObjectGetChildIfExists(execInfo.hiddenRoot, "bleHdl");
+    JsvObjectIterator it;
+    jsvObjectIteratorNew(&it, handles); // it's actually an array, but object iterator is ok
+    while (jsvObjectIteratorHasValue(&it)) {
+      int handleValue = jsvGetIntegerAndUnLock(jsvObjectIteratorGetKey(&it));
+      if ((handleValue >> BLEP_CENTRAL_NOTIFICATION_CONN_SHIFT) == centralIdx)
+        jsvObjectIteratorRemoveAndGotoNext(&it, handles);
+      else
+        jsvObjectIteratorNext(&it);
+    }
+    jsvObjectIteratorFree(&it);
+    jsvUnLock(handles);
     break;
   }
   case BLEP_CENTRAL_NOTIFICATION: {

libs/bluetooth/jswrap_bluetooth.c

@@ -4655,7 +4655,7 @@ JsVar *jswrap_ble_BluetoothRemoteGATTCharacteristic_startNotifications(JsVar *ch
   uint16_t handle = (uint16_t)jsvObjectGetIntegerChild(characteristic, "handle_value");
   JsVar *handles = jsvObjectGetChild(execInfo.hiddenRoot, "bleHdl", JSV_ARRAY);
   if (handles) {
-    jsvSetArrayItem(handles, handle, characteristic);
+    jsvSetArrayItem(handles, handle | (jsble_get_central_connection_idx(central_conn_handle) << BLEP_CENTRAL_NOTIFICATION_CONN_SHIFT), characteristic);
     jsvUnLock(handles);
   }
 
@@ -4705,7 +4705,7 @@ JsVar *jswrap_ble_BluetoothRemoteGATTCharacteristic_stopNotifications(JsVar *cha
   uint16_t handle = (uint16_t)jsvObjectGetIntegerChild(characteristic, "handle_value");
   JsVar *handles = jsvObjectGetChild(execInfo.hiddenRoot, "bleHdl", JSV_ARRAY);
   if (handles) {
-    jsvSetArrayItem(handles, handle, 0);
+    jsvSetArrayItem(handles, handle + (jsble_get_central_connection_idx(central_conn_handle) << BLEP_CENTRAL_NOTIFICATION_CONN_SHIFT), 0);
     jsvUnLock(handles);
   }
   JsVar *promise = jsvLockAgainSafe(blePromise);

targets/esp32/BLE/esp32_gattc_func.c

@@ -97,6 +97,7 @@ void gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp
     case ESP_GATTC_NOTIFY_EVT:
       // We've been notified of new data
       // p_data->notify.is_notify is whether it's notify or indicate
+      // FIXME: for >1 connection we need to add (jsble_get_central_connection_idx(central_conn_handle) << BLEP_CENTRAL_NOTIFICATION_CONN_SHIFT) to this
       jsble_queue_pending_buf(BLEP_CENTRAL_NOTIFICATION, p_data->notify.handle, (char*)p_data->notify.value, p_data->notify.value_len);
       // Do we have to send a confirmation if it's an indication?
       break;

targets/nrf5x/bluetooth.c

@@ -1623,7 +1623,7 @@ static void ble_evt_handler(ble_evt_t const * p_ble_evt, void * p_context) {
         // Notification/Indication
         const ble_gattc_evt_hvx_t *p_hvx = &p_ble_evt->evt.gattc_evt.params.hvx;
         // p_hvx->type is BLE_GATT_HVX_NOTIFICATION or BLE_GATT_HVX_INDICATION
-        jsble_queue_pending_buf(BLEP_CENTRAL_NOTIFICATION, p_hvx->handle, (char*)p_hvx->data, p_hvx->len);
+        jsble_queue_pending_buf(BLEP_CENTRAL_NOTIFICATION, p_hvx->handle | (jsble_get_central_connection_idx(p_ble_evt->evt.common_evt.conn_handle) << BLEP_CENTRAL_NOTIFICATION_CONN_SHIFT), (char*)p_hvx->data, p_hvx->len);
         if (p_hvx->type == BLE_GATT_HVX_INDICATION) {
           sd_ble_gattc_hv_confirm(p_ble_evt->evt.gattc_evt.conn_handle, p_hvx->handle);
         }