Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/fga/fix voip issues #4029

Merged
merged 3 commits into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/4019.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix sticky end call notification
1 change: 1 addition & 0 deletions changelog.d/4026.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix call screen stuck with some hanging up scenarios
1 change: 1 addition & 0 deletions changelog.d/4028.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix other call not always refreshed when ended
29 changes: 16 additions & 13 deletions vector/src/main/java/im/vector/app/core/services/CallService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private val loggerTag = LoggerTag("CallService", LoggerTag.VOIP)
class CallService : VectorService() {

private val connections = mutableMapOf<String, CallConnection>()
private val knownCalls = mutableSetOf<CallInformation>()
private val knownCalls = mutableMapOf<String, CallInformation>()
private val connectedCallIds = mutableSetOf<String>()

private lateinit var notificationManager: NotificationManagerCompat
Expand Down Expand Up @@ -190,28 +190,30 @@ class CallService : VectorService() {
} else {
notificationManager.notify(callId.hashCode(), notification)
}
knownCalls.add(callInformation)
knownCalls[callId] = callInformation
}

private fun handleCallTerminated(intent: Intent) {
val callId = intent.getStringExtra(EXTRA_CALL_ID) ?: ""
val endCallReason = intent.getSerializableExtra(EXTRA_END_CALL_REASON) as EndCallReason
val rejected = intent.getBooleanExtra(EXTRA_END_CALL_REJECTED, false)
alertManager.cancelAlert(callId)
val terminatedCall = knownCalls.firstOrNull { it.callId == callId }
val terminatedCall = knownCalls.remove(callId)
if (terminatedCall == null) {
Timber.tag(loggerTag.value).v("Call terminated for unknown call $callId$")
Timber.tag(loggerTag.value).v("Call terminated for unknown call $callId")
handleUnexpectedState(callId)
return
}
knownCalls.remove(terminatedCall)
val notification = notificationUtils.buildCallEndedNotification(false)
val notificationId = callId.hashCode()
startForeground(notificationId, notification)
if (knownCalls.isEmpty()) {
Timber.tag(loggerTag.value).v("No more call, stop the service")
stopForeground(true)
mediaSession?.isActive = false
myStopSelf()
}
val wasConnected = connectedCallIds.remove(callId)
val notification = notificationUtils.buildCallEndedNotification(terminatedCall.isVideoCall)
notificationManager.notify(callId.hashCode(), notification)
if (!wasConnected && !terminatedCall.isOutgoing && !rejected && endCallReason != EndCallReason.ANSWERED_ELSEWHERE) {
val missedCallNotification = notificationUtils.buildCallMissedNotification(terminatedCall)
notificationManager.notify(MISSED_CALL_TAG, terminatedCall.nativeRoomId.hashCode(), missedCallNotification)
Expand Down Expand Up @@ -243,7 +245,7 @@ class CallService : VectorService() {
} else {
notificationManager.notify(callId.hashCode(), notification)
}
knownCalls.add(callInformation)
knownCalls[callId] = callInformation
}

/**
Expand All @@ -267,18 +269,19 @@ class CallService : VectorService() {
} else {
notificationManager.notify(callId.hashCode(), notification)
}
knownCalls.add(callInformation)
knownCalls[callId] = callInformation
}

private fun handleUnexpectedState(callId: String?) {
Timber.tag(loggerTag.value).v("Fallback to clear everything")
callRingPlayerIncoming?.stop()
callRingPlayerOutgoing?.stop()
val notification = notificationUtils.buildCallEndedNotification(false)
if (callId != null) {
notificationManager.cancel(callId.hashCode())
startForeground(callId.hashCode(), notification)
} else {
startForeground(DEFAULT_NOTIFICATION_ID, notification)
}
val notification = notificationUtils.buildCallEndedNotification(false)
startForeground(DEFAULT_NOTIFICATION_ID, notification)
if (knownCalls.isEmpty()) {
mediaSession?.isActive = false
myStopSelf()
Expand Down Expand Up @@ -371,7 +374,7 @@ class CallService : VectorService() {
putExtra(EXTRA_END_CALL_REASON, endCallReason)
putExtra(EXTRA_END_CALL_REJECTED, rejected)
}
ContextCompat.startForegroundService(context, intent)
context.startService(intent)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class CurrentCallsViewPresenter {
this.currentCall = currentCall
this.currentCall?.addListener(tickListener)
this.calls = calls
val hasActiveCall = currentCall != null
val hasActiveCall = calls.isNotEmpty()
currentCallsView?.isVisible = hasActiveCall
currentCallsView?.render(calls, currentCall?.formattedDuration() ?: "")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class SharedKnownCallsViewModel @Inject constructor(
}
}

private val currentCallListener = object : WebRtcCallManager.CurrentCallListener {
private val callManagerListener = object : WebRtcCallManager.Listener {
override fun onCurrentCallChange(call: WebRtcCall?) {
val knownCalls = callManager.getCalls()
liveKnownCalls.postValue(knownCalls)
Expand All @@ -50,12 +50,17 @@ class SharedKnownCallsViewModel @Inject constructor(
it.addListener(callListener)
}
}

override fun onCallEnded(callId: String) {
val knownCalls = callManager.getCalls()
liveKnownCalls.postValue(knownCalls)
}
}

init {
val knownCalls = callManager.getCalls()
liveKnownCalls.postValue(knownCalls)
callManager.addCurrentCallListener(currentCallListener)
callManager.addListener(callManagerListener)
knownCalls.forEach {
it.addListener(callListener)
}
Expand All @@ -65,7 +70,7 @@ class SharedKnownCallsViewModel @Inject constructor(
callManager.getCalls().forEach {
it.removeListener(callListener)
}
callManager.removeCurrentCallListener(currentCallListener)
callManager.removeListener(callManagerListener)
super.onCleared()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,15 @@ class VectorCallViewModel @AssistedInject constructor(
} ?: VectorCallViewState.TransfereeState.UnknownTransferee
}

private val currentCallListener = object : WebRtcCallManager.CurrentCallListener {
private val callManagerListener = object : WebRtcCallManager.Listener {

override fun onCallEnded(callId: String) {
withState { state ->
if (state.otherKnownCallInfo?.callId == callId) {
setState { copy(otherKnownCallInfo = null) }
}
}
}

override fun onCurrentCallChange(call: WebRtcCall?) {
if (call != null) {
Expand All @@ -159,9 +167,7 @@ class VectorCallViewModel @AssistedInject constructor(
}

private fun updateOtherKnownCall(currentCall: WebRtcCall) {
val otherCall = callManager.getCalls().firstOrNull {
it.callId != currentCall.callId && it.mxCall.state is CallState.Connected
}
val otherCall = getOtherKnownCall(currentCall)
setState {
if (otherCall == null) {
copy(otherKnownCallInfo = null)
Expand All @@ -171,6 +177,12 @@ class VectorCallViewModel @AssistedInject constructor(
}
}

private fun getOtherKnownCall(currentCall: WebRtcCall): WebRtcCall? {
return callManager.getCalls().firstOrNull {
it.callId != currentCall.callId && it.mxCall.state is CallState.Connected
}
}

init {
setupCallWithCurrentState()
}
Expand All @@ -184,7 +196,7 @@ class VectorCallViewModel @AssistedInject constructor(
}
} else {
call = webRtcCall
callManager.addCurrentCallListener(currentCallListener)
callManager.addListener(callManagerListener)
webRtcCall.addListener(callListener)
val currentSoundDevice = callManager.audioManager.selectedDevice
if (currentSoundDevice == CallAudioManager.Device.Phone) {
Expand Down Expand Up @@ -230,7 +242,7 @@ class VectorCallViewModel @AssistedInject constructor(
}

override fun onCleared() {
callManager.removeCurrentCallListener(currentCallListener)
callManager.removeListener(callManagerListener)
call?.removeListener(callListener)
call = null
proximityManager.stop()
Expand Down Expand Up @@ -310,10 +322,10 @@ class VectorCallViewModel @AssistedInject constructor(
VectorCallViewEvents.ShowCallTransferScreen
)
}
VectorCallViewActions.TransferCall -> {
VectorCallViewActions.TransferCall -> {
handleCallTransfer()
}
is VectorCallViewActions.SwitchCall -> {
is VectorCallViewActions.SwitchCall -> {
setState { VectorCallViewState(action.callArgs) }
setupCallWithCurrentState()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -810,17 +810,19 @@ class WebRtcCall(
}
}

fun endCall(reason: EndCallReason = EndCallReason.USER_HANGUP) {
fun endCall(reason: EndCallReason = EndCallReason.USER_HANGUP, sendSignaling: Boolean = true) {
sessionScope?.launch(dispatcher) {
if (mxCall.state is CallState.Ended) {
return@launch
}
val reject = mxCall.state is CallState.LocalRinging
terminate(reason, reject)
if (reject) {
mxCall.reject()
} else {
mxCall.hangUp(reason)
if (sendSignaling) {
if (reject) {
mxCall.reject()
} else {
mxCall.hangUp(reason)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ class WebRtcCallManager @Inject constructor(
private val sessionScope: CoroutineScope?
get() = currentSession?.coroutineScope

interface CurrentCallListener {
fun onCurrentCallChange(call: WebRtcCall?) {}
fun onAudioDevicesChange() {}
interface Listener {
fun onCallEnded(callId: String) = Unit
fun onCurrentCallChange(call: WebRtcCall?) = Unit
fun onAudioDevicesChange() = Unit
}

val supportedPSTNProtocol: String?
Expand All @@ -106,13 +107,13 @@ class WebRtcCallManager @Inject constructor(
protocolsChecker?.removeListener(listener)
}

private val currentCallsListeners = CopyOnWriteArrayList<CurrentCallListener>()
private val currentCallsListeners = CopyOnWriteArrayList<Listener>()

fun addCurrentCallListener(listener: CurrentCallListener) {
fun addListener(listener: Listener) {
currentCallsListeners.add(listener)
}

fun removeCurrentCallListener(listener: CurrentCallListener) {
fun removeListener(listener: Listener) {
currentCallsListeners.remove(listener)
}

Expand Down Expand Up @@ -250,10 +251,13 @@ class WebRtcCallManager @Inject constructor(
callsByRoomId[webRtcCall.signalingRoomId]?.remove(webRtcCall)
callsByRoomId[webRtcCall.nativeRoomId]?.remove(webRtcCall)
transferees.remove(callId)
if (getCurrentCall()?.callId == callId) {
if (currentCall.get()?.callId == callId) {
val otherCall = getCalls().lastOrNull()
currentCall.setAndNotify(otherCall)
}
tryOrNull {
currentCallsListeners.forEach { it.onCallEnded(callId) }
}
// There is no active calls
if (getCurrentCall() == null) {
Timber.tag(loggerTag.value).v("Dispose peerConnectionFactory as there is no need to keep one")
Expand Down Expand Up @@ -424,7 +428,11 @@ class WebRtcCallManager @Inject constructor(

override fun onCallManagedByOtherSession(callId: String) {
Timber.tag(loggerTag.value).v("onCallManagedByOtherSession: $callId")
onCallEnded(callId, EndCallReason.ANSWERED_ELSEWHERE, false)
val call = callsByCallId[callId]
?: return Unit.also {
Timber.tag(loggerTag.value).w("onCallManagedByOtherSession for non active call? $callId")
}
call.endCall(EndCallReason.ANSWERED_ELSEWHERE, sendSignaling = false)
}

override fun onCallAssertedIdentityReceived(callAssertedIdentityContent: CallAssertedIdentityContent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,6 @@ class NotificationUtils @Inject constructor(private val context: Context,
setSmallIcon(R.drawable.ic_call_answer)
}
}
// This is a trick to make the previous notification with same id disappear as cancel notification is not working with Foreground Service.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So also remove the trick itself?

Copy link
Member Author

@ganfra ganfra Sep 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it's not really a trick anymore, cancelNotification work, but only when stopForeground is called. But timeout param is working very strangely...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks

.setTimeoutAfter(1)
.setColor(ThemeUtils.getColor(context, android.R.attr.colorPrimary))
.setCategory(NotificationCompat.CATEGORY_CALL)
Expand Down