1023079 - [NFC] Emulator support for triggering tech-lost

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Krzysztof Mioduszewski[:tauzen]]

We need additional command in the emulator to trigger tech-lost notification [1],[2]. Currently we simulate this in marionette web-api tests by calling toggleNFC(false) which disables nfc. We should be able to trigger tech-lost while having nfc enabled to test real world scenarios.

Once this will be solved a followup bug to fix the current tests will be required.

[1] - https://github.com/mozilla/gecko-dev/blob/master/dom/system/gonk/nfc_consts.js#L45
[2] - https://github.com/mozilla/gecko-dev/blob/master/dom/system/gonk/nfc_worker.js#L390

Comment 1

[Dimi Lee [:dimi]]

Attachment: Emulator support for triggering tech v1

Comment 2

[Dimi Lee [:dimi]]

Attachment: Testcase v1

Comment 3

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

Comment on attachment 8449217 [details] [diff] [review]
Emulator support for triggering tech v1

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

This looks very good, thank you. Just one trivial nit.

::: hw/nfc-nci.h
@@ +465,5 @@
>      NCI_RF_DEACT_IDLE_MODE = 0x00,
>      NCI_RF_DEACT_SLEEP_MODE = 0x01,
>      NCI_RF_DEACT_SLEEP_AF_MODE = 0x02,
> +    NCI_RF_DEACT_DISCOVERY = 0x03,
> +    NUMBER_OF_NCI_RF_DEACT_TYPE = 0x04

There shouldn't be an assignment here for NUMBER_OF_NCI_RF_DEACT_TYPE. These NUMBER_OF_ constants are not specified in the spec and should not have an assigned value. Let the compiler count.

@@ +473,5 @@
>      NCI_RF_DEACT_DH_REQUEST = 0x00,
>      NCI_RF_DEACT_ENDPOINT_REQUEST = 0x01,
>      NCI_RF_DEACT_RF_LINK_LOSS = 0x02,
> +    NCI_RF_DEACT_NFC_B_BAD_AFI = 0x03,
> +    NUMBER_OF_NCI_RF_DEACT_REASON = 0x04

Same as above.

Comment 4

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

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

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

::: dom/nfc/tests/marionette/test_nfc_manager_tech_lost.js
@@ +15,5 @@
> +  log('Received \'nfc-manager-tech-discovered\'');
> +  is(msg.type, 'techDiscovered', 'check for correct message type');
> +  is(msg.techList[0], 'P2P', 'check for correct tech type');
> +
> +  emulator.deactivate();

This sounds like you're deactivating the emulator and we might want to find a general solution for this problem in another bug. The naming of all these helper functions is not pretty. Maybe we should move them into individual objects; one object per protocol (e.g., NCI.deactivate(), LLPC.connect(), SNEP.put(), etc.).

Comment 5

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

After I deactivated an RE, I could not activate it again in P2P mode, because there was no active RE, according to the error message. After deactivation, will the emulator be in IDLE state, or will nfcd switch it to DISCOVERY?

Comment 6

[Dimi Lee [:dimi]]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #5)
> After I deactivated an RE, I could not activate it again in P2P mode,
> because there was no active RE, according to the error message. After
> deactivation, will the emulator be in IDLE state, or will nfcd switch it to
> DISCOVERY?

nfcd should reset it to discovery mode

Comment 7

[Dimi Lee [:dimi]]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #4)
> Comment on attachment 8449219 [details] [diff] [review]
> Testcase v1
> 
> Review of attachment 8449219 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/nfc/tests/marionette/test_nfc_manager_tech_lost.js
> @@ +15,5 @@
> > +  log('Received \'nfc-manager-tech-discovered\'');
> > +  is(msg.type, 'techDiscovered', 'check for correct message type');
> > +  is(msg.techList[0], 'P2P', 'check for correct tech type');
> > +
> > +  emulator.deactivate();
> 
> This sounds like you're deactivating the emulator and we might want to find
> a general solution for this problem in another bug. The naming of all these
> helper functions is not pretty. Maybe we should move them into individual
> objects; one object per protocol (e.g., NCI.deactivate(), LLPC.connect(),
> SNEP.put(), etc.).

Agree, I will create another bug and refine it.

Comment 8

[Dimi Lee [:dimi]]

Attachment: pull request for mozilla-b2g/platform_external_qemu

Comment 9

[Dimi Lee [:dimi]]

Attachment: Testcase v2

Comment 10

[Carsten Book [:Tomcat]]

master : https://github.com/mozilla-b2g/platform_external_qemu/commit/efd87a5797ca40fa2df256630c07e0dfb2f762dc

Comment 11

[Carsten Book [:Tomcat]]

also landed the testcase as https://hg.mozilla.org/integration/b2g-inbound/rev/ad6d3a034431

Comment 12

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

Comment on attachment 8450718 [details] [diff] [review]
Testcase  v2

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

::: dom/nfc/tests/marionette/test_nfc_manager_tech_lost.js
@@ +15,5 @@
> +  log('Received \'nfc-manager-tech-discovered\'');
> +  is(msg.type, 'techDiscovered', 'check for correct message type');
> +  is(msg.techList[0], 'P2P', 'check for correct tech type');
> +
> +  emulator.deactivate();

Dimi, I think you forgot to upload this function?

Comment 13

[Dimi Lee [:dimi]]

Attachment: Lost file for Testcase v2

Sorry after rebasing I forget add head.js to patch

Comment 14

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

Comment on attachment 8451412 [details] [diff] [review]
Lost file for Testcase v2

will do this in Bug 1035041

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ad6d3a034431