BelledonneCommunications / linphone-android

Linphone.org mirror for linphone-android (https://gitlab.linphone.org/BC/public/linphone-android)
https://linphone.org
GNU General Public License v3.0
1.12k stars 680 forks source link

"ping" SIP proxy when push received and ready to answer call #564

Closed brianjmurrell closed 5 years ago

brianjmurrell commented 5 years ago

It would be immensely helpful in programming a SIP proxy to send pushes to linphone if linphone would ping back to the SIP proxy when it is ready to receive the INVITE.

At worst this ping could be a registration but perhaps something more lightweight can be used, such as a NOTIFY or OPTIONS, if the SIP protocol allows such from a SIP client to a proxy.

Viish commented 5 years ago

Actually it does "ping back" using a REGISTER. Regarding the SIP protocol for that, there is a new RFC dedicated to push: https://datatracker.ietf.org/doc/draft-ietf-sipcore-sip-push/ I haven't read it yet but maybe there is something in it for that. Anyway we will integrate it, we just don't know the timeline yet.

brianjmurrell commented 5 years ago

Actually it does "ping back" using a REGISTER.

It doesn't actually. I pressume there is some kind of algorithm inside of linphone where it thinks/knows it's already registered and so doesn't send a REGISTER every time it's woken up with a push. Maybe when linphone is enabled for Background mode and/or Enable service notification.

Can we please reopen this ticket until this is resolved?

Regarding the SIP protocol for that, there is a new RFC dedicated to push: https://datatracker.ietf.org/doc/draft-ietf-sipcore-sip-push/

Yes, I am aware.

I haven't read it yet but maybe there is something in it for that. Anyway we will integrate it, we just don't know the timeline yet.

I have not read it yet either. But it's moot until there are implementations.

Viish commented 5 years ago

It's not in liblinphone, it's up to the app. Up until a few weeks we used to do setNetworkReachable(false) then true to force the sockets to be destroyed and re-created when a push was received. Now we don't, we only create and start the LinphoneService if it has been destroyed. Actually if a push is received, depending on the configuration of the sender of the push, it may indicate that the INVITE hasn't been received by the proper channel, so the only way to fix that is to REGISTER again, hence the "ping back" mechanism I mentioned.

Viish commented 5 years ago

I keep this issue closed because it's not really an issue, and it's not part of any SIP RFC (not even the one I mentioned) thus it won't be implemented.

brianjmurrell commented 5 years ago

we only create and start the LinphoneService if it has been destroyed.

So this means that a REGISTER is not sent on every push then, right?

depending on the configuration of the sender of the push, it may indicate that the INVITE hasn't been received by the proper channel, so the only way to fix that is to REGISTER again, hence the "ping back" mechanism I mentioned.

Can you expand on this since this does not seem to be happening for me.

Viish commented 5 years ago

It is easy to check. In your app code, take a look at: public void onMessageReceived(RemoteMessage remoteMessage) {} method in FirebaseMessaging class. Currently (feature/release-4.1 branch) we only start the service if it isn't running. In master branch, we do setNetworkReachable(false) then setNetworkReachable(true) to force the sockets to be re-created. You could also do a refreshRegister().

brianjmurrell commented 5 years ago

do setNetworkReachable(false) then setNetworkReachable(true) to force the sockets to be re-created

Yes, this seems to have the desired effect:

diff --git a/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java b/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
index 417bbccf..98a9eb30 100644
--- a/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
+++ b/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
@@ -24,7 +24,9 @@ import static android.content.Intent.ACTION_MAIN;
 import android.content.Intent;
 import com.google.firebase.messaging.FirebaseMessagingService;
 import com.google.firebase.messaging.RemoteMessage;
+import org.linphone.LinphoneManager;
 import org.linphone.LinphoneService;
+import org.linphone.mediastream.Log;
 import org.linphone.settings.LinphonePreferences;
 import org.linphone.utils.LinphoneUtils;

@@ -54,6 +56,20 @@ public class FirebaseMessaging extends FirebaseMessagingService {
             intent.setClass(this, LinphoneService.class);
             intent.putExtra("PushNotification", true);
             startService(intent);
+        } else if (LinphoneManager.isInstanciated() && LinphoneManager.getLc().getCallsNb() == 0) {
+            LinphoneUtils.dispatchOnUIThread(
+                    new Runnable() {
+                        @Override
+                        public void run() {
+                            Log.i(
+                                    "[Push Notification] Push notification received with LinphoneManager still alive");
+                            if (LinphoneManager.isInstanciated()
+                                    && LinphoneManager.getLc().getCallsNb() == 0) {
+                                LinphoneManager.getLc().setNetworkReachable(false);
+                                LinphoneManager.getLc().setNetworkReachable(true);
+                            }
+                        }
+                    });
         }
     }
 }

It can be much quicker to wait for the [re-]registration to happen after sending the push than just waiting some arbitrary (and worst-case long enough) timeout.

You could also do a refreshRegister().

That also works (applied to the above):

diff --git a/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java b/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
index 98a9eb30..2245c3e0 100644
--- a/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
+++ b/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
@@ -65,8 +65,7 @@ public class FirebaseMessaging extends FirebaseMessagingService {
                                     "[Push Notification] Push notification received with LinphoneManager still alive");
                             if (LinphoneManager.isInstanciated()
                                     && LinphoneManager.getLc().getCallsNb() == 0) {
-                                LinphoneManager.getLc().setNetworkReachable(false);
-                                LinphoneManager.getLc().setNetworkReachable(true);
+                                LinphoneManager.getLc().refreshRegisters();
                             }
                         }
                     });

Is this any better than the above for any reasons at all?

brianjmurrell commented 5 years ago

I have to say, having this re-registration "ping" is substantially better than the PBX just having to wait for worst-case timeout before sending the INVITE.

Is there any reason this sort of behaviour is not the default?

Without such a ping, how is a PBX supposed to know it's OK to send the INVITE -- that the client is ready to be INVITEd?

Since I have to build my own linphone client to have the google-services.json included anyway, it's not a big deal for me to carry a patch such as this, but this really seems like more generally useful behaviour for anyone trying to implement push solutions. At least prior to any kind of implementations (on both clients and servers) of https://datatracker.ietf.org/doc/draft-ietf-sipcore-sip-push/.

brianjmurrell commented 5 years ago

Any suggestions on how to implement this behaviour (always REGISTERing after receiving a push notification, even when already awake) now that LinphoneManager has been refactored in 4b846bcc?

Viish commented 5 years ago

If you want to force a register simply call core. refreshRegister()

brianjmurrell commented 5 years ago

Ah, yes, of course. As easy as:

--- a/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
+++ b/app/src/main/java/org/linphone/firebase/FirebaseMessaging.java
@@ -62,7 +62,7 @@ public class FirebaseMessaging extends FirebaseMessagingService {
             if (LinphoneManager.getInstance() != null) {
                 Core core = LinphoneManager.getCore();
                 if (core != null) {
-                    core.ensureRegistered();
+                    core.refreshRegisters();
                 }
             }
         }