flutter-webrtc / callkeep

iOS CallKit and Android ConnectionService for Flutter
MIT License
131 stars 142 forks source link

Remove PushKit code from iOS #40

Closed VictorUvarov closed 3 years ago

VictorUvarov commented 3 years ago

I don't think it is a good idea to handle pushkit notifications with this plugin.

cloudwebrtc commented 3 years ago

Pushkit must be used with Callkit. I think this is a perfect way. If you need to use fcm to wake up callkeep, you can also call displayIncomingCall from dart api.

https://stackoverflow.com/questions/59686121/using-pushkit-without-callkit-will-not-terminate-the-app-any-more-on-xcode-11-3

VictorUvarov commented 3 years ago

I understand, but the current implementation seems limited, what if we want to handle generic calls without numbers. Some things are hardcoded: hasVideo, localizedCallerName, handleType.

- (void)pushRegistry:(PKPushRegistry *)registry didReceiveIncomingPushWithPayload:(PKPushPayload *)payload forType:(NSString *)type {
    // Process the received push
    NSLog(@"didReceiveIncomingPushWithPayload payload = %@", payload.type);
    /* payload example.
     {
         "callkeep": {
             "title": "Incoming Call",
             "number": "+86186123456789"
         }
     }
    */
    NSDictionary *dic = payload.dictionaryPayload[@"callkeep"];
    NSString *number = dic[@"number"];
    [CallKeep reportNewIncomingCall:[self createUUID]
                             handle:number
                         handleType:@"number"
                           hasVideo:NO
                localizedCallerName:@"hello"
                        fromPushKit:YES
                            payload:payload.dictionaryPayload
              withCompletionHandler:^(){}];
}
ghenry commented 3 years ago

I understand, but the current implementation seems limited, what if we want to handle generic calls without numbers. Some things are hardcoded: hasVideo, localizedCallerName, handleType.

Yep, see #29

cloudwebrtc commented 3 years ago

@VictorUvarov @ghenry If we use this format, it should work.

     {
         "callkeep": {
             "title": "Incoming Call",
             "number": "+8618612345678",
             "localizedCallerName": "hello",
             "hasVideo": false,
             "handleType": "number", 
         }
     }
ghenry commented 3 years ago

Is this new or user error for incorrect payload?

On Wed, 23 Dec 2020, 21:52 CloudWebRTC, notifications@github.com wrote:

@VictorUvarov https://github.com/VictorUvarov @ghenry https://github.com/ghenry If we use this format, it should work.

 {
     "callkeep": {
         "title": "Incoming Call",
         "number": "+8618612345678",
         "localizedCallerName": "hello",
         "hasVideo": false,
         "handleType": "number",
     }
 }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flutter-webrtc/callkeep/issues/40#issuecomment-750487394, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG66FOZH67BN5AAIQSBL3SWJRCHANCNFSM4VGWLNCA .

cloudwebrtc commented 3 years ago

@ghenry I want to set a fixed format JSON payload for callkeep. Because the message content of offline calls is generally fixed, number or ID or email, title, display name, type (audio/video).

ghenry commented 3 years ago

Understood.

Don't we want callerid_name and callerid_number instead title and number?

Do we need the callkeep key?

On Wed, 23 Dec 2020, 23:24 CloudWebRTC, notifications@github.com wrote:

@ghenry https://github.com/ghenry I want to set a fixed format JSON payload for callkeep.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flutter-webrtc/callkeep/issues/40#issuecomment-750556094, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG66AAWLTDEN5CURP4FHTSWJ34PANCNFSM4VGWLNCA .

cloudwebrtc commented 3 years ago

@ghenry callerid_name and callerid_number are typical VOIP naming. I guess that the use of callkeep is not limited to VOIP. There may be other use of email or non-number strings. For example, use ion-conference-room1

use caller_id and caller_name ?

{
         "callkeep": {
             "title": "Incoming Call",
             "caller_id": "+8618612345678",
             "caller_name": "hello",
             "caller_id_type": "number", 
             "has_video": false,
         },
         "extra": {
             "foo": "bar",
             "key": "value",
         }
 }

callkeep key I think it can be kept, If the user wants to carry an extra payload in push, callkeep key can help isolate the data, making it more convenient to use.

ghenry commented 3 years ago

What do the other projects do?

It’s all a phone call whether voice or video. This is all VoIP. Voice over IP, Video over IP. Doesn’t matter if it’s SIP or WebRTC :)

-- Kind Regards,

Gavin Henry. Managing Director.

T +44 (0) 330 44 50 000 D +44 (0) 330 44 55 007 M +44 (0) 7930 323266 F +44 (0) 1224 824887 E ghenry@suretec.co.uk

Open Source. Open Solutions(tm).

http://www.suretecsystems.com/

Suretec Systems is a limited company registered in Scotland. Registered number: SC258005. Registered office: 24 Cormack Park, Rothienorman, Inverurie, Aberdeenshire, AB51 8GL.

Subject to disclaimer at http://www.suretecgroup.com/disclaimer.html

ghenry commented 3 years ago

I’m happy with that schema anyway.

On Wed, 23 Dec 2020 at 23:37, CloudWebRTC notifications@github.com wrote:

@ghenry https://github.com/ghenry callerid_name and callerid_number are typical VOIP naming. I guess that the use of callkeep is not limited to VOIP. There may be other use of email or non-number strings. For example, use ion-conference-room1

use caller_id and caller_name ?

{ "callkeep": { "title": "Incoming Call", "caller_id": "+8618612345678", "caller_name": "hello", "callerId_type": "number", "has_video": false, } }

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flutter-webrtc/callkeep/issues/40#issuecomment-750571712, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG66CN4GDXG2HJZVC6BH3SWJ5J3ANCNFSM4VGWLNCA .

-- Kind Regards,

Gavin Henry. Managing Director.

T +44 (0) 330 44 50 000 D +44 (0) 330 44 55 007 M +44 (0) 7930 323266 F +44 (0) 1224 824887 E ghenry@suretec.co.uk

Open Source. Open Solutions(tm).

http://www.suretecsystems.com/

Suretec Systems is a limited company registered in Scotland. Registered number: SC258005. Registered office: 24 Cormack Park, Rothienorman, Inverurie, Aberdeenshire, AB51 8GL.

Subject to disclaimer at http://www.suretecgroup.com/disclaimer.html

cloudwebrtc commented 3 years ago

What do the other projects do? It’s all a phone call whether voice or video. This is all VoIP. Voice over IP, Video over IP. Doesn’t matter if it’s SIP or WebRTC :) -- Kind Regards, Gavin Henry. Managing Director. T +44 (0) 330 44 50 000 D +44 (0) 330 44 55 007 M +44 (0) 7930 323266 F +44 (0) 1224 824887 E ghenry@suretec.co.uk Open Source. Open Solutions(tm). http://www.suretecsystems.com/ Suretec Systems is a limited company registered in Scotland. Registered number: SC258005. Registered office: 24 Cormack Park, Rothienorman, Inverurie, Aberdeenshire, AB51 8GL. Subject to disclaimer at http://www.suretecgroup.com/disclaimer.html

For example, I set up an important video conference on the server. The server may push video calls at a specified time. At this time, the id may not be a number. I don’t know if my understanding is correct. The one pushed is not necessarily number, it may be non-number id

cloudwebrtc commented 3 years ago

It’s all a phone call whether voice or video. This is all VoIP. Voice over IP, Video over IP. Doesn’t matter if it’s SIP or WebRTC :)

Sorry, I equate VOIP with SIP audio/video call. In fact, WebRTC is also voip.

cloudwebrtc commented 3 years ago

@VictorUvarov Do you have any thoughts on this payload format?

{
         "callkeep": {
             "caller_id": "+8618612345678",
             "caller_name": "hello",
             "caller_id_type": "number", 
             "has_video": false,
         },
         "extra": {
             "foo": "bar",
             "key": "value",
         }
 }
VictorUvarov commented 3 years ago

As long as we can handle every parameter the native callkit reportNewIncomingCall can handle we should be good.

Here is what i'm currently doing natively:

 private func processPush(with payload: PKPushPayload, type: PKPushType, and completion: (() -> Void)?) {
        print("Push received: \(payload)")
        PushKitService.shared.didReceiveIncomingPush(with: payload, forType: type)

        // get uuid and other needed information from payload
        guard let callUUID = payload.dictionaryPayload["id"] as? String,
              let callerName = payload.dictionaryPayload["caller_name"] as? String,
              let isVideoCall = payload.dictionaryPayload["is_video_call"] as? Bool,
              !FlutterCallkitPlugin.hasCall(with: UUID(uuidString: callUUID)!)
        else {
            completion?()
            return
        }

        // prepare call update
        let callUpdate = CXCallUpdate()
        callUpdate.localizedCallerName = callerName
        callUpdate.remoteHandle = CXHandle(type: .generic, value: callerName)
        callUpdate.supportsHolding = false
        callUpdate.supportsGrouping = false
        callUpdate.supportsUngrouping = false
        callUpdate.supportsDTMF = false
        callUpdate.hasVideo = isVideoCall

        // prepare provider configuration
        let configuration = CXProviderConfiguration(localizedName: "appName")
        configuration.iconTemplateImageData = UIImage(named: "call_logo")?.pngData()
        if #available(iOS 11.0, *) {
            configuration.includesCallsInRecents = false
        }
        configuration.supportsVideo = isVideoCall

        // send it to the plugin
        FlutterCallkitPlugin.reportNewIncomingCall(
            with: UUID(uuidString: callUUID)!,
            callUpdate: callUpdate,
            providerConfiguration: configuration,
            pushProcessingCompletion: completion
        )
    }