feat: configureCallKit before JS initialisation to avoid dropping incoming calls on cold start #193

vuletuanbt · 2021-03-12T07:24:20Z

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
 ...
  RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];
  _rnTwilioVoice = [bridge moduleForClass:[RNTwilioVoice class]];
  [_rnTwilioVoice initPushRegistry];
  [_rnTwilioVoice reRegisterWithTwilioVoice];

    NSDictionary *configCallkit = @{@"appName": @"AppName"};
  [_rnTwilioVoice configCallKit:configCallkit];
  
  // ---  Voip Push Notification
  // ===== (THIS IS OPTIONAL BUT RECOMMENDED) =====
  // --- register VoipPushNotification here ASAP rather than in JS. Doing this from the JS side may be too slow for some use cases
  // --- see: https://github.com/react-native-webrtc/react-native-voip-push-notification/issues/59#issuecomment-691685841
  [RNVoipPushNotificationManager voipRegistration];
  ...
  return YES;
}

jdegger · 2021-03-15T08:02:13Z

Thank you for your PR @vuletuanbt, we will review it as soon as possible!

@fabriziomoscon For review

fabriziomoscon · 2021-03-29T11:13:02Z

@vuletuanbt I will test this tonight

fabriziomoscon · 2021-03-30T07:18:00Z

@vuletuanbt I will continue testing this today, as last night I run into Signing certificates issues

fabriziomoscon · 2021-04-01T12:37:07Z

UPDATE:
@vuletuanbt I am currently installing the latest Xcode as I need it to test iOS 14

fabriziomoscon · 2021-04-01T14:40:53Z

UPDATE:
@vuletuanbt with Xcode 12.4 my app doesn't build anymore, so before I can test your code I need to fix this issue.

rkrk9 · 2021-04-01T18:11:57Z

@fabriziomoscon @vuletuanbt
I build successful with the following code.

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];
  
  id _rnTwilioVoice = [bridge moduleForClass:[RNTwilioVoice class]];

  [_rnTwilioVoice initPushRegistry];
  [_rnTwilioVoice reRegisterWithTwilioVoice];

fabriziomoscon · 2021-04-03T11:00:54Z

UPDATE:
I updated Xcode to 12.4, RN to 0.62.2 and fixed the iOS build issues. Next step for me would be to use these changes and test that I can receive calls in background

fabriziomoscon · 2021-04-03T16:38:51Z

ios/RNTwilioVoice/RNTwilioVoice.m

+}
+
+- (void) reRegisterWithTwilioVoice {
+    NSString *accessToken = [self fetchAccessToken];


this code has been copied and pasted form the function:
- (void)pushRegistry:(PKPushRegistry *)registry didUpdatePushCredentials:(PKPushCredentials *)credentials forType:(NSString *)type {

Please extract the code into a function so that it can be called in both occasions

fabriziomoscon · 2021-04-04T10:55:49Z

@vuletuanbt please could you let me know which pod TwilioVoice do you use?

vuletuan · 2021-04-04T11:06:38Z

@vuletuanbt please could you let me know which pod TwilioVoice do you use?

pod 'TwilioVoice', '~> 5.2.0'
It's your package default. What's your problem ?

fabriziomoscon · 2021-04-05T11:48:50Z

@vuletuan after reading these:
react-native-webrtc/react-native-voip-push-notification#59
react-native-webrtc/react-native-voip-push-notification#69
I understood the intention of your PR.

When receiving a background call, there could be a race condition between the native handling of the push and the JS initialisation:
native handling push notification: ====> 1ms
JS callkit initialisations: ============> 2.3 ms
that results in the call being not reported.
This issue is also present in Android and I am already working on a PR containing the fix: #164 (comment)

To fix this issue this PR should suggest to perform as much of the CallKit initialisation as possible in the main app.

vuletuanbt · 2021-04-06T12:25:22Z

@vuletuan after reading these:
react-native-webrtc/react-native-voip-push-notification#59
react-native-webrtc/react-native-voip-push-notification#69
I understood the intention of your PR.

When receiving a background call, there could be a race condition between the native handling of the push and the JS initialisation:
native handling push notification: ====> 1ms
JS callkit initialisations: ============> 2.3 ms
that results in the call being not reported.
This issue is also present in Android and I am already working on a PR containing the fix: #164 (comment)

To fix this issue this PR should suggest to perform as much of the CallKit initialisation as possible in the main app.

So, you mean I could use an event to communicate with JS instead of the directed way like I did to handle the incoming call

fabriziomoscon · 2021-04-06T18:38:09Z

No, I will integrate the necessary changes into #164

fabriziomoscon · 2021-04-07T07:52:15Z

@vuletuanbt I have added this commit on top of the new branch: c176602
feel free to test it.

But it doesn't work for me, because the to register for CallKit you need the accessToken which is provided by the server, which is contacted by JS, where the user session credentials are stored.

I see this errors in the Xcode debug log:

ERROR:Twilio:[Platform](0x102d46bc0): Registration failed: Error Domain=com.twilio.voice.error Code=31301 "Registration failed" UserInfo={NSLocalizedDescription=Registration failed, NSLocalizedFailureReason=Invalid Access Token}
2021-04-07 00:12:39.225224+0100 Hoxfon[336:12211] ERROR:Twilio:[Platform](0x102d46bc0): Inside register:deviceToken:completion:, failed to register for Twilio push notifications. Error:Registration failed
2021-04-07 00:12:39.227025+0100 Hoxfon[336:12211] An error occurred while registering: Registration failed
2021-04-07 00:12:39.228 [warn][tid:main][RCTEventEmitter.m:53] Sending `deviceNotReady` with no listeners registered

Are you and @reikireirikei experiencing the same issue? How did you make it work?

vuletuan · 2021-04-07T08:02:17Z

@vuletuanbt I have added this commit on top of the new branch: c176602
feel free to test it.

But it doesn't work for me, because the to register for CallKit you need the accessToken which is provided by the server, which is contacted by JS, where the user session credentials are stored.

I see this errors in the Xcode debug log:
ERROR:Twilio:[Platform](0x102d46bc0): Registration failed: Error Domain=com.twilio.voice.error Code=31301 "Registration failed" UserInfo={NSLocalizedDescription=Registration failed, NSLocalizedFailureReason=Invalid Access Token}
2021-04-07 00:12:39.225224+0100 Hoxfon[336:12211] ERROR:Twilio:[Platform](0x102d46bc0): Inside register:deviceToken:completion:, failed to register for Twilio push notifications. Error:Registration failed
2021-04-07 00:12:39.227025+0100 Hoxfon[336:12211] An error occurred while registering: Registration failed
2021-04-07 00:12:39.228 [warn][tid:main][RCTEventEmitter.m:53] Sending `deviceNotReady` with no listeners registered
Are you and @reikireirikei experiencing the same issue? How did you make it work?

I ran into it the first time launch app but it's ok. After that, you just init Twilio on JS like before. Then you can receive the call normally. Maybe it needs some enhance

fabriziomoscon · 2021-04-07T11:44:17Z

I see.
The title of this PR is a bit misleading: feat: support for receiving incoming call in background on IOS 14, because the current version of master allows for incoming calls in the background on iOS 14, I actually checked. So may I ask you to change the title to: configureCallKit on native app or something similar.

I have checked the Twilio quickstart project and I can see that it has a few important changes, so I think it is best for me to focus on migrating this module to align with the latest Twilio SDK version before reconsidering this PR.

I hope that you would agree with me that having errors at initialisations is not what we want. Feel free to use your own fork until then

vuletuan · 2021-04-07T12:28:37Z

@fabriziomoscon

I hope that you would agree with me that having errors at initialisations is not what we want

Sorry for my bad. I was missing to resolve this. I just focused to handle the incoming call in the background.

| The title of this PR is a bit misleading: feat: support for receiving incoming call in background on IOS 14, because the current version of master allows for incoming calls in the background on iOS 14, I actually checked.

Is that your master or Twilio ? Can you share with me?

I have checked the Twilio quickstart project and I can see that it has a few important changes, so I think it is best for me to focus on migrating this module to align with the latest Twilio SDK version before reconsidering this PR.

Is that the 5.5.2 version on IOS ?

fabriziomoscon · 2021-04-07T21:33:29Z

I will leave my experiment that uses your code here: https://github.com/hoxfon/react-native-twilio-programmable-voice/commits/feat/twilio-android-sdk-5-and-ios-bg-call-exp just in case you need to confirm the errors or try to fix it.
I am currently working on top of branch: feat/twilio-android-sdk-5, so please consider making PRs to that branch, if you find a fix.

Background calls on iOS work on the current master commit (and on branch feat/twilio-android-sdk-5) of this repo: fe40b2e
Obviously you need to have the correct push certificate setup in Twilio console to receive calls, and there is difference between debug and release certificates. When your certificates are mixed up, Twilio console (under Programmable Voice) has APN errors usually.

Twilio changelog is here: https://www.twilio.com/docs/voice/voip-sdk/ios/changelog
Twilio quickstart commits are here: https://github.com/twilio/voice-quickstart-ios/commits/master

My Apps file: AppDeletagate.m has these lines:

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];
  RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:bridge
                                                   moduleName:@"Hoxfon"
                                            initialProperties:nil];

  if ([FIRApp defaultApp] == nil) {
    [FIRApp configure];
  }

  rootView.backgroundColor = [[UIColor alloc] initWithRed:1.0f green:1.0f blue:1.0f alpha:1];

  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  UIViewController *rootViewController = [UIViewController new];
  rootViewController.view = rootView;
  self.window.rootViewController = rootViewController;
  [self.window makeKeyAndVisible];

 
  NSArray *allPngImageNames = [[NSBundle mainBundle] pathsForResourcesOfType:@"png" inDirectory:nil];
  for (NSString *imgName in allPngImageNames){
    if ([imgName containsString:@"LaunchImage"]){
      UIImage *img = [UIImage imageNamed:imgName];

      if (img.scale == [UIScreen mainScreen].scale && CGSizeEqualToSize(img.size, [UIScreen mainScreen].bounds.size)) {
        rootView.backgroundColor = [UIColor colorWithPatternImage:img];
      }
    }
  }

  NSLog(@"Twilio Voice Version: %@", [TwilioVoice sdkVersion]);

  return YES;
}

If you want to help with this project please be my guest. And I would really like if you could help with commits on branch feat/twilio-android-sdk-5. My plan is to update both Android and iOS to the latest version of Twilio Voice SDK by following the commit of Twilio engineers here:

Thank you for your help!

feat: support for receiving incoming call in background on IOS 14

f642522

jdegger requested a review from fabriziomoscon March 15, 2021 08:02

fabriziomoscon reviewed Apr 3, 2021

View reviewed changes

fabriziomoscon closed this Apr 7, 2021

fabriziomoscon changed the title ~~feat: support for receiving incoming call in background on IOS 14~~ feat: configureCallKit before JS initialisation to avoid dropping incoming calls before JS initialisation Jul 25, 2021

fabriziomoscon changed the title ~~feat: configureCallKit before JS initialisation to avoid dropping incoming calls before JS initialisation~~ feat: configureCallKit before JS initialisation to avoid dropping incoming calls on cold start Jul 25, 2021

feat: configureCallKit before JS initialisation to avoid dropping incoming calls on cold start #193

feat: configureCallKit before JS initialisation to avoid dropping incoming calls on cold start #193

Uh oh!

Conversation

vuletuanbt commented Mar 12, 2021

Uh oh!

jdegger commented Mar 15, 2021

Uh oh!

fabriziomoscon commented Mar 29, 2021

Uh oh!

fabriziomoscon commented Mar 30, 2021

Uh oh!

fabriziomoscon commented Apr 1, 2021

Uh oh!

fabriziomoscon commented Apr 1, 2021

Uh oh!

rkrk9 commented Apr 1, 2021

Uh oh!

fabriziomoscon commented Apr 3, 2021

Uh oh!

fabriziomoscon Apr 3, 2021

Choose a reason for hiding this comment

Uh oh!

fabriziomoscon commented Apr 4, 2021

Uh oh!

vuletuan commented Apr 4, 2021

Uh oh!

fabriziomoscon commented Apr 5, 2021

Uh oh!

vuletuanbt commented Apr 6, 2021

Uh oh!

fabriziomoscon commented Apr 6, 2021

Uh oh!

fabriziomoscon commented Apr 7, 2021

Uh oh!

vuletuan commented Apr 7, 2021

Uh oh!

fabriziomoscon commented Apr 7, 2021

Uh oh!

vuletuan commented Apr 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabriziomoscon commented Apr 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vuletuan commented Apr 7, 2021 •

edited

Loading