-
-
Notifications
You must be signed in to change notification settings - Fork 307
Improve answering calls #6015
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
Improve answering calls #6015
Changes from all commits
dcde0be
04606e3
be0bd8d
ed89739
9106d40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what was causing the lock screen to appear after answering a call. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,7 @@ import coil.request.ImageRequest | |
| import com.bluelinelabs.logansquare.LoganSquare | ||
| import com.nextcloud.talk.BuildConfig | ||
| import com.nextcloud.talk.R | ||
| import com.nextcloud.talk.activities.CallActivity | ||
| import com.nextcloud.talk.activities.MainActivity | ||
| import com.nextcloud.talk.api.NcApi | ||
| import com.nextcloud.talk.application.NextcloudTalkApplication | ||
|
|
@@ -61,6 +62,7 @@ import com.nextcloud.talk.models.json.participants.Participant | |
| import com.nextcloud.talk.models.json.participants.ParticipantsOverall | ||
| import com.nextcloud.talk.models.json.push.DecryptedPushMessage | ||
| import com.nextcloud.talk.models.json.push.NotificationUser | ||
| import com.nextcloud.talk.receivers.DeclineCallReceiver | ||
| import com.nextcloud.talk.receivers.DirectReplyReceiver | ||
| import com.nextcloud.talk.receivers.DismissRecordingAvailableReceiver | ||
| import com.nextcloud.talk.receivers.MarkAsReadReceiver | ||
|
|
@@ -95,7 +97,6 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_THREAD_ID | |
| import com.nextcloud.talk.utils.preferences.AppPreferences | ||
| import io.reactivex.Observable | ||
| import io.reactivex.Observer | ||
| import io.reactivex.android.schedulers.AndroidSchedulers | ||
| import io.reactivex.disposables.Disposable | ||
| import io.reactivex.schedulers.Schedulers | ||
| import okhttp3.JavaNetCookieJar | ||
|
|
@@ -268,11 +269,65 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor | |
| } | ||
| ) | ||
|
|
||
| val pendingIntentFlags = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { | ||
| PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT | ||
| } else { | ||
| PendingIntent.FLAG_UPDATE_CURRENT | ||
| } | ||
|
|
||
| val answerVoiceBundle = Bundle(bundle).apply { putBoolean(BundleKeys.KEY_CALL_VOICE_ONLY, true) } | ||
| val answerVoicePendingIntent = PendingIntent.getActivity( | ||
| applicationContext, | ||
| requestCode + ANSWER_VOICE_REQUEST_OFFSET, | ||
| Intent(applicationContext, CallActivity::class.java).apply { | ||
| putExtras(answerVoiceBundle) | ||
| addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) | ||
| }, | ||
| pendingIntentFlags | ||
| ) | ||
|
|
||
| val answerVideoBundle = Bundle(bundle).apply { putBoolean(BundleKeys.KEY_CALL_VOICE_ONLY, false) } | ||
| val answerVideoPendingIntent = PendingIntent.getActivity( | ||
| applicationContext, | ||
| requestCode + ANSWER_VIDEO_REQUEST_OFFSET, | ||
| Intent(applicationContext, CallActivity::class.java).apply { | ||
| putExtras(answerVideoBundle) | ||
| addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) | ||
| }, | ||
| pendingIntentFlags | ||
| ) | ||
|
|
||
| val declinePendingIntent = PendingIntent.getBroadcast( | ||
| applicationContext, | ||
| requestCode + DECLINE_CALL_REQUEST_OFFSET, | ||
| Intent(applicationContext, DeclineCallReceiver::class.java).apply { | ||
| putExtra(KEY_NOTIFICATION_TIMESTAMP, pushMessage.timestamp.toInt()) | ||
| }, | ||
| pendingIntentFlags | ||
| ) | ||
|
|
||
| val soundUri = getCallRingtoneUri(applicationContext, appPreferences) | ||
| val notificationChannelId = NotificationUtils.NotificationChannels.NOTIFICATION_CHANNEL_CALLS_V4.name | ||
| val uri = signatureVerification.user!!.baseUrl!!.toUri() | ||
| val baseUrl = uri.host | ||
|
|
||
| val callerPersonBuilder = Person.Builder() | ||
| .setName(conversation.displayName) | ||
| .setImportant(true) | ||
| if (conversation.type == ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL) { | ||
| val avatarUrl = ApiUtils.getUrlForAvatar( | ||
| signatureVerification.user!!.baseUrl!!, | ||
| conversation.name, | ||
| false, | ||
| darkMode = DisplayUtils.isDarkModeOn(applicationContext) | ||
| ) | ||
| loadAvatarSync(avatarUrl, applicationContext)?.let { callerPersonBuilder.setIcon(it) } | ||
| } | ||
| val callerPerson = callerPersonBuilder.build() | ||
|
|
||
| val isVideoCall = (conversation.callFlag and Participant.InCallFlags.WITH_VIDEO) > 0 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Afaik call flags are still like this: Would be less annoying if #708 would be solved. @SystemKeeper @Ivansss how is this done on iOS?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my testing it worked as intended: That may not handle edge cases, like the caller turning off their video, or switching from audio to video, but I think the most common use case is handled. I'll wait to hear how it's handled on iOS though.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For iOS it's the same behavior that a call is offered as videocall. In the I think this is the expected behavior for most people.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in be0bd8d I will say, however, that I personally don't like this behavior. I'm not used to having to click an extra button to enable video for video calls. I think a setting would be nice for this. I can create an issue for it after this is merged.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand for some people it's not the expected behavior but as it's privacy related we should disable it initially. Some background: the call flag just tells that there is a device existing that could transmit video. It does not say video is enabled. So most of the time the called person will see "incoming video call", but actually the other participant does not have video enabled. In this case, most people will be confused to unveil their own video after accepting the call but don't the the other participants video. |
||
| val primaryAnswerIntent = if (isVideoCall) answerVideoPendingIntent else answerVoicePendingIntent | ||
|
|
||
| val notification = | ||
| NotificationCompat.Builder(applicationContext, notificationChannelId) | ||
| .setPriority(NotificationCompat.PRIORITY_HIGH) | ||
|
|
@@ -289,6 +344,11 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor | |
| .setContentIntent(fullScreenPendingIntent) | ||
| .setFullScreenIntent(fullScreenPendingIntent, true) | ||
| .setSound(soundUri) | ||
| .setStyle( | ||
| NotificationCompat.CallStyle | ||
| .forIncomingCall(callerPerson, declinePendingIntent, primaryAnswerIntent) | ||
| .setIsVideo(isVideoCall) | ||
| ) | ||
| .build() | ||
| notification.flags = notification.flags or Notification.FLAG_INSISTENT | ||
|
|
||
|
|
@@ -299,7 +359,7 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor | |
|
|
||
| chatNetworkDataSource?.getRoom(userBeingCalled, roomToken = pushMessage.id!!) | ||
| ?.subscribeOn(Schedulers.io()) | ||
| ?.observeOn(AndroidSchedulers.mainThread()) | ||
| ?.observeOn(Schedulers.io()) | ||
| ?.subscribe(object : Observer<ConversationModel> { | ||
| override fun onSubscribe(d: Disposable) { | ||
| // unused atm | ||
|
|
@@ -1107,5 +1167,8 @@ class NotificationWorker(context: Context, workerParams: WorkerParameters) : Wor | |
| private const val TIMER_COUNT = 12 | ||
| private const val TIMER_DELAY: Long = 5 | ||
| private const val LINEBREAK: String = "\n" | ||
| private const val ANSWER_VOICE_REQUEST_OFFSET = 1 | ||
| private const val ANSWER_VIDEO_REQUEST_OFFSET = 2 | ||
| private const val DECLINE_CALL_REQUEST_OFFSET = 3 | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /* | ||
| * Nextcloud Talk - Android Client | ||
| * | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: GPL-3.0-or-later | ||
| */ | ||
| package com.nextcloud.talk.receivers | ||
|
|
||
| import android.content.BroadcastReceiver | ||
| import android.content.Context | ||
| import android.content.Intent | ||
| import androidx.core.app.NotificationManagerCompat | ||
| import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_NOTIFICATION_TIMESTAMP | ||
|
|
||
| class DeclineCallReceiver : BroadcastReceiver() { | ||
|
|
||
| override fun onReceive(context: Context, intent: Intent) { | ||
| val notificationId = intent.getIntExtra(KEY_NOTIFICATION_TIMESTAMP, 0) | ||
| NotificationManagerCompat.from(context).cancel(notificationId) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should work and fix the issue with the ongoing ringing.
however it should have been covered by the call of
cancelExistingNotificationsForRoommethod inperformCall, but there seems to be a bug that this is not triggered sometimes.I will continue debugging in the evening or tomorrow to sort things out..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have some ideas what could go wrong, but it's not settled.
To simplify things, i suggest to keep your approach, but to remove the block
from the function getRoomAndContinue completely.
So even when other things go wrong, the ringing should definitely stop.
Just let me know if you want to do this or if i should take over from here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mahibi feel free to take over