979891 - B2G NFC: NFC Daemon shall support HCI Event EVT-TRANSACTION handling

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Ming Yin]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20140212131424

Comment 1

[Ming Yin]

Attachment: EVT_TRANSACTION Handling_in_Payment_Scenario.png

Comment 2

[Ming Yin]

Please refer to 2 relevant documents.

1.  GSMA NFC Handset APIs Requirement Specification v4.1(http://www.gsma.com/digitalcommerce/wp-content/uploads/2013/12/TS26v4-1.pdf)

In this document, similar EVT_TRANSACTION handling is specified for Android and J2ME mobile platform.

A mapping of the above handling for Firefox OS can be found in [attachment 8386125 [details]].

2. ETSI TS 102.622

In this document, the SWP/HCI based communication is described between NFC controller and UICC. Also the various HCI events including EVT_TRANSACTION are specified.

The payload of the EVT_TRANSACTION event data is made up of  a list of parameters coded in BER-TLV fields. The tag and length of these fields are defined in following table.

Description |  Tag | Length
--------------------------------
AID         | '81' | 5 to 16
--------------------------------
Parameters  | '82' | 0 to 255
--------------------------------

The above event data needs to be forwarded by NFCd to Gecko without any modification.

Comment 3

[Ken Chang[:kenkai]]

Hi Ming, NFCD should be able to receive this event. But we need to pass this event to gecko and gaia.

Hi Yoshi, do we define a parcel type for this event and send this event to gecko from NFCD?

Comment 4

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

We plan to, but we are still working on daemon side (Bug 959434), we still need to make sure our daemon could work on dev-phones (Flame, Nexus 4, Nexus 5,...etc) from different NCI libraries, then we could define the detail parcel format could work on these platforms.

Comment 5

[Ming Yin]

Do we need to create a new bug for the TODO: define a parcel type for this event(HCI EVT_TRANSACTION event)  and send this event to gecko from NFCD? Or is it part of this bug?

Comment 6

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

Sure we could file a new bug for this, or even maybe do it in this bug.
It's just that there will not be just only one notification from NFC daemon, there will be several. Although filing bugs are cheap, however filing too many and then duplicate them will also waste our time. The data passing to Gecko is definely in our TODO list, but I'd like to wait until we have a clearer view when the daemon is ready, and hope at the time we already some WebAPI ready, so we could discuss what engineering bugs needs to be filed and clearly identify the resposibility of each engineering bug.

Comment 7

[Dimi Lee [:dimi]]

In my testing nfcd now we can receive HCI_EVENT_TRANSACTION.
But the correct data format should be:
|'81'|AID Length|AID|'82'|Parameters|

But in flame we received 
|'81'|AID Length|AID|'81'|AID Length|AID|'82'|Parameters|

Now we are checking the behavior of Nexus5

Comment 8

[Dimi Lee [:dimi]]

Attachment: Part1. Implement receiving HCI_EVENT_TRANSACTION

Comment 9

[Dimi Lee [:dimi]]

Attachment: Part1. Implement receiving HCI_EVENT_TRANSACTION

Comment 10

[Dimi Lee [:dimi]]

Attachment: Part2. Notify HCI_EVENT_TRANSACTION to gecko

Comment 11

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

Comment on attachment 8452159 [details] [diff] [review]
Part1. Implement receiving HCI_EVENT_TRANSACTION

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

::: src/nci/SecureElement.cpp
@@ +92,5 @@
>  
> +  // If the controller has an HCI Network, register for that.
> +  for (size_t i = 0; i < mActualNumEe; i++) {
> +    if ((mEeInfo[i].num_interface > 0) &&
> +        (mEeInfo[i].ee_interface[0] == NCI_NFCEE_INTERFACE_HCI_ACCESS)) {

bail out early.

@@ +638,5 @@
> +  ALOGD("%s: event=0x%X", __FUNCTION__, event);
> +
> +  switch (event)
> +  {
> +  case NFA_HCI_REGISTER_EVT:

The style is different from the others,
check coding style on MDN for the style of 'switch'

Comment 12

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

Comment on attachment 8452161 [details] [diff] [review]
Part2. Notify HCI_EVENT_TRANSACTION to gecko

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

::: src/NfcGonkMessage.h
@@ +361,5 @@
> +   * NFC_NOTIFICATION_TRANSACTION_EVENT
> +   *
> +   * To notify a transaction event from secure element.
> +   *
> +   * data is char* [aid length][aid][payload length][payload]

This doesn't match what you did in MessageHandler.cpp

::: src/NfcService.cpp
@@ +152,5 @@
>    NfcService::Instance()->mQueue.push_back(event);
>    sem_post(&thread_sem);
>  }
>  
> +void NfcService::notifySETransactionListeners(TransactionEvent* pEvent)

notifySETransactionEvent will be a better name since there's no listener here.

::: src/interface/DeviceHost.cpp
@@ +13,5 @@
>  {
>    ALOGE("%s: not implement", __FUNCTION__);
>  }
>  
> +void DeviceHost::notifyTransactionListeners(TransactionEvent* pEvent)

ditto

::: src/nci/SecureElement.cpp
@@ +340,3 @@
>  }
>  
>  void SecureElement::notifyTransactionListeners(const uint8_t* payload, uint32_t len)

The naming also looks strange as there's no listener in this class.

Comment 13

[Dimi Lee [:dimi]]

Attachment: Part1. Implement receiving HCI_EVENT_TRANSACTION v2

Comment 14

[Dimi Lee [:dimi]]

Attachment: Part2. Notify HCI_EVENT_TRANSACTION to gecko v2

Comment 15

[Dimi Lee [:dimi]]

Attachment: pull request to mozilla-b2g/platform_system_nfcd

Comment 16

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

https://github.com/mozilla-b2g/platform_system_nfcd/commit/8400a1a850d19f28137880b31582efa3416223c3