Feat/v4 Twilio iOS v5 #138

lonnylot · 2019-12-28T19:12:12Z

Upgrading to use Twilio 5.x iOS SDK.

This is an important update as the Twilio iOS 2.x SDK is unsupported as of January 1st 2020 and they recommend using the iOS 5.x SDK for incoming calls on iOS >= 13.x.

I am not an iOS dev. and I am giving this my best shot. I believe there are bugs, but I wanted to get it out publicly to ask for help from anyone who knows iOS/Objective-C.

EDIT: Here are some links related to the breaking changes:

enaluz · 2019-12-28T23:52:42Z

Great work! I was just getting started on this on my own, but glad to see someone made some headway already.

After some initial testing on my app, it seems that when the call is ended on either end of the call, the call isn't terminated for the other party. Perhaps a bug with the performEndCallAction method on line 543? Will try to investigate further.

lonnylot · 2019-12-29T00:25:09Z

Ethan, please make sure you have the latest version updated about 2 hours ago. I believe I found and corrected the issue.

…

Sent from my iPhone

On Dec 28, 2019, at 11:52 PM, Ethan Naluz ***@***.***> wrote: After some initial testing on my app, it seems that when the call is ended on either end of the call, the call isn't terminated for the other party. Perhaps a bug with the performEndCallAction method on line 543? Will try to investigate further. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

lonnylot · 2019-12-29T00:26:36Z

Put correctly: I found an issue with canceling incoming calls. Make sure you have that fix when testing and let me know if that fixes your issue.

…

Sent from my iPhone

On Dec 28, 2019, at 11:52 PM, Ethan Naluz ***@***.***> wrote: After some initial testing on my app, it seems that when the call is ended on either end of the call, the call isn't terminated for the other party. Perhaps a bug with the performEndCallAction method on line 543? Will try to investigate further. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

enaluz · 2019-12-29T01:14:23Z

That fix resolved the issue! The only small issue I found was that at the top of performEndCallActionWithUUID, there used to be a conditional check for the uuid that would just return if uuid == nil. An exception would be raised at the JS layer because that check was removed in your changes, however it wouldn't crash the app.

Not sure how important the uuid is (and in what situations it would be important), but my app is taking calls successfully without having the uuid set. Other than that, everything seems to check out in dev for me.

Would appreciate it if anybody else could do more testing for their use cases and report back!

lonnylot · 2019-12-29T13:28:01Z

Fixes #89

fabriziomoscon

Just a couple of clarifications.
I will need to test it on iOS before merging into master.

README.md

ios/RNTwilioVoice/RNTwilioVoice.m

enaluz · 2020-01-02T08:14:11Z

After playing around with this a bit, I found a bug when doing back to back calls. The second call doesn't terminate for the callee when the caller terminates the call. In the second call, two logs are missing that showed up during the first call: provider:performEndCallAction and provider:didDeactivateAudioSession. Seems like an easy fix, will try to fix this on my end today or tomorrow.

enaluz · 2020-01-03T01:20:12Z

The bug I described above was introduced in part, because of self.userInitiatedDisconnect = YES; in line 121 of RNTwilioVoice.m. This change broke for me because in the JS layer I was executing the disconnect method under the connectionDidDisconnect event listener (I don't use a CallUI in my app, I just use the CallKit UI). Although removing the disconnect method in the JS layer would make this PR work for me, I believe this could still break for other people. Not sure what the best move if that is the case.

Thoughts @fabriziomoscon @gitstud ?

Happy to clarify if this was a vague or confusing explanation.

fabriziomoscon · 2020-01-03T08:59:27Z

I will try to verify this and help during the weekend. Unfortunately my app conversion to RN 0.61 is taking forever, but once I am in a position to make and receive calls I will help 100% with implantation

lonnylot · 2020-01-03T11:53:50Z

@enaluz May I ask why you're calling the disconnect method from the connectionDidDisconnect event?

Also, could you please provide the NSLogs so we can see what code path is being taken? I suspect it's from activeCall or activeCallInvites not being cleared at the appropriate time.

…ected

lonnylot · 2020-01-07T10:57:46Z

I finally finished my dialer yesterday and was able to test all of the events.

@fabriziomoscon what can I do to help get this committed so we can continue to bug fix and add helpful features to this library?

fabriziomoscon

Hi, this is a very long PR. It is impossible to merge without me properly testing manually. Also because this library doesn't have tests :/
The fact that you changed some variable names, formatting and swapped lines, probably makes a lot of sense, but unfortunately it makes it tricky to complete a review.

Could I ask you to squash all commits into one and stock to the original formatting and variable names?
A future commit where you can for variable names and formatting should be sent separately.

lonnylot · 2020-01-08T15:28:01Z

Yeah, that's fare. Sorry about that - the initial implementation was just me copying the updated Twilio QuickStart, which is probably the wrong way to do it.

I'll update it again with just the changes required for updating to the new version of the iOS SDK.

* Correctly find callInvite to cancel I'm a novice at iOS development, but in my attempt to copy this QuickStart implementation into hoxfon/react-native-twilio-programmable-voice#138 I needed to make this change to get the cancel to work correctly. I wanted to share this change with you, but I have not tested it in this repo myself. Please confirm and merge if it is an issue. * Updated key name to uuid

fabriziomoscon · 2020-01-18T14:15:15Z

This was automatically closed by me deleting the base branch feature/android-skd-4.
The new base branch is: #144

Please do not remove https://github.com/lonnylot/react-native-twilio-programmable-voice/tree/feat/v4-ios-v5 as I will include it into #144

Initial iOS 5.x SDK integration

96f1f1c

lonnylot changed the base branch from master to feat/v4 December 28, 2019 19:14

lonnylot changed the title ~~Feat/v4 Twilio ios v5~~ Feat/v4 Twilio iOS v5 Dec 28, 2019

Corrected how cancelled call invites are found

f268668

lonnylot mentioned this pull request Dec 28, 2019

Correctly find callInvite to cancel twilio/voice-quickstart-objc#198

Merged

1 task

aniravi24 requested review from gitstud and fabriziomoscon December 31, 2019 01:11

fabriziomoscon requested changes Dec 31, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

ios/RNTwilioVoice/RNTwilioVoice.m Outdated Show resolved Hide resolved

lonnylot added 2 commits January 3, 2020 11:34

Minor refactoring

db3447e

Updated README

f41bb34

lonnylot requested a review from fabriziomoscon January 3, 2020 11:54

lonnylot added 3 commits January 6, 2020 22:04

Fixed setMuted and setSpeakerPhone

3132dc2

Implemented 'deviceDidReceiveIncoming' for iOS; added missing callRej…

401cfa1

…ected

Updated param keys in getActiveCall response

059f519

fabriziomoscon requested changes Jan 7, 2020

View reviewed changes

mysport12 mentioned this pull request Jan 18, 2020

Open PR Bundle - iOS and Android to v5 #145

Closed

fabriziomoscon closed this Jan 18, 2020

fabriziomoscon mentioned this pull request Jan 18, 2020

Android twilio v3 #144

Merged

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/v4 Twilio iOS v5 #138

Feat/v4 Twilio iOS v5 #138

lonnylot commented Dec 28, 2019 •

edited

Loading

enaluz commented Dec 28, 2019 •

edited

Loading

lonnylot commented Dec 29, 2019 via email

lonnylot commented Dec 29, 2019 via email

enaluz commented Dec 29, 2019 •

edited

Loading

lonnylot commented Dec 29, 2019

fabriziomoscon left a comment

enaluz commented Jan 2, 2020 •

edited

Loading

enaluz commented Jan 3, 2020 •

edited

Loading

fabriziomoscon commented Jan 3, 2020

lonnylot commented Jan 3, 2020

lonnylot commented Jan 7, 2020

fabriziomoscon left a comment

lonnylot commented Jan 8, 2020

fabriziomoscon commented Jan 18, 2020

Feat/v4 Twilio iOS v5 #138

Feat/v4 Twilio iOS v5 #138

Conversation

lonnylot commented Dec 28, 2019 • edited Loading

enaluz commented Dec 28, 2019 • edited Loading

lonnylot commented Dec 29, 2019 via email

lonnylot commented Dec 29, 2019 via email

enaluz commented Dec 29, 2019 • edited Loading

lonnylot commented Dec 29, 2019

fabriziomoscon left a comment

Choose a reason for hiding this comment

enaluz commented Jan 2, 2020 • edited Loading

enaluz commented Jan 3, 2020 • edited Loading

fabriziomoscon commented Jan 3, 2020

lonnylot commented Jan 3, 2020

lonnylot commented Jan 7, 2020

fabriziomoscon left a comment

Choose a reason for hiding this comment

lonnylot commented Jan 8, 2020

fabriziomoscon commented Jan 18, 2020

lonnylot commented Dec 28, 2019 •

edited

Loading

enaluz commented Dec 28, 2019 •

edited

Loading

enaluz commented Dec 29, 2019 •

edited

Loading

enaluz commented Jan 2, 2020 •

edited

Loading

enaluz commented Jan 3, 2020 •

edited

Loading