RDKEMW-15846 : Elite classic controller is not disconnect on standby.#87
RDKEMW-15846 : Elite classic controller is not disconnect on standby.#87natrajmuthusamy wants to merge 6 commits intodevelopfrom
Conversation
Reason for change : Sets the operation to modify the device's blocked property, enabling dynamic block/unblock control via this function. Priority: P1 Test Procedure: Follow the steps provided in description Risks: High Signed-off-by:Natraj Muthusamy <Natraj_Muthusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Bluetooth Manager’s gamepad startup/standby handling to dynamically toggle the “blocked” state for a specific Xbox Elite controller variant, aiming to prevent reconnect/disconnect issues across standby transitions.
Changes:
- Unblocks a targeted Xbox Elite HID device during gamepad startup processing.
- Blocks the same targeted device when entering standby/light-sleep in the SysDiag power-state callback.
- Adds a new firmware/device-id constant used to identify the targeted controller variant.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/ifce/btrMgr.c | Adds block/unblock state updates around startup and standby transitions; adjusts recreated event fields. |
| include/btmgr.h | Adds a constant for an additional Xbox Elite “default firmware/device id” value. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| BTRMGRLOG_INFO("Entered standby mode, disconnecting gamepads\n"); | ||
| gbGamepadStandbyMode = TRUE; | ||
| //stop any gamepads from connecting whilst in standby mode | ||
| BTRCore_clearLEActionListForGamepads(ghBTRCoreHdl); | ||
| //Disconnect all connected gamepads | ||
| lstConnectedDevices = malloc(sizeof(BTRMGR_ConnectedDevicesList_t)); | ||
| if (!lstConnectedDevices) | ||
| { | ||
| BTRMGRLOG_ERROR("Run out memory for connected devices list\n"); | ||
| return eBTRMgrFailure; | ||
| } | ||
| if ((lenBtrMgrResult = BTRMGR_GetConnectedDevices(0, lstConnectedDevices)) == BTRMGR_RESULT_SUCCESS) { | ||
| BTRMGRLOG_DEBUG ("Connected Devices = %d\n", lstConnectedDevices->m_numOfDevices); | ||
|
|
||
| for (ui16LoopIdx = 0; ui16LoopIdx < lstConnectedDevices->m_numOfDevices; ui16LoopIdx++) { | ||
| unsigned int ui32confirmIdx = 2; | ||
| enBTRCoreDeviceType lenBtrCoreDevTy = enBTRCoreUnknown; | ||
| enBTRCoreDeviceClass lenBtrCoreDevCl = enBTRCore_DC_Unknown; | ||
| stBTRCoreBTDevice stDeviceInfo = {0}; | ||
|
|
||
| btrMgr_GetDeviceDetails(lstConnectedDevices->m_deviceProperty[ui16LoopIdx].m_deviceHandle,&stDeviceInfo); | ||
| if ((stDeviceInfo.enDeviceType == BTRMGR_DEVICE_TYPE_HID || | ||
| stDeviceInfo.enDeviceType == BTRMGR_DEVICE_TYPE_HID_GAMEPAD) && | ||
| (stDeviceInfo.ui16DevAppearanceBleSpec != BTRMGR_HID_GAMEPAD_LE_APPEARANCE) && | ||
| (stDeviceInfo.ui32ModaliasVendorId == BTRMGR_XBOX_ELITE_VENDOR_ID) && | ||
| (stDeviceInfo.ui32ModaliasProductId == BTRMGR_XBOX_ELITE_PRODUCT_ID) && | ||
| (stDeviceInfo.ui32ModaliasDeviceId == BTRMGR_XBOX_ELITE_DEFAULT_FIRMWARE2)) { | ||
| lenBtrCoreRet = btrCore_UpdateDeviceBlockState (ghBTRCoreHdl, stDeviceInfo.tDeviceId, stDeviceInfo.enDeviceType,1); | ||
| if (lenBtrCoreRet != enBTRCoreSuccess) { |
There was a problem hiding this comment.
New standby-mode behavior adds device blocking/unblocking for a specific controller variant, but existing unit tests for btrMgr_SDStatusCb only cover ON/OFF paths and do not exercise the standby/light-sleep branch or the btrCore_UpdateDeviceBlockState call. Please add/extend tests to cover entering standby (block + disconnect) and exiting standby (unblock on next startup/power-on) for both matching and non-matching devices.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| strncpy(stRecreatedEvent.deviceAddress, stDeviceInfo.pcDeviceAddress, BTRMGR_NAME_LEN_MAX - 1); | ||
| stRecreatedEvent.deviceId = stDeviceInfo.tDeviceId; | ||
| stRecreatedEvent.eDeviceClass = stDeviceInfo.enDeviceType; | ||
| stRecreatedEvent.eDeviceClass = stDeviceInfo.enDeviceClass; |
There was a problem hiding this comment.
stRecreatedEvent.eDeviceClass is later consumed as an enBTRCoreDeviceClass (e.g., mapped via btrMgr_MapDeviceTypeFromCore(p_StatusCB->eDeviceClass) in btrMgr_MapDevstatusInfoToEventInfo). Assigning stDeviceInfo.ui32DevClassBtSpec here changes the meaning/type to a Bluetooth class-of-device bitfield, which will cause incorrect device-type mapping (likely UNKNOWN) for the recreated DEVICE_FOUND flow at startup. Set eDeviceClass back to the core device class (the same value previously used, i.e., stDeviceInfo.enDeviceType) and keep the BT-spec class in ui32DevClassBtSpec (already set elsewhere on the struct).
| stRecreatedEvent.eDeviceClass = stDeviceInfo.enDeviceClass; | |
| stRecreatedEvent.eDeviceClass = stDeviceInfo.enDeviceType; |
| if (btrMgr_GetDeviceDetails(lstConnectedDevices->m_deviceProperty[ui16LoopIdx].m_deviceHandle, &stDeviceInfo) != 0) { | ||
| BTRMGRLOG_ERROR ("btrMgr_GetDeviceDetails failed for device handle %llu\n", | ||
| lstConnectedDevices->m_deviceProperty[ui16LoopIdx].m_deviceHandle); | ||
| continue; | ||
| } | ||
| /* Map BTRCore device type to BTRMGR device type before comparison */ | ||
| BTRMGR_DeviceType_t enMgrDevType = btrMgr_MapDeviceTypeFromCore(stDeviceInfo.enDeviceType); | ||
|
|
There was a problem hiding this comment.
This standby-path block-state update likely never triggers: stDeviceInfo.enDeviceType is a core device class (it is mapped elsewhere via btrMgr_MapDeviceTypeFromCore(stDeviceInfo.enDeviceType)), but here it's compared directly against BTRMGR_DEVICE_TYPE_* values. Use the mapped BTRMGR device type (or compare against the corresponding core enum) to ensure the HID/gamepad check works.
Also, btrMgr_GetDeviceDetails(...) return value is ignored; on failure stDeviceInfo remains zeroed and the block/unblock logic silently skips. Check the return and log/continue on failure (as done in BTRMGR_ConnectGamepads_StartUp).
| BTRMGR_DeviceType mappedDevType = btrMgr_MapDeviceTypeFromCore(stDeviceInfo.enDeviceType); | ||
|
|
||
| if ((mappedDevType == BTRMGR_DEVICE_TYPE_HID || | ||
| mappedDevType == BTRMGR_DEVICE_TYPE_HID_GAMEPAD) && | ||
| (stDeviceInfo.ui16DevAppearanceBleSpec != BTRMGR_HID_GAMEPAD_LE_APPEARANCE) && | ||
| (stDeviceInfo.ui32ModaliasVendorId == BTRMGR_XBOX_ELITE_VENDOR_ID) && | ||
| (stDeviceInfo.ui32ModaliasProductId == BTRMGR_XBOX_ELITE_PRODUCT_ID) && | ||
| (stDeviceInfo.ui32ModaliasDeviceId == BTRMGR_XBOX_ELITE_DEFAULT_FIRMWARE2)) { | ||
| lenBtrCoreRet = btrCore_UpdateDeviceBlockState(ghBTRCoreHdl,stDeviceInfo.tDeviceId,stDeviceInfo.enDeviceType,0); | ||
| if (lenBtrCoreRet != enBTRCoreSuccess) { | ||
| BTRMGRLOG_INFO("Failed to update the block state of the device ...\n"); | ||
| } else { | ||
| BTRMGRLOG_INFO("Updated the block state of the device successfully ...\n"); | ||
| } |
There was a problem hiding this comment.
The Xbox Elite "classic" identification + block/unblock logic is duplicated in both the startup connect loop and the standby disconnect loop. Consider extracting a small helper (e.g., btrMgr_IsXboxEliteClassic(...) and/or btrMgr_UpdateBlockStateIfXboxEliteClassic(...)) to keep the device-match criteria consistent and reduce the risk of future divergence.
| #define BTRMGR_NINTENDO_GAMESIR_VENDOR_ID 0x057E | ||
| #define BTRMGR_HID_GAMEPAD_LE_APPEARANCE 0x3C4 | ||
| #define BTRMGR_XBOX_ELITE_DEFAULT_FIRMWARE 0x0407 | ||
| #define BTRMGR_XBOX_ELITE_DEFAULT_FIRMWARE2 0x0903 |
There was a problem hiding this comment.
BTRMGR_XBOX_ELITE_DEFAULT_FIRMWARE2 doesn’t indicate what the value represents (it’s compared to ui32ModaliasDeviceId, which is typically a device/revision id rather than firmware). Consider renaming this macro to reflect the field it matches (e.g., ..._DEVICE_ID_0903 / ..._REVISION_0903) or adding a short comment describing what 0x0903 corresponds to, to avoid confusion with actual firmware versions.
| #define BTRMGR_XBOX_ELITE_DEFAULT_FIRMWARE2 0x0903 | |
| #define BTRMGR_XBOX_ELITE_DEFAULT_FIRMWARE2 0x0903 /* used as Xbox Elite modalias device/revision ID, not a firmware version */ |
Reason for change : Sets the operation to modify the device's blocked property, enabling dynamic block/unblock control via this function.
Priority: P1
Test Procedure: Follow the steps provided in description
Risks: High
Signed-off-by:Natraj Muthusamy Natraj_Muthusamy@comcast.com