feat: iOS AVS calling support [WPB-24959]#4130
Conversation
Garzas
requested review from
emmaoke-w,
ohassine,
saleniuk,
typfel and
yamilmedina
and removed request for
a team
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4130 +/- ##
==========================================
Coverage 61.55% 61.55%
Complexity 4022 4022
==========================================
Files 2068 2068
Lines 67436 67436
Branches 6651 6651
==========================================
Hits 41513 41513
Misses 23277 23277
Partials 2646 2646 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Bencher Report
saleniuk
left a comment
saleniuk
left a comment
There was a problem hiding this comment.
I have a few (possibly silly) questions 🙇
| matching { sourceSet -> | ||
| sourceSet.name.startsWith("ios") && sourceSet.name.endsWith("Main") | ||
| }.all { | ||
| dependencies { | ||
| api(libs.avsKmp) | ||
| } | ||
| } |
There was a problem hiding this comment.
Can't we use the same approach as for other platforms?
Something like:
val iosMain by getting {
dependencies {
api(libs.avsKmp)
}
}
|
|
||
| # avs | ||
| avs = { module = "com.wire:avs", version.ref = "avs" } | ||
| avsKmp = { module = "com.wire:avs-kmp", version.ref = "avs" } |
There was a problem hiding this comment.
Wouldn't be better to separate the version numbers for regular and kmp?
| } | ||
|
|
||
| fun notifyNetworkChangedIfAvailable(): Boolean { | ||
| if (!startIfAvailable()) return false |
There was a problem hiding this comment.
Why only some functions are secured by this if (!startIfAvailable()) return false?
Others, like setNetworkQualityInterval can still be called before it's started and fail in that case, right?
Is it because some of them are called inside the withCalling block? So maybe either we should secure all of them just to be sure or maybe we should make it so that they are all scoped and can be called only inside the withCalling block?
| import kotlin.uuid.Uuid | ||
|
|
||
| @Suppress("LongParameterList", "TooManyFunctions") | ||
| internal class CallManagerImpl internal constructor( |
There was a problem hiding this comment.
Is it basically almost 1:1 copy of JvmAndroid's CallManagerImpl? It will take quite a bit more work to maintain them separately and make sure they stay consistent, so would it be possible to extract the common code and reuse it?
|
3 participants
https://wearezeta.atlassian.net/browse/WPB-24959
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
avs-kmponly into iOS source sets to avoid resolving unavailable macOS variants.Causes (Optional)
Apple calling code was still using placeholder/no-op implementations for call manager, media manager, and flow manager paths. Adding
avs-kmpat the wider Apple source set level also caused KMP variant resolution to request macOS artifacts, while the current AVS KMP artifact is iOS-only.Solutions
AppleAvsInteropto bridge Kalium’s Apple calling code to AVS KMP / AVS C APIs.CallManagerImplwith a real implementation for:GlobalCallManageron Apple to create realCallManagerImpl,FlowManagerServiceImpl, andMediaManagerServiceImplinstances.FlowManagerServiceImplto start AVS media/flow managers, attach iOS video views, and switch camera capture device.PlatformViewto carry aUIView?for AVS video attachment.avs-kmpdependency alias and scoped it toios*Mainsource sets only, avoiding unsupported macOS variant resolution.Notes
UIViewinstances for AVS video preview/rendering.MediaManagerServiceImpl; speaker state currently remains a no-op/false path.AVSFlowManagerAPI does not expose the equivalent operation.avs-kmpis scoped to iOS source sets only because the current artifact does not publish macOS variants.