eopeter / flutter_twilio_voice

BSD 3-Clause "New" or "Revised" License
14 stars 22 forks source link

Update TwilioVoice to 5.x #5

Open markathomas opened 4 years ago

markathomas commented 4 years ago

Need to update to TwilioVoice 5.x for iOS 13 support. Details at https://github.com/twilio/twilio-voice-ios/blob/Releases/iOS-13-Migration-Guide.md

ghost commented 4 years ago

Hi @markathomas is it supported in your fork? what's the main improvements in your fork over the original? regards

markathomas commented 4 years ago

My fork uses TwilioVoice 5.x and adds features lacking in the original such as putting a call on hold, sending DTMF, explicit answer (for upcoming android support) and returning whether or not one is on call. I also changed the plugin API to return an enum instead of raw strings so the user doesn't have to parse things themselves. Additionally, for my use case, I allow the voice tokens to be supplied to the plugin rather than having the plugin request one with every call; the plugin makes HTTP requests with no authentication so I chose to provide the tokens to the plugin whenever it changes. as I'm using OAuth2 + JWT for security on my API server.

ghost commented 4 years ago

thanks @markathomas

I git clone your repo. When I try to run the example/ I get this:

...  
    Using `ARCHS` setting to build architectures of target `Pods-Runner`: (``)
    Finding Podfile changes
      - Flutter
      - flutter_twilio_voice
    Fetching external sources
    -> Fetching podspec for `Flutter` from `.symlinks/flutter/ios`
    -> Fetching podspec for `flutter_twilio_voice` from `.symlinks/plugins/flutter_twilio_voice/ios`
    Resolving dependencies of `Podfile`
      CDN: trunk Relative path: CocoaPods-version.yml exists! Returning local because checking is only perfomed in repo update
    [!] CocoaPods could not find compatible versions for pod "TwilioVoice":
      In snapshot (Podfile.lock):
        TwilioVoice (= 4.1.0, ~> 4.1)
      In Podfile:
        flutter_twilio_voice (from `.symlinks/plugins/flutter_twilio_voice/ios`) was resolved to 0.0.3, which depends on
          TwilioVoice (~> 5.1.1)
    Specs satisfying the `TwilioVoice (= 4.1.0, ~> 4.1), TwilioVoice (~> 5.1.1)` dependency were found, but they required a higher minimum deployment target.

do you know how can I fix this?

markathomas commented 4 years ago

i didn't update the example so check the Podfile and Podfile.lock in it for discrepancies. The example is from the main repo and has almost no functionality

ghost commented 4 years ago

ok thanks!

ghost commented 4 years ago

@markathomas Did you ever got app crash at open in the iOS Simulator? I tried with Xcode 10.3 and 11.3.1 with no luck

App crash with SIGILL

markathomas commented 4 years ago

No, haven't seen that but honestly I tested on a real phone. Do you have a stack trace? Perhaps I can be of assistance

ghost commented 4 years ago

ok, I will try on a real phone first I see no stack trace. thank you

markathomas commented 4 years ago

Did you init the plugin with a valid Twilio VoiceGrant token? You do so like this: FlutterTwilioVoice.tokens(accessToken: voiceToken, fcmToken: value); where accessToken is the Twilio VoiceGrant token and fcmToken is for Firebase Cloud Messaging (for Android only)

ghost commented 4 years ago

hmm! you may be right I generated the jwt token with a python script but I forgot to add it to the code I think

ghost commented 4 years ago

do you have a small working example I could just git clone and run?

markathomas commented 4 years ago

no, my project is proprietary

ghost commented 4 years ago

ok thanks

josh-burton commented 3 years ago

@markathomas is your fork still available somewhere? Can't seem to find it

markathomas commented 3 years ago

https://github.com/markathomas/flutter_twilio_voice

josh-burton commented 3 years ago

@markathomas thanks :)

diegogarciar commented 3 years ago

Hello, I have created a new plugin, based on this one with android support. I have also just added null-safety support https://pub.dev/packages/twilio_voice

markathomas commented 3 years ago

Why a new plugin instead of just creating a PR? This will only confuse users

diegogarciar commented 3 years ago

Because @eopeter seems away, I don't want to depend on the author's approval each time for improvements

markathomas commented 3 years ago

Hence this repo; perhaps you're unsure how Git works. This repo is independent of @eopeter as he/she is non-responsive as you say. Android support was already added in this repo which you would have known had you simply asked.

diegogarciar commented 3 years ago

I think you are misunderstanding, this issue is at @eopeter’s main repository, which hasn’t been updated since aug. It doesn’t include android support nor null safety. I am aware your branch does have Android as I contributed myself, but I find it harder for new users to find that the project is still alive and that there’s a working branch (yours) instead of simply searching it on pub.dev.

eopeter commented 3 years ago

If you make a pull request, I can merge it. I am not away. So don't make claims you don't know about.

eopeter commented 3 years ago

I have never received a pull request on this repo even though it's being forked so if you have a contribution to improve it, please make a pull request.

diegogarciar commented 3 years ago

@eopeter Apologies then, I can submit a PR with Android Support and Null safety. I restructured the plugin calls on mine, I can rollback those changes so there's no breaking change with yours.

eopeter commented 3 years ago

I am glad to take a PR. Too many forks with no one giving back and it shouldn’t be that way.

On Mar 6, 2021, at 12:52 PM, Diego Garcia notifications@github.com wrote:

 @eopeter Apologies then, I can submit a PR with Android Support and Null safety. I restructured the plugin calls on mine, I can rollback those changes so there's no breaking change with yours.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.