-
Notifications
You must be signed in to change notification settings - Fork 739
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
select answer should be used for outgoing calls. #4625
Conversation
…eature/dla/outgoing_pstn_call_fails
...x-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/call/model/MxCallImpl.kt
Outdated
Show resolved
Hide resolved
@@ -203,8 +203,10 @@ internal class MxCallImpl( | |||
|
|||
override fun selectAnswer() { | |||
Timber.tag(loggerTag.value).v("select answer $callId") | |||
if (isOutgoing) return | |||
state = CallState.Answering |
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.
do we still set this state somewhere else or does CallState.Answering
no longer need to exist?
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.
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.
thanks for explaining
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 guess the initial issue was due to a quick copy paste of the accept
fun (https://github.com/vector-im/element-android/blob/main/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/call/model/MxCallImpl.kt#L179)
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.
small question around removing the state, otherwise looking good! 💯
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.
Thanks for the fix, my fault for that :/
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.
LGTM
@@ -203,8 +203,10 @@ internal class MxCallImpl( | |||
|
|||
override fun selectAnswer() { | |||
Timber.tag(loggerTag.value).v("select answer $callId") | |||
if (isOutgoing) return | |||
state = CallState.Answering |
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 guess the initial issue was due to a quick copy paste of the accept
fun (https://github.com/vector-im/element-android/blob/main/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/call/model/MxCallImpl.kt#L179)
Fixes #4621
Use case is:
The reason being that if bob tries to accept on two devices, it's clear which one alice is actually trying to connect to.
I believe this problem might have just surfaced recently due to changes at the sip bridge adding a timeout for the
selectAnswer