NFC enabled apps are likely to need a basic, but largely similar needs to manipulate NDEF Records in tag and P2P read and write scenarios. Create a shared javascript library file for apps to use.

Thanks, Garner. I think Michal need this lib for developing contacts sharing. So when do you plan to finish this?

A library would need to be defined for a basic subset useful for all apps, but I can get a first pass out for comments by the certified app module owners.

WIP refactoring patch. nfc_util.js is now shared for all apps to use, and nfc_manager_util.js has additional manager and handover specific functions. Device handover unit testing needed. I have no NFC handover type devices currently.

Update. Made NDEF const, and updated Ndef --> NDEF function names.

Good work on some much needed cleanup. Some quick comments/suggestions: - Remove NfcManagerUtil.parseNDEF() and use NfcUtil.parseNDEF() instead. - The comment explaining the usage of NfcUtil.parseNDEF() is not accurate anymore. - The comment in NfcUtil should also mention the other exported functions. - In NfcUtil.doParseNDEF() and other functions you reference the NDEF constants via "this.NDEF.*". I think you need to remove "this." since you moved 'NDEF' to the global namespace. - In an earlier version of the HandoverManager implementation I also wrote an encoding of NDEF messages. Evelyn asked me to remove it since it wasn't used in the HandoverManager. If we create a general purpose library, perhaps we can add this to NfcUtil? If you agree, I will email you my implementation.

(In reply to arno from comment #6) > - In an earlier version of the HandoverManager implementation I also wrote > an encoding of NDEF messages. Evelyn asked me to remove it since it wasn't > used in the HandoverManager. If we create a general purpose library, perhaps > we can add this to NfcUtil? If you agree, I will email you my implementation. Please attach/email the encoding impl.

Updated with Arno's comments. Fixed SMARTPOSTER_ACTION capitalization. Arno, can you post the NDEFRecord encode function somewhere?

Comment on attachment 8371151 [details] (v1) Create shared library for NFC I recall that I said move the class declaration out of object method and it seems not occur. Please do. And strongly acquire unit test for such big change.

Garner: just emailed you my NDEF.encode() implementation.

Comment on attachment 8370940 [details] [diff] [review] (v3) 0001-WIP-Update-system-and-nfc-demo-app-to-use-new-shared.patch Marking old patch obsolete. the github pull request is newer, and debugged for NFC handover.

Comment on attachment 8371151 [details] (v1) Create shared library for NFC Alive: Rebased. Buffer moved out of NfcUtil into it's own global scope object. NFC Handover is now pairing and working in the latest set against an NFC enabled portable speaker (manual connect after pairing is needed, but of of scope of this bug). All previous functionality was unit tested as working. Arno: I added your encodeNDEF function. Please check the util function comments for accuracy.

Comment on attachment 8371151 [details] (v1) Create shared library for NFC When I say unit test I don't mean manually testing. We NEED nfc_manager_test under gaia/apps/system/test/unit. Next time please come up with that because I have seen many typo-like error and the test coverage of nfc_manager is exactly, 0%. If you have trouble writing unit test please tell me.

(In reply to Alive Kuo [:alive][NEEDINFO!][:艾莉芙] from comment #14) > Comment on attachment 8371151 [details] > (v1) Create shared library for NFC > We NEED nfc_manager_test under gaia/apps/system/test/unit. I hear you, although nfc_manager realistically also invloves the lower code stack too due to session management. The new shared libray code can use automated existance and unit tests however (open a new bug?). We should make that a topic for the next weekly meeting (I mentioned the need for automated testing of reading/writing/NFC on/NFC off last week as well).

(In reply to Garner Lee from comment #13) > Comment on attachment 8371151 [details] > Arno: I added your encodeNDEF function. Please check the util function > comments for accuracy. I would propose the following comment: "encodeNDEF: takes an array of NDEFRecords and returns an Array of octets representing the binary encoding of the NDEF message according to the NFC Forum"

I've updated the names of the shared NfcUtil to NfcUtils to be more in line with other utility files in the existing gaia code. This may need re-review. This of breaks the other gaia app code that has included the "NfcUtil" class, but those should be easy to fix at the same time switching to use the shared version. Unit test will now also pass on Travis CI (with MockMozNDEFRecord) that doesn't have NFC compiled in. Followup Bug 973133 for navigator DOM level tests. https://travis-ci.org/mozilla-b2g/gaia/builds/19696896

Please request review again if you think it's necessary. It's unclear what's going to be checked to me.

Alive: aside from the new unit tests, no major changes (just renames). Hi Ken, We can still land to master, or should we wait?

It is for gaia part and there is a bubble-tea branch for Gaia, I think Alive may be able to provide more clear answer. Alive, what do you think? Can we land this patch in Gaia?

Garner, please create another pull request to bubble-tea and ask for my review. Thanks!

Comment on attachment 8384796 [details] bubble-tea branch pull req (duplicate). r+ with nits

Updated. test_context_menu_activity_picker seems broken on bubble-tea currently on the lastest head (probably unrelated). What is step 5 here: https://wiki.mozilla.org/Gaia/Team/Taipei/BubbleTea 5. Merge your patch. (<-- Who should do that merge?)

test_context_menu_activity_picker failing will be fixed by bug 981965.

All green: https://travis-ci.org/mozilla-b2g/gaia/builds/20709966 Bubble-tea merge needed.

bubble-tea is freezed right now, we will do merge next week after v1.4 is branched.

Merger(s): When 1.4 is branched, Check-in needed.

bubble-tea: cd6f2ee78b577f769f09dc13f80591b93e134711 Leaving this open because I assume we aren't resolving bugs like these until they hit master.

Ken: when is bubble-tea merging into master (a few dependencies are waiting for it).

Alive, which patches in bubble-tea this patch is depend on? Can we have a list so we could land this group of patches together? I believe Yuren and Fred is working on this too....

pull request to master: https://github.com/mozilla-b2g/gaia/pull/17369 travis-ci: https://travis-ci.org/mozilla-b2g/gaia/builds/21163018 try server: https://tbpl.mozilla.org/?tree=Try&rev=f86b9af3bd7b

travis-ci is passed, merged. https://github.com/mozilla-b2g/gaia/commit/3ee5d595f8c347e6479856b3efa72fa079faacf6