diff --git a/src/bt-ifce/btrCore_dbus_bluez5.c b/src/bt-ifce/btrCore_dbus_bluez5.c index e17320e..05bd5a6 100644 --- a/src/bt-ifce/btrCore_dbus_bluez5.c +++ b/src/bt-ifce/btrCore_dbus_bluez5.c @@ -4697,7 +4697,6 @@ BtrCore_BTGetPairedDeviceInfo ( DBusMessage* lpDBusMsg = NULL; DBusMessage* lpDBusReply = NULL; DBusMessageIter rootIter; - DBusError lDBusErr; bool adapterFound = FALSE; char* pdeviceInterface = BT_DBUS_BLUEZ_DEVICE_PATH; @@ -4713,15 +4712,16 @@ BtrCore_BTGetPairedDeviceInfo ( int d = 0; - if (!apstBtIfceHdl || !apBtAdapter || !pPairedDeviceInfo) + if (!apstBtIfceHdl || !apBtAdapter || !pPairedDeviceInfo || + !pstlhBtIfce || !pstlhBtIfce->pDBusConn) { + BTRCORELOG_ERROR ("NULL check failed.\n"); return -1; + } - dbus_error_init(&lDBusErr); lpDBusReply = btrCore_BTSendMethodCall(pstlhBtIfce->pDBusConn, "/", DBUS_INTERFACE_OBJECT_MANAGER, "GetManagedObjects"); if (!lpDBusReply) { - BTRCORELOG_ERROR ("org.bluez.Manager.ListAdapters returned an error: '%s'\n", lDBusErr.message); - dbus_error_free(&lDBusErr); + BTRCORELOG_ERROR ("GetManagedObjects call failed\n"); return -1; } @@ -4792,14 +4792,15 @@ BtrCore_BTGetPairedDeviceInfo ( ++b; } else if (DBUS_TYPE_BOOLEAN == dbus_message_iter_get_arg_type(&innerDictEntryIter3)) { - bool *device_prop = FALSE; + dbus_bool_t device_prop = 0; dbus_message_iter_get_basic(&innerDictEntryIter3, &device_prop); if (dbusObject2) { if (strcmp(dbusObject2, "Paired") == 0 && device_prop) { - if(adapter_path) + if ((adapter_path) && (adapter_path[0] != '\0') && (d < BT_MAX_NUM_DEVICE)) { strncpy(&paths[d][0], adapter_path, (strlen(adapter_path) < BT_MAX_DEV_PATH_LEN) ? strlen(adapter_path) : BT_MAX_DEV_PATH_LEN - 1); - ++d; + ++d; + } } } } @@ -4852,33 +4853,45 @@ BtrCore_BTGetPairedDeviceInfo ( for ( i = 0; i < num; i++) { DBusPendingCall* lpDBusPendC = NULL; + if (pPairedDeviceInfo->devicePath[i][0] == '\0') { + BTRCORELOG_INFO("Skipping empty device path at idx %d\n", i); + continue; + } + lpDBusMsg = dbus_message_new_method_call(BT_DBUS_BLUEZ_PATH, pPairedDeviceInfo->devicePath[i], DBUS_INTERFACE_PROPERTIES, "GetAll"); - dbus_message_append_args(lpDBusMsg, DBUS_TYPE_STRING, &pdeviceInterface, DBUS_TYPE_INVALID); - dbus_error_init(&lDBusErr); - - // Check if message creation was successful and the connection is not closed yet. - if (lpDBusMsg == NULL || pstlhBtIfce == NULL || pstlhBtIfce->pDBusConn == NULL) { + if (lpDBusMsg == NULL) { BTRCORELOG_ERROR ("Failed to create message ...\n"); return -1; } + if (!dbus_message_append_args(lpDBusMsg, DBUS_TYPE_STRING, &pdeviceInterface, DBUS_TYPE_INVALID)) { + BTRCORELOG_ERROR ("Failed to append arguments to message ...\n"); + dbus_message_unref(lpDBusMsg); + lpDBusMsg = NULL; + return -1; + } + if (!dbus_connection_send_with_reply(pstlhBtIfce->pDBusConn, lpDBusMsg, &lpDBusPendC, -1)) { BTRCORELOG_ERROR ("failed to send message"); + dbus_message_unref(lpDBusMsg); + lpDBusMsg = NULL; return -1; } dbus_connection_flush(pstlhBtIfce->pDBusConn); dbus_message_unref(lpDBusMsg); + lpDBusMsg = NULL; // CID 342201: Unused value (UNUSED_VALUE) if (lpDBusPendC != NULL) { dbus_pending_call_block(lpDBusPendC); lpDBusReply = dbus_pending_call_steal_reply(lpDBusPendC); dbus_pending_call_unref(lpDBusPendC); + lpDBusPendC = NULL; if (lpDBusReply != NULL) { stBTDeviceInfo apstBTDeviceInfo; @@ -4887,6 +4900,7 @@ BtrCore_BTGetPairedDeviceInfo ( if (0 != btrCore_BTParseDevice(lpDBusReply, &apstBTDeviceInfo)) { BTRCORELOG_ERROR ("Parsing the device %s failed..\n", pPairedDeviceInfo->devicePath[i]); dbus_message_unref(lpDBusReply); + lpDBusReply = NULL; return -1; } else { @@ -4894,11 +4908,11 @@ BtrCore_BTGetPairedDeviceInfo ( } dbus_message_unref(lpDBusReply); + lpDBusReply = NULL; } } } - BTRCORELOG_TRACE ("Exiting\n"); return 0; diff --git a/unitTest/test_btrCore.c b/unitTest/test_btrCore.c index be1cd55..b5b47da 100644 --- a/unitTest/test_btrCore.c +++ b/unitTest/test_btrCore.c @@ -71,6 +71,14 @@ typedef struct _stBTRCoreDevStateInfo { enBTRCoreDeviceState eDeviceCurrState; } stBTRCoreDevStateInfo; +typedef struct _stBTRCorePendingHidNameInfo { + BOOLEAN bActive; + tBTRCoreDevId btrCoreDevId; + enBTRCoreDeviceType enBTRCoreDevType; + GThread* pTimeoutThread; + stBTDeviceInfo stBTDevInfo; +} stBTRCorePendingHidNameInfo; + typedef struct _stBTRCoreHdl { @@ -127,8 +135,18 @@ typedef struct _stBTRCoreHdl { unsigned short batteryLevelRefreshInterval; GCond batteryLevelCond; BOOLEAN batteryLevelThreadExit; + + + GMutex hidNameWaitMutex; + GCond hidNameWaitCond; + BOOLEAN hidNameWaitInitialized; + stBTRCorePendingHidNameInfo stPendingHidNameInfo[BTRCORE_MAX_NUM_BT_DISCOVERED_DEVICES]; } stBTRCoreHdl; +typedef struct _stBTRCoreHidNameTimeoutData { + stBTRCoreHdl* pBTRCoreHdl; + tBTRCoreDevId btrCoreDevId; +} stBTRCoreHidNameTimeoutData; void test_BTRCore_Init_NULL() { @@ -6101,6 +6119,12 @@ void test_btrCore_BTDeviceStatusUpdateCb_DeviceLost(void) { stBTDeviceInfo* deviceInfo= (stBTDeviceInfo*)malloc(sizeof(stBTDeviceInfo));; int ret; + memset(btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(deviceInfo, 0, sizeof(stBTDeviceInfo)); + g_mutex_init(&btrCoreHdl->hidNameWaitMutex); + g_cond_init(&btrCoreHdl->hidNameWaitCond); + btrCoreHdl->hidNameWaitInitialized = TRUE; + // Set up the known device btrCoreHdl->numOfPairedDevices = 1; btrCoreHdl->stKnownDevicesArr[0].tDeviceId = 73588229205; @@ -6125,6 +6149,8 @@ void test_btrCore_BTDeviceStatusUpdateCb_DeviceLost(void) { TEST_ASSERT_EQUAL(0, ret); printf("Test case for device lost passed!\n"); + g_mutex_clear(&btrCoreHdl->hidNameWaitMutex); + g_cond_clear(&btrCoreHdl->hidNameWaitCond); free(btrCoreHdl); free(deviceInfo); } @@ -6135,6 +6161,12 @@ void test_btrCore_BTDeviceStatusUpdateCb_enBTDevStPropChanged(void) { stBTDeviceInfo* deviceInfo= (stBTDeviceInfo*)malloc(sizeof(stBTDeviceInfo));; int ret; + memset(btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(deviceInfo, 0, sizeof(stBTDeviceInfo)); + g_mutex_init(&btrCoreHdl->hidNameWaitMutex); + g_cond_init(&btrCoreHdl->hidNameWaitCond); + btrCoreHdl->hidNameWaitInitialized = TRUE; + // Set up the known device btrCoreHdl->numOfPairedDevices = 1; btrCoreHdl->stKnownDevicesArr[0].tDeviceId = 73588229205; @@ -6159,6 +6191,8 @@ void test_btrCore_BTDeviceStatusUpdateCb_enBTDevStPropChanged(void) { TEST_ASSERT_EQUAL(0, ret); printf("Test case for device lost passed!\n"); + g_mutex_clear(&btrCoreHdl->hidNameWaitMutex); + g_cond_clear(&btrCoreHdl->hidNameWaitCond); free(btrCoreHdl); free(deviceInfo); } @@ -6774,6 +6808,12 @@ void test_btrCore_BTDeviceStatusUpdateCb_DeviceLost_fail(void) { stBTDeviceInfo* deviceInfo= (stBTDeviceInfo*)malloc(sizeof(stBTDeviceInfo));; int ret; + memset(btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(deviceInfo, 0, sizeof(stBTDeviceInfo)); + g_mutex_init(&btrCoreHdl->hidNameWaitMutex); + g_cond_init(&btrCoreHdl->hidNameWaitCond); + btrCoreHdl->hidNameWaitInitialized = TRUE; + // Set up the known device // btrCoreHdl->numOfPairedDevices = 0; btrCoreHdl->numOfScannedDevices=2; @@ -6798,6 +6838,8 @@ void test_btrCore_BTDeviceStatusUpdateCb_DeviceLost_fail(void) { TEST_ASSERT_EQUAL(0, ret); printf("Test case for device lost passed!\n"); + g_mutex_clear(&btrCoreHdl->hidNameWaitMutex); + g_cond_clear(&btrCoreHdl->hidNameWaitCond); free(btrCoreHdl); free(deviceInfo); } @@ -6807,6 +6849,12 @@ void test_btrCore_BTDeviceStatusUpdateCb_enBTDevStPropChanged1(void) { stBTDeviceInfo* deviceInfo= (stBTDeviceInfo*)malloc(sizeof(stBTDeviceInfo));; int ret; + memset(btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(deviceInfo, 0, sizeof(stBTDeviceInfo)); + g_mutex_init(&btrCoreHdl->hidNameWaitMutex); + g_cond_init(&btrCoreHdl->hidNameWaitCond); + btrCoreHdl->hidNameWaitInitialized = TRUE; + // Set up the known device // btrCoreHdl->numOfPairedDevices = 1; @@ -6833,6 +6881,8 @@ void test_btrCore_BTDeviceStatusUpdateCb_enBTDevStPropChanged1(void) { TEST_ASSERT_EQUAL(0, ret); printf("Test case for device lost passed!\n"); + g_mutex_clear(&btrCoreHdl->hidNameWaitMutex); + g_cond_clear(&btrCoreHdl->hidNameWaitCond); free(btrCoreHdl); free(deviceInfo); } @@ -6854,6 +6904,151 @@ void test_btrCore_BTDeviceStatusUpdateCb_DeviceFound1(void) { printf("Test case for device found passed!\n"); } + +/* Tests for the HID name-wait thread introduced in commit e60bfcf + * (RDKEMW-13897: XBOX gen3 controller takes time to discover). + * + * When an HID GamePad/Joystick is discovered with no valid name (device name + * equals its MAC address), a background thread is started that waits up to + * BTRCORE_HID_NAME_WAIT_TIMEOUT_SEC seconds for the name to arrive. + * The mutex and cond on the handle MUST be properly initialised before any + * code path that touches them is exercised. + */ + +/* Verify that when an HID GamePad is found with no valid name the name-wait + * thread is started and can be cleanly cancelled by a subsequent Lost event. */ +void test_btrCore_BTDeviceStatusUpdateCb_HIDGamePadFoundNoName(void) { + stBTRCoreHdl btrCoreHdl; + stBTDeviceInfo deviceInfo; + int ret; + + memset(&btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(&deviceInfo, 0, sizeof(stBTDeviceInfo)); + + /* Initialise mutex and cond so the HID-name-wait thread can use them */ + g_mutex_init(&btrCoreHdl.hidNameWaitMutex); + g_cond_init(&btrCoreHdl.hidNameWaitCond); + btrCoreHdl.hidNameWaitInitialized = TRUE; + + /* HID GamePad class (0x508 = enBTRCore_DC_HID_GamePad) with device name + * identical to the MAC address triggers the name-wait path. */ + deviceInfo.ui32Class = enBTRCore_DC_HID_GamePad; + strncpy(deviceInfo.pcAddress, "AA:BB:CC:DD:EE:FF", sizeof(deviceInfo.pcAddress) - 1); + strncpy(deviceInfo.pcName, "AA:BB:CC:DD:EE:FF", sizeof(deviceInfo.pcName) - 1); + + /* enBTDevStFound with name==address should start the name-wait thread */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStFound, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + /* Cancel the wait by triggering enBTDevStLost for the same device. + * btrCore_ClearPendingControllerNameInfo() will signal the cond and + * join the thread, so this call blocks only briefly. */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStLost, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + g_mutex_clear(&btrCoreHdl.hidNameWaitMutex); + g_cond_clear(&btrCoreHdl.hidNameWaitCond); +} + +/* Same as above but with a Joystick class instead of GamePad. */ +void test_btrCore_BTDeviceStatusUpdateCb_HIDJoystickFoundNoName(void) { + stBTRCoreHdl btrCoreHdl; + stBTDeviceInfo deviceInfo; + int ret; + + memset(&btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(&deviceInfo, 0, sizeof(stBTDeviceInfo)); + + g_mutex_init(&btrCoreHdl.hidNameWaitMutex); + g_cond_init(&btrCoreHdl.hidNameWaitCond); + btrCoreHdl.hidNameWaitInitialized = TRUE; + + deviceInfo.ui32Class = enBTRCore_DC_HID_Joystick; + strncpy(deviceInfo.pcAddress, "11:22:33:44:55:66", sizeof(deviceInfo.pcAddress) - 1); + strncpy(deviceInfo.pcName, "11:22:33:44:55:66", sizeof(deviceInfo.pcName) - 1); + + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStFound, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + /* Cancel via Lost event */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStLost, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + g_mutex_clear(&btrCoreHdl.hidNameWaitMutex); + g_cond_clear(&btrCoreHdl.hidNameWaitCond); +} + +/* Verify that a second Found event for the same HID device while a name-wait + * is already in progress simply updates the stored device info without + * spawning a duplicate thread, and the wait can still be cancelled cleanly. */ +void test_btrCore_BTDeviceStatusUpdateCb_HIDGamePadFoundDuplicate(void) { + stBTRCoreHdl btrCoreHdl; + stBTDeviceInfo deviceInfo; + int ret; + + memset(&btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(&deviceInfo, 0, sizeof(stBTDeviceInfo)); + + g_mutex_init(&btrCoreHdl.hidNameWaitMutex); + g_cond_init(&btrCoreHdl.hidNameWaitCond); + btrCoreHdl.hidNameWaitInitialized = TRUE; + + deviceInfo.ui32Class = enBTRCore_DC_HID_GamePad; + strncpy(deviceInfo.pcAddress, "CC:DD:EE:FF:00:11", sizeof(deviceInfo.pcAddress) - 1); + strncpy(deviceInfo.pcName, "CC:DD:EE:FF:00:11", sizeof(deviceInfo.pcName) - 1); + + /* First Found - starts name-wait thread */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStFound, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + /* Second Found for the same device - should update stored info and return */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStFound, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + /* Cancel via Lost event - signals cond and joins thread */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStLost, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + g_mutex_clear(&btrCoreHdl.hidNameWaitMutex); + g_cond_clear(&btrCoreHdl.hidNameWaitCond); +} + +/* Verify the NameChanged path: when a valid name arrives for an HID GamePad + * that is pending a name-wait, the thread is cancelled early and a discovery + * callback is queued with the real device name. */ +void test_btrCore_BTDeviceStatusUpdateCb_HIDGamePadNameChanged(void) { + stBTRCoreHdl btrCoreHdl; + stBTDeviceInfo deviceInfo; + int ret; + + memset(&btrCoreHdl, 0, sizeof(stBTRCoreHdl)); + memset(&deviceInfo, 0, sizeof(stBTDeviceInfo)); + + g_mutex_init(&btrCoreHdl.hidNameWaitMutex); + g_cond_init(&btrCoreHdl.hidNameWaitCond); + btrCoreHdl.hidNameWaitInitialized = TRUE; + + /* pGAQueueOutTask is left NULL; btrCore_OutTaskAddOp handles NULL gracefully */ + deviceInfo.ui32Class = enBTRCore_DC_HID_GamePad; + strncpy(deviceInfo.pcAddress, "DD:EE:FF:00:11:22", sizeof(deviceInfo.pcAddress) - 1); + strncpy(deviceInfo.pcName, "DD:EE:FF:00:11:22", sizeof(deviceInfo.pcName) - 1); + + /* Start the name-wait thread */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStFound, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + /* Simulate the name arriving: set a valid name different from the address */ + strncpy(deviceInfo.pcName, "Xbox Wireless Controller", sizeof(deviceInfo.pcName) - 1); + + /* enBTDevStNameChanged should cancel the pending wait because name != address, + * signal the cond, and join the thread. */ + ret = btrCore_BTDeviceStatusUpdateCb(enBTDevUnknown, enBTDevStNameChanged, &deviceInfo, &btrCoreHdl); + TEST_ASSERT_EQUAL(0, ret); + + g_mutex_clear(&btrCoreHdl.hidNameWaitMutex); + g_cond_clear(&btrCoreHdl.hidNameWaitCond); +} + void test_btrCore_BTLeStatusUpdateCb_InvalidArg(void){ int ret; ret = btrCore_BTLeStatusUpdateCb(0, NULL, NULL);