Skip to content

Fixed bug preventing delayed events from working #85

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

Closed

Conversation

jonastelzio
Copy link

When using [RNVoipPushNotificationManager voipRegistration]; in AppDelegate.m the RNVoipPushNotificationManager.m constructor (or init function or whatever Objective-C calls this), is executed twice, once I suppose by us interacting with RNVoipPushNotificationManager prior to the RN Bridge having instantiated it, and then again later when the bridge is going to use it and start observing.

The initialization process is a bit of a mystery to me, but in any regard, the _delayedEvents array is overridden by the 2nd initialization, causing us to loose all the delayed events, effectively causing didLoadWithEvents to never fire, and the events being lost in the void that exists between IOS and React Native :(

This fixes that, but as I'm not really an objective-c developer, I don't know if I'm just hacking this together, instead of addressing some underlaying issue.

@danjenkins danjenkins requested a review from zxcpoiu April 9, 2021 08:56
@zxcpoiu
Copy link
Member

zxcpoiu commented Apr 14, 2021

Will do, just give me few days more.

@danjenkins you guys also observed this issue?

@danjenkins
Copy link

@zxcpoiu @jonastelzio is a client of mine and I worked through the issue with him - a) I can't see any harm from the change and b) seems to fix the issues we were facing :) I just requested your review as I know Github is noisy :D

@zxcpoiu
Copy link
Member

zxcpoiu commented Apr 16, 2021

I did some tests but can't reproduce the issue.
Didn't see any duplicated logs.

@jonastelzio Do you see each log twice?

- (instancetype)init
{
    NSLog(@"DEBUG [RNVoipPushNotification][init] ENTER");
    if (self = [super init]) {
        NSLog(@"DEBUG [RNVoipPushNotification][init] INITIALIZE");
        _delayedEvents = [NSMutableArray array];
    }
    return self;
}

// --- I only saw these logs once:
//DEBUG [RNVoipPushNotification][init] ENTER
//DEBUG [RNVoipPushNotification][init] INITIALIZE


It is really strange the instance be initialized twice.
Although it won't hurt to merge this, and can be merged for sure.

But if you have this duplicate initialization issue, then react-native-callkeep would be affected as well. (and likely the other modules)

Not sure what is the root cause.

@manuquentin pardon me to ping you, but do you have any idea?

@zxcpoiu
Copy link
Member

zxcpoiu commented Apr 16, 2021

@thymikee Seems you have the same issue, do you have any clues?

@jonastelzio
Copy link
Author

But if you have this duplicate initialization issue, then react-native-callkeep would be affected as well. (and likely the other modules)

Not sure what is the root cause.

Indeed and it is! I've made the exact same patch in that repo, but yeah I don't fully understand why it's necessary either, but my insight into Objective-C isn't quite deep enough to speculate.

I'll run the debugging code monday for good measure. I keep my macbook at the office so have no way to run it at home.

@Romick2005
Copy link
Contributor

I can confirm that init is called twice. I have added some more logs and my output is the following:
image
Also I had a problem that startObserving not called on application metro bundler restart.

@zxcpoiu
Copy link
Member

zxcpoiu commented May 19, 2021

please take a look at #87 and test if that solves this problem :)

@zxcpoiu
Copy link
Member

zxcpoiu commented Jul 26, 2023

This may be solved by #87, reopen if there is still issues

@zxcpoiu zxcpoiu closed this Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants