fix: android callingx calls should handle audio through through telecom#2324
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (8)
📝 WalkthroughWalkthroughAdds Telecom-backed audio endpoint support across react-native-callingx and react-native-sdk, including native snapshot storage, JS event/type plumbing, Telecom-managed routing, recovery handling, and sample call-creation updates. ChangesCallingx Android/JS audio endpoint support
Estimated code review effort: 4 (Complex) | ~60 minutes react-native-sdk Telecom-managed CallManager integration
Estimated code review effort: 4 (Complex) | ~55 minutes Sample app call creation tweaks
Estimated code review effort: 1 (Trivial) | ~3 minutes Sequence Diagram(s)sequenceDiagram
participant TelecomCallRepository
participant CallService
participant AudioEndpointStore
participant CallingxModuleImpl
participant JSLayer as JS Layer
TelecomCallRepository->>CallService: onCallAudioEndpointsChanged(callId)
CallService->>CallService: build snapshot JSON
CallService->>AudioEndpointStore: setSnapshot(callId, snapshotJson)
CallService->>CallingxModuleImpl: broadcast CALL_AUDIO_ENDPOINTS_CHANGED_ACTION
CallingxModuleImpl->>JSLayer: emit didChangeAudioEndpoints(snapshot)
JSLayer->>JSLayer: normalizeEventParams(snapshot)
sequenceDiagram
participant CallManager
participant CallingxModule
participant StreamInCallManagerModule
participant AudioDeviceManager
CallManager->>CallManager: check Telecom-managed state
alt Telecom-managed
CallManager->>CallingxModule: getAvailableAudioEndpoints(callId)
CallingxModule-->>CallManager: AudioEndpointsSnapshot
CallManager->>CallingxModule: setDefaultAudioDeviceEndpointType(type)
CallManager->>StreamInCallManagerModule: setTelecomManagedMode(true)
CallManager->>AudioDeviceManager: start() with routing disabled
else Classic mode
CallManager->>StreamInCallManagerModule: setTelecomManagedMode(false)
CallManager->>AudioDeviceManager: setup()/start()
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bundle sizeBuilt package output. Sizes in KB; delta vs
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/utils/AudioEndpointUtils.kt (1)
21-27: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueFuture Telecom endpoint types collapse into "unknown".
androidx.core.telecom.CallEndpointCompatmay expose additional types beyond earpiece/speaker/wired_headset/bluetooth (e.g.TYPE_STREAMINGon newer platform versions). Theelse -> TYPE_UNKNOWNbranch silently maps any such type to the same generic "unknown" string the JS layer would use for a genuinely unrecognized type, losing information. This is a safe fallback today, so low priority.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/utils/AudioEndpointUtils.kt` around lines 21 - 27, The endpointTypeToString mapping in AudioEndpointUtils.kt collapses any future CallEndpointCompat types into TYPE_UNKNOWN, which loses information for newer supported endpoint values. Update endpointTypeToString to explicitly handle the known telecom constants in this class and preserve additional recognized types with distinct string values instead of falling through to the generic unknown case. Keep TYPE_UNKNOWN only for truly unrecognized values so the JS layer can distinguish future endpoint types.packages/react-native-callingx/src/CallingxModule.ts (2)
158-174: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate JSON-parse-with-fallback logic.
getAvailableAudioEndpointshere duplicates the same "parse snapshot JSON with try/catch fallback to empty snapshot" logic present inEventManager.ts'snormalizeEventParams. Consider extracting a sharedparseAudioEndpointsSnapshot(json: string | undefined)helper to keep both call sites in sync if the snapshot shape changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-callingx/src/CallingxModule.ts` around lines 158 - 174, The JSON parsing and empty-snapshot fallback in getAvailableAudioEndpoints duplicates the same logic used by normalizeEventParams in EventManager.ts. Extract a shared helper such as parseAudioEndpointsSnapshot(json: string | undefined) that returns an AudioEndpointsSnapshot with the current fallback shape, and update both getAvailableAudioEndpoints and normalizeEventParams to call it so future snapshot shape changes stay consistent.
151-185: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew methods use method syntax, not arrow-function class fields.
getRegisteredCallIds,getAvailableAudioEndpoints, andrequestAudioEndpointChangeare added as class methods rather than arrow-function class fields. This matches the file's pre-existing (non-compliant) pattern used throughout the class, so fixing only the new methods would create inconsistency without meaningfully improvingthis-binding safety here (all calls are viathis.method()/ a module instance).As per coding guidelines, "All class methods must be arrow-function class fields, not method syntax — including private/protected methods."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-callingx/src/CallingxModule.ts` around lines 151 - 185, The new CallingxModule APIs are currently declared with method syntax instead of the required arrow-function class field style. Update getRegisteredCallIds, getAvailableAudioEndpoints, and requestAudioEndpointChange in CallingxModule to match the class’s arrow-function field convention so they are consistent with the coding guidelines and the rest of the class structure.Source: Coding guidelines
packages/react-native-sdk/src/modules/call-manager/CallManager.ts (1)
55-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the exported Callingx endpoint types here
packages/react-native-callingx/src/types.tsalready exportsAudioEndpointandAudioEndpointsSnapshot; reusing them avoids a duplicated shape that can drift from the native payload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/modules/call-manager/CallManager.ts` around lines 55 - 59, The CallManager module currently defines duplicate Callingx audio endpoint snapshot types instead of reusing the exported types from the Callingx package. Update the type aliases in CallManager.ts to import and use AudioEndpoint and AudioEndpointsSnapshot from packages/react-native-callingx/src/types so the shape stays aligned with the native payload and avoids drift.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModuleImpl.kt`:
- Around line 319-321: The tracked call IDs cleanup in CallingxModuleImpl is
incomplete because removeTrackedCall() only runs for source == "app", leaving
system or remote-ended calls in CallRegistrationStore. Update the call-end flow
so every terminal call path removes the call from the tracked set, and use
getRegisteredCallIds()/CallRegistrationStore consistently to verify no ended
call remains registered; if app-only cleanup is intentional, add a separate
cleanup path for non-app ends.
In `@packages/react-native-sdk/src/modules/call-manager/CallManager.ts`:
- Around line 108-118: The routing flow in CallManager is swallowing native
failures by using an empty catch after
getAvailableAudioEndpoints/requestAudioEndpointChange, which hides rejected
Telecom routing requests. Update the promise chain in CallManager to handle
rejections explicitly by logging the error (with enough context like callId and
endpointName) and, if appropriate, rethrowing or propagating it instead of
ignoring it; apply the same fix to the other similar routing block referenced by
the review.
In `@packages/react-native-sdk/src/utils/internal/callingx/callingx.ts`:
- Around line 27-42: The fallback in callingx() should not switch back to
classic StreamInCallManager audio when Telecom has already registered or owns a
call. Add a guard in the callingx.ts recovery path, using the existing
CallingxModule and StreamInCallManagerNativeModule checks, so the classic
recovery block only runs when Telecom is not already managing an active call. If
a Telecom registration/start sequence has partially succeeded, skip the
stop/setTelecomManagedMode/setup/start fallback and leave Telecom ownership
intact.
In `@packages/react-native-sdk/src/utils/internal/registerSDKGlobals.ts`:
- Line 94: `registerSDKGlobals` is calling
`StreamInCallManagerNativeModule.setTelecomManagedMode(false)` unconditionally,
which can crash on iOS because that native method is Android-only. Add the same
platform check used in `CallManager.start` around this call so it only runs on
Android, and keep the `StreamInCallManagerNativeModule` reference confined to
the guarded path.
---
Nitpick comments:
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/utils/AudioEndpointUtils.kt`:
- Around line 21-27: The endpointTypeToString mapping in AudioEndpointUtils.kt
collapses any future CallEndpointCompat types into TYPE_UNKNOWN, which loses
information for newer supported endpoint values. Update endpointTypeToString to
explicitly handle the known telecom constants in this class and preserve
additional recognized types with distinct string values instead of falling
through to the generic unknown case. Keep TYPE_UNKNOWN only for truly
unrecognized values so the JS layer can distinguish future endpoint types.
In `@packages/react-native-callingx/src/CallingxModule.ts`:
- Around line 158-174: The JSON parsing and empty-snapshot fallback in
getAvailableAudioEndpoints duplicates the same logic used by
normalizeEventParams in EventManager.ts. Extract a shared helper such as
parseAudioEndpointsSnapshot(json: string | undefined) that returns an
AudioEndpointsSnapshot with the current fallback shape, and update both
getAvailableAudioEndpoints and normalizeEventParams to call it so future
snapshot shape changes stay consistent.
- Around line 151-185: The new CallingxModule APIs are currently declared with
method syntax instead of the required arrow-function class field style. Update
getRegisteredCallIds, getAvailableAudioEndpoints, and requestAudioEndpointChange
in CallingxModule to match the class’s arrow-function field convention so they
are consistent with the coding guidelines and the rest of the class structure.
In `@packages/react-native-sdk/src/modules/call-manager/CallManager.ts`:
- Around line 55-59: The CallManager module currently defines duplicate Callingx
audio endpoint snapshot types instead of reusing the exported types from the
Callingx package. Update the type aliases in CallManager.ts to import and use
AudioEndpoint and AudioEndpointsSnapshot from
packages/react-native-callingx/src/types so the shape stays aligned with the
native payload and avoids drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c25b2857-fb3d-4e30-a764-a07122358c48
📒 Files selected for processing (29)
packages/react-native-callingx/android/src/main/AndroidManifest.xmlpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/AudioEndpointStore.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallRegistrationStore.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallService.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModuleImpl.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/model/CallAction.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/CallRepository.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/utils/AudioEndpointUtils.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/utils/SettingsStore.ktpackages/react-native-callingx/android/src/newarch/java/io/getstream/rn/callingx/CallingxModule.ktpackages/react-native-callingx/android/src/oldarch/java/io/getstream/rn/callingx/CallingxModule.ktpackages/react-native-callingx/ios/Callingx.mmpackages/react-native-callingx/src/CallingxModule.tspackages/react-native-callingx/src/EventManager.tspackages/react-native-callingx/src/spec/NativeCallingx.tspackages/react-native-callingx/src/types.tspackages/react-native-callingx/src/utils/constants.tspackages/react-native-sdk/__tests__/call-manager/CallManager.test.tspackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/audio/AudioDeviceManager.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/callmanager/StreamInCallManagerModule.ktpackages/react-native-sdk/src/modules/call-manager/CallManager.tspackages/react-native-sdk/src/modules/call-manager/native-module.d.tspackages/react-native-sdk/src/utils/StreamVideoRN/types.tspackages/react-native-sdk/src/utils/internal/callingx/callingx.tspackages/react-native-sdk/src/utils/internal/registerSDKGlobals.tspackages/react-native-sdk/src/utils/push/libs/callingx.tssample-apps/react-native/dogfood/src/screens/Call/JoinCallScreen.tsxsample-apps/react-native/expo-video-sample/components/CreateRingingCall.tsx
|
🎉 The changes from this pull request have been released. Shipped with:
|
💡 Overview
Primarily this PR implements the advice below from Android core-telecom docs
Other changes:
Fixes #2320
📝 Implementation notes
Incallmanager delegates Audio routing to callingx. But still supports reading state through it and calling its methods.
🎫 Ticket: https://linear.app/stream/issue/XYZ-123
📑 Docs: https://github.com/GetStream/docs-content/pull/
Summary by CodeRabbit
didChangeAudioEndpoints, including listing available endpoints and switching endpoints.defaultDeviceEndpointTypeto set the starting/default audio endpoint for Telecom-managed calls; added native APIs to check Telecom mode, get registered call IDs, and manage endpoints.didChangeAudioEndpoints.