1109456 - Support NFC tag transceive WebAPI

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Dimi Lee [:dimi]]

To read proprietary data in different NFC card, we need to support transceive WebAPI for App

Comment 1

[Dimi Lee [:dimi]]

bug 1105666 may take a while, remove me from Assignee

Comment 2

[Dimi Lee [:dimi]]

Attachment: Part1. Transceive WebIDL patch v1

Comment 3

[Dimi Lee [:dimi]]

Attachment: Part2. WebIDL for message between gecko and nfcd v1

Comment 4

[Dimi Lee [:dimi]]

Attachment: Part3. GECKO implementation. patch1

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8538943 [details] [diff] [review]
Part1. Transceive WebIDL patch v1

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

We just need the trasceive interface in MozNFCTag, other interfaces should be done in other bugs in Bug 1082992.

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8538943 [details] [diff] [review]
Part1. Transceive WebIDL patch v1

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

::: dom/webidl/MozNFCTag.webidl
@@ +141,5 @@
> +  /**
> +   * Send raw command to tag and receive the response.
> +   */
> +  [Throws, ChromeOnly]
> +  Promise<sequence<byte>> transceive(NFCTechType tech, sequence<byte> command);

request feedback from smuag for whether we should use
Uint8Array or sequence<byte> here.

Or can you help to explain their difference ?

Thanks

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8538947 [details] [diff] [review]
Part3. GECKO implementation. patch1

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

::: dom/nfc/gonk/NfcMessageHandler.cpp
@@ +260,5 @@
> +
> +  uint32_t length = aOptions.mCommand.Length();
> +  aParcel.writeInt32(length);
> +  for (uint32_t i = 0; i < length; i++) {
> +    aParcel.writeInt32(aOptions.mCommand[i]);

Should write an ByteArray, here you write Uint32Array.

@@ +419,5 @@
> +  uint32_t length = aParcel.readInt32();
> +  aOptions.mResponse.SetCapacity(length);
> +
> +  for (uint32_t i = 0; i < length; i++) {
> +    uint8_t data = aParcel.readInt32();

Ditto, here you read an Uint32Array.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8538947 [details] [diff] [review]
Part3. GECKO implementation. patch1

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

Hi, smaug
we'd like to request for your feedback here.

For the use of Promise API, I found that it only lists the type returned in OnResolve, but doesn't list the type in OnRejected.

For example, in this bug, 

Promise<sequeuce<byte>> transceive(...), sequence<byte> is the type returned in OnResolve, but what about the type returned in OnRejected?
So what's the common pattern we use in onReject? return DOMError , Error, or just DOMString ? 
And how do we help app developers to know how to do error handling of this interface?

Thanks

::: dom/nfc/nsNfc.js
@@ +89,5 @@
>        debug("can not find promise resolver for id: " + this._requestId +
>             ", errormsg: " + aErrorMsg);
>        return;
>      }
>      resolver.reject(aErrorMsg);

Here we reject with DOMString.

@@ +198,5 @@
>    },
>  
> +  transceive: function transceive(tech, cmd) {
> +    if (this.isLost) {
> +      throw new this._window.DOMError("InvalidStateError", "NFCTag object is invalid");

Here we reject with DOMError.

Comment 9

[Dimi Lee [:dimi]]

Attachment: Part3. GECKO implementation. patch v2

Fix not using byte array in NfcMessageHandler.cpp

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8538943 [details] [diff] [review]
Part1. Transceive WebIDL patch v1

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

I check current m-c mostly we use Uint8Array for this.

Comment 11

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8538945 [details] [diff] [review]
Part2. WebIDL for message between gecko and nfcd v1

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

::: dom/webidl/NfcOptions.webidl
@@ +22,5 @@
>    boolean isP2P;
>    sequence<MozNDEFRecordOptions> records;
> +
> +  NFCTechType technology;
> +  sequence<byte> command;

Use Uint8Array.

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8538947 [details] [diff] [review]
Part3. GECKO implementation. patch1

Found Bug 1016560 has processed DOMError/String.

Comment 13

[Dimi Lee [:dimi]]

Attachment: Part1. Transceive WebIDL patch v2

Use Uint8Array

Comment 14

[Dimi Lee [:dimi]]

Attachment: Part2. WebIDL for message between gecko and nfcd v2

Use Uint8Array

Comment 15

[Dimi Lee [:dimi]]

Attachment: Part1. Transceive WebIDL patch v3

Change in this patch
- Rebase
- use Uint8Array

Comment 16

[Dimi Lee [:dimi]]

Attachment: Part2. WebIDL for message between gecko and nfcd v3

Change in this patch:
- Use Uint8Array

Comment 17

[Dimi Lee [:dimi]]

Attachment: Part3. GECKO implementation patch v3

Change in this patch
- rebase

Comment 18

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8545122 [details] [diff] [review]
Part1. Transceive WebIDL patch v3

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

::: dom/nfc/nsINfcContentHelper.idl
@@ +174,5 @@
> +   * @param command
> +   *        Command to send
> +   */
> +  void transceive(in DOMString sessionToken,
> +                  in nsINfcRequestCallback callback,

callback should be last arg.

Comment 19

[Dimi Lee [:dimi]]

Attachment: Part1. Transceive WebIDL patch v4

Comment 20

[Dimi Lee [:dimi]]

Comment on attachment 8545125 [details] [diff] [review]
Part2. WebIDL for message between gecko and nfcd v3

Add r? to smug because of WebIDL change

Comment 21

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8545191 [details] [diff] [review]
Part1. Transceive WebIDL patch v4

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

::: dom/webidl/MozNFCTag.webidl
@@ +104,5 @@
> +  /**
> +   * Send raw command to tag and receive the response.
> +   */
> +  [Throws, ChromeOnly]
> +  Promise<Uint8Array> transceive(NFCTechType tech, Uint8Array command);

Move this interface to the partial interface below.
Also I prefer move Throws to the last attr although this is not mandatory.

Or if you'd like to merge the partial interface into one is also fine by me.

Comment 22

[Dimi Lee [:dimi]]

Attachment: Support NFC tag transceive WebAPI

Comment 23

[Dimi Lee [:dimi]]

try result : https://treeherder.mozilla.org/#/jobs?repo=try&revision=584bff5a91d1

Comment 24

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/b2g-inbound/rev/0c39bc01f634

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0c39bc01f634