-
Notifications
You must be signed in to change notification settings - Fork 152
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
Add support for specifying a delegate FirebaseMessagingService #191
base: master
Are you sure you want to change the base?
Conversation
@fabriziomoscon, please help us to review this PR |
Thank you for this PR @vuletuan I will also notify @fabriziomoscon about this one |
@vuletuan which FIrebase library version is your app using? When testing this I received a crash for an incoming call: java.lang.RuntimeException: Unable to instantiate service com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService: java.lang.InstantiationException: java.lang.Class<com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService> has no zero argument constructor
at android.app.ActivityThread.handleCreateService(ActivityThread.java:4057)
at android.app.ActivityThread.access$1800(ActivityThread.java:231)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1968)
at android.os.Handler.dispatchMessage(Handler.java:107)
at android.os.Looper.loop(Looper.java:214)
at android.app.ActivityThread.main(ActivityThread.java:7682)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:516)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:950) I guess that in this PR you should also update the build.gradle file and the README.md https://firebase.google.com/support/release-notes/android#messaging_v21-0-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
I'm sorry for my missing. I will update build.gradle and README.md |
Thanks @vuletuanbt you need to change this line: |
@fabriziomoscon I'm using firebase-bom(26.4.0) to install firebase-messaging. I guess my firebase-messaging version could be 21.0.1 |
I did it. |
@vuletuan I followed your instruction and update my app implementation platform('com.google.firebase:firebase-bom:26.7.0')
implementation 'com.google.firebase:firebase-messaging' And upon receiving a call the app crashes with: java.lang.RuntimeException: Unable to instantiate service com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService: java.lang.InstantiationException: java.lang.Class<com.hoxfon.react.RNTwilioVoice.fcm.VoiceFirebaseMessagingService> has no zero argument constructor By reading this https://firebase.google.com/docs/android/learn-more I can see that using the BoM is a major change in the way Firebase dependencies are handled. So I would ask you that you would submit a PR to migrate Firebase to the latest version (before this PR), so we can test that properly and see that calls in all scenarios can be received. |
It' ok. I will to help you migrate firebase to lastest version. But first, could you try firebase-bom |
@vuletuan by reading the error message I attempted to add an empty public VoiceFirebaseMessagingService() {}
public VoiceFirebaseMessagingService(FirebaseMessagingService delegate) {
super();
this.mFirebaseServiceDelegate = delegate;
callNotificationManager = new CallNotificationManager();
} and now I can receive calls with both firebase-bom: |
@fabriziomoscon I did not run into it. I will add it to my PR. What about the others? |
We're using multiple push providers and thus need to use a delegate service for handling our notifications.
This PR adds the ability to specify a delegate service during instantiation as this library will otherwise throw an NPE when attempting to invoke getReactNativeHost() when handling a remote message.