Emulator should support processing type 3 tag - reading function should be implemented in this bug

Currently blocked by bug 1021180.

Comment on attachment 8439094 [details] [diff] [review] Part1. Set rf mode according to protocol v1 Review of attachment 8439094 [details] [diff] [review]: ----------------------------------------------------------------- I'm r-'ing this patch, because I'm not convinced by the dynamic approach. The current RF mode is quick hack I put in when I needed to get something working. Let's first discuss on IRC how to clean this up. ::: android/console.c @@ +4001,5 @@ > } else { > nfc->active_rf = nfc->rf + param->rf; > } > + > + nfc_set_rf_mode_by_protocol(nfc->active_rf, param->re->rfproto); This value should rather be set as part of the initialization of nfc_rf. Or is there a reason why this _must_ be set dynamically? I'd rather prefer to add new entries to nfc->rf. ::: hw/nfc-rf.c @@ -22,4 @@ > rf->iface = iface; > switch (iface) { > case NCI_RF_INTERFACE_FRAME: > - // TODO: this is also depend on protocol You should pass in the protocol as well and do a more complex setup here. ::: hw/nfc.c @@ +78,5 @@ > return NULL; > } > > +int > +nfc_set_rf_mode_by_protocol(struct nfc_rf* rf, enum nci_rf_protocol proto) This function should be located in nfc_rf.c

Comment on attachment 8439097 [details] [diff] [review] Part2 Support t3t polling command v1 Review of attachment 8439097 [details] [diff] [review]: ----------------------------------------------------------------- Nits only. Looks good! ::: hw/nfc-nci.c @@ +781,5 @@ > +{ > + /* We do nothing here except send notification to HOST */ > + struct nfc_t3t_polling_ntf_param* ntf; > + > + ntf = qemu_malloc(sizeof(struct nfc_t3t_polling_ntf_param)); Use 'sizeof(*ntf)'. It's less likely to break if the data structures get renamed or refactored. ::: hw/nfc-nci.h @@ +488,5 @@ > +struct nci_t3t_polling_cmd { > + uint16_t sc; > + uint8_t rc; > + uint8_t tsn; > +}; Please add ' __attribute__((packed))' right before the semicolon. It's not strictly necessary here, but certainly safer in the long run. @@ +494,5 @@ > +struct nci_t3t_polling_ntf { > + uint8_t status; > + uint8_t nres; > + uint8_t res[]; > +}; Again, packed.

Comment on attachment 8439098 [details] [diff] [review] Part3 add type 3 tag as a remote end point v1 Review of attachment 8439098 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: hw/nfc-re.c @@ +22,4 @@ > #include "llcp-snep.h" > #include "nfc-re.h" > > +/* NFCID2 is deifned in [Digital] Table44 */ 'defined' @@ +24,5 @@ > > +/* NFCID2 is deifned in [Digital] Table44 */ > +struct nfc_re nfc_res[5] = { > + INIT_NFC_RE([0], NCI_RF_PROTOCOL_NFC_DEP, NCI_RF_NFC_F_PASSIVE_LISTEN_MODE, > + NULL, "deadbeaf0", "\x01\xfe\x0\x0\x0\x0\x1", nfc_res+0), Start counting at 0, instead of 1, so that indices in the names are equal. ::: hw/nfc-tag.h @@ +142,5 @@ > struct nfc_t2t_format format; > }; > > +/** > + * There is no specefic size defined in T3T sepc. 'specific'

Comment on attachment 8439764 [details] [diff] [review] Part4. Process type 3 tag check command v2 Review of attachment 8439764 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, except for some the missing enum values. If we define constants from a standard, we should define all that belong together. Will make it easier to later see what's missing. As a side note, nfc-re.c keeps growing and we should think about splitting it up. ::: hw/nfc-tag.h @@ +90,5 @@ > + > +enum t3t_command_set { > + CHECK_COMMAND = 0x06, > + UPDATE_COMMAND = 0x08 > +}; [T3TOP] also lists the command 'POLLING'.

Comment on attachment 8439094 [details] [diff] [review] Part1. Set rf mode according to protocol v1 Review of attachment 8439094 [details] [diff] [review]: ----------------------------------------------------------------- We land this now and improve the RF and RE handling in bug 1026422.

Please run this through Try (and post a link here) before requesting checkin. Thanks! :)

sorry, i forget this should be a pull request

master: https://github.com/mozilla-b2g/platform_external_qemu/commit/973367035a1f2545f3dad6e40e354463dc56a7f4

Test case try server result : https://tbpl.mozilla.org/?tree=Try&rev=5521312d4b27 Please help merge test case for this bug, thanks

remove checkin-need base testcase need to update according to latest head.js change

checkin-needed for rebased testcase