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

WIP: Feature/add streaming #54

Merged
merged 6 commits into from
Oct 13, 2019
Merged

WIP: Feature/add streaming #54

merged 6 commits into from
Oct 13, 2019

Conversation

henrysachs
Copy link
Collaborator

I had the Problem that with my NFC Tag i could only scan a Tag once and not more than that. So I implemented the Eventchannel methods that just emit the onTagDiscovered values to dart. Therefore a User can now listen to the stream and scan as much nfc tags as he wants. I updated the example implementation and also the README to state how it works.

Open for discussion why i marked this PR first as WIP.

@henrysachs
Copy link
Collaborator Author

fixes #30 if I'm right.

@matteocrippa
Copy link
Collaborator

Looks good to me @henrysachs the only issue we have to figure out is we can do the same behaviour for iOS, or we can do something like a switch between the two type passing an extra param.

What do you think? like continousScan

@henrysachs
Copy link
Collaborator Author

the problem is I'm currently just starting to get into ios developement, but I can take a look into options on the Weekend. I would suggest lets wait till than and just keep up the WIP status.

@matteocrippa
Copy link
Collaborator

As far as I remember it should not be possible, because iOS rely on its proper system that show a modal that take care of that, but think it should work only one time.
Need to look in docs

- streaming in ios isnt really possible because every scan is shown to the user through a bottom sheet
- So we just use the streaming channel and write the read result to it
@henrysachs
Copy link
Collaborator Author

so as expected on ios you cant have a continous scan. Every scan triggers the bottom sheet. So I took the read result and write it to the Eventsink. So one doesnt actually need the read function because he can just subscribe to the stream and let that handle everything. But on IOS when you want to scan another tag you need to trigger the read function again but will get the result in the stream.

Also all messages i wrote onto my nfc tags made problems when parsing the message. So i used a library which can parse all NFC Types of messages which will help us in the future. The types could also be passed to the nfc data element to trigger something like an url open action.

@henrysachs
Copy link
Collaborator Author

henrysachs commented Oct 12, 2019

Also the current implementation cant read the id of the card which is for example crucial for my use case. This is because the current delegate cant do this. I would like to create some follow up issues for this. For example

  • Pass Tag ID to Flutter (#25)
  • Pass Type to Flutter (#56)
  • Write NFC Tag on IOS (already exists #48)

@henrysachs
Copy link
Collaborator Author

If you got an Idea how we could solve this better let us discuss about it i used my Saturday to learn IOS and this was the only thing i came up with, but it was fun! :D

@matteocrippa
Copy link
Collaborator

Great work @henrysachs , yep I see you use VYNFCKit, think is ok because it use MIT license so is ok with current plugin one.

Think we can split the feature, and if this provide the continous scan we can start releasing this and issue a new version on the pub repo too.

Then be atomic with little PR for specific other feature, what do you think?

@henrysachs
Copy link
Collaborator Author

I have some problems understanding the further plans. So did you mean we merge this into master -> release the plugin -> improve the library further

That’s what I think would be great.

@matteocrippa
Copy link
Collaborator

yep, idea is to have incremental release if possible, so who use this lib start benefit of your great improvement already

@matteocrippa matteocrippa merged commit 9078ee9 into dotintent:master Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants