addressing review comments

chirag-silabs · chirag-silabs · commit 7ddd0df61259 · 2024-09-17T22:47:33.000+05:30
diff --git a/src/platform/silabs/BLEManagerImpl.h b/src/platform/silabs/BLEManagerImpl.h
@@ -59,6 +59,8 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla
     void HandleBootEvent(void);
 
 #if (SLI_SI91X_ENABLE_BLE || RSI_BLE_ENABLE)
+    // Used for posting the event in the BLE queue
+    void BlePostEvent(BleEvent_t * event);
     void HandleConnectEvent(sl_wfx_msg_t * evt);
     void HandleConnectionCloseEvent(sl_wfx_msg_t * evt);
     void HandleWriteEvent(sl_wfx_msg_t * evt);
@@ -94,6 +96,14 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla
     // the implementation methods provided by this class.
     friend BLEManager;
 
+#if (SLI_SI91X_ENABLE_BLE || RSI_BLE_ENABLE)
+    // rs91x BLE task handling 
+    osMessageQueueId_t sBleEventQueue = NULL;
+    static void sl_ble_event_handling_task(void * args);
+    void sl_ble_init();
+    void ProcessEvent(BleEvent_t inEvent);
+#endif
+
     // ===== Members that implement the BLEManager internal interface.
 
     CHIP_ERROR _Init(void);
diff --git a/src/platform/silabs/rs911x/BLEManagerImpl.cpp b/src/platform/silabs/rs911x/BLEManagerImpl.cpp
@@ -80,32 +80,43 @@ using namespace ::chip;
 using namespace ::chip::Ble;
 using namespace ::chip::DeviceLayer::Internal;
 
-void sl_ble_init()
-{
-    uint8_t randomAddrBLE[RSI_BLE_ADDR_LENGTH] = { 0 };
-    uint64_t randomAddr                        = chip::Crypto::GetRandU64();
-    memcpy(randomAddrBLE, &randomAddr, RSI_BLE_ADDR_LENGTH);
-    // Set the two least significant bits as the first 2 bits of the address has to be '11' to ensure the address is a random
-    // non-resolvable private address
-    randomAddrBLE[(RSI_BLE_ADDR_LENGTH - 1)] |= 0xC0;
+namespace chip {
+namespace DeviceLayer {
+namespace Internal {
 
-    // registering the GAP callback functions
-    rsi_ble_gap_register_callbacks(NULL, NULL, rsi_ble_on_disconnect_event, NULL, NULL, NULL, rsi_ble_on_enhance_conn_status_event,
-                                   NULL, NULL, NULL);
+namespace {
 
-    // registering the GATT call back functions
-    rsi_ble_gatt_register_callbacks(NULL, NULL, NULL, NULL, NULL, NULL, NULL, rsi_ble_on_gatt_write_event, NULL, NULL,
-                                    rsi_ble_on_read_req_event, rsi_ble_on_mtu_event, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                                    NULL, rsi_ble_on_event_indication_confirmation, NULL);
+#define CHIP_ADV_DATA_TYPE_FLAGS 0x01
+#define CHIP_ADV_DATA_TYPE_UUID 0x03
+#define CHIP_ADV_DATA_TYPE_NAME 0x09
+#define CHIP_ADV_DATA_TYPE_SERVICE_DATA 0x16
 
-    //  Exchange of GATT info with BLE stack
-    rsi_ble_add_matter_service();
-    rsi_ble_set_random_address_with_value(randomAddrBLE);
-    InitBleEventQueue();
-    chip::DeviceLayer::Internal::BLEMgrImpl().HandleBootEvent();
-}
+#define CHIP_ADV_DATA_FLAGS 0x06
+
+#define CHIP_ADV_DATA 0
+#define CHIP_ADV_SCAN_RESPONSE_DATA 1
+#define CHIP_ADV_SHORT_UUID_LEN 2
+
+#define MAX_RESPONSE_DATA_LEN 31
+#define MAX_ADV_DATA_LEN 31
+
+// Timer Frequency used.
+#define TIMER_CLK_FREQ ((uint32_t) 32768)
+
+// Convert msec to timer ticks.
+#define TIMER_MS_2_TIMERTICK(ms) ((TIMER_CLK_FREQ * ms) / 1000)
+#define TIMER_S_2_TIMERTICK(s) (TIMER_CLK_FREQ * s)
 
-void ProcessEvent(BleEvent_t inEvent)
+const uint8_t UUID_CHIPoBLEService[]      = { 0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
+                                              0x00, 0x10, 0x00, 0x00, 0xF6, 0xFF, 0x00, 0x00 };
+const uint8_t ShortUUID_CHIPoBLEService[] = { 0xF6, 0xFF };
+
+} // namespace
+
+BLEManagerImpl BLEManagerImpl::sInstance;
+
+
+void BLEManagerImpl::ProcessEvent(BleEvent_t inEvent)
 {
     switch (inEvent.eventType)
     {
@@ -154,7 +165,17 @@ void ProcessEvent(BleEvent_t inEvent)
     }
 }
 
-void sl_ble_event_handling_task(void * args)
+void BLEManagerImpl::BlePostEvent(BleEvent_t * event)
+{
+    sl_status_t status = osMessageQueuePut(sInstance.sBleEventQueue, event, 0, 0);
+    if (status != osOK)
+    {
+        ChipLogError(DeviceLayer, "BlePostEvent: failed to post event: 0x%lx", status);
+        // TODO: Handle error, requeue event depending on queue size or notify relevant task, Chipdie, etc.
+    }
+}
+
+void BLEManagerImpl::sl_ble_event_handling_task(void * args)
 {
     sl_status_t status;
     BleEvent_t bleEvent;
@@ -163,15 +184,15 @@ void sl_ble_event_handling_task(void * args)
     osSemaphoreAcquire(sl_rs_ble_init_sem, osWaitForever);
 
     // This function initialize BLE and start BLE advertisement.
-    sl_ble_init();
+    sInstance.sl_ble_init();
 
     // Application event map
     while (1)
     {
-        status = osMessageQueueGet(GetBleEventQueue(), &bleEvent, NULL, osWaitForever);
+        status = osMessageQueueGet(sInstance.sBleEventQueue, &bleEvent, NULL, osWaitForever);
         if (status == osOK)
         {
-            ProcessEvent(bleEvent);
+            sInstance.ProcessEvent(bleEvent);
         }
         else
         {
@@ -180,48 +201,41 @@ void sl_ble_event_handling_task(void * args)
     }
 }
 
-namespace chip {
-namespace DeviceLayer {
-namespace Internal {
-
-namespace {
-
-#define CHIP_ADV_DATA_TYPE_FLAGS 0x01
-#define CHIP_ADV_DATA_TYPE_UUID 0x03
-#define CHIP_ADV_DATA_TYPE_NAME 0x09
-#define CHIP_ADV_DATA_TYPE_SERVICE_DATA 0x16
-
-#define CHIP_ADV_DATA_FLAGS 0x06
-
-#define CHIP_ADV_DATA 0
-#define CHIP_ADV_SCAN_RESPONSE_DATA 1
-#define CHIP_ADV_SHORT_UUID_LEN 2
-
-#define MAX_RESPONSE_DATA_LEN 31
-#define MAX_ADV_DATA_LEN 31
-
-// Timer Frequency used.
-#define TIMER_CLK_FREQ ((uint32_t) 32768)
+void BLEManagerImpl::sl_ble_init()
+{
+    uint8_t randomAddrBLE[RSI_BLE_ADDR_LENGTH] = { 0 };
+    uint64_t randomAddr                        = chip::Crypto::GetRandU64();
+    memcpy(randomAddrBLE, &randomAddr, RSI_BLE_ADDR_LENGTH);
+    // Set the two least significant bits as the first 2 bits of the address has to be '11' to ensure the address is a random
+    // non-resolvable private address
+    randomAddrBLE[(RSI_BLE_ADDR_LENGTH - 1)] |= 0xC0;
 
-// Convert msec to timer ticks.
-#define TIMER_MS_2_TIMERTICK(ms) ((TIMER_CLK_FREQ * ms) / 1000)
-#define TIMER_S_2_TIMERTICK(s) (TIMER_CLK_FREQ * s)
+    // registering the GAP callback functions
+    rsi_ble_gap_register_callbacks(NULL, NULL, rsi_ble_on_disconnect_event, NULL, NULL, NULL, rsi_ble_on_enhance_conn_status_event,
+                                   NULL, NULL, NULL);
 
-const uint8_t UUID_CHIPoBLEService[]      = { 0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
-                                              0x00, 0x10, 0x00, 0x00, 0xF6, 0xFF, 0x00, 0x00 };
-const uint8_t ShortUUID_CHIPoBLEService[] = { 0xF6, 0xFF };
+    // registering the GATT call back functions
+    rsi_ble_gatt_register_callbacks(NULL, NULL, NULL, NULL, NULL, NULL, NULL, rsi_ble_on_gatt_write_event, NULL, NULL,
+                                    rsi_ble_on_read_req_event, rsi_ble_on_mtu_event, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+                                    NULL, rsi_ble_on_event_indication_confirmation, NULL);
 
-} // namespace
+    //  Exchange of GATT info with BLE stack
+    rsi_ble_add_matter_service();
+    rsi_ble_set_random_address_with_value(randomAddrBLE);
+    
+    sInstance.sBleEventQueue = osMessageQueueNew(WFX_QUEUE_SIZE, sizeof(WfxEvent_t), NULL);
+    VerifyOrDie(sInstance.sBleEventQueue != nullptr);
 
-BLEManagerImpl BLEManagerImpl::sInstance;
+    chip::DeviceLayer::Internal::BLEMgrImpl().HandleBootEvent();
+}
 
 CHIP_ERROR BLEManagerImpl::_Init()
 {
     CHIP_ERROR err;
 
     sl_rs_ble_init_sem = osSemaphoreNew(1, 0, NULL);
 
-    sBleThread = osThreadNew(sl_ble_event_handling_task, NULL, &kBleTaskAttr);
+    sBleThread = osThreadNew(sInstance.sl_ble_event_handling_task, NULL, &kBleTaskAttr);
 
     VerifyOrReturnError(sBleThread != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
diff --git a/src/platform/silabs/rs911x/wfx_sl_ble_init.cpp b/src/platform/silabs/rs911x/wfx_sl_ble_init.cpp
@@ -22,6 +22,8 @@
  */
 #include <lib/support/logging/CHIPLogging.h>
 #include <platform/CHIPDeviceLayer.h>
+// #include <platform/internal/BLEManager.h>
+#include <platform/silabs/BLEManagerImpl.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -32,30 +34,7 @@ extern "C" {
 #endif
 
 // Global Variables
-rsi_ble_t att_list;
 BleEvent_t bleEvent;
-static osMessageQueueId_t sBleEventQueue = NULL;
-
-void InitBleEventQueue()
-{
-    sBleEventQueue = osMessageQueueNew(WFX_QUEUE_SIZE, sizeof(WfxEvent_t), NULL);
-    VerifyOrDie(sBleEventQueue != nullptr);
-}
-
-osMessageQueueId_t GetBleEventQueue()
-{
-    return sBleEventQueue;
-}
-
-void BlePostEvent(BleEvent_t * event)
-{
-    sl_status_t status = osMessageQueuePut(sBleEventQueue, event, 0, 0);
-    if (status != osOK)
-    {
-        ChipLogError(DeviceLayer, "BlePostEvent: failed to post event: 0x%lx", status);
-        // TODO: Handle error, requeue event depending on queue size or notify relevant task, Chipdie, etc.
-    }
-}
 
 /*==============================================*/
 /**
@@ -70,7 +49,7 @@ void rsi_ble_on_mtu_event(rsi_ble_event_mtu_t * rsi_ble_mtu)
 {
     bleEvent.eventType = RSI_BLE_MTU_EVENT;
     memcpy(&bleEvent.eventData->rsi_ble_mtu, rsi_ble_mtu, sizeof(rsi_ble_event_mtu_t));
-    BlePostEvent(&bleEvent);
+    chip::DeviceLayer::Internal::BLEMgrImpl().BlePostEvent(&bleEvent);
 }
 
 /*==============================================*/
@@ -88,7 +67,7 @@ void rsi_ble_on_gatt_write_event(uint16_t event_id, rsi_ble_event_write_t * rsi_
     bleEvent.eventType           = RSI_BLE_GATT_WRITE_EVENT;
     bleEvent.eventData->event_id = event_id;
     memcpy(&bleEvent.eventData->rsi_ble_write, rsi_ble_write, sizeof(rsi_ble_event_write_t));
-    BlePostEvent(&bleEvent);
+    chip::DeviceLayer::Internal::BLEMgrImpl().BlePostEvent(&bleEvent);
 }
 
 /*==============================================*/
@@ -106,7 +85,7 @@ void rsi_ble_on_enhance_conn_status_event(rsi_ble_event_enhance_conn_status_t *
     bleEvent.eventData->connectionHandle = 1;
     bleEvent.eventData->bondingHandle    = 255;
     memcpy(bleEvent.eventData->resp_enh_conn.dev_addr, resp_enh_conn->dev_addr, RSI_DEV_ADDR_LEN);
-    BlePostEvent(&bleEvent);
+    chip::DeviceLayer::Internal::BLEMgrImpl().BlePostEvent(&bleEvent);
 }
 
 /*==============================================*/
@@ -123,7 +102,7 @@ void rsi_ble_on_disconnect_event(rsi_ble_event_disconnect_t * resp_disconnect, u
 {
     bleEvent.eventType         = RSI_BLE_DISCONN_EVENT;
     bleEvent.eventData->reason = reason;
-    BlePostEvent(&bleEvent);
+    chip::DeviceLayer::Internal::BLEMgrImpl().BlePostEvent(&bleEvent);
 }
 
 /*==============================================*/
@@ -140,7 +119,7 @@ void rsi_ble_on_event_indication_confirmation(uint16_t resp_status, rsi_ble_set_
     bleEvent.eventType              = RSI_BLE_GATT_INDICATION_CONFIRMATION;
     bleEvent.eventData->resp_status = resp_status;
     memcpy(&bleEvent.eventData->rsi_ble_event_set_att_rsp, rsi_ble_event_set_att_rsp, sizeof(rsi_ble_set_att_resp_t));
-    BlePostEvent(&bleEvent);
+    chip::DeviceLayer::Internal::BLEMgrImpl().BlePostEvent(&bleEvent);
 }
 
 /*==============================================*/
@@ -158,7 +137,7 @@ void rsi_ble_on_read_req_event(uint16_t event_id, rsi_ble_read_req_t * rsi_ble_r
     bleEvent.eventType           = RSI_BLE_EVENT_GATT_RD;
     bleEvent.eventData->event_id = event_id;
     memcpy(&bleEvent.eventData->rsi_ble_read_req, rsi_ble_read_req, sizeof(rsi_ble_read_req_t));
-    BlePostEvent(&bleEvent);
+    chip::DeviceLayer::Internal::BLEMgrImpl().BlePostEvent(&bleEvent);
 }
 
 /*==============================================*/
@@ -260,7 +239,7 @@ void rsi_ble_add_char_val_att(void * serv_handler, uint16_t handle, uuid_t att_t
                               uint8_t data_len, uint8_t auth_read)
 {
     rsi_ble_req_add_att_t new_att = { 0 };
-
+    rsi_ble_t att_list;
     memset(&new_att, 0, sizeof(rsi_ble_req_add_att_t));
     //! preparing the attributes
     new_att.serv_handler  = serv_handler;
