Closed
Bug 963541
Opened 11 years ago
Closed 10 years ago
B2G NFC: remove NFCTag.connect and NFCTag.close
Categories
(Firefox OS Graveyard :: NFC, defect)
Tracking
(tracking-b2g:backlog)
People
(Reporter: allstars.chh, Assigned: allstars.chh)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
We need to have a clear definition of what does NFCTag.connect() and NFCTag.close() should do precisely. What does connect/close do? and what happened if we don't call them correctly. And should we really need them ? connect() maybe still needed as it is used to switch RF interface. But what about close? should we rename it to disconnect() ? If the app calls NFCTag.close(), actually NFC library will close the physical location to that tag, however since Tag is still in proximity another tech-discovered will be fired immediately, this will cause a bad user experience. And what will happen if we calls two connect()? like : tag.connect(NFC_A); // didn't call close before calling next connect() tag.connect(NFC_B);
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → allstars.chh
Assignee | ||
Comment 1•11 years ago
|
||
(In reply to Yoshi Huang[:allstars.chh] from comment #0) > What does connect/close do? and what happened if we don't call them > correctly. > And should we really need them ? > > connect() maybe still needed as it is used to switch RF interface. > connect is used to switch to IsoDep or Frame RF interfaces. Currently we don't support IsoDep yet. See Bug 916428 > But what about close? should we rename it to disconnect() ? From Android http://developer.android.com/reference/android/nfc/tech/TagTechnology.html#close() This function actually calls NfcService.reconnect(); So back to our WebNFC API: Should we rename close to reconnect? And under what circumstance we will need this 'reconnect' ? > And what will happen if we calls two connect()? > like : > > tag.connect(NFC_A); // didn't call close before calling next connect() > tag.connect(NFC_B); In the example above, no error should be thrown, as NFC_A and NFC_B use the same Frame RF interface. However what should happen if tag.connect(NFC_A); tag.connect(ISO_DEP); Should the second connect could switch to IsoDep RF interface successfully? or it should throw because not calling close() before ?
Comment 2•11 years ago
|
||
I think it makes sense to track (re)connect/close() states, and throw an JS exception if it isn't closed before switching technologies so it can potentially also properly finish whatever it was doing. It may also encourage proper application cleanup (close ICC channels, etc.), for future feature development. This happen whether or not the application remembers whether it is connected or not.
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Garner Lee from comment #2) > I think it makes sense to track (re)connect/close() states, I think you mean 'connect/close(reconnect)' > and throw an JS > exception if it isn't closed before switching technologies Yeah, again, define 'close'. I think you're still talking 'reconnect' from my previous comment. However I agree that we need to throw error at this case. We need some TagTechnology support first, like NfcA or IsoDep(Bug 916428). But before they are supported, I think NFCTag.connect/close are not neccesary.
Depends on: 916428
Updated•11 years ago
|
Blocks: b2g-NFC-2.0
Updated•11 years ago
|
No longer blocks: b2g-NFC-2.0
Updated•11 years ago
|
blocking-b2g: --- → backlog
Assignee | ||
Updated•11 years ago
|
Assignee: allstars.chh → allstars.chh
Assignee | ||
Updated•11 years ago
|
Assignee: allstars.chh → allstars.chh
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Blocks: b2g-nfc-privilege
Assignee | ||
Comment 7•10 years ago
|
||
These functions are actually useless now, since when the tag is discovered nfcd already connects to it. So in this bug I'll remove these two interfaces(connect/close) from MozNFCTag.webidl, but still keep the internal impl because we might still need this when we support multi-protocol tags.
Assignee | ||
Updated•10 years ago
|
Summary: B2G NFC: Have a clear definition with NFCTag.connect and NFCTag.close → B2G NFC: remove NFCTag.connect and NFCTag.close
Assignee | ||
Comment 8•10 years ago
|
||
NFCTechType will still be used in Bug 1057264, so I don't remove them for now.
Updated•10 years ago
|
Attachment #8477275 -
Flags: review?(dlee) → review+
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8477275 [details] [diff] [review] Patch Hi, Smaug Jonas has asked the purpose of these two interfaces earlier this year. These two interfaces are added when WebNFC firstly landed Bug 674741, and actually it uses the same interface from Android. See [1] After investigation we found that these interfaces are used to switch RF interface for different Technologies. However right now we didn't support any NFC Tag Technology yet, and nfc daemon already doing the RF-connect when the tag is discovered.(Like when a MozNFCPeer is discoverd, nfcd will connect to it by itself). So actully the two interaces are dummy, so we propose to remove these two. [1]: http://developer.android.com/reference/android/nfc/tech/TagTechnology.html#connect()
Attachment #8477275 -
Flags: review?(bugs)
Updated•10 years ago
|
Attachment #8477275 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Add r=smaug, dimi
Attachment #8477275 -
Attachment is obsolete: true
Attachment #8479842 -
Flags: review+
Assignee | ||
Comment 12•10 years ago
|
||
add checkin-needed for b2g-inbound is close.
Keywords: checkin-needed
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/8af301918828
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8af301918828
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S3 (29aug)
Assignee | ||
Updated•10 years ago
|
No longer blocks: b2g-nfc-privilege
Assignee | ||
Updated•10 years ago
|
Blocks: b2g-nfc-privilege
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•