Skip to content

Pass data from VoIP Notification down to RN #149

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

Merged

Conversation

rcidt
Copy link

@rcidt rcidt commented Jan 16, 2020

This PR allows a developer to pass data received in the VoIP notification payload, down to the RN layer, when reporting a new incoming call.

Ricardo Corrie added 2 commits January 16, 2020 12:03
@danjenkins
Copy link
Collaborator

Can you explain why this is useful? This should be handled by a notification library not by adding to callkeep

@rcidt
Copy link
Author

rcidt commented Jan 16, 2020

@danjenkins We don't rely on SIP Registration to receive the INVITE.

The way we have implemented Inbound Calls, is by using a "rendezvous" type model. Meaning, the VoIP Notification, contains a telephone number, which we need to then use to make an outbound call, which will connect both ends.

This change allows us to pass that data down to the RN app as part of the didDisplayIncomingCall event parameters.

I notice now that this PR, as is, will be a breaking change. I will push a change to overload the method so that the payload parameter is optional.

@danjenkins
Copy link
Collaborator

But even so shouldn't it make its way into React Native using a notification library rather than shoehorning it into callkeep? Not saying this doesn't have merit, just being cautious about not adding things that should live elsewhere into the lib

@rcidt
Copy link
Author

rcidt commented Jan 16, 2020

We made this change a while back, so my memory is not very clear as to details, but we experienced issues attempting to do just that. I believe that the behavior was inconsistent and we attributed it to Apple's overall crackdown on the way VoIP Notifications are used and handled.

PS. I just pushed a change that prevents this PR from breaking the current CallKeep API.

@rcidt rcidt force-pushed the notification-payload branch 3 times, most recently from 5bd6055 to e6a29d1 Compare February 27, 2020 18:06
@manuquentin manuquentin merged commit 4ba799d into react-native-webrtc:master Mar 10, 2020
@manuquentin
Copy link
Contributor

Thanks @rcidt, I'll update the index.ts file.

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.

3 participants