fix(voip): Android audio-route sync + outgoing ringback on comm path#7273
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:
WalkthroughAdds Android-only audio-route sync start/stop hooks to native VoIP and exposes them via the TurboModule surface with JS fallbacks; wires a non‑iOS VoipCommunicationDeviceChanged event into MediaCallEvents to update call-store speaker state; iOS implementations are no-op promise resolvers. Changes
Sequence DiagramsequenceDiagram
participant App as App / useCallStore
participant Native as NativeVoip (TurboModule)
participant Android as Android VoipModule
participant Media as MediaCallEvents
participant Store as Call Store
Note over App,Store: call start
App->>Native: startAudioRouteSync()
Native->>Android: startAudioRouteSync()
Android->>Android: register OnCommunicationDeviceChangedListener
Android->>Native: emit current speaker state
Native-->>App: Promise resolved
Note over App,Store: device change
Android->>Android: communicationDevice changed
Android->>Native: emit VoipCommunicationDeviceChanged({ isSpeaker })
Native->>Media: event dispatched
Media->>Store: setState({ isSpeakerOn })
Note over App,Store: call end
App->>Native: stopAudioRouteSync()
Native->>Android: stopAudioRouteSync()
Android->>Android: unregister listener & clear reference
Native-->>App: Promise resolved
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt`:
- Around line 170-208: startAudioRouteSync and stopAudioRouteSync call
AudioManager methods without handling exceptions so the JS promise and module
state can drift; wrap the native operations (getSystemService,
addOnCommunicationDeviceChangedListener,
removeOnCommunicationDeviceChangedListener and accessing
audioManager.communicationDevice) in try/catch blocks and call
promise.reject(...) with the caught Throwable message when an exception occurs,
while ensuring communicationDeviceListener and audioRouteSyncActive are only set
or cleared on successful operations (or reverted in the catch) so module state
stays consistent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 275b61b3-176a-4894-83c4-6c70a7bf4234
📒 Files selected for processing (9)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktapp/lib/native/NativeVoip.tsapp/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/useCallStore.tsios/Libraries/VoipModule.mm
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/native/NativeVoip.tsapp/lib/services/voip/useCallStore.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms
Files:
app/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/native/NativeVoip.tsapp/lib/services/voip/useCallStore.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/native/NativeVoip.tsapp/lib/services/voip/useCallStore.ts
app/lib/services/voip/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Files:
app/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/services/voip/useCallStore.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Applied to files:
app/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/MediaCallEvents.tsios/Libraries/VoipModule.mmapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/native/NativeVoip.tsandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktapp/lib/services/voip/useCallStore.ts
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.
Applied to files:
app/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.ios.test.ts
📚 Learning: 2026-03-05T06:06:12.277Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RCTWatchModule.mm:19-24
Timestamp: 2026-03-05T06:06:12.277Z
Learning: Do not re-activate or reset the WCSession singleton in iOS Objective-C/Swift bridge modules. Ensure WCSession is activated and its delegate is set in a single, central place (e.g., ios/RocketChat Watch App/Loaders/WatchSession.swift) and avoid duplicating activation or delegate assignment in other iOS bridge files like ios/RCTWatchModule.mm. If WCSession is already activated via the central loader, relying on WCSession.defaultSession is sufficient and maintains a single session lifecycle.
Applied to files:
ios/Libraries/VoipModule.mm
📚 Learning: 2026-03-31T11:59:31.061Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: android/build.gradle:3-8
Timestamp: 2026-03-31T11:59:31.061Z
Learning: In the RocketChat/Rocket.Chat.ReactNative repository, the React Native upgrade helper (https://react-native-community.github.io/upgrade-helper/?from=0.79.4&to=0.81.5) recommends kotlinVersion = "2.1.20", compileSdkVersion = 36, targetSdkVersion = 36, and buildToolsVersion = "36.0.0" in android/build.gradle for the RN 0.79.4 → 0.81.5 upgrade. These are the sanctioned values for this upgrade path and should not be flagged as compatibility concerns.
Applied to files:
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt
🔇 Additional comments (10)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt (1)
210-223: Good defensive event emission path.The active-instance guard + try/catch keeps native-to-JS event delivery resilient.
app/lib/services/voip/MediaCallEvents.ts (1)
189-196: Android listener wiring looks correct.The non-iOS gate and
call == nullguard prevent stale speaker-state writes.Based on learnings: "Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration".
app/lib/services/voip/useCallStore.ts (1)
175-179: Call lifecycle sync integration is well-scoped.Starting on
setCalland stopping onreset(Android-only) is a clean mapping for listener lifetime.Based on learnings: "Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration".
Also applies to: 321-325
app/lib/services/voip/MediaCallEvents.test.ts (1)
34-40: Nice coverage for speaker-route event syncing.The suite validates both state transitions and the null-call no-op guard.
Also applies to: 473-506
app/lib/services/voip/useCallStore.test.ts (1)
24-46: Good Android-side lifecycle assertions.These tests lock in the expected
startAudioRouteSync/stopAudioRouteSyncbehavior.Also applies to: 320-338
app/lib/services/voip/MediaCallEvents.ios.test.ts (1)
356-371: Platform guard test is solid.Verifying no iOS subscription for
VoipCommunicationDeviceChangedis exactly the right regression check.app/lib/services/voip/useCallStore.ios.test.ts (1)
1-102: iOS guard suite looks good.The tests correctly enforce that audio-route sync calls stay Android-only.
app/lib/native/NativeVoip.ts (1)
45-57: TurboModule surface extension is consistent and safe.Typed spec plus fallback no-ops avoids runtime gaps when native isn’t present.
Also applies to: 82-83
ios/Libraries/VoipModule.mm (2)
120-122:startAudioRouteSynciOS no-op behavior looks correct.Resolving
@NOhere aligns with the intended iOS contract for this feature.
124-126:stopAudioRouteSyncimplementation is consistent with the iOS stub contract.Returning
@NOas a no-op is appropriate for parity with the new cross-platform API surface.
|
Good diff. Solid impl, tests comprehensive. A few findings:
|
|
[codex] Addressed in c261d73:
|
c261d73 to
dfba14a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt (1)
170-206:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard the listener registration path.
getSystemService(AudioManager::class.java)and the listener add/remove calls are still unchecked here, so a native failure can crash the module or leave the JS promise unsettled. Please wrap both methods defensively and reject the Promise on failure. As per coding guidelines, use explicit error handling with try/catch blocks for async operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt` around lines 170 - 206, Wrap the native calls in startAudioRouteSync and stopAudioRouteSync with defensive try/catch and null checks: ensure reactApplicationContext.getSystemService(AudioManager::class.java) is checked for null before calling addOnCommunicationDeviceChangedListener/removeOnCommunicationDeviceChangedListener, catch any thrown exceptions around addOnCommunicationDeviceChangedListener and removeOnCommunicationDeviceChangedListener, set communicationDeviceListener only after successful registration, and call promise.reject(...) with a descriptive error when any failure occurs (instead of leaving the promise unresolved); keep emitCommunicationDeviceChanged and promise.resolve(null) behavior only on successful paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt`:
- Around line 170-206: Wrap the native calls in startAudioRouteSync and
stopAudioRouteSync with defensive try/catch and null checks: ensure
reactApplicationContext.getSystemService(AudioManager::class.java) is checked
for null before calling
addOnCommunicationDeviceChangedListener/removeOnCommunicationDeviceChangedListener,
catch any thrown exceptions around addOnCommunicationDeviceChangedListener and
removeOnCommunicationDeviceChangedListener, set communicationDeviceListener only
after successful registration, and call promise.reject(...) with a descriptive
error when any failure occurs (instead of leaving the promise unresolved); keep
emitCommunicationDeviceChanged and promise.resolve(null) behavior only on
successful paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f49d3301-9464-4392-a1cc-4de158f964cc
📒 Files selected for processing (9)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktapp/lib/native/NativeVoip.tsapp/lib/services/voip/MediaCallEvents.ios.test.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/useCallStore.tsios/Libraries/VoipModule.mm
✅ Files skipped from review due to trivial changes (1)
- app/lib/services/voip/useCallStore.ios.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/lib/services/voip/MediaCallEvents.ios.test.ts
- app/lib/services/voip/useCallStore.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/services/voip/MediaCallEvents.tsapp/lib/native/NativeVoip.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms
Files:
app/lib/services/voip/MediaCallEvents.tsapp/lib/native/NativeVoip.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/services/voip/MediaCallEvents.tsapp/lib/native/NativeVoip.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.test.ts
app/lib/services/voip/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Files:
app/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Applied to files:
app/lib/services/voip/MediaCallEvents.tsios/Libraries/VoipModule.mmapp/lib/native/NativeVoip.tsapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.test.tsandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt
📚 Learning: 2026-03-05T06:06:12.277Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RCTWatchModule.mm:19-24
Timestamp: 2026-03-05T06:06:12.277Z
Learning: Do not re-activate or reset the WCSession singleton in iOS Objective-C/Swift bridge modules. Ensure WCSession is activated and its delegate is set in a single, central place (e.g., ios/RocketChat Watch App/Loaders/WatchSession.swift) and avoid duplicating activation or delegate assignment in other iOS bridge files like ios/RCTWatchModule.mm. If WCSession is already activated via the central loader, relying on WCSession.defaultSession is sufficient and maintains a single session lifecycle.
Applied to files:
ios/Libraries/VoipModule.mm
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.
Applied to files:
app/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/useCallStore.test.ts
🔇 Additional comments (7)
app/lib/services/voip/MediaCallEvents.ts (1)
190-197: Android speaker-state sync is scoped correctly.The listener is only attached off-iOS, updates state only while a call is active, and will be removed by the existing teardown path.
ios/Libraries/VoipModule.mm (1)
127-133: iOS stubs match the new contract cleanly.These Promise-resolving no-ops keep the TurboModule surface aligned without changing iOS behavior.
app/lib/services/voip/MediaCallEvents.test.ts (2)
34-40: Nice cleanup hook for the test double.The dedicated
setStatewrapper keeps the new assertions isolated.
473-529: Good Android coverage for speaker sync.These assertions validate the new
VoipCommunicationDeviceChangedstate updates and teardown behavior without disturbing the existing accept/hold coverage.app/lib/native/NativeVoip.ts (1)
54-58: Spec and fallback additions line up well.The new TurboModule methods and JS no-op fallbacks match the native implementations and keep non-native environments safe.
Also applies to: 75-89
app/lib/services/voip/useCallStore.test.ts (2)
29-46: Mocking the new native methods is clean.The test double now matches the expanded TurboModule surface.
413-438: Android lifecycle assertions look good.These tests cover the new
setCall/resetsync hooks without changing iOS behavior.
dfba14a to
5071f9a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt`:
- Around line 179-217: The catch blocks in startAudioRouteSync and
stopAudioRouteSync currently set communicationDeviceListener = null even when
cleanup/teardown failed, risking native listeners remaining registered; update
logic so communicationDeviceListener is only cleared when registration or
removal actually succeeds: in startAudioRouteSync, move assignment
communicationDeviceListener = listener to after the seed-read succeeds (or set
it back if seed/read fails) and do not null it in the catch; in
stopAudioRouteSync, only set communicationDeviceListener = null after
removeOnCommunicationDeviceChangedListener completes without throwing (leave it
intact on exception) and let the catch log/reject without clearing the handle.
Reference: startAudioRouteSync, stopAudioRouteSync, communicationDeviceListener,
addOnCommunicationDeviceChangedListener,
removeOnCommunicationDeviceChangedListener.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71440d03-2d80-472a-b88e-69e337c444b0
📒 Files selected for processing (6)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktapp/lib/services/voip/MediaCallEvents.test.tsapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/useCallStore.ios.test.tsapp/lib/services/voip/useCallStore.test.tsapp/lib/services/voip/useCallStore.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/lib/services/voip/MediaCallEvents.ts
- app/lib/services/voip/MediaCallEvents.test.ts
- app/lib/services/voip/useCallStore.ios.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/services/voip/useCallStore.tsapp/lib/services/voip/useCallStore.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
**/*.{ts,tsx}: Use TypeScript with strict mode enabled and baseUrl set to app/ for module imports
Support iOS 13.4+ and Android 6.0+ as minimum target platforms
Files:
app/lib/services/voip/useCallStore.tsapp/lib/services/voip/useCallStore.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use tabs for indentation with single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses when possible
Use ESLint with@rocket.chat/eslint-configbase including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/services/voip/useCallStore.tsapp/lib/services/voip/useCallStore.test.ts
app/lib/services/voip/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Files:
app/lib/services/voip/useCallStore.tsapp/lib/services/voip/useCallStore.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
📚 Learning: 2026-04-22T22:57:58.545Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-22T22:57:58.545Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP features in app/lib/services/voip/ directory using Zustand stores for WebRTC peer-to-peer audio calls with native CallKit (iOS) and Telecom (Android) integration
Applied to files:
app/lib/services/voip/useCallStore.tsapp/lib/services/voip/useCallStore.test.tsandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.
Applied to files:
app/lib/services/voip/useCallStore.test.ts
🔇 Additional comments (2)
app/lib/services/voip/useCallStore.ts (1)
175-180: Android sync is wired in the right place.The
!isIOSguard and fire-and-forget native calls fit the store flow cleanly here.Also applies to: 326-330
app/lib/services/voip/useCallStore.test.ts (1)
413-437: Good coverage for the new native bridge calls.This Android suite exercises both the
setCall()andreset()paths and keeps the new mocks isolated from the existing store tests.
Registers OnCommunicationDeviceChangedListener (API 31+) for the call lifetime so external audio route changes (headset plug/unplug, BT connect/disconnect, system audio panel) mirror into useCallStore.isSpeakerOn and keep the in-call speaker icon accurate.
5071f9a to
0992f76
Compare
|
Android Build Available Rocket.Chat Experimental 4.72.0.108713 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQmWY_Xi8-WREp8CwbPMrvi5IbThj1soI8bOKp6QMOg-nBo4Jfqbly2FxaqU0T5t2CBP7KcpcFsffhpEUrI |
|
Android Build Available Rocket.Chat Experimental 4.72.0.108713 |
expo-av's Audio.Sound plays through STREAM_MUSIC, which routes to the loudspeaker by default and is unaffected by setCommunicationDevice. Result: outgoing dialtone was loud regardless of isSpeakerOn, and toggleSpeaker had no audible effect during ringback. Native VoipModule.startRingback uses MediaPlayer with AudioAttributes USAGE_VOICE_COMMUNICATION + CONTENT_TYPE_SONIFICATION, so the dialtone follows the active comm device (earpiece/speaker/BT). iOS keeps expo-av Ringer (CallKit + AVAudioSession handle routing).
|
iOS Build Available Rocket.Chat Experimental 4.72.0.108734 |
|
Android Build Available Rocket.Chat Experimental 4.72.0.108735 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNS7OyHEy1CmL4d5YiMjQR3J7O2lNRG8Wq-XUwlf7PB9KRz5vCic400QzDfgsO8ojKOr1iHNsog-bt05acME |
Proposed changes
Two related Android audio-routing fixes for the new VoIP stack.
1. Sync
isSpeakerOnwith system audio route changes (mid-call)Mid-call on Android, when audio route changed via something other than the in-app Speaker button (headset plug/unplug, BT connect/disconnect, system audio panel),
useCallStore.isSpeakerOnstayed stale and the speaker icon misrepresented the live route.Registers
AudioManager.OnCommunicationDeviceChangedListener(API 31+) for the call's lifetime. The listener emitsVoipCommunicationDeviceChanged { isSpeaker: boolean }to JS;MediaCallEventsmirrors it intouseCallStore.isSpeakerOn. A synthetic event is emitted once after registration so the icon is accurate at t=0 (covers calls accepted with speaker already engaged).The app uses a self-managed
PhoneAccount, soConnection.setAudioRoute()is a no-op and CallKeep audio-route APIs are dead — audio routing must go throughAudioManagerdirectly.2. Play outgoing-call ringback on the voice-communication audio path
expo-av'sAudio.Soundplays throughSTREAM_MUSICon Android, which routes to the loudspeaker by default and is unaffected bysetCommunicationDevice. Result: the outgoing dialtone was loud regardless ofisSpeakerOn, andtoggleSpeakerhad no audible effect during ringback.VoipModule.startRingback/stopRingbackuseMediaPlayerwithAudioAttributes USAGE_VOICE_COMMUNICATION + CONTENT_TYPE_SONIFICATIONso the dialtone follows the active communication device (earpiece by default, speaker/BT when selected).CallViewcalls native ringback on Android only; iOS keeps the existingexpo-avRinger (CallKit + AVAudioSession route it correctly).Issue(s)
https://rocketchat.atlassian.net/browse/VMUX-99
How to test or reproduce
Manual on Android device:
Audio-route sync (mid-call)
adb logcat -s RocketChat.VoipModule:Dshows listener firing on each route changeOutgoing ringback routing
6. Place an outgoing call → dialtone plays through earpiece (not loudspeaker) by default
7. Tap Speaker during ringback → dialtone audibly reroutes to loudspeaker
8. Tap Speaker again → dialtone returns to earpiece
9. Connect a BT headset before placing the call → dialtone plays through BT
10.
adb logcat -s RocketChat.VoipModule:DshowsstartRingback/stopRingbacklog linesScreenshots
Types of changes
Checklist
Further comments
startAudioRouteSync/stopAudioRouteSyncare idempotent (no-op when listener already registered / not registered).stopAudioRouteSyncis safe to call N times —resetfires 2–3× per call ending.startAudioRouteSyncresolvesnulland skips listener registration — no crash, no regression on legacy devices.startRingbackis idempotent; a second call while already playing is a no-op.stopRingbackis safe to call when not playing.VoipCommunicationDeviceChangedis never emitted;supportedEventsarray untouched.