Closed Bug 970251 Opened 6 years ago Closed 6 years ago

B2G NFC: Use API to enable/disable NFC hardware

Categories

(Firefox OS Graveyard :: NFC, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

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

RESOLVED FIXED
1.4 S6 (25apr)
feature-b2g 2.0
tracking-b2g backlog

People

(Reporter: allstars.chh, Assigned: allstars.chh)

References

Details

(Whiteboard: [p=3])

Attachments

(3 files, 3 obsolete files)

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.
What's wrong with using the Settings API?
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?
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.
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: 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.
This bug will focus on Gecko part first, I'll handle Gaia changes in Bug 995101.
Whiteboard: [p=3]
Target Milestone: --- → 1.4 S6 (25apr)
Attached patch Part 3: Marionette tests. v2. (obsolete) — Splinter Review
update the sequence of tests.
Attachment #8405235 - Attachment is obsolete: true
Attachment #8405235 - Flags: review?(dlee)
Attachment #8405239 - Flags: review?(dlee)
Attachment #8405239 - Attachment is obsolete: true
Attachment #8405239 - Flags: review?(dlee)
Attachment #8405245 - Flags: review?(dlee)
Attachment #8405233 - Flags: review?(dlee) → review+
blocking-b2g: --- → backlog
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.
Attached patch Part 2: Impl. v2Splinter Review
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
Flags: in-moztrap-
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.