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

Join Reader and Writer objects #561

Closed
kenchris opened this issue Apr 22, 2020 · 10 comments
Closed

Join Reader and Writer objects #561

kenchris opened this issue Apr 22, 2020 · 10 comments

Comments

@kenchris
Copy link
Contributor

I take the blame for this. It originally seemed like a good idea with separate objects, but I am getting more and more convinced that it was a bad idea.

  • If we enhance to be able to select tag in the case of multiple tags read, this is required
  • TNEP also requires this
  • We can get rid of ignoreRead
  • User is more in control of what happens.

Suggested name NDEFPoller or NDEFPollerDevice

@OlivierGrenoble
Copy link

If we enhance to be able to select tag in the case of multiple tags read, this is required

On Android, it is not possible to select one tag among several tags. There should be only one in the RF field.

@reillyeon
Copy link
Member

I vote for calling the combined object NDEFReader. The physical hardware components for NFC seem to usually be called a "reader" even though they can also write. It doesn't seem necessary to find a new (potentially awkward) word to describe it.

@kenchris
Copy link
Contributor Author

I need to get hand on the new NFC TNEP spec, but it looks like they used poller device (vs tag device) but I need to confirm

@OlivierGrenoble
Copy link

I vote for calling the combined object NDEFReader. The physical hardware components for NFC seem to usually be called a "reader" even though they can also write. It doesn't seem necessary to find a new (potentially awkward) word to describe it.

I agree

@kenchris
Copy link
Contributor Author

I checked the new spec and it seems that the Poller thing was just what Nordic named it in their API.

Currently the following NFC Forum Devices are defined:

  • NFC Universal Device
  • NFC Tag Device
  • NFC Reader Device.

@zolkis
Copy link
Contributor

zolkis commented Apr 26, 2020

I vote for calling the combined object NDEFReader

Currently the following NFC Forum Devices are defined:
NFC Universal Device
NFC Tag Device
NFC Reader Device

Ergo what about NFCReader, bearing in mind possible future support for NDEF+other tech for a given device?

Edit.
In fact, pondering whether NFCDevice or NDEFDevice would be better. Think about e.g. Example 2.

const writer = new NDEFWriter();
writer.write(
  "Hello World"
).then(() => {
  console.log("Message written.");
}).catch(error => {
  console.log(`Write failed :-( try again: ${error}.`);
});

If we used NDEFReader, we created reader and use it for writing.

const reader = new NDEFReader();
reader.write(
  "Hello World"
).then(() => {
  console.log("Message written.");
}).catch(error => {
  console.log(`Write failed :-( try again: ${error}.`);
});

I know we could call it const ndef = new NDEFReader(); as well, but maybe would look better this way:

const ndef = new NFCDevice();
ndef.write(
  "Hello World"
).then(() => {
  console.log("Message written.");
}).catch(error => {
  console.log(`Write failed :-( try again: ${error}.`);
});

@reillyeon
Copy link
Member

I'd be cautious about assuming that the existing API can be extended to support future technologies. Keeping the API focused on NDEF allows us to only consider a subset of all possible NFC interactions and built the best API for that.

@zolkis
Copy link
Contributor

zolkis commented Apr 27, 2020

Discussed with Kenneth that we'd likely stick with NDEFReader that will have a write() method :).

zolkis added a commit to zolkis/web-nfc that referenced this issue Apr 30, 2020
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
@kenchris
Copy link
Contributor Author

kenchris commented May 25, 2020

Fixed on branch: 00509fc

@kenchris kenchris reopened this May 25, 2020
@kenchris
Copy link
Contributor Author

kenchris commented Dec 8, 2020

Merged upstream

@kenchris kenchris closed this as completed Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants