Implement BLE Manager Shutdown for nimble host (project-chip#33109)

* [ESP32] Implement BLE Manager Shutdown for nimble host - Replace ble deinit imple in Esp32AppServer with BLEMgr().Shutdown() - Replace few ESP_LOG with ChipLog in Esp32AppServer - Move ble deinit kCommissioningComplete switch case - Make USE_BLE_ONLY_FOR_COMMISSIONING depends on BT_ENABLED * Restyled by clang-format * address reviews * Add checks for timer handler --------- Co-authored-by: Restyled.io <[email protected]>
Jerry-ESP · Apr 25, 2024 · b80b867 · b80b867
1 parent cb9f02f
commit b80b867
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 48 deletions.
diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig
@@ -632,12 +632,13 @@ menu "CHIP Device Layer"
                 When set, WoBLE advertisements will stop while a WoBLE connection is active.
 
         config USE_BLE_ONLY_FOR_COMMISSIONING
-	    bool "Use BLE only for commissioning"
-	    default y
-	    help
-            Disable this flag if BLE is used for any other purpose than commissioning.
-            When enabled, it deinitialized the BLE on successful commissioning, and on
-            bootup do not initialize the BLE if device is already provisioned with Wi-Fi/Thread credentials.
+            depends on BT_ENABLED
+            bool "Use BLE only for commissioning"
+            default y
+            help
+                Disable this flag if BLE is used for any other purpose than commissioning.
+                When enabled, it deinitialized the BLE on successful commissioning, and on
+                bootup do not initialize the BLE if device is already provisioned with Wi-Fi/Thread credentials.
 
     endmenu
 

diff --git a/examples/platform/esp32/common/CommonDeviceCallbacks.cpp b/examples/platform/esp32/common/CommonDeviceCallbacks.cpp
@@ -51,7 +51,6 @@ void CommonDeviceCallbacks::DeviceEventCallback(const ChipDeviceEvent * event, i
 
     case DeviceEventType::kCHIPoBLEConnectionClosed:
         ESP_LOGI(TAG, "CHIPoBLE disconnected");
-        Esp32AppServer::DeInitBLEIfCommissioned();
         break;
 
     case DeviceEventType::kDnssdInitialized:
@@ -67,6 +66,7 @@ void CommonDeviceCallbacks::DeviceEventCallback(const ChipDeviceEvent * event, i
 
     case DeviceEventType::kCommissioningComplete: {
         ESP_LOGI(TAG, "Commissioning complete");
+        Esp32AppServer::DeInitBLEIfCommissioned();
     }
     break;
 

diff --git a/examples/platform/esp32/common/Esp32AppServer.cpp b/examples/platform/esp32/common/Esp32AppServer.cpp
@@ -115,46 +115,12 @@ static size_t hex_string_to_binary(const char * hex_string, uint8_t * buf, size_
 
 void Esp32AppServer::DeInitBLEIfCommissioned(void)
 {
-#if CONFIG_BT_ENABLED && CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING
+#ifdef CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING
     if (chip::Server::GetInstance().GetFabricTable().FabricCount() > 0)
     {
-        esp_err_t err = ESP_OK;
-
-#if CONFIG_BT_NIMBLE_ENABLED
-        if (!ble_hs_is_enabled())
-        {
-            ESP_LOGI(TAG, "BLE already deinited");
-            return;
-        }
-        if (nimble_port_stop() != 0)
-        {
-            ESP_LOGE(TAG, "nimble_port_stop() failed");
-            return;
-        }
-        vTaskDelay(100);
-        nimble_port_deinit();
-
-#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)
-        err = esp_nimble_hci_and_controller_deinit();
-#endif
-#endif /* CONFIG_BT_NIMBLE_ENABLED */
-
-#if CONFIG_IDF_TARGET_ESP32
-        err |= esp_bt_mem_release(ESP_BT_MODE_BTDM);
-#elif CONFIG_IDF_TARGET_ESP32C2 || CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32S3 || CONFIG_IDF_TARGET_ESP32H2
-        err |= esp_bt_mem_release(ESP_BT_MODE_BLE);
-#endif
-
-        if (err != ESP_OK)
-        {
-            ESP_LOGE(TAG, "BLE deinit failed");
-        }
-        else
-        {
-            ESP_LOGI(TAG, "BLE deinit successful and memory reclaimed");
-        }
+        chip::DeviceLayer::Internal::BLEMgr().Shutdown();
     }
-#endif /* CONFIG_BT_ENABLED && CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING */
+#endif /* CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING */
 }
 
 void Esp32AppServer::Init(AppDelegate * sAppDelegate)
@@ -165,7 +131,7 @@ void Esp32AppServer::Init(AppDelegate * sAppDelegate)
     if (hex_string_to_binary(CONFIG_TEST_EVENT_TRIGGER_ENABLE_KEY, sTestEventTriggerEnableKey,
                              sizeof(sTestEventTriggerEnableKey)) == 0)
     {
-        ESP_LOGE(TAG, "Failed to convert the EnableKey string to octstr type value");
+        ChipLogError(DeviceLayer, "Failed to convert the EnableKey string to octstr type value");
         memset(sTestEventTriggerEnableKey, 0, sizeof(sTestEventTriggerEnableKey));
     }
     static OTATestEventTriggerDelegate testEventTriggerDelegate{ ByteSpan(sTestEventTriggerEnableKey) };
@@ -189,7 +155,7 @@ void Esp32AppServer::Init(AppDelegate * sAppDelegate)
     if (chip::DeviceLayer::ConnectivityMgr().IsThreadProvisioned() &&
         (chip::Server::GetInstance().GetFabricTable().FabricCount() != 0))
     {
-        ESP_LOGI(TAG, "Thread has been provisioned, publish the dns service now");
+        ChipLogProgress(DeviceLayer, "Thread has been provisioned, publish the dns service now");
         chip::app::DnssdServer::Instance().StartServer();
     }
 #endif

diff --git a/src/platform/ESP32/BLEManagerImpl.h b/src/platform/ESP32/BLEManagerImpl.h
@@ -156,7 +156,7 @@ class BLEManagerImpl final : public BLEManager,
     // ===== Members that implement the BLEManager internal interface.
 
     CHIP_ERROR _Init(void);
-    void _Shutdown() {}
+    void _Shutdown();
     bool _IsAdvertisingEnabled(void);
     CHIP_ERROR _SetAdvertisingEnabled(bool val);
     bool _IsAdvertising(void);
@@ -296,6 +296,7 @@ class BLEManagerImpl final : public BLEManager,
 
     void DriveBLEState(void);
     CHIP_ERROR InitESPBleLayer(void);
+    void DeinitESPBleLayer(void);
     CHIP_ERROR ConfigureAdvertisingData(void);
     CHIP_ERROR StartAdvertising(void);
 
@@ -330,6 +331,9 @@ class BLEManagerImpl final : public BLEManager,
     static void HandleGAPEvent(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t * param);
 
 #elif CONFIG_BT_NIMBLE_ENABLED
+    CHIP_ERROR DeinitBLE();
+    static void ClaimBLEMemory(System::Layer *, void *);
+
     void HandleRXCharRead(struct ble_gatt_char_context * param);
     void HandleRXCharWrite(struct ble_gatt_char_context * param);
     void HandleTXCharWrite(struct ble_gatt_char_context * param);

diff --git a/src/platform/ESP32/nimble/BLEManagerImpl.cpp b/src/platform/ESP32/nimble/BLEManagerImpl.cpp
@@ -246,6 +246,21 @@ CHIP_ERROR BLEManagerImpl::_Init()
     return err;
 }
 
+void BLEManagerImpl::_Shutdown()
+{
+    BleLayer::Shutdown();
+
+    // selectively setting kGATTServiceStarted flag, in order to notify the state machine to stop the CHIPoBLE GATT service
+    mFlags.ClearAll().Set(Flags::kGATTServiceStarted);
+    mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled;
+
+#if CONFIG_ENABLE_ESP32_BLE_CONTROLLER
+    OnChipBleConnectReceived = nullptr;
+#endif // CONFIG_ENABLE_ESP32_BLE_CONTROLLER
+
+    PlatformMgr().ScheduleWork(DriveBLEState, 0);
+}
+
 CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
@@ -806,7 +821,8 @@ void BLEManagerImpl::DriveBLEState(void)
     // Stop the CHIPoBLE GATT service if needed.
     if (mServiceMode != ConnectivityManager::kCHIPoBLEServiceMode_Enabled && mFlags.Has(Flags::kGATTServiceStarted))
     {
-        // TODO: Not supported
+        DeinitESPBleLayer();
+        mFlags.ClearAll();
     }
 
 exit:
@@ -932,6 +948,56 @@ CHIP_ERROR BLEManagerImpl::InitESPBleLayer(void)
     return err;
 }
 
+void BLEManagerImpl::DeinitESPBleLayer()
+{
+    VerifyOrReturn(DeinitBLE() == CHIP_NO_ERROR);
+    BLEManagerImpl::ClaimBLEMemory(nullptr, nullptr);
+}
+
+void BLEManagerImpl::ClaimBLEMemory(System::Layer *, void *)
+{
+    TaskHandle_t handle = xTaskGetHandle("nimble_host");
+    if (handle)
+    {
+        ChipLogDetail(DeviceLayer, "Schedule ble memory reclaiming since nimble host is still running");
+
+        // Rescheduling it for later, 2 seconds is an arbitrary value, keeping it a bit more so that
+        // we dont have to reschedule it again
+        SystemLayer().StartTimer(System::Clock::Seconds32(2), ClaimBLEMemory, nullptr);
+    }
+    else
+    {
+        // Free up all the space occupied by ble and add it to heap
+        esp_err_t err = ESP_OK;
+
+#if CONFIG_IDF_TARGET_ESP32
+        err = esp_bt_mem_release(ESP_BT_MODE_BTDM);
+#elif CONFIG_IDF_TARGET_ESP32C2 || CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32S3 || CONFIG_IDF_TARGET_ESP32H2 ||          \
+    CONFIG_IDF_TARGET_ESP32C6
+        err = esp_bt_mem_release(ESP_BT_MODE_BLE);
+#endif
+
+        VerifyOrReturn(err == ESP_OK, ChipLogError(DeviceLayer, "BLE deinit failed"));
+        ChipLogProgress(DeviceLayer, "BLE deinit successful and memory reclaimed");
+        // TODO: post an event when ble is deinitialized and memory is added to heap
+    }
+}
+
+CHIP_ERROR BLEManagerImpl::DeinitBLE()
+{
+    VerifyOrReturnError(ble_hs_is_enabled(), CHIP_ERROR_INCORRECT_STATE, ChipLogProgress(DeviceLayer, "BLE already deinited"));
+    VerifyOrReturnError(0 == nimble_port_stop(), MapBLEError(ESP_FAIL), ChipLogError(DeviceLayer, "nimble_port_stop() failed"));
+
+    esp_err_t err = nimble_port_deinit();
+    VerifyOrReturnError(err == ESP_OK, MapBLEError(err));
+
+#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)
+    err = esp_nimble_hci_and_controller_deinit();
+#endif
+
+    return MapBLEError(err);
+}
+
 CHIP_ERROR BLEManagerImpl::ConfigureAdvertisingData(void)
 {
     CHIP_ERROR err;