997576 - [NFC] Testcase for reading empty NFC tag

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Dimi Lee [:dimi]]

Write a test case for empty NFC tag because of bug 997094

Comment 1

[Ken Chang[:kenkai]]

Please this into backlog.

Comment 2

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

defer to sprint3.

Comment 3

[Dimi Lee [:dimi]]

Attachment: Add tag clear command

Comment 4

[Dimi Lee [:dimi]]

Attachment: NFC add tag clear command v1

Comment 5

[Dimi Lee [:dimi]]

Attachment: Testcase v1

Comment 6

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8450816 [details] [diff] [review]
NFC add tag clear command v1

Review of attachment 8450816 [details] [diff] [review]:
-----------------------------------------------------------------

This looks very good already. Thanks for the patch.

::: hw/nfc-tag.c
@@ +79,5 @@
>  {
>      ssize_t offset = 0;
>      uint8_t* data;
>  
>      assert(tag);

Please use 'assert(ndef_msg || !len);'. This tests for either an NDEF message, or no message.

@@ +91,5 @@
>  
>      data[offset++] = NDEF_MESSAGE_TLV;
>      data[offset++] = len;
>  
> +    if (len) {

I'm not sure, but I think you can just use '0' as length argument for memcpy.

@@ +92,5 @@
>      data[offset++] = NDEF_MESSAGE_TLV;
>      data[offset++] = len;
>  
> +    if (len) {
> +        assert(ndef_msg);

Should be removed with comment above addressed.

@@ +103,5 @@
>      memset(data + offset, 0, sizeof(tag->t.t1.format.data) - offset);
>  }
>  
>  static void
>  set_t2t_data(struct nfc_tag* tag, const uint8_t* ndef_msg, ssize_t len)

See my comments for |set_t1t_data|.

@@ +128,5 @@
>      memset(data + offset, 0, sizeof(tag->t.t2.format.data) - offset);
>  }
>  
>  static void
>  set_t3t_data(struct nfc_tag* tag, const uint8_t* ndef_msg, ssize_t len)

See my comments for |set_t1t_data|.

@@ +158,4 @@
>  }
>  
>  static void
>  set_t4t_data(struct nfc_tag* tag, const uint8_t* ndef_msg, ssize_t len)

See my comments for |set_t1t_data|.

Comment 7

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8450819 [details] [diff] [review]
Testcase v1

Review of attachment 8450819 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good. Thank you.

::: dom/nfc/tests/marionette/test_nfc_read_tag.js
@@ +9,5 @@
> +// TODO : Get this from emulator console command.
> +const T1T_RE_INDEX = 2;
> +const T2T_RE_INDEX = 3;
> +const T3T_RE_INDEX = 4;
> +const T4T_RE_INDEX = 5;

Would it make sense to move these constants to head.js in another cleanup bug? There could also be P2P_RE_INDEX_*.

@@ +103,5 @@
> +    testUrlT1TDiscover,
> +    testUrlT2TDiscover,
> +    testUrlT3TDiscover,
> +    testUrlT4TDiscover
> +];

Would it make sense to first run the Url test and afterwards run the Empty tests? This way, the 'clear' operation would really *clear* an NDEF message from the tag, and not just set an empty message.

Comment 8

[Dimi Lee [:dimi]]

Attachment: pull request for mozilla-b2g/platform_external_qemu

Comment 9

[Dimi Lee [:dimi]]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #7)
> Comment on attachment 8450819 [details] [diff] [review]
> Testcase v1
> 
> Review of attachment 8450819 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks good. Thank you.
> 
> ::: dom/nfc/tests/marionette/test_nfc_read_tag.js
> @@ +9,5 @@
> > +// TODO : Get this from emulator console command.
> > +const T1T_RE_INDEX = 2;
> > +const T2T_RE_INDEX = 3;
> > +const T3T_RE_INDEX = 4;
> > +const T4T_RE_INDEX = 5;
> 
> Would it make sense to move these constants to head.js in another cleanup
> bug? There could also be P2P_RE_INDEX_*.
> 
I will create another cleanup bug for it.

Comment 10

[Dimi Lee [:dimi]]

Attachment: Testcase v2

Comment 11

[Dimi Lee [:dimi]]

Attachment: Testcase v3

Address Thomas's comment

Comment 12

[Carsten Book [:Tomcat]]

merged: https://github.com/mozilla-b2g/platform_external_qemu/commit/c61e5f15fd62888f2c33d7d542b5b65c38102e8b

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/861a8e0774f3

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/861a8e0774f3