1112478 - remove nfc-tag-discovered MozActivity

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Like Bug 1108973 we also need to check nfc-tag-discovered MozActivity.
It seems trying to pass techList and tech, but I don't know what's the purpose of this.
I guess it's just copied from Android TAG_DISCOVERED.

Comment 1

[Krzysztof Mioduszewski[:tauzen]]

I assume this was introduced when some other tag technologies (ISODEP?) and proprietary data exchange formats were considered. 

Right now this is fired when there aren't any NDEF records found on the tag. This activity could be used by the developer to write some data on the tag. It would be a similar use case as in getting nfc-ndef-discovered with tnf/type "empty".

Comment 2

[Krzysztof Mioduszewski[:tauzen]]

My mistake, without ontagfound handler it's not possible to write data on empty tag right now.
Since we're removing techList and tech in bug 1110060, this activity won't carry any additional data. 

Except for notification purpose this activity is useless, If a developer would like to work with NFC tag he should use ontagfound instead. Yoshi, what do you think about this? Should we remove this activity?

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I think so.
At least I don't know what's this activity designed for.

Even we can do like Android pass the tag object to another app by MozActivity (Android uses Intent), 
1. Should we fire ontagfound to the app launched by MozActivity?
In the URL case, System app fire a view MozActivity to System Browser, then it gets the URL, however it now gets another ontagfound, if System Browser has implemented ontagfound, it would open the URL again, so the same URL could be opened twice. If System Browser doesn't implement ontagfound, then the URL could be fallback to System to launch the 'view' MozActivity, loop again.


2. our API has ontaglost, what should happen if the tag is removed away?

I think there are some design problem for this and we haven't figured it out clearly yet.
Before we sort it out we should remove it, otherwise it has been there for a while and still, nobody is using it, or nobody knows how to use it.

Comment 4

[Krzysztof Mioduszewski[:tauzen]]

Ok, as per comment 3. I'm removing nfc-tag-discovered activity. Will try to land this before branch date, since this is not a big change and requires mostly clean-up.

Comment 5

[Krzysztof Mioduszewski[:tauzen]]

Attachment: pull-request-1112478.txt

Hello Greg, once more not a big change with clean-up. This is related to the introduction of ontagfound, which should be used by app developer to work with tags. Tests are progressing. Thanks.

Comment 6

[Greg Weng [:snowmantw][:gweng][:λ]]

However I noticed there are some Gij intermittent errors. For safety you should try to re-trigger each job (if you have the permission) maybe 5 or 6 times to make sure there is no harness error. Thanks.

Comment 7

[Krzysztof Mioduszewski[:tauzen]]

Comment on attachment 8545835 [details] [review]
pull-request-1112478.txt

[Bug caused by] (feature/regressing bug #):
None, NfcManager clean up related to recent changes in NFC API.
[User impact] if declined: None
[Testing completed]: On device testing completed, gaia-try all green.
[Risk to taking this patch] (and alternatives if risky): None
[String changes made]: None

As per comment 3, it would be good to have this landed on 2.2 so the situation would be clear for 3rd party developers, who will get access to NFC API in 2.2 (Bug 1102019).

Comment 8

[Krzysztof Mioduszewski[:tauzen]]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #6)
> However I noticed there are some Gij intermittent errors. For safety you
> should try to re-trigger each job (if you have the permission) maybe 5 or 6
> times to make sure there is no harness error. Thanks.

Thanks Greg, I've re-triggered the failed jobs. Now everything is green.

Comment 9

[bhavana bajaj [:bajaj]]

Needs to land on master first, please seek approval once your patch is landed and stuck on master without backouts etc..

Comment 10

[Krzysztof Mioduszewski[:tauzen]]

(In reply to bhavana bajaj [:bajaj] from comment #9)
> Needs to land on master first, please seek approval once your patch is
> landed and stuck on master without backouts etc..

Ok, understood. Will add the flag once this will be landed.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/a1622a84a5a62a21716b6096475d9486d4ca5dd6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/d49bf352caa9848d859924bc2e1b610044cbf7ec