Skip to content
This repository has been archived by the owner on Jun 23, 2024. It is now read-only.

Include raw payload #54

Merged
merged 6 commits into from
Jul 18, 2020
Merged

Include raw payload #54

merged 6 commits into from
Jul 18, 2020

Conversation

laurensfischer
Copy link
Contributor

Hey @semlette ,

thanks for your work and your awesome library. It works like a charm.
We need to read NDEFText tags where the first byte as well as the language code was not written correctly. Therefore we need some mechanism to access the raw payload, which we implemented in this pull request. Because we don't think it should be provided by default, we added it as optional parameter to the startReading method.
We hope you appreciate our work and approve it to be merged. If not, please tell us what we should improve.

Best regards,
Laurens

@avbk
Copy link

avbk commented Jun 19, 2020

👍

Awesome, I was just looking for this. I also have badly formatted NFC tags. Hope this gets merged soon 🙂

@semlette
Copy link
Owner

This seems like a good change.
The removal of the first byte stems back to v1, when I didn't even know what it was for - only in v2 did I realise.

Would the dart:typed_data Uint8List be a better data type than String?

Is there a particular reason why this should be controlled by an argument instead of always being set?

@semlette semlette added the enhancement New feature or request label Jun 23, 2020
@laurensfischer
Copy link
Contributor Author

Thanks for the input, Uint8List is indeed better.
We thought some people won't need the raw payload, but it's probably easier to always provide the raw payload in case someone needs it.

semlette
semlette reviewed Jun 24, 2020
View reviewed changes
Copy link
Owner

@semlette semlette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
I will do some testing later before I merge.

lib/src/api.dart Show resolved Hide resolved
@laurensfischer
Copy link
Contributor Author

Were you already able to test the changes?

@semlette
Copy link
Owner

Afraid I don't have time to test it.

Will happily merge and release if you confirm that does indeed work.

@avbk, what do you think of this?

@avbk
Copy link

avbk commented Jul 16, 2020

I just tried the branch and it seems to work pretty well 🙂

@semlette semlette merged commit 753534c into semlette:master Jul 18, 2020
@semlette
Copy link
Owner

Included in v2.0.5.

haizadvnet pushed a commit to haizadvnet/nfc_in_flutter that referenced this pull request Apr 5, 2024
@timnew @Harry-Chen
Add new method makeNdefReadOnly (fix semlette#54)
61d1f37 
flutter pub get

Add makeNdefReadOnly

Implement the method on both iOS and Android
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@semlette semlette semlette approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@laurensfischer @avbk @semlette