B2G NFC: Use API to enable/disable NFC hardware

RESOLVED FIXED in 1.4 S6 (25apr)

Status

Firefox OS
NFC
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: allstars, Unassigned)

Tracking

unspecified
1.4 S6 (25apr)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(feature-b2g:2.0, tracking-b2g:backlog)

Details

(Whiteboard: [p=3])

Attachments

(3 attachments, 3 obsolete attachments)

I think this is mentioned by Alive and Evelyn before.
Also I mentioned this in https://bugzilla.mozilla.org/show_bug.cgi?id=866907#c5, 
https://bugzilla.mozilla.org/show_bug.cgi?id=866907#c7,
and https://bugzilla.mozilla.org/show_bug.cgi?id=866907#c9

Right now we use Settings API to turn on/off NFC HW.

This bug is filed to discuss should we use API for this.
(Assignee)

Updated

4 years ago
Blocks: 860906
What's wrong with using the Settings API?

Comment 2

4 years ago
There's 2 halves.

1) The UI sets the user preference
2)
  a) Settings restore startup time: nsISettingsServiceCallback (Nfc.js) in system/gonk watches that setting. This then fires off the Config message to the nfcd system daemon.
  b) nsIObserver (Nfc.js): watches in system/gonk to fire off the UI changes. Fires off Config message to nfcd.

What improvements are we discussing? Move it up from Gonk to something higher in the stack?

Comment 3

4 years ago
The question is whether we need to temporarily turn off NFC. Like the working mode between WIFI and data call, although the setting of data call is turned on, the data call should not be established temporarily when wifi is connected.

Comment 4

4 years ago
Okay. The bug is to look at improving the NFC suspend feature to be more API based, and/or less ambiguous? Suspend currently available as a feature only for airplane mode, as with wifi and bluetooth, via the "nfc.suspend" setting.
(Assignee)

Updated

4 years ago
Blocks: 970263
(Assignee)

Updated

4 years ago
Assignee: nobody → allstars.chh
I'll add three interfaces into NfcManager

DOMRequest startPoll();
DOMRequest stopPoll();
DOMRequest powerOff();

Which are from W3C NFC, except here we return DOMRequest, also powerOn is removed.
(Assignee)

Updated

4 years ago
Blocks: 995101
This bug will focus on Gecko part first, I'll handle Gaia changes in Bug 995101.
(Assignee)

Updated

4 years ago
Whiteboard: [p=3]
Target Milestone: --- → 1.4 S6 (25apr)
Created attachment 8405233 [details] [diff] [review]
Part 1: WebIDL change
Created attachment 8405235 [details] [diff] [review]
Part 3: Marionette tests.
(Assignee)

Updated

4 years ago
Attachment #8405233 - Flags: review?(dlee)
(Assignee)

Updated

4 years ago
Attachment #8405234 - Flags: review?(dlee)
(Assignee)

Updated

4 years ago
Attachment #8405235 - Flags: review?(dlee)
Created attachment 8405239 [details] [diff] [review]
Part 3: Marionette tests. v2.

update the sequence of tests.
Attachment #8405235 - Attachment is obsolete: true
Attachment #8405235 - Flags: review?(dlee)
Attachment #8405239 - Flags: review?(dlee)
Created attachment 8405245 [details] [diff] [review]
Part 3: Marionette tests. v2.
Attachment #8405239 - Attachment is obsolete: true
Attachment #8405239 - Flags: review?(dlee)
Attachment #8405245 - Flags: review?(dlee)

Updated

4 years ago
Attachment #8405233 - Flags: review?(dlee) → review+
blocking-b2g: --- → backlog

Updated

4 years ago
Attachment #8405245 - Flags: review?(dlee) → review+
Comment on attachment 8405233 [details] [diff] [review]
Part 1: WebIDL change

Hi smaug

The three interfaces added here are referenced from W3C NFC API. [1]
I didn't add powerOn because I think startPoll can cover it.

Thanks

[1]: http://www.w3.org/TR/nfc/#nfcmanager-interface
Attachment #8405233 - Flags: superreview?(bugs)
Comment on attachment 8405233 [details] [diff] [review]
Part 1: WebIDL change

Could you file a followup bug to investigate if the API shouldn't use 
DOMRequests (which it is already using), but Promises.
Attachment #8405233 - Flags: superreview?(bugs) → superreview+
Comment on attachment 8405234 [details] [diff] [review]
Part 2: Impl

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

::: dom/system/gonk/nsINfcContentHelper.idl
@@ +135,5 @@
>    void notifySendFileStatus(in nsIDOMWindow window,
>                              in octet status,
>                              in DOMString requestId);
> +
> +

Nit.
Extra empty line here, it will also be good if we add comment here
since startPoll menas turn on nfc and start polling
Attachment #8405234 - Flags: review?(dlee) → review+
(In reply to Olli Pettay [:smaug] from comment #13)
> Comment on attachment 8405233 [details] [diff] [review]
> Part 1: WebIDL change
> 
> Could you file a followup bug to investigate if the API shouldn't use 
> DOMRequests (which it is already using), but Promises.

Thanks for the review.
Filed Bug 996397 for this.
Created attachment 8406675 [details] [diff] [review]
Part 2: Impl. v2

Address Dimi's comments:
remove extra line,
add comments in nsINfcContentHelper.idl
Attachment #8405234 - Attachment is obsolete: true
Attachment #8406675 - Flags: review+
Please fix your Mercurial configuration so that your patches contain your full name.
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Flags: needinfo?(allstars.chh)
fixed now, sorry for the confusion.
Flags: needinfo?(allstars.chh)
feature-b2g: --- → 2.0

Updated

4 years ago
Flags: in-moztrap-
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.