becvert / cordova-plugin-zeroconf

Cordova ZeroConf Plugin
MIT License
81 stars 57 forks source link

[Android, iOS] Expose init to accommodate wifi AP changes #51

Closed emcniece closed 6 years ago

emcniece commented 6 years ago

Associated PR: https://github.com/becvert/cordova-plugin-zeroconf/pull/50

I am dealing with a scenario in which users are required to change their Wifi AP connection while the app is running. This app uses Wifi to connect to a hardware device in AP mode - the hardware device then broadcasts services, and the app scans the "network" (really just the phone and the single hardware device) for services to connect to. The app tells users how to change Wifi networks, and guides them towards the proper Wifi settings screen in their OS. The hardware devices can also be added to an existing network (ie. they can operate in STA mode), but our primary setup for now is to have the hardware create an access point.

We have discovered that scanning for services is very unreliable when switching Wifi networks. Each OS "caches" scan results a bit differently, but in general the services discovered when the app first starts persist even when Wifi networks are changed.

My testing scenario looks like this: DEVICE_1 and DEVICE_2 exist in the office. DEVICE_1 is STA mode and connects to OFFICE_AP, while DEVICE_2 is in AP mode and creates DEVICE_2_AP. The phone/app first connects to OFFICE_AP and discovered DEVICE_1, then the user changes the phone Wifi connection over to DEVICE_2_AP. At this point, DEVICE_2 does not get discovered, and DEVICE_1 remains in the scan results list, even if zeroconf.watch() is called again.

This is problematic even for only 1 device, as the device will never be discovered if the user changes Wifi networks while the app is open. Right now we show users a big warning that tells them to completely close and re-open the app - doing so gets the scan to find the correct services.

I believe that this is primarily due to the Zeroconf browser instantiation being performed on app init, particularly in Android's case. https://github.com/becvert/cordova-plugin-zeroconf/blob/master/src/android/net/becvert/cordova/ZeroConf.java#L66 is only called on app startup, and as such the Wifi instances here seem to be bound to the network that is connected during boot.

I have tested the close() method and discovered that it throws an error if you call it after changing Wifi networks. Here is an example of me calling watch(), changing Wifi networks, then calling close():

[ 11-20 17:36:04.961  4334: 4458 D/ZeroConf ]
Watch _emcniece._tcp.local.

[ 11-20 17:36:05.017  4334: 5606 E/ZeroConf ]
setsockopt failed: ENODEV (No such device)
java.net.SocketException: setsockopt failed: ENODEV (No such device)
    at libcore.io.IoBridge.setSocketOption(IoBridge.java:321)
    at java.net.PlainDatagramSocketImpl.setOption(PlainDatagramSocketImpl.java:189)
    at java.net.PlainDatagramSocketImpl.join(PlainDatagramSocketImpl.java:128)
    at java.net.MulticastSocket.joinGroup(MulticastSocket.java:149)
    at javax.jmdns.impl.JmDNSImpl.openMulticastSocket(JmDNSImpl.java:474)
    at javax.jmdns.impl.JmDNSImpl.<init>(JmDNSImpl.java:423)
    at javax.jmdns.JmDNS.create(JmDNS.java:155)
    at net.becvert.cordova.ZeroConf$BrowserManager.<init>(ZeroConf.java:434)
    at net.becvert.cordova.ZeroConf$4.run(ZeroConf.java:276)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588)
    at java.lang.Thread.run(Thread.java:818)
Caused by: android.system.ErrnoException: setsockopt failed: ENODEV (No such device)
    at libcore.io.Posix.setsockoptGroupReq(Native Method)
    at libcore.io.ForwardingOs.setsockoptGroupReq(ForwardingOs.java:153)
    at libcore.io.IoBridge.setSocketOptionErrno(IoBridge.java:391)
    at libcore.io.IoBridge.setSocketOption(IoBridge.java:319)
    ... 11 more

The solution to this may be to create a method in which we can re-init the plugin. https://github.com/becvert/cordova-plugin-zeroconf/pull/50/files#diff-a9beaeb5801c88e06e5384461d466bd3R337 is a (probably not great) example of how this can be accomplished - here I am directly calling the destroy and init functions for Android.

... and it works! With this, I am able to watch network changes with @ionic-native/network and call zeroconf.reInit() - the previously results are cleared, and the new results are detected immediately.

iOS seems to have this problem as well, but I have done less testing there as my iOS device has been absent for a few days. In any case, this PR clones the behaviour in iOS to ensure symmetry.

Though I can't confirm it, this proposal may help with a few issues:

To test this, I have been using my fork with a hacked @ionic-native/zeroconf version. The Ionic Native module needs a few things added to it:

node_modules/@ionic-native/zeroconf/index.d.ts:

// tack on at the bottom
reInit(): Promise<void>;

node_modules/@ionic-native/zeroconf/index.js (generated, I know - just not sure when or where):

// existing code...
__decorate([
        Cordova(),
        __metadata("design:type", Function),
        __metadata("design:paramtypes", []),
        __metadata("design:returntype", Promise)
    ], Zeroconf.prototype, "close", null);
// new stuff here:
    __decorate([
        Cordova(),
        __metadata("design:type", Function),
        __metadata("design:paramtypes", []),
        __metadata("design:returntype", Promise)
    ], Zeroconf.prototype, "reInit", null);

Since I want to use the Rx model provided with @ionic-native/zeroconf, this is a bit of a pain to develop around... my CI pipe is unusable until I fork @ionic-native/zeroconf or until this proposal gets accepted and integrated.

What are your thoughts on this @becvert ?

becvert commented 6 years ago

I had a quick read. It makes sense. I'll take care of this sometime. A bit busy right now. thank you

emcniece commented 6 years ago

Thanks for taking a look! I figured out how to PR and build the @ionic-native stuff so I think we have a clear path forward.

I need this ASAP as it is blocking a release - either I have to fork this and @ionic-native (and modify my CI & deploys), or wait until you have time. As a busy dev myself, I sympathize and very much appreciate your attention 😄

What can I do to help you incorporate this change? Would you be interested in moving this plugin to an organization?

becvert commented 6 years ago

I've made you a collaborator for the time being. You should be able to merge your PR. Please update version number to 1.3.0 And edit the README and CHANGELOG accordingly. Thank you