search

chariotsolutions / phonegap-nfc

PhoneGap NFC Plugin
MIT License
706 stars 556 forks source link

remove listeners: the correct way #270

Closed wesleyvandevoorde closed 6 years ago

wesleyvandevoorde commented 7 years ago

The listeners were not deleted correctly.

don commented 6 years ago

@wesleyvandevoorde can you create a new pull request with just the changes for removing the listeners? There are too many unrelated changes for me to merge this pull request.

jeremycoppey commented 6 years ago

This would solve my issue: https://github.com/chariotsolutions/phonegap-nfc/issues/257 Can you please make the changes asap? @wesleyvandevoorde

jeremycoppey commented 6 years ago

Any updates on this!! Would like to have some update on the remove listeners! Kind regards Jérémy

don commented 6 years ago

@wesleyvandevoorde Good catch on fixing the removeFromTechList implementation to iterate through the item. https://github.com/chariotsolutions/phonegap-nfc/pull/270/commits/8c9a89544314536ba2dc9820dc3f70da34d40f51#diff-48a9acb6beb13451194fb8d899ae4294R565 however removeFromTechList and removeIntentFilter shouldn't modify the list. They need to remove through the iterator to avoid a potential ConcurrentModificationException.

The biggest problem is that the NfcAdapter doesn't know about the new list and intent filter until for foreground listener is re-added. For now when listeners are removed, I restart nfc.

Closing this pull request without merging. The fixes have been added to master with https://github.com/chariotsolutions/phonegap-nfc/commit/77bf91895768fd46d530d29d7524fe65b6fd0c1e

@jeremycoppey this will be in the next release, until then you can run from cordova plugin add https://github.com/chariotsolutions/phonegap-nfc