VoIPGRID / VialerSIPLib

An Objective-c wrapper for PJSIP
GNU General Public License v3.0
133 stars 69 forks source link

IPAddressMonitor doesn't work correctly #30

Closed smolskyaleksey closed 7 years ago

smolskyaleksey commented 7 years ago

You have 2 callbacks endpointConfig.cb.on_incoming_call = &onIncomingCall;

endpointConfig.cb.on_call_state = &onCallState;

onCallState called before onIncomingCall as result you post notification VSLCallStateChangedNotification and call checkNetworkMonitoring method. Inside this method

- (IPAddressMonitor *)ipAddressMonitor {
    if (!_ipAddressMonitor) {
        VSLAccount *activeAccount;
        for (VSLAccount *account in self.accounts) {
            if ([account firstActiveCall]) {
                activeAccount = account;
                break;
            }
        }

        NSString *reachabilityServer = @"sipproxy.voipgrid.nl";
        if (activeAccount) {
            reachabilityServer = activeAccount.accountConfiguration.sipProxyServer;
        }
        _ipAddressMonitor = [[IPAddressMonitor alloc] initWithHost:reachabilityServer];
    }
    return _ipAddressMonitor;
}

we don't have active call yet and this one never execute

 if (activeAccount) {
           reachabilityServer = activeAccount.accountConfiguration.sipProxyServer;
 }

active call will set inside onIncomingCall

static void onIncomingCall(pjsua_acc_id acc_id, pjsua_call_id call_id, pjsip_rx_data *rdata) {
    VSLEndpoint *endpoint = [VSLEndpoint sharedEndpoint];
    VSLAccount *account = [endpoint lookupAccount:acc_id];
    if (account) {
        DDLogInfo(@"Detected inbound call(%d) for account:%d", call_id, acc_id);
        VSLCall *call = [[VSLCall alloc] initInboundCallWithCallId:call_id account:account];
        if (call) {
            [[[VialerSIPLib sharedInstance] callManager] addCall:call];
            if ([VSLEndpoint sharedEndpoint].incomingCallBlock) {
                [VSLEndpoint sharedEndpoint].incomingCallBlock(call);
            }
        }
    } else {
        DDLogWarn(@"Could not accept incoming call. No account found with ID:%d", acc_id);
    }
}

This code

NSString *newIPAddress = [self getIPAddress:YES];
    if (![self.currentIPAddress isEqualToString:newIPAddress]) {
        self.currentIPAddress = newIPAddress;
        [[NSNotificationCenter defaultCenter] postNotificationName:IPAddressMonitorChangedNotification object:nil];
    }

was deleted in 0085628e0c7737fee7cf80cba587261a966e944e ip address still the same. Now you post IPAddressMonitorChangedNotification and execute

- (void)ipAddressChanged:(NSNotification *)notification {
    DDLogInfo(@"network changed");
    if (self.state == VSLEndpointStarted) {
       if ([self shutdownTransport]) {
            for (VSLAccount *account in self.accounts) {
                [account reregisterAccount];
            }
        }
    }
}
smolskyaleksey commented 7 years ago

And How can you make reregisterAccount after shutdownTransport ? ))

2016-12-16 11:49:22:663 FoilChat[2678:619641] Internet connection changed
2016-12-16 11:49:22:664 FoilChat[2678:619641] network changed
2016-12-16 11:49:22:664 FoilChat[2678:619641] onTransportState
2016-12-16 11:49:22:664 FoilChat[2678:619641] Shuting down transport
2016-12-16 11:49:22:664 FoilChat[2678:619641] Releasing transport
2016-12-16 11:49:22:666 FoilChat[2678:619641]    pjsua_acc.c !Acc 0: setting unregistration..
2016-12-16 11:49:22:667 FoilChat[2678:619641] tsx0x10213bca8  ..Failed to send Request msg REGISTER/cseq=33362 (tdta0x10213c400)! err=171060 (Unsupported transport (PJSIP_EUNSUPTRANSPORT))
2016-12-16 11:49:22:668 FoilChat[2678:619641]    pjsua_acc.c  ....SIP registration failed, status=503 (Unsupported transport (PJSIP_EUNSUPTRANSPORT))
2016-12-16 11:49:22:668 FoilChat[2678:619641]    pjsua_acc.c  ....Scheduling re-registration retry for acc 0 in 6 seconds..
2016-12-16 11:49:22:668 FoilChat[2678:619641] onRegState2
2016-12-16 11:49:22:668 FoilChat[2678:619641] AccountState will change from VSLAccountStateConnected(2) to VSLAccountStateDisconnected(3)
2016-12-16 11:49:22:668 FoilChat[2678:619641] Account valid: YES
2016-12-16 11:49:22:669 FoilChat[2678:619641] Sending registration for account: 0
2016-12-16 11:49:22:670 FoilChat[2678:619641]    pjsua_acc.c  ....Acc 0: setting registration..
2016-12-16 11:49:22:670 FoilChat[2678:619641]    pjsua_acc.c  .....Unable to generate suitable Contact header for registration: Unsupported transport (PJSIP_EUNSUPTRANSPORT) [status=171060]
2016-12-16 11:49:22:768 FoilChat[2678:619641]    pjsua_acc.c  .....Unable to create registration: Unsupported transport (PJSIP_EUNSUPTRANSPORT) [status=171060]
2016-12-16 11:49:22:770 FoilChat[2678:619641] Account registration failed
2016-12-16 11:49:22:779 FoilChat[2678:619641] Unable to re-register account
2016-12-16 11:49:22:779 FoilChat[2678:619641]      sip_reg.c  .Error sending request, status=171060
2016-12-16 11:49:22:779 FoilChat[2678:619641]    pjsua_acc.c  .Unable to create/send REGISTER: Unsupported transport (PJSIP_EUNSUPTRANSPORT) [status=171060]
2016-12-16 11:49:22:784 FoilChat[2678:619641] Account unregistration failed
bobvoorneveld commented 7 years ago

I'm working on a fix for the network change detection. The problem with the current code is that the IPAddress never changes, the lookup seems to be broken. As a fix, I stop watching the ip address and just register again after a network change. As a result, the current develop branch will register a couple of times because the network switches go back and forth a couple of times when you switch. I'm working on a small delay in this branch: https://github.com/VoIPGRID/VialerSIPLib/tree/feature/network-monitoring

It waits 0.1 sec before it will register again. I don't understand what you mean with the first part? I think it will always find an active call to setup the network monitoring or am I wrong?

Don't know how that last one happen. Can you give an example of what you did so I can reproduce?

smolskyaleksey commented 7 years ago

I think it will always find an active call to setup the network monitoring or am I wrong?

it is wrong. Callback onCallState(inside this callback you create ip monitor "checkNetworkMonitoring") called before callback onIncomingCall (inside this callback you set active call)

bobvoorneveld commented 7 years ago

you're right, but then it will use our proxy server instead of the server that is used for the account. Is that a problem for now? And do you have a solution to do it differently? I think there are 2 solutions:

smolskyaleksey commented 7 years ago

Yes, it is real big problem for me. I don't use sip proxy server. I made hotfix

  if (!self.monitoringCalls) {
                VSLAccount *activeAccount;
                for (VSLAccount *account in self.accounts) {
                    if ([account firstActiveCall]) {
                        activeAccount = account;
                        break;
                    }
                }
                if (activeAccount) {
                    DDLogVerbose(@"Starting network monitor");
                    [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(ipAddressChanged:) name:IPAddressMonitorChangedNotification object:nil];
                    [self.ipAddressMonitor startMonitoring];
                    self.monitoringCalls = YES;
                }
            }
bobvoorneveld commented 7 years ago

please put it in a PR, we'll accept. You could put the second if statement within the for loop and break out. (without the if statement of course).

smolskyaleksey commented 7 years ago

done! https://github.com/VoIPGRID/VialerSIPLib/pull/31

bobvoorneveld commented 7 years ago

fixed in https://github.com/VoIPGRID/VialerSIPLib/pull/34