Closed emcniece closed 7 years ago
emcniece
commented
7 years ago Android: plugin discovers services, but nothing has IP addresses associated with it. Tried setting watchAddressFamily = 'ipv4'; with no success.
iOS: finds services with IPs but crashes on an error:
objc[479]: __weak variable at 0x170222c90 holds 0x100f457a0 instead of 0x1742c52b0. This is probably incorrect use of objc_storeWeak() and objc_loadWeak(). Break on objc_weak_error to debug.Rolling back to 1.2.5 works as expected, so we are proceeding with that. More in-depth debugging will happen this coming week.
becvert
commented
7 years ago There's a new resolved event in watch
emcniece
commented
7 years ago @becvert what does the resolved event indicate, and how is it different from added?
becvert
commented
7 years ago Having added and resolved events is more inline with the operations of the underlying lilbraries.
And as discussed in #38 it will prevent a few duplicates.
In details, added tells you a device is present but you don't have yet the IPs or the hostname and so on. You just got the mdns name. Then they are resolved events, that give you the hostname and IPs and txtRecords...
So Just switch from added to resolved. Hopefully that's just what's triggering your issue.
emcniece
commented
7 years ago @becvert thank you so much for the clarification! This was the source of the problem.
We are wrapping this plugin in an RxJS layer, like so:
...
@Effect({dispatch: false})
startScan$: Observable<Action> = this.actions$
.ofType(app.ActionTypes.START_SCAN)
.do(()=>{
const stopScan$ = this.actions$.ofType(app.ActionTypes.STOP_SCAN);
this.zeroConf.startScan()
.takeUntil(stopScan$)
.filter((found)=> {
return found.service.type === WATCH_SERVICE_TYPE
})
.switchMap((found) => {
let host = found.service;
if(found.action === 'added') {
return Observable.of(new app.AddAction(host));
}
else if (found.action === 'removed') {
return Observable.of(new app.RemoveAction(host));
}
})
.subscribe(
(action)=>this.store.dispatch(action),
()=>this.store.dispatch(new app.ScanFailedAction())
);
});
...Performing the string change to resolved was easy enough, but the bigger problem is the .subscribe() after the .switchMap() - the SwitchMap was not finding the new action, thus it didn't return the required observable. The following subscribe block dispatches a handled error, but it doesn't actually pass the error forward to ScanFailedAction(). This combination caused the app to fail silently, show a "scan failed" message, and display no results.
The fix here was to ensure that all 3 conditions are dealt with properly.
The iOS crash is a semi-documented condition in which the code attempts to access an out-of-scope variable, or something that has been garbage-collected. Red herring.
I like this new paradigm of added/resolved/removed, but perhaps the version bump from 1.2.5 to 1.2.6 is misleading. In Semantic Versioning the right-most two digits (MINOR and PATCH) can be incremented to indicate backward-compatible changes, while PATCH alone signifies bugfixes.
This change introduces a new feature, so this version bump should have been at least a MINOR increment. It could be argued that since the added action no longer contains IP addresses (and many of us expect it to) that this is an "... incompatible API [change]", warranting a MAJOR version increase. It could also be argued that this project is not using Semantic Versioning 😄
Many thanks for your continued efforts on this project!
becvert
commented
7 years ago I'll be more careful next time with the version number 👍
After the 1.2.6 upgrade, our app started failing to produce scan results. We are manually filtering for Ipv4 addresses, so Android shows nothing (more details in the following comments), while iOS seems to be
unwatching when we don't expect it to. This issue may be caused by us handling this plugin in a way that was not intended, and it might be because of some recent changes.Currently investigating, wanted to send a heads up. I updated this plugin today and it has started failing on every scan. Our recent build moved
cordova-plugin-zeroconffrom 1.2.5 to 1.2.6. Details to follow asap.