Closed
Bug 1050186
Opened 10 years ago
Closed 10 years ago
[NFC] Emulator support tag format command
Categories
(Firefox OS Graveyard :: NFC, defect)
Tracking
(b2g-v1.4 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)
RESOLVED
FIXED
2.1 S2 (15aug)
People
(Reporter: dlee, Assigned: dlee)
References
Details
(Whiteboard: [p=2])
Attachments
(1 file, 4 obsolete files)
For bug 1049429, testcase will test tag support NDEF and NON-NDEF format. So emulator will have to support function to set tag data back to original format.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8469183 -
Flags: review?(tzimmermann)
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8469183 [details] [diff] [review] Bug 1050186 patch v1 This version may have bug when running get ndef detail for type 4 tag
Attachment #8469183 -
Attachment is obsolete: true
Attachment #8469183 -
Flags: review?(tzimmermann)
Assignee | ||
Comment 3•10 years ago
|
||
Before ndef data is set to type 4 tag, use proprietary cc instead of ndef cc. Otherwise we will detect type 4 tag as NDEF format but there is no ndef data actually.
Attachment #8469774 -
Flags: review?(tzimmermann)
Assignee | ||
Comment 4•10 years ago
|
||
Support format command to rollback tag data to the state without setting NDEF data
Attachment #8469776 -
Flags: review?(tzimmermann)
Comment 5•10 years ago
|
||
Comment on attachment 8469774 [details] [diff] [review] Part1. Use proprietarty cc as the initial value of t4t Review of attachment 8469774 [details] [diff] [review]: ----------------------------------------------------------------- Just a quick question. I thought the tags wouldn't be detected before we set a value. So I don't understand where this problem shows up. ::: hw/nfc-tag.c @@ +46,5 @@ > > /* [Type 4 Tag Operation Specification] */ > static enum t4t_file_select t4t_file_sel = NONE; > > /* [T4TOP] Table5 */ Maybe also refer to table 7, to signal difference between NDEF and proprietary values. @@ +157,5 @@ > assert(tag); > assert(ndef_msg || !len); > assert(len + 2 < sizeof(tag->t.t4.format.data)); > > + uint8_t cc[] = T4T_NDEF_CC; Please move this line to the op of the function and mark it as 'static const'.
Comment 6•10 years ago
|
||
Comment on attachment 8469776 [details] [diff] [review] Part2. Emulator support tag format command Review of attachment 8469776 [details] [diff] [review]: ----------------------------------------------------------------- r- for now because there is a bug. ::: hw/nfc-tag.c @@ +194,5 @@ > +int > +nfc_tag_format(struct nfc_tag* tag) > +{ > + switch (tag->type) { > + case T1T: { Please remove the braces here and put them into the format macros. @@ +210,5 @@ > + case T4T: { > + FORMAT_NFC_T4T(tag, T4T_PROPRIETARY_CC) > + break; > + } > + default: { No braces at all in the default branch. ::: hw/nfc-tag.h @@ +278,5 @@ > uint8_t mem[T3T_MEMORY_SIZE]; > }; > > struct nfc_t3t_format { > + uint8_t ver[1]; I don't understand this. Why do you use |ver[]|, instead of |ver|? The memcpy in FORMAT_NFC_... should be able to use |&ver|. @@ +354,5 @@ > .t.t4.format.cc = cc_, \ > } > > +#define FORMAT_NFC_T1T(tag_, uid_, res_) \ > + memset(tag, 0, sizeof(struct nfc_tag)); \ I'd prefer to not have these memsets in the macros. We're writing most fields anyway afterwards. If you need specific fields to be cleared, please do that instead. @@ +356,5 @@ > > +#define FORMAT_NFC_T1T(tag_, uid_, res_) \ > + memset(tag, 0, sizeof(struct nfc_tag)); \ > + tag_->type = T1T; \ > + uint8_t uid[] = uid_; memcpy(tag_->t.t1.format.uid, uid, sizeof(uid)); \ Again, please move these arrays to the top of the macro and mark them as 'static const'. Here and below. @@ +370,5 @@ > +#define FORMAT_NFC_T3T(tag_, v_, r_, w_, nb_, u_, wf_, rw_, ln_, cs_) \ > + memset(tag, 0, sizeof(struct nfc_tag)); \ > + tag_->type = T3T; \ > + uint8_t v[] = v_; memcpy(tag_->t.t3.format.ver, v, sizeof(v)); \ > + uint8_t r[] = r_; memcpy(tag_->t.t3.format.ver, r, sizeof(r)); \ You are writing to |ver| over and over. ;)
Attachment #8469776 -
Flags: review?(tzimmermann) → review-
Updated•10 years ago
|
Attachment #8469774 -
Flags: review?(tzimmermann) → review+
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #5) > Comment on attachment 8469774 [details] [diff] [review] > Part1. Use proprietarty cc as the initial value of t4t > > Review of attachment 8469774 [details] [diff] [review]: > ----------------------------------------------------------------- > > Just a quick question. I thought the tags wouldn't be detected before we set > a value. So I don't understand where this problem shows up. > The problem is that I want to write a testcase trying to get information from a NON-NDEF formatted tag. In testcase test_nfc_read_tag.js it will set tag with NDEF data, so i will need a way to rollback the tag to original state before doing my new testcase.
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8469776 [details] [diff] [review] Part2. Emulator support tag format command Review of attachment 8469776 [details] [diff] [review]: ----------------------------------------------------------------- ::: hw/nfc-tag.h @@ +278,5 @@ > uint8_t mem[T3T_MEMORY_SIZE]; > }; > > struct nfc_t3t_format { > + uint8_t ver[1]; just want to make the FORMAT... MACRO looks uniform. I will remove this change in next patch. @@ +370,5 @@ > +#define FORMAT_NFC_T3T(tag_, v_, r_, w_, nb_, u_, wf_, rw_, ln_, cs_) \ > + memset(tag, 0, sizeof(struct nfc_tag)); \ > + tag_->type = T3T; \ > + uint8_t v[] = v_; memcpy(tag_->t.t3.format.ver, v, sizeof(v)); \ > + uint8_t r[] = r_; memcpy(tag_->t.t3.format.ver, r, sizeof(r)); \ Ah...thanks for finding this. I am really too uncareful :(
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8469776 -
Attachment is obsolete: true
Attachment #8471307 -
Flags: review?(tzimmermann)
Comment 10•10 years ago
|
||
Comment on attachment 8471307 [details] [diff] [review] Part2. Emulator support tag format command v2 Review of attachment 8471307 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8471307 -
Flags: review?(tzimmermann) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8469774 -
Attachment is obsolete: true
Attachment #8471307 -
Attachment is obsolete: true
Attachment #8472234 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [p=2]
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 12•10 years ago
|
||
b2g-jellybean: https://github.com/mozilla-b2g/platform_external_qemu/commit/30d0dfa566651fea8031551e86cec6018b7bbb12 Oh my, we're running off the same rev from this repo for 1.4+? That doesn't necessarily seem....good...
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•