becvert / cordova-plugin-zeroconf

Cordova ZeroConf Plugin
MIT License
81 stars 57 forks source link

Replace library to RxDnssd #17

Closed NeoLSN closed 5 years ago

NeoLSN commented 7 years ago

@becvert I've done the Android library replacement. Because I changed the plugin interface and I don't know swift 3. Can you help me for the iOS part?

NeoLSN commented 7 years ago

This is the ionic-native plugin file.

https://gist.github.com/NeoLSN/b8a598dd3bcdbef9115d6d84d6ae6049

becvert commented 7 years ago

Hi @NeoLSN. That looks good.

I started to switch over to RxDNSSD too. I'm not that enthusiast quite yet. There's kind of a leak. The services keep appearing and disappearing. I'm seeing this with the Bonjour Browser app as well. So I guess it's not coming from my code.

How's that working for you? What do you see in the log of watch()? How often the onNext callback is called?

Also is there a Rx implementation for iOS or is it just a Java stuff?

becvert commented 7 years ago

The following is what happens to me:

first, a service is discovered OK

Sending result: {"action":"added","service":{"domain":"local.","port":48084,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","hostname":"d18a95ec-77b4-4769-88c9-0800772daa79.local.","type":"_belote-rebelote._tcp.","txtRecord":{"nom":"Becvert's Galaxy","joueur":"Tab"},"addresses":["192.168.1.82"]}}

then it's removed and re-added twice in a row

Sending result: {"action":"removed","service":{"domain":"local.","port":0,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","type":"_belote-rebelote._tcp.","txtRecord":{},"addresses":[]}}
Sending result: {"action":"added","service":{"domain":"local.","port":48084,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","hostname":"d18a95ec-77b4-4769-88c9-0800772daa79.local.","type":"_belote-rebelote._tcp.","txtRecord":{"nom":"Becvert's Galaxy","joueur":"Tab"},"addresses":["192.168.1.82"]}}
Sending result: {"action":"added","service":{"domain":"local.","port":48084,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","hostname":"d18a95ec-77b4-4769-88c9-0800772daa79.local.","type":"_belote-rebelote._tcp.","txtRecord":{"nom":"Becvert's Galaxy","joueur":"Tab"},"addresses":["192.168.1.82"]}}

then it's removed and re-added 3 times in a row

Sending result: {"action":"removed","service":{"domain":"local.","port":0,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","type":"_belote-rebelote._tcp.","txtRecord":{},"addresses":[]}}
Sending result: {"action":"added","service":{"domain":"local.","port":48084,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","hostname":"d18a95ec-77b4-4769-88c9-0800772daa79.local.","type":"_belote-rebelote._tcp.","txtRecord":{"nom":"Becvert's Galaxy","joueur":"Tab"},"addresses":["192.168.1.82"]}}
Sending result: {"action":"added","service":{"domain":"local.","port":48084,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","hostname":"d18a95ec-77b4-4769-88c9-0800772daa79.local.","type":"_belote-rebelote._tcp.","txtRecord":{"nom":"Becvert's Galaxy","joueur":"Tab"},"addresses":["192.168.1.82"]}}
Sending result: {"action":"added","service":{"domain":"local.","port":48084,"name":"20782720-d8ae-8e5a-cd76-2b1024947b70","hostname":"d18a95ec-77b4-4769-88c9-0800772daa79.local.","type":"_belote-rebelote._tcp.","txtRecord":{"nom":"Becvert's Galaxy","joueur":"Tab"},"addresses":["192.168.1.82"]}}

and so on

Weird behavior, isn't it? Not expected?

JmDNS is stable in comparison

NeoLSN commented 7 years ago

1) Also is there a Rx implementation for iOS or is it just a Java stuff? I don't know there is or not Rx implementation for iOS. Rx stuff is included in the RxDnssd library and it is another java library.

2) How's that working for you? I just do add/remove the service information.

3) What do you see in the log of watch()? I saw the same thing like you. I've used RxDnssd on the Android native app, so I was thinking this is a normal behavior until now.

4) How often the onNext callback is called? It depends on service timeout. The appearing and disappearing is because service will re-broadcast the alive message on the network. There is a cache mechanism on JmDNS library (there is a bug), so JmDNS doesn't show blinking. The below code is what I've done on the native app.

@Override
public void onNext(BonjourService bonjourService) {
    Log.i(TAG, "bonjourService -> " + bonjourService);
    if (!bonjourService.isLost()) {
        mServiceInfos.put(bonjourService.getServiceName(), bonjourService);
        Log.i(TAG, "add -> " + bonjourService.getServiceName());
    } else {
        mServiceInfos.remove(bonjourService.getServiceName());
        Log.i(TAG, "remove -> " + bonjourService.getServiceName());
    }
}

I just know RxDnssd used the same library like iOS used (on C code), so what the value it can return is the same as iOS did. Is iOS also stable?

becvert commented 7 years ago

Thanks for the info.

Still it's a bit odd to me that "removed" should be triggered. But let's say it's the way it's working, nonetheless "added" is called too many times.

I'm going to check if that happens on iOS too.

becvert commented 7 years ago

iOS is stable.

There's definitely something wrong with RxDNSSD. My app crashes after a while. I'll put this on hold until you work this out with andriydruk/RxDNSSD

I'll try to change the javascript interface though.

Thanks.

emcniece commented 7 years ago

Can I buy you guys a pile of coffee or beer or something? Really stoked to start testing this after #14-#17 are merged. Shoot me an email if you get thirsty.

becvert commented 7 years ago

We are waiting on some news from Andriy Druk at RxDNSSD. @NeoLSN could you send him an email andriy.druk@gmail.com

hubertperron commented 7 years ago

I'm seeing the same behaviour on the Android side on my app. The watcher callback seem to cache the results so i'm still seeing devices even after they have been disconnected from the network.

Is there a fix available?

NeoLSN commented 7 years ago

@becvert I sent the mail last Friday, but didn't get any response since that time.

becvert commented 7 years ago

@NeoLSN Thanks. We just have to wait then. Also I was wondering how difficult would it be to use apple's dnssd without the Rx paradigm. Maybe it's worth a shot.

andriydruk commented 7 years ago

Hi, there

I’m so sorry for the delay. I spent the morning debugging your code and found a bad mistake. I saw that you just copied my code from Bonjour Browser, but it's not so simple. When I created this I made some assumption, but it wouldn't work for you.

First of all your code never unregister services. It happens because of 'equals' function of BonjourService. I guess that it was my mistake include 'flags' to 'equals'. Please use another key for your hashmap.

I'm going to investigate your browse function too bit later.

@becvert yes, you can use Apple DNSSD API with my library. For your needs, it can be much better.

andriydruk commented 7 years ago

Browse works ok, but there are few notes:

  1. RxDNSSD provides you a flow of events that happened in a network. There is a bunch of flags about services state that you can get from BonjourService.flags (and 'service was lost' is one of them). At current implementation, you use operation 'resolve'. Actually, it's a merge of 2 another operations - resolveIPV4 and resolveIPV6, that's why you can discover one service twice (with one address and with both addresses). If you don't like this you can switch to IPV4/IPV6 only or don't notify your users about finds before you don't get both addresses.
  2. At current implementation, your plugin shows services that were registered on owner's device. From my perspective of view, you shouldn't notify your users about their own registrations.
bugnano commented 7 years ago

The part that shows your own device's registered services can be good as it is right now. Let me explain:

First of all, it has always worked like that, so it comes as no surprise for every user of this plugin.

Other than that, it can be useful for debugging purposes that the service you registered gets shown when browsing (you don't need two separate devices to debug the most trivial network operations).

Lastly, it is not hard for the user software to check whether the discovered service is the one that has been registered on this device, or it is a service discovered from a remote device (if you don't like this idea, a possible solution can be to enable or disable such behaviour by a configuration flag).

Other than that, there is still the bug that you mentioned elsewhere:... =>Move to #16

As a side note, I know that this is not the right place to ask, but I don't know where to ask elsewhere, is the cordova-plugin-websocket-server ipv6-ready (in order to be compliant with Apple's ipv6-only networking)? => WebSocketServer#15

becvert commented 7 years ago

@bugnano (and others) Please keep this PR clean. It's about AppleDNSSD and Rx. Thanks

becvert commented 7 years ago

At current implementation, your plugin shows services that were registered on owner's device.

This plugin is meant to be just a wrapper around underlying libraries. It does what they do. I agree check must happen at application level. Anyway, please open a new issue if you want fo follow up on this particular point. Thanks.

becvert commented 7 years ago

@NeoLSN Please let me know if you're happy with the latest changes and if it's worth a new try for me. Thanks

@andriydruk thanks for your feedback.

NeoLSN commented 7 years ago

@becvert I'm ok with that changes. I think it is worth for a try. We need to find a way to merge IPv4 and IPv6 records.

becvert commented 7 years ago

@NeoLSN ok I'll try to do some tests again sometime, although I'm unsure whether fixes were made on the 'browse' side?

About merging ipv4 and ipv6 I don't know. I like to think that cordova plugins are just bridges between native and webviews. So if JmDNS or dnssd send so many events then your application should receive just as many events. And in the network layer of your app, you decide what to do. That's what you would do if were doing native apps. And I don't want to do any kind of magic that needs time to fix and support afterwards. Does it make sense? Anyway, please open a new issue if you want to follow up on that remark.

So if there are 2 'resolved' events per service it's fine with me. The issue I pointed out was more about some kind of memory leak.

bugnano commented 7 years ago

I 100% agree with you in the sense that a cordova plugin should be as a thin as a layer can be, but it also needs to take into account the platform differences (e.g. if Bonjour on iOS and RxDnssd on Android have a slightly different behaviour, it would be better to normalize these differences into the plugin itself, in order to have a consistent behaviour on the JS side).

P.S. sorry for the off-topic contents of yesterday, but I don't know where to discuss such issues.

NeoLSN commented 7 years ago

I'll try to merge that on Java code and update the latest version let you know.

becvert commented 7 years ago

Hi @NeoLSN what's the status on this PR?

NeoLSN commented 7 years ago

@becvert No. The blink speed is still too fast, so the mdns information just merge a second then delete again. For a user perspective, the merge information didn't help anything.

becvert commented 7 years ago

OK. Thanks, By curiosity, I think I'll try to implement Apple's Bonjour for Java without the Rx paradigm, but it won't happen anytime soon.

NeoLSN commented 7 years ago

https://github.com/driftyco/ionic-native/commit/68d9946

becvert commented 5 years ago

You can try ou the RxDNSSD branch

becvert commented 5 years ago

Closing old PR