Closed
Bug 1016777
Opened 10 years ago
Closed 10 years ago
[NFC] Emulator support for reading NDEF data from type 1 tag
Categories
(Firefox OS Graveyard :: NFC, defect)
Tracking
(b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)
RESOLVED
FIXED
2.0 S3 (6june)
People
(Reporter: dlee, Assigned: dlee)
References
Details
(Whiteboard: [p=2])
Attachments
(2 files, 6 obsolete files)
Emulator should support processing type 1 tag - reading function should be implemented in this bug
Assignee | ||
Updated•10 years ago
|
Whiteboard: [p=2]
Assignee | ||
Updated•10 years ago
|
Summary: Bug 993836 - [NFC] Emulator support for reading NDEF data from type 1 tag → [NFC] Emulator support for reading NDEF data from type 1 tag
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8429949 -
Flags: review?(tzimmermann)
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8429950 -
Flags: review?(tzimmermann)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8429956 -
Flags: review?(tzimmermann)
Comment 4•10 years ago
|
||
Comment on attachment 8429949 [details] [diff] [review] Part1. Add type 1 tag as a remote end point v1 Review of attachment 8429949 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: hw/nfc-re.c @@ +26,4 @@ > INIT_NFC_RE([0], NCI_RF_PROTOCOL_NFC_DEP, NCI_RF_NFC_F_PASSIVE_LISTEN_MODE, NULL, "deadbeaf0", nfc_res+0), > INIT_NFC_RE([1], NCI_RF_PROTOCOL_NFC_DEP, NCI_RF_NFC_F_PASSIVE_LISTEN_MODE, NULL, "deadbeaf1", nfc_res+1), > + INIT_NFC_RE([2], NCI_RF_PROTOCOL_T1T, NCI_RF_NFC_A_PASSIVE_LISTEN_MODE, nfc_tags+0, "deadbeaf2", nfc_res+2), > + INIT_NFC_RE([3], NCI_RF_PROTOCOL_T2T, NCI_RF_NFC_A_PASSIVE_LISTEN_MODE, nfc_tags+1, "deadbeaf3", nfc_res+3) It looks like the attached test case handles the index change correctly. ::: hw/nfc-tag.c @@ +60,5 @@ > + > + data = tag->t1.format.data; > + > + /* [Type 1 Tag Operation Specificatio] 6.1 NDEF Management */ > + memcpy(data + offset, t1t_cc, ARRAY_SIZE(t1t_cc)); Since |t1t_cc| has static const qualifiers, I wonder if you could just init data with it's value and only append the NDEF message here.
Attachment #8429949 -
Flags: review?(tzimmermann) → review+
Comment 5•10 years ago
|
||
Comment on attachment 8429950 [details] [diff] [review] Part2. Process type 1 tag read command v1 Review of attachment 8429950 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: hw/nfc-re.c @@ +703,5 @@ > + * But from libnfc-nci it seems proprietary parameters is required for > + * t1t tag. > + */ > +static size_t > +nfc_re_create_activated_ntf_t1t(struct nfc_re* re, uint8_t* act) Since this is static, you should drop the prefix 'nfc_re_', here and below. @@ +721,1 @@ > size_t 'static' ::: hw/nfc-tag.c @@ +115,5 @@ > } > > +static size_t > +process_t1t_rid(struct nfc_tag* tag, const struct t1t_rid_command* cmd, > + uint8_t* consumed, struct t1t_rid_response* rsp) Weird indention. This line should be aligned with the first argument above. Here and below. @@ +125,5 @@ > + > + rsp->hr[0] = T1T_HRO; > + rsp->hr[1] = T1T_HR1; > + > + memcpy(rsp->uid, tag->t1.format.uid, ARRAY_SIZE(rsp->uid)); sizeof(rsp->uid). It shouldn't make a difference here, but sizeof is more correct than ARRAY_SIZE in this case. ::: hw/nfc-tag.h @@ +34,5 @@ > + RALL_COMMAND = 0x00, > + RID_COMMAND = 0x78 > +}; > + > +struct t1t_rall_command { Maybe add reference to RALL spec like you did below for RID.
Attachment #8429950 -
Flags: review?(tzimmermann) → review+
Comment 6•10 years ago
|
||
Comment on attachment 8429956 [details] [diff] [review] Testcase for bug 1016777 Review of attachment 8429956 [details] [diff] [review]: ----------------------------------------------------------------- This probably requires a small update. ::: dom/nfc/tests/marionette/test_nfc_tag.js @@ +59,5 @@ > > toggleNFC(false, runNextTest); > }); > > toggleNFC(true, function() { This function now returns a Promise.
Attachment #8429956 -
Flags: review?(tzimmermann) → review+
Comment 7•10 years ago
|
||
Thanks a lot, Dimi! There are just nits and that one interface change in the test case.
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8429949 [details] [diff] [review] Part1. Add type 1 tag as a remote end point v1 Review of attachment 8429949 [details] [diff] [review]: ----------------------------------------------------------------- ::: hw/nfc-tag.c @@ +60,5 @@ > + > + data = tag->t1.format.data; > + > + /* [Type 1 Tag Operation Specificatio] 6.1 NDEF Management */ > + memcpy(data + offset, t1t_cc, ARRAY_SIZE(t1t_cc)); The value of CC field here only indicate that there is NDEF data. So i think configure it when set NDEF would be better. I will change the naming of t1t_cc to t1t_cc_ndef for better naming.
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8429956 -
Attachment is obsolete: true
Attachment #8430505 -
Flags: review+
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8429949 -
Attachment is obsolete: true
Attachment #8429950 -
Attachment is obsolete: true
Attachment #8430513 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 11•10 years ago
|
||
Remove checkin-need because the coding style here may cause build fail in MAC environment. Reference bug : bug 1017038
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8430513 -
Attachment is obsolete: true
Attachment #8431300 -
Flags: review+
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #8430505 -
Attachment is obsolete: true
Attachment #8431303 -
Flags: review+
Assignee | ||
Comment 14•10 years ago
|
||
Set checkin-needed because bug 1017038 landed
Keywords: checkin-needed
Comment 15•10 years ago
|
||
Master: https://github.com/mozilla-b2g/platform_external_qemu/commit/0a2b7e94dce4989a3740fea6f6e3152978216c88
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
Flags: in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Updated•10 years ago
|
status-b2g-v1.3:
--- → fixed
status-b2g-v1.3T:
--- → fixed
Comment 16•10 years ago
|
||
Changes since v3: - rebased onto latest Gecko master
Attachment #8431303 -
Attachment is obsolete: true
Attachment #8437532 -
Flags: review+
Comment 17•10 years ago
|
||
Landing missing test case https://hg.mozilla.org/integration/b2g-inbound/rev/30e01a6763ad https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=30e01a6763ad
Blocks: b2g-nfc
You need to log in
before you can comment on or make changes to this bug.
Description
•