Skip to content

Support self managed capability #237

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
wants to merge 14 commits into from
Closed

Support self managed capability #237

wants to merge 14 commits into from

Conversation

luxiliu
Copy link

@luxiliu luxiliu commented Jul 6, 2020

This is to support self-managed video call based on @danjenkins's changes.

@codetheweb
Copy link

I tried this PR but kept getting the below error:

Screen Shot 2020-07-06 at 11 06 19 AM

Do I need to change any methods or setup calls from the current version of this package?

RNCallKeep.setup({
      ios: {
        appName: 'Sample App',
      },
      android: {
        alertTitle: 'Permissions required',
        alertDescription: 'Sample App needs to be able to accept incoming calls.',
        cancelButton: 'Cancel',
        okButton: 'ok',
        imageName: 'phone_account_icon',
        selfManaged: true,
      },
    });

@danjenkins
Copy link
Collaborator

danjenkins commented Jul 6, 2020 via email

@luxiliu luxiliu changed the base branch from master to allow-non-numbers-and-android-video July 9, 2020 12:52
@luxiliu
Copy link
Author

luxiliu commented Jul 9, 2020

I tried this PR but kept getting the below error:

Screen Shot 2020-07-06 at 11 06 19 AM

Do I need to change any methods or setup calls from the current version of this package?

RNCallKeep.setup({
      ios: {
        appName: 'Sample App',
      },
      android: {
        alertTitle: 'Permissions required',
        alertDescription: 'Sample App needs to be able to accept incoming calls.',
        cancelButton: 'Cancel',
        okButton: 'ok',
        imageName: 'phone_account_icon',
        selfManaged: true,
      },
    });

Hi @codetheweb , do you still see this error?

@codetheweb
Copy link

Sorry, I forgot to reply.
We're no longer using this package so I can't currently test it.

@luxiliu luxiliu mentioned this pull request Jul 10, 2020
2 tasks
@meliodev
Copy link

Hey @luxiliu ! I have just tried your latest PR, and I've got the same error 'selfManaged' as @codetheweb. Do you have please a fix in mind for this ?

@luxiliu
Copy link
Author

luxiliu commented Jul 13, 2020

Hi @meliodev, sorry, just updated, can you have a try again?

@meliodev
Copy link

meliodev commented Jul 14, 2020

Hi @luxiliu !
Here is what I did:

  1. AndroidManifest.xml
<uses-permission android:name="android.permission.MANAGE_OWN_CALLS"/>
<uses-permission android:name="android.permission.READ_CALL_LOG"/>
    android: {
        . . .
        selfManaged: true,  // <- I added this 
      }

But now nothing happens when I call
RNCallKeep.displayIncomingCall(callUUID, 'X', 'Y', 'number', true);

I don't know if I have to use some other new methods/listeners. Can you please share a snippet so I can test it. Thanks for your support 💯

@luxiliu
Copy link
Author

luxiliu commented Jul 14, 2020

@meliodev , RNCallKeep.displayIncomingCall() is to trigger system native incoming call ui. Here because it's selfManaged, it won't use the native ui. Yes, there's a new callback. I'll post a snippet later

@luxiliu
Copy link
Author

luxiliu commented Jul 14, 2020

@meliodev , I've updated the example, hope it could help you.

@sboily
Copy link
Member

sboily commented Jul 14, 2020

Hello @luxiliu, i like your approach to solve this new feature, thank you for this update. What is the next step to merge this PR? Do you think it's correct for resolving the video issue on callkeep with Android? I haven't tested but i read the documentation on the Android SDK and it looks like a good solution. Any objections here about this PR?

@luxiliu
Copy link
Author

luxiliu commented Jul 14, 2020

@sboily , next step is probably to implement VideoProvider, which is to use system managed ui, but that could be in another PR. @danjenkins , do you have any concerns about this PR?

@luxiliu luxiliu requested a review from sboily July 14, 2020 03:12
@meliodev
Copy link

Hi again @luxiliu after updating the example I have got this error
react-native-callkeep-error 1

@luxiliu
Copy link
Author

luxiliu commented Jul 14, 2020

@meliodev are you able to run the example? I feel like it's the problem with your code.

@meliodev
Copy link

meliodev commented Jul 14, 2020

@luxiliu I just run the example as it is.
So when I pressed on 'Display incoming call now', showIncomingCall event was triggered. But no ringtone and pop up for answer/decline call shows up. Honestly, I don't know exactly the calling flow I have to use. Am I supposed to use another library to show an answer/call notification with ringtone ? Can you please clarify to me how I can proceed. Thank you again for your help.

@luxiliu
Copy link
Author

luxiliu commented Jul 14, 2020

@meliodev , I'm afraid you need to have a look https://developer.android.com/guide/topics/connectivity/telecom/selfManaged

@meliodev
Copy link

meliodev commented Jul 15, 2020

@luxiliu first thank you for the awesome work you have provided to this library.
Please I just want some directives as I am stuck with this issue since long time :'( . I have checked the link you gave to me but I couldn't achieve positive results. Can you please tell me briefly the next steps I have to do before redirecting the user to my own videoCall UI.
Here is the flow I am expecting and please correct me if I am wrong.

  1. The callee receives a firebase cloud notification which triggers a ringing UI (even when the app is killed and phone is locked)
  2. The callee accepts or rejects the call.
  3. The callee is redirected to the video call UI if he accepts the call.

Now my main question is

  • How can I build an incomingCall UI with ringtone. Is there an existing react native library for that ?

@luxiliu
Copy link
Author

luxiliu commented Jul 15, 2020

@meliodev, sorry, I don't know about such libraries. I'm afraid you have to explore it yourself.

@sboily
Copy link
Member

sboily commented Jul 29, 2020

Hello, i checked this PR and it add self managed capability to callkeep and not specially concentrated to the video support. I propose to renamed it to reflect this interesting work. If you are interesting about video support on callkeep, i open this PR #251. It add video support on callkeep. So basically it replace the Answer button by the video button etc ... This i not yet finished, but feedback is welcome.

@sboily sboily changed the title Allow non numbers and android video Support self managed capability Jul 29, 2020
@sboily
Copy link
Member

sboily commented Jul 29, 2020

Please note there is also a work from @Telzio guy here about self managed capability : https://github.com/Telzio/react-native-callkeep/tree/feature-android-self-managed

@luxiliu luxiliu closed this Aug 9, 2020
@danjenkins
Copy link
Collaborator

@luxiliu Why'd you close this? :)

@meliodev
Copy link

Yes I still had hope for somebody to help with that :( I really need that feature. I would love help and contribute but unfortunately I don't have enough knowledge on that.

@luxiliu luxiliu deleted the allow-non-numbers-and-android-video branch August 10, 2020 10:50
@luxiliu
Copy link
Author

luxiliu commented Aug 10, 2020

I'll create a new one

@luxiliu luxiliu restored the allow-non-numbers-and-android-video branch September 24, 2020 12:49
@luxiliu luxiliu reopened this Sep 24, 2020
@luxiliu
Copy link
Author

luxiliu commented Sep 24, 2020

I was hoping I could polish the code and raise another PR with some video support, but it turned out I couldn't find myself enough time to do the coding, so I think it's better to reopen it. Please provide feedback and re-review it, thanks.

@luxiliu
Copy link
Author

luxiliu commented Nov 2, 2020

A new pull request is raised to just focus self-managed call support, #310

@luxiliu luxiliu closed this Nov 2, 2020
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.

8 participants