B2G NFC: remove NFCTag.connect and NFCTag.close

RESOLVED FIXED in 2.1 S3 (29aug)

Status

RESOLVED FIXED
5 years ago
4 years ago

People

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

Tracking

unspecified
2.1 S3 (29aug)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(tracking-b2g:backlog)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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);
Blocks: 948721
Assignee: nobody → allstars.chh
(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

5 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.
(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
Depends on: 976457

Updated

5 years ago
Blocks: 978707

Updated

5 years ago
Blocks: 949293

Updated

5 years ago
No longer blocks: 949293
blocking-b2g: --- → backlog
Assignee: allstars.chh → allstars.chh
Assignee: allstars.chh → allstars.chh
No longer depends on: 916428
Blocks: 976457
No longer depends on: 976457
Bug 1043889 is unrelated to this - typo?
No longer blocks: 1043889
yeah, typo. sorry for my fat finger. :P
Blocks: 1043899
Np :-)
Blocks: 1042851
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.
Summary: B2G NFC: Have a clear definition with NFCTag.connect and NFCTag.close → B2G NFC: remove NFCTag.connect and NFCTag.close
NFCTechType will still be used in Bug 1057264, so I don't remove them for now.
Posted patch Patch (obsolete) — Splinter Review
Attachment #8477275 - Flags: review?(dlee)
Attachment #8477275 - Flags: review?(dlee) → review+
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)
Attachment #8477275 - Flags: review?(bugs) → review+
Posted patch PatchSplinter Review
Add r=smaug, dimi
Attachment #8477275 - Attachment is obsolete: true
Attachment #8479842 - Flags: review+
add checkin-needed for b2g-inbound is close.
Keywords: checkin-needed
Blocks: 964194
https://hg.mozilla.org/mozilla-central/rev/8af301918828
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S3 (29aug)
No longer blocks: 1042851
Blocks: 1042851
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
You need to log in before you can comment on or make changes to this bug.