RDK-60621 - Data Collection Through T2 Events For Components Logging To btmgrlog.txt#75
RDK-60621 - Data Collection Through T2 Events For Components Logging To btmgrlog.txt#75ANANTHMARIMUTHU wants to merge 32 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds telemetry instrumentation to the Bluetooth Manager (btrMgr) to track various Bluetooth operations and error conditions. The changes integrate a telemetry library (bt-telemetry.h) and add 22 telemetry markers throughout the codebase to monitor connection, pairing, unpairing, discovery, and error events.
Changes:
- Added bt-telemetry.h include and initialized telemetry in BTRMGR_Init
- Instrumented 22 telemetry markers for tracking BT operations (connect/disconnect, pair/unpair, discovery errors, adapter errors, GamePad RFC values)
- Added test simulation function (commented out with #if 0) to verify all telemetry markers
- Changed RFC GamePad parameter logging from DEBUG to INFO level
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/ifce/btrMgr.c
Outdated
| char btmgr_name[] = "btmgr"; | ||
| telemetry_init(btmgr_name); | ||
| /* Run telemetry test simulation if BTRMGR_TELEMETRY_TEST=1 is set */ | ||
| // btrMgr_TelemetryTestSimulation(); |
There was a problem hiding this comment.
Commented-out function call. This commented-out call to btrMgr_TelemetryTestSimulation should be removed. Commented-out code reduces code readability and should not be committed to the codebase. If this is needed for future testing, consider using a feature flag or environment variable check instead.
| // btrMgr_TelemetryTestSimulation(); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //Inside this function there is a failure logs so, did not any failures logs here. | ||
| btrMgr_GetDeviceDetails(ahBTRMgrDevHdl,&stDeviceInfo); | ||
| const char* lpcDeviceName = stDeviceInfo.pcDeviceName ? stDeviceInfo.pcDeviceName : "Unknown"; | ||
|
|
||
| if (lenBtrCoreRet != enBTRCoreSuccess) { | ||
| //This is telemetry log. If we change this print,need to change and configure the telemetry string in xconf server. | ||
| char buffer[256]; | ||
| snprintf(buffer, sizeof(buffer), "Failed to Disconnect from this device - Confirmed name,class,apperance,modalias: %s,%u,%u,v%04Xp%04Xd%04X", | ||
| lpcDeviceName, stDeviceInfo.ui32DevClassBtSpec, stDeviceInfo.ui16DevAppearanceBleSpec, | ||
| stDeviceInfo.ui32ModaliasVendorId, stDeviceInfo.ui32ModaliasProductId, stDeviceInfo.ui32ModaliasDeviceId); | ||
| telemetry_event_s("BTDiscFail_split", buffer); | ||
| BTRMGRLOG_ERROR ("Failed to Disconnect from this device - Confirmed name,class,apperance,modalias: %s,%u,%u,v%04Xp%04Xd%04X\n", | ||
| stDeviceInfo.pcDeviceName, stDeviceInfo.ui32DevClassBtSpec, stDeviceInfo.ui16DevAppearanceBleSpec, | ||
| lpcDeviceName, stDeviceInfo.ui32DevClassBtSpec, stDeviceInfo.ui16DevAppearanceBleSpec, | ||
| stDeviceInfo.ui32ModaliasVendorId, stDeviceInfo.ui32ModaliasProductId, stDeviceInfo.ui32ModaliasDeviceId); | ||
| BTRMGRLOG_ERROR ("Disconnect failure device MAC %s\n", stDeviceInfo.pcDeviceAddress); |
There was a problem hiding this comment.
Disconnect telemetry/log formatting has the same issue as connect/unpair: stDeviceInfo is not zero-initialized and the result of btrMgr_GetDeviceDetails is ignored. If that call fails, pcDeviceName/pcDeviceAddress may be garbage and the telemetry payload may contain undefined values. Please MEMSET_S + check the return, and fall back to safe defaults before calling snprintf/telemetry_event_*.
| /* | ||
| * TEST SIMULATION FUNCTION FOR TELEMETRY MARKERS | ||
| * This function simulates all 22 telemetry markers for testing | ||
| * Set BTRMGR_TELEMETRY_TEST=1 environment variable to enable testing | ||
| */ | ||
| #if 1 | ||
| STATIC void | ||
| btrMgr_TelemetryTestSimulation ( |
There was a problem hiding this comment.
The telemetry test simulation block is compiled in unconditionally (#if 1) despite the comment saying it should be enabled via an environment variable. This risks shipping a test-only routine in production builds. Please gate this behind a compile-time flag (e.g., #ifdef BTRMGR_TELEMETRY_TEST) and/or implement the documented env-var check inside the function.
| #include "btrMgr_SysDiag.h" | ||
| #include "btrMgr_Columbo.h" | ||
| #include "btrMgr_LEOnboarding.h" | ||
|
|
||
| #include "bt-telemetry.h" |
There was a problem hiding this comment.
bt-telemetry.h is included unconditionally, but this repository does not provide that header in the normal include paths (it only exists under unitTest/support/include). This will break non-unit-test builds unless the header/library is guaranteed to come from an external dependency. Please add a proper configure-time check + conditional include (or ship/install the header and link the telemetry library when enabled).
| //Inside this function there is a failure logs so, did not any failures logs here. | ||
| btrMgr_GetDeviceDetails(ahBTRMgrDevHdl,&stDeviceInfo); | ||
|
|
||
| if (enBTRCoreSuccess != BTRCore_UnPairDevice(ghBTRCoreHdl, ahBTRMgrDevHdl)) { | ||
| //This is telemetry log. If we change this print,need to change and configure the telemetry string in xconf server. | ||
| char buffer[256]; | ||
| snprintf(buffer, sizeof(buffer), "Failed to unpair name,class,apperance,modalias: %s,%u,%u,v%04Xp%04Xd%04X", | ||
| stDeviceInfo.pcDeviceName, stDeviceInfo.ui32DevClassBtSpec, stDeviceInfo.ui16DevAppearanceBleSpec, | ||
| stDeviceInfo.ui32ModaliasVendorId, stDeviceInfo.ui32ModaliasProductId, stDeviceInfo.ui32ModaliasDeviceId); | ||
| telemetry_event_s("BTUnpairFail_split", buffer); |
There was a problem hiding this comment.
Unpair telemetry/log formatting uses stDeviceInfo without checking whether btrMgr_GetDeviceDetails succeeded, and without NULL-safety for pcDeviceName/pcDeviceAddress. Please zero-init stDeviceInfo, check the return from btrMgr_GetDeviceDetails, and use safe fallbacks ("Unknown" / 0) when details are unavailable to avoid crashes and bogus telemetry payloads.
No description provided.