Skip to content
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

Upgrade to Swift 3 #221

Closed
wants to merge 3 commits into from
Closed

Upgrade to Swift 3 #221

wants to merge 3 commits into from

Conversation

marcelgoya
Copy link

No description provided.

@saghul
Copy link
Collaborator

saghul commented Oct 31, 2016

Thanks for working on this! I'm on vacation at the moment, but I'll check
it once I'm back.

Saúl

On Oct 29, 2016 07:10, "Marcel Goya" notifications@github.com wrote:


You can view, comment on, or merge this pull request online at:

#221
Commit Summary

  • Upgrade to Swift 3

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#221, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATYGHQQOwBX8FV2-avlyWF0O-l66c2eks5q4o8BgaJpZM4Kj9_9
.

@stormbkk87
Copy link

@marcelgoya I'm getting the error below after merging your changes and trying to create a RTCPeerConnection. Any ideas? Seems simple like it's not exposing the plugin functions. However, I'm seeing the onReset write to the log. I'm sure I'm having a brain freeze but cannot for the life of me figure it out with Swift 3.

iosrtcPlugin#onReset() | doing nothing
ERROR: Method 'new_RTCPeerConnection:' not defined in Plugin 'iosrtcPlugin'

@marcelgoya
Copy link
Author

@stormbkk87 You're right. I'm getting the same error. No idea where this is coming from.

@stormbkk87
Copy link

I've been testing other cordova plugins that use audio and worked before I upgraded to Swift 3 and a few don't work anymore. So, looks like Swift 3 changes things. Apple is really annoying me lately. lol

@marcelgoya
Copy link
Author

You're absolutely not alone! What really pisses me off is that it's impossible to compile Frameworks who run V3 and V2x all together. I mean, they have I dunno how many thousands of devs working for them and they can't even add some shitty compatibility mode?!

@marcelgoya
Copy link
Author

I'm currently working on a cross platform app which uses WebRTC for the video chat feature and honestly, the amount of time I had wasted just to work around Apple's shit is seriously getting me to the point where I'm considering to stop supporting their platform because it requires such a vast amount of time to fix all their incompatibility problems and to create workarounds. I'm not saying that Android & co are much better, they have their problems too but the level of arrogance coming from Apple is really winding me up.

@stormbkk87
Copy link

Aha! Found it. The signature of the CDVPlugin methods are just a bit off so Cordova throws that error. Common cordova error I guess when Apple decides to do the evil signature change of functions. Argh.

I haven't made audio work yet. But I'm getting candidates. @marcelgoya Let me know if audio works for you.

Public cordova function signatures should look like this:

open func new_RTCPeerConnection(_ command: CDVInvokedUrlCommand)

@stormbkk87
Copy link

stormbkk87 commented Nov 1, 2016

Confirmed audio works on device iPhone 6 iOS 10. Back in the saddle. By the way, whiskey works wonders when dealing with stupid Apple changes. :)

Also, I'm using Meteor 1.4.2 for those having issues upgrading.

@marcelgoya
Copy link
Author

@stormbkk87 Yup, works for me too

@ostrichegret
Copy link

btw, can i use this for Sinch SDK?
https://github.com/sinch/sinch-js-rtc

Thanks

@marcelgoya
Copy link
Author

When are you planning to merge the Swift 3 upgrade into the main branch? It's been almost a month now and I'd like to use the official branch rather than my own.

@ibc
Copy link
Collaborator

ibc commented Nov 21, 2016

When are you planning to merge the Swift 3 upgrade into the main branch? It's been almost a month now and I'd like to use the official branch rather than my own.

Both @saghul and me have been busy in some conference events lately.

@saghul
Copy link
Collaborator

saghul commented Nov 21, 2016

@marcelgoya I'm back! Since I merged the getStats PR today this no longer applies cleanly. Can you please rebase it? Thanks!

@marcelgoya
Copy link
Author

@saghul Sure, but I won't be able to do it before Friday. Cheers

@saghul
Copy link
Collaborator

saghul commented Nov 23, 2016

@marcelgoya No problem at all, thanks!

@marcelgoya
Copy link
Author

@saghul I've just updated the PluginRTCPeerConnection file. I noticed a couple of things:

  • the following delegates were missing:
    func peerConnection(onRenegotiationNeeded peerConnection: RTCPeerConnection!)
    func peerConnection(_ peerConnection: RTCPeerConnection!, didGetStats stats: [Any]!)

  • func peerConnection(peerConnection: RTCPeerConnection!,
    didGetStats stats: [AnyObject]!) {
    needs to be rewritten. You're generating an NSDictionary although an NSArray is required in self.onGetStatsCallback. Also the type casting in the for loop is incorrect.

@marcelgoya
Copy link
Author

@saghul Any updates on this one?

@ibc
Copy link
Collaborator

ibc commented Dec 1, 2016

Holidays :)

@marcelgoya
Copy link
Author

Yeah, me too now :)

@marcelgoya marcelgoya closed this Dec 1, 2016
@saghul
Copy link
Collaborator

saghul commented Dec 2, 2016

So, why did you close this?

@saghul saghul reopened this Dec 5, 2016
@janowsiany
Copy link

Hi, wanted to ask about the status on merging this into master and releasing next version to npm.

@saghul
Copy link
Collaborator

saghul commented Dec 9, 2016

It will be ready when it's ready, not sooner and not later.

@ibc
Copy link
Collaborator

ibc commented Dec 9, 2016

Hi, wanted to ask about the status on merging this into master and releasing next version to npm.

Please, don't disturb into the issue just to make us hurry up.

@saghul
Copy link
Collaborator

saghul commented Mar 25, 2017

I tried to use this PR, but the change is too invasive and didn't work, so I switched to using the syntax converter and fixing all errors. Did some light testing (GUM only) and things seem to work.

Please see #263

@saghul saghul closed this Mar 25, 2017
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.

None yet

6 participants