Merge pull request #14747 from paul-szczepanek-arm/fix-adv-con

BLE: fix advertising set termination event

Author: 0xc0170

connectivity/FEATURE_BLE/include/ble/common/blecommon.h

@@ -216,7 +216,17 @@ enum ble_error_t {
     /**
      * Data not found or there is nothing to return.
      */
-    BLE_ERROR_NOT_FOUND = 13
+    BLE_ERROR_NOT_FOUND = 13,
+
+    /**
+     * Specified timeout expired.
+     */
+    BLE_ERROR_TIMEOUT = 14,
+
+    /**
+     * Specified limit expired.
+     */
+    BLE_ERROR_LIMIT_REACHED = 15
 };
 
 /**

connectivity/FEATURE_BLE/include/ble/gap/Events.h

@@ -627,10 +627,15 @@ struct AdvertisingEndEvent {
     /** Create an extended advertising end event.
      *
      * @param advHandle Advertising set handle.
-     * @param connection Connection handle.
-     * @param completed_events Number of events created during before advertising end.
+     * @param connection Connection handle - only valid if connected is True.
+     * @param completed_events Number of events created during before advertising end - only valid
+     *        if advertising end has been caused by BLE_ERROR_LIMIT_REACHED, not the local user.
+     *        Check getStatus().
      * @param connected True if connection has been established.
-     * @param status Error code if stop command failed.
+     * @param status Error code showing the reason for event. BLE_ERROR_LIMIT_REACHED if set number
+     *        of events have been reached. BLE_ERROR_TIMEOUT if set time has elapsed.
+     *        BLE_ERROR_SUCCESS if connection occurred or user ended the set. Check isConnected()
+     *        to determine which.
      */
     AdvertisingEndEvent(
         advertising_handle_t advHandle,

connectivity/FEATURE_BLE/libraries/cordio_stack/ble-host/sources/stack/dm/dm_adv_ae.c

@@ -31,6 +31,7 @@
 #include "dm_adv.h"
 #include "dm_dev.h"
 #include "dm_main.h"
+#include "dm_conn.h"
 
 /**************************************************************************************************
   Macros
@@ -1215,15 +1216,20 @@ void dmExtAdvActHciEnableCmpl(hciEvt_t *pEvent)
   dmEvt_t dmMsg;
 
   memcpy(&dmMsg, &pEvent->hdr, sizeof(wsfMsgHdr_t));
-  dmMsg.advSetStart.numSets = 0;
+
+  /* we have to handle stopping and starting separately as it uses the same message memory */
 
   for ( i= 0; i < DM_NUM_ADV_SETS; i++)
   {
     switch(dmAdvCb.advState[i])
     {
     case DM_ADV_STATE_STOPPING:
     case DM_ADV_STATE_STOPPING_DIRECTED:
+      /* prepare the message for callback */
+      dmMsg.advSetStop.handle = DM_ADV_HCI_HANDLE_NONE;
+      dmMsg.advSetStop.status = dmMsg.hdr.status;
       dmMsg.advSetStop.advHandle = i;
+
       advType = dmAdvCb.advType[i];
 
       if (dmMsg.hdr.status == HCI_SUCCESS)
@@ -1243,10 +1249,24 @@ void dmExtAdvActHciEnableCmpl(hciEvt_t *pEvent)
       /* if not connectable directed advertising */
       if ((advType != DM_ADV_NONE) && !DM_ADV_CONN_DIRECTED(advType))
       {
-        cbackEvent = DM_ADV_SET_STOP_IND;
+        /* we have to dispatch callbacks one by one as msg only has space for one set of parameters */
+        dmMsg.hdr.event = DM_ADV_SET_STOP_IND;
+        (*dmCb.cback)((dmEvt_t *) &dmMsg);
       }
       break;
 
+    default:
+      break;
+    }
+  }
+
+  /* safe to write as message is only used by starting, removing and clearing does not send a message */
+  dmMsg.advSetStart.numSets = 0;
+
+  for ( i= 0; i < DM_NUM_ADV_SETS; i++)
+  {
+    switch(dmAdvCb.advState[i])
+    {
     case DM_ADV_STATE_STARTING:
     case DM_ADV_STATE_STARTING_DIRECTED:
       dmMsg.advSetStart.advHandle[dmMsg.advSetStart.numSets++] = i;
@@ -1355,6 +1375,13 @@ void dmExtAdvHciHandler(hciEvt_t *pEvent)
       if (!DM_ADV_CONN_DIRECTED(advType))
       {
         pEvent->hdr.event = DM_ADV_SET_STOP_IND;
+        if (pEvent->leAdvSetTerm.status == HCI_SUCCESS) {
+            /* translate the handle to conn id */
+            dmConnCcb_t* ccb = dmConnCcbByHandle(pEvent->leAdvSetTerm.handle);
+            if (ccb) {
+                pEvent->hdr.param = ccb->connId;
+            }
+        }
         (*dmCb.cback)((dmEvt_t *) pEvent);
       }
       /* else if low duty cycle directed advertising failed to create connection */

connectivity/FEATURE_BLE/source/cordio/source/PalGapImpl.cpp

@@ -869,10 +869,17 @@ void PalGap::gap_handler(const wsfMsgHdr_t *msg)
                 break;
             }
 
+            connection_handle_t connection_handle = DM_CONN_ID_NONE;
+            /* the way we distinguish between local close and connection is the invalid HCI conn handle */
+            if (evt->status == HCI_SUCCESS && evt->handle != DM_CONN_HCI_HANDLE_NONE) {
+                /* use the translated conn handle */
+                connection_handle = evt->hdr.param;
+            }
+
             handler->on_advertising_set_terminated(
                 hci_error_code_t(evt->status),
                 evt->advHandle,
-                evt->handle,
+                connection_handle,
                 evt->numComplEvts
             );
         }   break;

connectivity/FEATURE_BLE/source/generic/GapImpl.cpp

@@ -2461,6 +2461,8 @@ ble_error_t Gap::startAdvertising(
 #if BLE_FEATURE_EXTENDED_ADVERTISING
 void Gap::process_enable_queue()
 {
+    _process_enable_queue_pending = false;
+
     tr_info("Evaluating pending advertising sets to be started");
     if (!_advertising_enable_command_params.number_of_handles) {
         /* no set pending to be enabled */
@@ -2506,7 +2508,6 @@ void Gap::process_enable_queue()
     }
 
     _advertising_enable_command_params.number_of_handles = 0;
-    _process_enable_queue_pending = false;
 }
 #endif //BLE_FEATURE_EXTENDED_ADVERTISING
 
@@ -3480,16 +3481,34 @@ void Gap::on_advertising_set_terminated(
             to_string(status),
             number_of_completed_extended_advertising_events);
 
-    _active_sets.clear(advertising_handle);
-    _pending_sets.clear(advertising_handle);
+    ble_error_t error_code = BLE_ERROR_UNSPECIFIED;
+    bool connected = false;
 
-    // If this is part of the address refresh start advertising again.
-    if (_address_refresh_sets.get(advertising_handle) && !connection_handle) {
-        _address_refresh_sets.clear(advertising_handle);
-        tr_info("Part of the address refresh, restarting advertising");
-        startAdvertising(advertising_handle);
-        _adv_started_from_refresh.set(advertising_handle);
-        return;
+    /* translate HCI error into BLE API error code */
+    if (status == hci_error_code_t::SUCCESS) {
+        error_code = BLE_ERROR_NONE;
+        /* self cancelled set will have the handle set to invalid value */
+        if (connection_handle != DM_CONN_ID_NONE) {
+            connected = true;
+        }
+    } else if (status == hci_error_code_t::ADVERTISING_TIMEOUT) {
+        error_code = BLE_ERROR_TIMEOUT;
+    } else if (status == hci_error_code_t::LIMIT_REACHED) {
+        error_code = BLE_ERROR_LIMIT_REACHED;
+    }
+
+    if (error_code != BLE_ERROR_UNSPECIFIED) {
+        _active_sets.clear(advertising_handle);
+        _pending_sets.clear(advertising_handle);
+
+        // If this is part of the address refresh start advertising again.
+        if (_address_refresh_sets.get(advertising_handle) && !connection_handle) {
+            _address_refresh_sets.clear(advertising_handle);
+            tr_info("Part of the address refresh, restarting advertising");
+            startAdvertising(advertising_handle);
+            _adv_started_from_refresh.set(advertising_handle);
+            return;
+        }
     }
 
     /* postpone as other events may still be pending */
@@ -3508,7 +3527,8 @@ void Gap::on_advertising_set_terminated(
             advertising_handle,
             connection_handle,
             number_of_completed_extended_advertising_events,
-            status == hci_error_code_t::SUCCESS
+            connected,
+            error_code
         )
     );
 }

connectivity/FEATURE_BLE/source/pal/PalGap.h

@@ -193,9 +193,9 @@ struct PalGapEventHandler {
 
     /** Called when advertising set stops advertising.
      *
-     * @param status SUCCESS if connection has been established.
+     * @param status SUCCESS if connection has been established or if stopped by user.
      * @param advertising_handle Advertising set handle.
-     * @param advertising_handle Connection handle.
+     * @param advertising_handle Connection handle. Set to invalid handle if no connection made.
      * @param number_of_completed_extended_advertising_events Number of events created during before advertising end.
      */
     virtual void on_advertising_set_terminated(