Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify NFCReaders of failures when trying to get a message from a tag? #279

Closed
leonhsl opened this issue Aug 6, 2019 · 14 comments · Fixed by #432
Closed

Notify NFCReaders of failures when trying to get a message from a tag? #279

leonhsl opened this issue Aug 6, 2019 · 14 comments · Fixed by #432
Labels
Origin Trial Issues that would be good to solve before OT

Comments

@leonhsl
Copy link
Contributor

leonhsl commented Aug 6, 2019

See
https://developer.android.com/reference/android/nfc/tech/Ndef.html#getNdefMessage().
Once a tag is discovered, we call android.nfc.tech.Ndef.getNdefMessage() to try to get a message from it, which may

  • return null if the tag is in the INITIALIZED state, i.e. formatted but empty
  • throw exceptions: TagLostException, IOException, or FormatException

In such cases do we need to let all active NFCReaders be aware of these failures so that they can choose how to proceed next? i.e. dispatching some error events properly to NFCReaders.

Currently http://w3c.github.io/web-nfc/#the-nfc-reading-algorithm only describes the case that a ndef message has already been gotten from the tag, so does the implementation in Chromium.

@kenchris
Copy link
Contributor

kenchris commented Aug 6, 2019

So I assume the problem we have here is that 1) is an empty formatted tag that doesn't even have the empty record?

@leonhsl
Copy link
Contributor Author

leonhsl commented Aug 6, 2019

Exactly.

@kenchris
Copy link
Contributor

kenchris commented Aug 6, 2019

Could we return an empty record?

@riju
Copy link
Collaborator

riju commented Aug 6, 2019

For context, this came up in review. In chromium implementation, I do see a kNfcEmpytMessage used to throw an exception while pushing empty message to tag. We could use something similar while reading also ?

@kenchris
Copy link
Contributor

kenchris commented Aug 6, 2019

For TagLostException, we can just avoid dispatching the event as no tag was fully read.

@kenchris
Copy link
Contributor

kenchris commented Aug 6, 2019

We should probably do like Generic Sensor and have an onerror event handler as well, which somehow can handle the error cases

@leonhsl
Copy link
Contributor Author

leonhsl commented Aug 7, 2019

So, for the null ndef message case we dispatch a reading event with a ndef message containing just an empty record; for IOException and FormatException cases we dispatch an error event. Right?

Note: These events are dispatched to ALL active readers, without any message filtering for matching their reader options.

@kenchris
Copy link
Contributor

kenchris commented Aug 7, 2019

Sounds good, but I think we might want to consider adding a "formatted" type, similarly to the "empty" type

@riju riju added the Origin Trial Issues that would be good to solve before OT label Aug 23, 2019
@beaufortfrancois
Copy link
Collaborator

According to #367, reading non-formatted tags result in a onreading event with empty records array for NDEFMessage.
I think we can close this issue right?

@beaufortfrancois
Copy link
Collaborator

Ooops! We should not close it. Dispatching event errors in IOException and FormatException cases is not supported anymore as we got rid of NDEFErrorEvent.

@kenchris
Copy link
Contributor

So you suggest we readd the error event for that? Or other solutions?

@beaufortfrancois
Copy link
Collaborator

I didn't find any other solution sadly that satisfies me but re-adding the NDEFError event.
@zolkis for thoughts as well.

@zolkis
Copy link
Contributor

zolkis commented Oct 28, 2019

I didn't find any other solution sadly that satisfies me but re-adding the NDEFError event.

I think so, too.

@beaufortfrancois
Copy link
Collaborator

For info, I have a PR at #432 that attempts to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Origin Trial Issues that would be good to solve before OT
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants