Closed
Bug 959109
Opened 11 years ago
Closed 11 years ago
B2G NFC: Disable NFC when screen is off.
Categories
(Firefox OS Graveyard :: NFC, defect)
Tracking
(feature-b2g:2.0)
VERIFIED
FIXED
feature-b2g | 2.0 |
People
(Reporter: allstars.chh, Assigned: dimi)
References
Details
(Whiteboard: [FT:RIL])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
Some NFC hardware functionalities (like discovery) should be disabled when screen is off.
Reporter | ||
Comment 1•11 years ago
|
||
When screen is off, Gaia will send a ContentEvent https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/nfc_manager.js#L158
Comment 3•11 years ago
|
||
My mistake. 952212 was not the right dependency. The bug depends Gaia change in Bug 950269.
Updated•11 years ago
|
Assignee: nobody → dgarnerlee
Reporter | ||
Comment 4•11 years ago
|
||
Hi Garner I thought this should be done in Gecko part, if you think the Gaia part is not correct (acceptNfcEvents()), can you kindly explain or maybe file another bug?
Comment 5•11 years ago
|
||
Comment on attachment 8360120 [details]
Check screen and lock state at the same time.
See github
Attachment #8360120 -
Flags: review?(alive) → review+
Comment 6•11 years ago
|
||
Comment on attachment 8360120 [details]
Check screen and lock state at the same time.
It's already done in acceptNFCEvents(), please explain why this change is made.
Attachment #8360120 -
Flags: review+
Comment 7•11 years ago
|
||
It relates to 959104, where "vibrate forever" is techDiscovered or techLost being fired continuously (and seems to have some effect on ShrinkUI in p2p mode). In handleEvent, acceptNFCEvents() was used by the screenchange case, which might send a conflicting value (NFC_HW_STATE_ENABLE_DISCOVERY, NFC_HW_STATE_DISABLE_DISCOVERY) with screen locked. It is by independent, but closely related to screenchange. Change that case to only check screen on/off fixed the vibrate forever problem. Other potential behavior issues in gecko/nfcd might be hidden however. If you see other issues, or intended to fix something else in this bug, please let me know. I'll check Alive's review comment, and investigate some more.
Comment 8•11 years ago
|
||
Alive's change request applied and still working. There's still some ocassional *extra* discovery event from the hardware (nfcd debounce didn't catch it?), but that seems to be a different bug. If OK, check-in needed.
Flags: needinfo?(alive)
Reporter | ||
Comment 9•11 years ago
|
||
As discussed with Garner, we don't have to ask Alive for comments.
Flags: needinfo?(alive)
Assignee | ||
Updated•11 years ago
|
QA Contact: dlee
Updated•11 years ago
|
Whiteboard: [FT:RIL]
Updated•11 years ago
|
Attachment #8360120 -
Attachment description: Check only screen state when handling screen state. → Check screen and lock state at the same time.
Updated•11 years ago
|
Blocks: b2g-NFC-2.0
Updated•11 years ago
|
Attachment #8360120 -
Attachment is obsolete: true
Updated•11 years ago
|
Assignee: dgarnerlee → dlee
Reporter | ||
Comment 11•11 years ago
|
||
Comment on attachment 8363600 [details] [diff] [review] [WIP] Enable/disable discovery when screen on/off Review of attachment 8363600 [details] [diff] [review]: ----------------------------------------------------------------- Add r=me ::: b2g/chrome/content/shell.js @@ +747,5 @@ > KeyboardHelper.handleEvent(detail); > break; > + case 'nfc-hardware-state-change': > + Services.obs.notifyObservers(null, 'nfc-hardware-state-change', > + JSON.stringify({ nfcHardwareState: detail.nfcHardwareState})); extra space before nfcHarwardState. or either also put a space at the end. ::: dom/system/gonk/Nfc.js @@ +639,5 @@ > + case NFC.TOPIC_HARDWARE_STATE: > + let state = JSON.parse(data); > + if (state) { > + let level = this.hardwareStateToPowerlevel(state.nfcHardwareState); > + this.setConfig({powerLevel:level}); put a space after : powerLevel: level
Attachment #8363600 -
Flags: feedback?(allstars.chh) → review+
Assignee | ||
Comment 12•11 years ago
|
||
Try server result : https://tbpl.mozilla.org/?tree=Try&rev=d8198d43baf8
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 14•11 years ago
|
||
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #13) > address nits first and add r=me Sorry, i forget to attach updated patch...
Assignee | ||
Comment 15•11 years ago
|
||
Comment 17•11 years ago
|
||
Since Bug 962539 has been landed, should we land this now?
Flags: needinfo?(dlee)
Assignee | ||
Updated•11 years ago
|
Attachment #8363600 -
Attachment is obsolete: true
Flags: needinfo?(dlee)
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Reporter | ||
Updated•11 years ago
|
Summary: B2G NFC: Disable NFC when screen in off. → B2G NFC: Disable NFC when screen is off.
Reporter | ||
Comment 18•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/2cd547d09fcf
Keywords: checkin-needed
Reporter | ||
Comment 19•11 years ago
|
||
update the subject of the patch. Add r=allstars.chh
Attachment #8366453 -
Attachment is obsolete: true
Comment 20•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2cd547d09fcf
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 22•11 years ago
|
||
(In reply to Tomoaki Konno from comment #21) > Is NFC payment disabled when screen is off? NFC payment should not be off in this case. I will pay attention to this use case when implement payment related function
Comment 24•11 years ago
|
||
This bug is conflicting to bug 973480, bug 979868, and bug 979888. We should add a standby mode for NFC when NFC is on but screen is off. Filing bug 984207 for tracking.
Flags: needinfo?(ming.yin)
Comment 25•11 years ago
|
||
(In reply to Tomoaki Konno from comment #21) > Is NFC payment disabled when screen is off? In current design, the answer is yes. But we should add a suspend/standby mode for NFC. Please check bug 984207.
Comment 26•11 years ago
|
||
Please refer to: https://bugzilla.mozilla.org/show_bug.cgi?id=973480#c8 Also, some of these bugs look like duplicates. Can these be appropriately duplicated?
Flags: needinfo?(whuang)
Comment 27•11 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=973480#c11 It depends on how partner wants to track it. I'm fine either way.
Flags: needinfo?(whuang)
Comment 28•11 years ago
|
||
(In reply to Sandip Kamat from comment #26) > Please refer to: https://bugzilla.mozilla.org/show_bug.cgi?id=973480#c8 > > Also, some of these bugs look like duplicates. Can these be appropriately > duplicated? I don't think so. Bug 984207 is proposing a standby/low power mode for NFC. But it doesn't define when device should enter this mode, which depends on the user story and what partners want.
Updated•10 years ago
|
feature-b2g: --- → 2.0
Comment 29•10 years ago
|
||
https://moztrap.mozilla.org/manage/cases/?filter-tag=2786&pagenumber=1&pagesize=50&sortfield=created_on&sortdirection=desc
Flags: in-moztrap+
Comment 30•10 years ago
|
||
Verify on Gaia f3b5d74dd3428c89cab06db734c62f3c9dbb8c4d Gecko https://hg.mozilla.org/mozilla-central/rev/e86a0d92d174 BuildID 20140525160203 Version 32.0a1
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•